Bug 1022983 - Update to Go 1.2
Update to Go 1.2
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: golang (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Adam Goode
Fedora Extras Quality Assurance
http://tip.golang.org/doc/go1.2
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-24 07:59 EDT by Matthew Miller
Modified: 2014-01-14 15:56 EST (History)
7 users (show)

See Also:
Fixed In Version: golang-1.2-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-01-14 03:44:31 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Matthew Miller 2013-10-24 07:59:31 EDT
Go 1.2 is in code freeze, with a release date scheduled for December 1.

Go 1.2 release candidate 2 was released last week: https://groups.google.com/forum/#!topic/Golang-Nuts/5enxY7gSLB4

I think we should update golang in Rawhide (but not F20 or F19) to the release candidate, so we can start testing builds against it and making any necessary adjustments.

For docker's in-container init process, we need the go-net libs but we also want the binary to be static; this isn't possible in 1.1 but will be in 1.2.
Comment 1 Peter Lemenkov 2013-10-24 10:12:15 EDT
Love the idea! Actually I'd rather update golang in F20 as well not just in F21/Rawhide.
Comment 2 Matthew Miller 2013-10-24 13:25:59 EDT
(In reply to Peter Lemenkov from comment #1)
> Love the idea! Actually I'd rather update golang in F20 as well not just in
> F21/Rawhide.

I think that's a little risky with the F20 schedule, where our release is scheduled for just 2 days after theirs (and our bits will have to all be in place the week before, for mirroring). So, we'd be shipping a RC with F20. We've done such things before, but in my opinion it'd best to do a post-release update.

But if the RC does seem to be stable and works with the packages we're shipping (currently to my knowledge, just etcd and docker real-soon-now), maybe it's worth the risk.
Comment 3 Matthew Miller 2013-10-24 14:27:52 EDT
Although I see that as I was writing this, the beta slipped by a week, pushing everything back. If it slips by _another_ week and Go stays on schedule (and from their docs it looks like they have a hard commitment to doing so), there may be no issue at all. But I suppose we shouldn't plan on that. :)

We could do the update in Rawhide now and then decide on what to do for F20 after the beta has shipped. Normally I'd frown on that (because we are totally going around the whole release management process) but this is pretty clearly on the fringe/bleeding edge and we know it doesn't affect anything else.
Comment 4 Adam Miller 2013-10-24 15:33:36 EDT
Go 1.2 builds on F20 but appears to fail one of the tests ... I'm going to look into it more as I find time but didn't want to remain silent on the BZ.
Comment 5 Matthew Miller 2013-10-24 19:36:45 EDT
(In reply to Adam Miller from comment #4)
> Go 1.2 builds on F20 but appears to fail one of the tests ... I'm going to
> look into it more as I find time but didn't want to remain silent on the BZ.

In the specfile (in both %build and %check) we have 

export PATH="$PATH":"$GOROOT"/bin

which means that the system path is first, and so if you have another go package installed that gets called first in the tests. Is that what you're running into? Try making it

export PATH=$GOROOT/bin:$PATH

(Quotes shouldn't be necessary, but whatever.)

On my F19 and F20 systems, with that switched, the tests pass. (And they don't pass if I have the older golang installed and the path the other way.)
Comment 6 Adam Miller 2013-10-24 19:40:52 EDT
I was building in mock which shouldn't have had any other version of Go installed in the chroot.
Comment 7 Adam Miller 2013-10-24 19:47:15 EDT
For reference, this is the issue I'm seeing building in mock:

--- FAIL: TestCgoTraceback (0.83 seconds)
        crash_cgo_test.go:33: expected "OK\n", but got "# command-line-arguments\n/tmp/go-build563671405/main.go: In 
function '_cgo_65eb2f8ba551_Cfunc_foo':\n/tmp/go-build563671405/main.go:31:49: warning: unused variable 'a' [-Wunused
-variable]\nOK\n"
FAIL
FAIL    runtime 15.451s
Comment 8 Matthew Miller 2013-10-24 19:49:40 EDT
(In reply to Adam Miller from comment #6)
> I was building in mock which shouldn't have had any other version of Go
> installed in the chroot.

Aw, too bad, because that's an easy fix. (It should probably be changed in the specfile anyway.)  I'll dig deeper.
Comment 9 Adam Miller 2013-10-24 20:00:52 EDT
Also, because I'm bad at making a complete thought before commenting on this particular BZ here is the SRPM I have that fails that test in mock for both F20 and Rawhide

http://maxamillion.fedorapeople.org/golang-1.2rc2-1.fc19.src.rpm
Comment 10 Matthew Miller 2013-10-24 20:04:31 EDT
(In reply to Adam Miller from comment #7)
> For reference, this is the issue I'm seeing building in mock:
> 
> --- FAIL: TestCgoTraceback (0.83 seconds)
>         crash_cgo_test.go:33: expected "OK\n", but got "#
> command-line-arguments\n/tmp/go-build563671405/main.go: In 
> function
> '_cgo_65eb2f8ba551_Cfunc_foo':\n/tmp/go-build563671405/main.go:31:49:
> warning: unused variable 'a' [-Wunused
> -variable]\nOK\n"
> FAIL
> FAIL    runtime 15.451s

Okay, yeah, I can reproduce this when I'm trying to update the RPM rather than just building it by hand. When I comment out the "mygcc" part, though, it works fine. So, it's something to do with that.
Comment 11 Matthew Miller 2013-11-08 16:56:58 EST
RC3 is out now, by the way https://groups.google.com/forum/#!msg/golang-nuts/7FXy8x7Oqyg/_mfUwUdnyXIJ
Comment 12 Matthew Miller 2013-11-08 17:10:41 EST
Same error happens in rc3, btw.
Comment 13 Matthew Miller 2013-11-08 17:42:54 EST
This is especially fun to debug because go cleans up the temporary directory it was using to build right after it exits, making it really hard to find where the error is. Neat.
Comment 14 Matthew Miller 2013-11-08 19:19:19 EST
Okay, so, here is the test code that is failing:

main.go:
-----------------
package main

/* void foo(void) {} */
import "C"

import (
        "fmt"
        "runtime"
)

func main() {
        C.foo()
        buf := make([]byte, 1)
        runtime.Stack(buf, true)
        fmt.Printf("OK\n")
}
--------------------
$ go run main.go
# command-line-arguments
./main.go: In function ‘_cgo_92e4c70c8af1_Cfunc_foo’:
./main.go:30:49: warning: unused variable ‘a’ [-Wunused-variable]
OK

but only if CC points to the version which wraps the $RPM_OPT_FLAGS
Comment 15 Matthew Miller 2013-11-08 19:38:15 EST
And in the generated code, here is our unused "a":

void
_cgo_92e4c70c8af1_Cfunc_foo(void *v)
{
        struct {
                char unused;
        } __attribute__((__packed__, __gcc_struct__)) *a = v;
        foo();
}

Now, without going deeply into what that's all about, I've got no idea if this is a bug or intentional in this test.
Comment 16 Matthew Miller 2013-11-08 19:59:40 EST
Soooooo, this means that:

1. Unsetting CC at the top of the %check file should work
2. This is probably the right thing to do, because we're not using a wrapper at runtime

EXCEPT

unsetting CC didn't work. I had to set CC to /usr/bin/gcc to make it work. That's confusing (and why I jumped through the hoops above). Why's that? Uh oh: 

$ grep mygcc . -r
Binary file ./bin/go matches
./src/cmd/go/zdefaultcc.go:const defaultCC = `/home/mattdm/rpmbuild/BUILD/go/zz/mygcc`
./src/cmd/cgo/zdefaultcc.go:const defaultCC = `/home/mattdm/rpmbuild/BUILD/go/zz/mygcc`
Binary file ./pkg/tool/linux_amd64/cgo matches


We've got the path to the gcc wrapper in the build tree built into the whole thing -- looks like golang 1.2 wants to keep using the wrapper it was built with. That seems logical, but raises some... interesting questions.

a) Should all go programs built by the go compiler use our RPM flags? (That is, should we _ship_ the mygcc wrapper?)
b) If so, might that introduce horrible bugs?
c) If not, should we seek an exception for building golang itself?
Comment 17 Vincent Batts 2013-11-10 08:00:33 EST
Another note, the upgrade to go1.2 is going to require a couple of additional things. Two subcommands currently in core 'golang' package and source repo are being moved their go.tools repo (`go doc` and `go vet`), namely so they can have a separate release from the compiler.
So making a spec for code.google.com/p/go.tools, that provide sub packages that will include these commands.
  code.google.com/p/go.tools/cmd/godoc
  code.google.com/p/go.tools/cmd/vet
Comment 19 Vincent Batts 2013-11-27 12:00:30 EST
update: i have separated out the godoc from the golang into subpackage golang-godoc. And 'golang' will require /usr/bin/godoc. Seamless for current fedora/epel releases.

We have the review in progess for the go.tools https://bugzilla.redhat.com/show_bug.cgi?id=1029068 which will provide it's own -godoc. 
This way an upgrade of golang to go1.2, will no long produce a golang-godoc, and could install either existing golang-godoc or that from go.tools. We could even have an explicit require on the go.tools subpackage.
Comment 20 Matthew Miller 2013-12-02 09:25:53 EST
Any comment on the "mygcc" issue?
Comment 21 Matthew Miller 2013-12-02 13:01:38 EST
Official release is here: http://blog.golang.org/go12
Comment 22 Vincent Batts 2013-12-02 14:30:48 EST
release! cool.

So the failure on unused variables is tied up in $RPM_OPT_FLAGS $RPM_LD_FLAGS.
I've tried a variety of ways to work around this, but I feel that seeking an exception for this package will be best.
Comment 23 Vincent Batts 2013-12-02 16:46:23 EST
raised the discussion here https://groups.google.com/forum/#!topic/golang-nuts/SJn48BkCUqA

Will open issue against golang, based on response from Ian Taylor.
Comment 24 Fedora Update System 2013-12-04 14:31:17 EST
golang-1.2-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/golang-1.2-1.el6
Comment 25 Fedora Update System 2013-12-04 14:33:32 EST
golang-1.2-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/golang-1.2-1.fc19
Comment 26 Fedora Update System 2013-12-04 14:39:00 EST
golang-1.2-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/golang-1.2-1.fc20
Comment 27 Fedora Update System 2013-12-04 19:42:03 EST
Package golang-1.2-1.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing golang-1.2-1.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-22742/golang-1.2-1.fc20
then log in and leave karma (feedback).
Comment 28 Fedora Update System 2014-01-14 03:44:31 EST
golang-1.2-1.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 29 Fedora Update System 2014-01-14 03:45:11 EST
golang-1.2-1.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 30 Fedora Update System 2014-01-14 15:56:32 EST
golang-1.2-1.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.