code review 217620043: libgo: Build and install

libgo: Build and install libnetgo.a libnetgo.a provides the net package built with the netgo tag enabled. This provides the netgo fallback solution for gccgo. This lib must be explicitly linked in using the -gccgoflags, so is not included by default. Fixes gcc BZ 63731.

[laboger, 2015-04-01 14:11:15]

Hello gofrontend-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/gofrontend

[iant, 2015-04-03 18:20:10]

Please fix the first line of the description to be on a single line. Any thoughts on where this should be documented? https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am File libgo/Makefile.am (left): https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#oldcode1 libgo/Makefile.am:1: # Makefile.am -- Go library Makefile. Why remove this line? https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am File libgo/Makefile.am (right): https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode823 libgo/Makefile.am:823: go_netgo_files = \ Rather than duplicating the list entirely, let's make another variable that holds all the common files, and use that variable in both go_net_files and go_netgo_files. https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode... libgo/Makefile.am:2104: $(LTGOCOMPILE) -I . -c -fgo-pkgpath=net -o $@ $$files If you are using LTGOCOMPILE you should be generating a .lo file, not a .o file. In this case, since you aren't creating a shared library, it should be fine to generate a .o file and use $(GOCOMPILE).

[laboger, 2015-04-07 13:12:32]

As far as documentation, I tried to find some documentation on build tags in general and netgo specifically because it seems like this should be documented there, but did not find much. Here are some ideas: - add something to the 'go help build' output about the use of the netgo tag in general and how it would be used to work around the static linking warning/problem against libnss and the workarounds for gc and gccgo - if there is something somewhere else that describes the netgo tag and when to use it, then add something about how to achieve the same effect in gccgo - seems like it would be good to have documentation describing the differences in using gccgo vs. gc and this would be included in that, however that is a bigger work item and would require more thought on what to include in such a document. I'll work on the other changes. On 04/03/2015 01:20 PM, iant@golang.org wrote: > Please fix the first line of the description to be on a single line. > > Any thoughts on where this should be documented? > > > https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am > File libgo/Makefile.am (left): > > https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#oldcode1 > > libgo/Makefile.am:1: # Makefile.am -- Go library Makefile. > Why remove this line? > > https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am > File libgo/Makefile.am (right): > > https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode823 > > libgo/Makefile.am:823: go_netgo_files = \ > Rather than duplicating the list entirely, let's make another variable > that holds all the common files, and use that variable in both > go_net_files and go_netgo_files. > > https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode... > > libgo/Makefile.am:2104: $(LTGOCOMPILE) -I . -c -fgo-pkgpath=net -o $@ > $$files > If you are using LTGOCOMPILE you should be generating a .lo file, not a > .o file. In this case, since you aren't creating a shared library, it > should be fine to generate a .o file and use $(GOCOMPILE). > > https://codereview.appspot.com/217620043/ > > > 

[laboger, 2015-04-07 15:53:47]

Please review changes. https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am File libgo/Makefile.am (left): https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#oldcode1 libgo/Makefile.am:1: # Makefile.am -- Go library Makefile. On 2015/04/03 18:20:10, iant wrote: > Why remove this line? Sorry shouldn't have been. https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am File libgo/Makefile.am (right): https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode823 libgo/Makefile.am:823: go_netgo_files = \ On 2015/04/03 18:20:10, iant wrote: > Rather than duplicating the list entirely, let's make another variable that > holds all the common files, and use that variable in both go_net_files and > go_netgo_files. Good idea. Done. https://codereview.appspot.com/217620043/diff/40001/libgo/Makefile.am#newcode... libgo/Makefile.am:2104: $(LTGOCOMPILE) -I . -c -fgo-pkgpath=net -o $@ $$files On 2015/04/03 18:20:10, iant wrote: > If you are using LTGOCOMPILE you should be generating a .lo file, not a .o file. > In this case, since you aren't creating a shared library, it should be fine to > generate a .o file and use $(GOCOMPILE). Tried this multiple ways and forgot to change this. Fixed.

[iant, 2015-04-07 18:05:38]

LGTM

[iant, 2015-04-07 18:08:44]

*** Submitted as https://code.google.com/p/gofrontend/source/detail?r=46a450318709 *** libgo: Build and install libnetgo.a libnetgo.a provides the net package built with the netgo tag enabled. This provides the netgo fallback solution for gccgo. This lib must be explicitly linked in using the -gccgoflags, so is not included by default. Fixes gcc BZ 63731. LGTM=iant R=gofrontend-dev, iant CC=gofrontend-dev https://codereview.appspot.com/217620043 Committer: Ian Lance Taylor <iant@golang.org>