code review 213570043: cmd: Fix dependencies for 'go get'

cmd: Fix dependencies for 'go get'
with gccgo

Problem described in GCC BZ 65462.
Generate the list of the standard GO package names based on what was built into libgo in the libgo Makefile.
Change the var name from reqPkgSrc to reqStdPkgSrc to clarify it only affects standard GO packages.
Skip the attempted loading of a package only if it is a standard GO package and the flag is set indicating its source is not required to be available.
This requires a corresponding change to gotools to build and link in the new file containing the list of standard GO package names that was generated by the libgo Makefile.

[laboger, 2015-03-20 21:00:11]

Hello gofrontend-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/gofrontend

[iant, 2015-03-20 22:51:06]

https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am
File libgo/Makefile.am (right):

https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode970
libgo/Makefile.am:970: s-version: Makefile zstdpkglist.go
Instead of adding the dependency here, add a line

noinst_DATA = zstdpkglist.go

just before the zstdpkglist.go target.

https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode990
libgo/Makefile.am:990: cat libgopkg-objs | sed 's/\.lo /\"\: true,\n/g' | sed
's/\.lo/\"\:true,/' | sed 's/-go//' | grep -v _c | sed 's/^/\t\"/' | sort | uniq
>> zstdpkglist.go.tmp
It doesn't look like you need libgopkg-objs at all.  Just
    echo $(libgo_go_objs) unsafe.lo | sed ...

Why are you backslash quoting the colons?

Instead of two .lo seds, could you use one?
    sed 's/\.lo ?/\": true,\n/g'

https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go
File libgo/go/cmd/go/build.go (right):

https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go#...
libgo/go/cmd/go/build.go:135: // Require the source for GO std packages
s/GO/Go/

[dave_cheney.net, 2015-03-21 02:23:48]

Is it possible to get this patch landed upstream in golang/go ?

On Sat, Mar 21, 2015 at 9:51 AM,  <iant@golang.org> wrote:
>
> https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am
> File libgo/Makefile.am (right):
>
>
https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode970
> libgo/Makefile.am:970: s-version: Makefile zstdpkglist.go
> Instead of adding the dependency here, add a line
>
> noinst_DATA = zstdpkglist.go
>
> just before the zstdpkglist.go target.
>
>
https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode990
> libgo/Makefile.am:990: cat libgopkg-objs | sed 's/\.lo /\"\: true,\n/g'
> | sed 's/\.lo/\"\:true,/' | sed 's/-go//' | grep -v _c | sed 's/^/\t\"/'
> | sort | uniq >> zstdpkglist.go.tmp
> It doesn't look like you need libgopkg-objs at all.  Just
>     echo $(libgo_go_objs) unsafe.lo | sed ...
>
> Why are you backslash quoting the colons?
>
> Instead of two .lo seds, could you use one?
>     sed 's/\.lo ?/\": true,\n/g'
>
> https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go
> File libgo/go/cmd/go/build.go (right):
>
>
https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go#...
> libgo/go/cmd/go/build.go:135: // Require the source for GO std packages
> s/GO/Go/
>
> https://codereview.appspot.com/213570043/
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to gofrontend-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[dave_cheney.net, 2015-03-23 04:50:05]

I think I've also hit this issue, https://github.com/golang/go/issues/10217. 

Lynn, did you raise a bug about this so I can close mine as a duplicate ?

[dave_cheney.net, 2015-03-23 06:24:00]

On 2015/03/23 04:50:05, dfc wrote:
> I think I've also hit this issue, https://github.com/golang/go/issues/10217. 
> 
> Lynn, did you raise a bug about this so I can close mine as a duplicate ?

This didn't build for me, 

/home/dfc/devel/build/./gcc/gccgo -B/home/dfc/devel/build/./gcc/ 
-static-libstdc++ -static-libgcc  -L ../x86_64-unknown-linux-gnu/libgo -L
../x86_64-unknown-linux-gnu/libgo/.libs -static-libgo -o go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/build.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/clean.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/context.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/discovery.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/env.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/fix.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/fmt.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/generate.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/get.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/go11.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/help.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/http.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/list.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/main.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/pkg.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/run.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/signal.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/signal_unix.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/test.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/testflag.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/tool.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/vcs.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/version.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/vet.go zdefaultcc.go 
/home/dfc/devel/build/./gcc/gccgo -B/home/dfc/devel/build/./gcc/ 
-static-libstdc++ -static-libgcc  -L ../x86_64-unknown-linux-gnu/libgo -L
../x86_64-unknown-linux-gnu/libgo/.libs -static-libgo -o cgo
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/ast.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/doc.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/gcc.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/godefs.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/main.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/out.go
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/util.go zdefaultcc.go 
../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/pkg.go:116:16: error:
reference to undefined name ‘stdpkg’
   p.Standard = stdpkg[p.ImportPath]
                ^
make[2]: *** [go] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/home/dfc/devel/build/gotools'
make[1]: *** [all-gotools] Error 2
make[1]: Leaving directory `/home/dfc/devel/build'
make: *** [all] Error 2

the file that defined map stdpkg wasn't generated correctly.

[laboger, 2015-03-23 13:41:05]

On 03/23/2015 01:24 AM, dave@cheney.net wrote:
> On 2015/03/23 04:50:05, dfc wrote:
>> I think I've also hit this issue,
> https://github.com/golang/go/issues/10217.
>
>> Lynn, did you raise a bug about this so I can close mine as a
> duplicate ?
>
No, I did not see the issue you mentioned.  This bug was reported to me 
as a gccgo bug from another bugzilla component so I created a gcc 
bugzilla to fix it.  I wanted this to get into gcc 5.0 if possible so 
that is why I started with a bug against gccgo.  I can submit it to the 
main golang repository too.  I actually didn't know that issues for 
gccgo would be submitted as golang issues.
> This didn't build for me,

The original patch was added to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462.  Ian asked me to 
create a code review for the patches that are for libgo against the 
gofrontend repository.  There is also a change needed to gotools for 
this to build correctly.  I mention that in the description.   Did you 
include the patch for gotools in your build?

I just tried a build on an x86 and it worked for me with the gotools 
patch and this patch.

>
> /home/dfc/devel/build/./gcc/gccgo -B/home/dfc/devel/build/./gcc/
> -static-libstdc++ -static-libgcc  -L ../x86_64-unknown-linux-gnu/libgo
> -L ../x86_64-unknown-linux-gnu/libgo/.libs -static-libgo -o go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/build.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/clean.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/context.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/discovery.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/env.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/fix.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/fmt.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/generate.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/get.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/go11.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/help.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/http.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/list.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/main.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/pkg.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/run.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/signal.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/signal_unix.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/test.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/testflag.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/tool.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/vcs.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/version.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/vet.go zdefaultcc.go
>
> /home/dfc/devel/build/./gcc/gccgo -B/home/dfc/devel/build/./gcc/
> -static-libstdc++ -static-libgcc  -L ../x86_64-unknown-linux-gnu/libgo
> -L ../x86_64-unknown-linux-gnu/libgo/.libs -static-libgo -o cgo
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/ast.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/doc.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/gcc.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/godefs.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/main.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/out.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/cgo/util.go
> zdefaultcc.go
> ../../gcc/branches/gccgo/gotools/../libgo/go/cmd/go/pkg.go:116:16:
> error: reference to undefined name ‘stdpkg’
>    p.Standard = stdpkg[p.ImportPath]
>                 ^
> make[2]: *** [go] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[2]: Leaving directory `/home/dfc/devel/build/gotools'
> make[1]: *** [all-gotools] Error 2
> make[1]: Leaving directory `/home/dfc/devel/build'
> make: *** [all] Error 2
>
> the file that defined map stdpkg wasn't generated correctly.
>
> https://codereview.appspot.com/213570043/

[laboger, 2015-03-23 13:54:08]

On 2015/03/20 22:51:06, iant wrote:
> https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am
> File libgo/Makefile.am (right):
> 
>
https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode970
> libgo/Makefile.am:970: s-version: Makefile zstdpkglist.go
> Instead of adding the dependency here, add a line
> 
> noinst_DATA = zstdpkglist.go
> 
> just before the zstdpkglist.go target.

OK

> 
>
https://codereview.appspot.com/213570043/diff/40001/libgo/Makefile.am#newcode990
> libgo/Makefile.am:990: cat libgopkg-objs | sed 's/\.lo /\"\: true,\n/g' | sed
> 's/\.lo/\"\:true,/' | sed 's/-go//' | grep -v _c | sed 's/^/\t\"/' | sort |
uniq
> >> zstdpkglist.go.tmp
> It doesn't look like you need libgopkg-objs at all.  Just
>     echo $(libgo_go_objs) unsafe.lo | sed ...
> 
> Why are you backslash quoting the colons=
> 
> Instead of two .lo seds, could you use one?
>     sed 's/\.lo ?/\": true,\n/g'

I had some issues at first getting the file to get generated correctly so this
is what I tried to make it work.  You're right it's not necessary to have the \:
or use the extra file so I will change that.

> 
> https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go
> File libgo/go/cmd/go/build.go (right):
> 
>
https://codereview.appspot.com/213570043/diff/40001/libgo/go/cmd/go/build.go#...
> libgo/go/cmd/go/build.go:135: // Require the source for GO std packages
> s/GO/Go/

OK

[iant, 2015-03-23 15:22:01]

Run "hg upload" when your new version is ready.

[laboger, 2015-03-23 16:46:30]

Hello gofrontend-dev@googlegroups.com, iant@golang.org, dave@cheney.net (cc:
gofrontend-dev@googlegroups.com),

Please take another look.

[iant, 2015-03-24 19:43:46]

LGTM

Thanks.

[iant, 2015-03-24 19:44:02]

*** Submitted as
https://code.google.com/p/gofrontend/source/detail?r=6ed23fc8eac4 ***

cmd: Fix dependencies for 'go get'
with gccgo

Problem described in GCC BZ 65462.
Generate the list of the standard GO package names based on what was built into
libgo in the libgo Makefile.
Change the var name from reqPkgSrc to reqStdPkgSrc to clarify it only affects
standard GO packages.
Skip the attempted loading of a package only if it is a standard GO package and
the flag is set indicating its source is not required to be available.
This requires a corresponding change to gotools to build and link in the new
file containing the list of standard GO package names that was generated by the
libgo Makefile.

LGTM=iant
R=gofrontend-dev, iant, dave
CC=gofrontend-dev
https://codereview.appspot.com/213570043

Committer: Ian Lance Taylor <iant@golang.org>