Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: run more buildmode=shared tests #16602

Open
crawshaw opened this issue Aug 4, 2016 · 26 comments

Comments

@crawshaw
Copy link
Contributor

commented Aug 4, 2016

#16590 was found very late in the 1.7 cycle and would have been caught much earlier if standard library tests for buildmode=shared were being run somewhere.

@crawshaw crawshaw added this to the Go1.8 milestone Aug 4, 2016

@bradfitz bradfitz added the Builders label Aug 4, 2016

@bradfitz bradfitz self-assigned this Aug 4, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Aug 4, 2016

What's the the environment variable I can set? GO_GCFLAGS=...., GO_LDFLAGS=.... ?

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2016

I don't think there's an environment variable that will do the job (yet). The standard library needs to be installed with -buildmode=shared, and then the binary built with -linkshared. Those are go binary flags, not compiler/linker flags.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

Looks like an environment variable already exists: $GO_FLAGS:

make.bash does:

CC=$CC_FOR_TARGET "$GOTOOLDIR"/go_bootstrap install $GO_FLAGS -gcflags "$GO_GCFLAGS" -ldflags "$GO_LDFLAGS" -v std cmd

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2016

You can't install the standard library linked against itself though.

On 5 August 2016 at 13:10, Brad Fitzpatrick notifications@github.com
wrote:

Looks like an environment variable already exists: $GO_FLAGS:

make.bash does:

CC=$CC_FOR_TARGET "$GOTOOLDIR"/go_bootstrap install $GO_FLAGS -gcflags
"$GO_GCFLAGS" -ldflags "$GO_LDFLAGS" -v std cmd


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#16602 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AApBFqjc5DLyAZpGwGyF45ypqltHaME7ks5qco11gaJpZM4JcnyH
.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

I don't follow. Tell me what to do and I'll set up builder.

@bradfitz bradfitz removed their assignment Aug 5, 2016

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2016

Doing "cd src && ./make.bash && go install -buildmode=shared std && cd ../test && go run run.go -linkshared" would be one useful thing. I'm not sure if that's what @crawshaw meant, sorry for hijacking if not.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2016

@crawshaw, what command should the buildmode=shared builder run?

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2016

Ideally the stdlib tests, against a dynamically linked stdlib:

./make.bash
go install -buildmode=shared std
go test -short -linkshared std

As a bonus, what @mwhudson suggested:

cd ../test && go run run.go -linkshared

This is mostly just adding the -linkshared flag to the usual tests, but cmd/dist has to call go install somewhere first.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 22, 2016

@quentinmit, any interest in setting up a new builder for this?

@bradfitz bradfitz removed the WaitingForInfo label Dec 2, 2016

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2016

@bradfitz I probably won't have cycles to do this before 1.8 ships.

@quentinmit quentinmit added the NeedsFix label Dec 6, 2016

@quentinmit quentinmit modified the milestones: Go1.8Maybe, Go1.8 Jan 9, 2017

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2017

I'm guessing this won't happen until 1.9.

@crawshaw crawshaw modified the milestones: Go1.9, Go1.8Maybe Jan 30, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jan 30, 2017

I'd still like to do it, especially now that we have two more weeks.

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2017

Ah, the release schedule shifted? Where should I look for it?

@crawshaw crawshaw modified the milestones: Go1.8Maybe, Go1.9 Jan 30, 2017

@dmitshur

This comment has been minimized.

Copy link
Member

commented Jan 31, 2017

@rsc

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2017

Ping @crawshaw

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2017

I don't really have time to fit this in before any deadline. @bradfitz were you going to look at it? If not, I guess we bump it to 1.9.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2017

It's not much work. I went to try the instructions above just locally, but they don't work.

On the Go 1.8 release branch, the make.bash and go test -short -linkshared std work fine, but then:

$ go test -short -linkshared std
# bufio_test
bufio/scan_test.go:23: undefined: IsSpace
bufio/scan_test.go:194: s.MaxTokenSize undefined (type *bufio.Scanner has no field or method MaxTokenSize, but does have bufio.maxTokenSize)
bufio/scan_test.go:230: s.MaxTokenSize undefined (type *bufio.Scanner has no field or method MaxTokenSize, but does have bufio.maxTokenSize)
bufio/scan_test.go:354: s.ErrOrEOF undefined (type *bufio.Scanner has no field or method ErrOrEOF)
bufio/scan_test.go:409: scanner.MaxTokenSize undefined (type *bufio.Scanner has no field or method MaxTokenSize, but does have bufio.maxTokenSize)
# testmain
/tmp/go-build353620918/archive/tar/_test/_testmain.go:24: undefined: tar.TestReader
/tmp/go-build353620918/archive/tar/_test/_testmain.go:26: undefined: tar.TestPartialRead
/tmp/go-build353620918/archive/tar/_test/_testmain.go:28: undefined: tar.TestSparseFileReader
/tmp/go-build353620918/archive/tar/_test/_testmain.go:30: undefined: tar.TestReadOldGNUSparseMap
/tmp/go-build353620918/archive/tar/_test/_testmain.go:32: undefined: tar.TestReadGNUSparseMap0x1
/tmp/go-build353620918/archive/tar/_test/_testmain.go:34: undefined: tar.TestReadGNUSparseMap1x0
/tmp/go-build353620918/archive/tar/_test/_testmain.go:36: undefined: tar.TestUninitializedRead
/tmp/go-build353620918/archive/tar/_test/_testmain.go:38: undefined: tar.TestReadTruncation
/tmp/go-build353620918/archive/tar/_test/_testmain.go:40: undefined: tar.TestReadHeaderOnly
/tmp/go-build353620918/archive/tar/_test/_testmain.go:42: undefined: tar.TestMergePAX
/tmp/go-build353620918/archive/tar/_test/_testmain.go:42: too many errors
FAIL    archive/tar [build failed]
# testmain
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:22: undefined: bzip2.TestReader
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:24: undefined: bzip2.TestBitReader
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:26: undefined: bzip2.TestMTF
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:32: undefined: bzip2.BenchmarkDecodeDigits
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:34: undefined: bzip2.BenchmarkDecodeTwain
/tmp/go-build353620918/compress/bzip2/_test/_testmain.go:36: undefined: bzip2.BenchmarkDecodeRand
# bytes_test
bytes/bytes_test.go:59: undefined: EqualPortable
bytes/bytes_test.go:237: undefined: IndexBytePortable
bytes/bytes_test.go:435: undefined: IndexBytePortable
bytes/bytes_test.go:509: undefined: EqualPortable
# testmain
/tmp/go-build353620918/archive/zip/_test/_testmain.go:24: undefined: zip.TestReader
/tmp/go-build353620918/archive/zip/_test/_testmain.go:26: undefined: zip.TestInvalidFiles
/tmp/go-build353620918/archive/zip/_test/_testmain.go:28: undefined: zip.TestIssue8186
/tmp/go-build353620918/archive/zip/_test/_testmain.go:30: undefined: zip.TestIssue10957
/tmp/go-build353620918/archive/zip/_test/_testmain.go:32: undefined: zip.TestIssue10956
/tmp/go-build353620918/archive/zip/_test/_testmain.go:34: undefined: zip.TestIssue11146
/tmp/go-build353620918/archive/zip/_test/_testmain.go:36: undefined: zip.TestIssue12449
/tmp/go-build353620918/archive/zip/_test/_testmain.go:38: undefined: zip.TestWriter
/tmp/go-build353620918/archive/zip/_test/_testmain.go:40: undefined: zip.TestWriterOffset
/tmp/go-build353620918/archive/zip/_test/_testmain.go:42: undefined: zip.TestWriterFlush
/tmp/go-build353620918/archive/zip/_test/_testmain.go:42: too many errors
FAIL    archive/zip [build failed]
FAIL    bufio [build failed]
FAIL    bytes [build failed]
FAIL    compress/bzip2 [build failed]
# testmain
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:22: undefined: lzw.TestReader
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:24: undefined: lzw.TestWriter
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:26: undefined: lzw.TestWriterReturnValues
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:28: undefined: lzw.TestSmallLitWidth
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:34: undefined: lzw.BenchmarkDecoder
/tmp/go-build353620918/compress/lzw/_test/_testmain.go:36: undefined: lzw.BenchmarkEncoder
# testmain
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:24: undefined: zlib.TestDecompressor
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:26: undefined: zlib.TestWriter
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:28: undefined: zlib.TestWriterBig
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:30: undefined: zlib.TestWriterDict
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:32: undefined: zlib.TestWriterReset
/tmp/go-build353620918/compress/zlib/_test/_testmain.go:34: undefined: zlib.TestWriterDictIsUsed
# testmain
/tmp/go-build353620918/compress/flate/_test/_testmain.go:24: undefined: flate.TestBulkHash4
/tmp/go-build353620918/compress/flate/_test/_testmain.go:26: undefined: flate.TestDeflate
/tmp/go-build353620918/compress/flate/_test/_testmain.go:28: undefined: flate.TestVeryLongSparseChunk
/tmp/go-build353620918/compress/flate/_test/_testmain.go:30: undefined: flate.TestDeflateInflate
/tmp/go-build353620918/compress/flate/_test/_testmain.go:32: undefined: flate.TestReverseBits
/tmp/go-build353620918/compress/flate/_test/_testmain.go:34: undefined: flate.TestDeflateInflateString
/tmp/go-build353620918/compress/flate/_test/_testmain.go:36: undefined: flate.TestReaderDict
/tmp/go-build353620918/compress/flate/_test/_testmain.go:38: undefined: flate.TestWriterDict
/tmp/go-build353620918/compress/flate/_test/_testmain.go:40: undefined: flate.TestRegression2508
/tmp/go-build353620918/compress/flate/_test/_testmain.go:42: undefined: flate.TestWriterReset
/tmp/go-build353620918/compress/flate/_test/_testmain.go:42: too many errors
...
@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2017

@crawshaw, any tips?

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2017

Sorry, my instructions were bad. (If you install a .so with a package in it, and then try to run the tests of that package, you don't have the extra symbols added by the _test.go files.)

If you install the minimal .so of runtime and sync/atomic, then you can run the tests of every package other than runtime and sync/atomic. E.g.

go install -buildmode=shared runtime sync/atomic
go test -short -linkshared archive/tar

Unfortunately that means the std shorthand won't work, because the runtime package tests won't pass this way.

Interestingly the tests for package reflect fail. That may actually be a bug.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2017

I don't fully understand.

What's the pseudo code English for what I should run?

Foreach package $PKG in "std", install something (?), then go test -short -linkshared $PKG?

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 8, 2017

Um, it just hangs for me:

bradfitz@gdev:~/go/src$ go install -buildmode=shared runtime sync/atomic
bradfitz@gdev:~/go/src$ go test -short -linkshared -v -x archive/tar 
WORK=/tmp/go-build944900597
mkdir -p $WORK/archive/tar/_test/archive/
...
...
... (hangs)
....
.....

Process tree shows nothing interesting:

23468 pts/13   Ss+    0:00  \_ /bin/bash
 9339 pts/4    Ss     0:06  \_ /bin/bash
22609 pts/4    Sl+    0:00  |   \_ /home/bradfitz/go/bin/go test -short -linkshared -v -x archive/tar
12028 pts/16   Ss+    0:00  \_ /bin/bash
 2763 pts/17   Ss+    0:00  \_ /bin/bash


$ sudo strace -f -p 22609
Process 22609 attached with 6 threads
[pid 22612] futex(0xc42002ed10, FUTEX_WAIT, 0, NULL <unfinished ...>
[pid 22614] futex(0xc42011a110, FUTEX_WAIT, 0, NULL <unfinished ...>
[pid 22613] futex(0xab1a00, FUTEX_WAIT, 0, NULL <unfinished ...>
[pid 22611] futex(0xc42002e910, FUTEX_WAIT, 0, NULL <unfinished ...>
[pid 22609] futex(0xa95d10, FUTEX_WAIT, 0, NULL <unfinished ...>
[pid 22610] restart_syscall(<... resuming interrupted call ...>

@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

$ git sync
$ rm -rf ../pkg
$ ./make.bash
...
$ go install -buildmode=shared runtime sync/atomic
$ go test -short -linkshared  archive/tar
ok  	archive/tar	0.043s
$

I also tried on release-branch.go1.8 and the same sequence works. So that it fails for you is pretty interesting. Do you have something funky in your pkg directory? If not, what kind of linux are you using?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 9, 2017

Do you have something funky in your pkg directory?

I don't look at my pkg directory. I'm a little disgusted that rm -rf ../pkg is a necessary step, ever. I dream of a proper build cache. (#4719)

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 9, 2017

Okay, it works now after nuking pkg.

Is this the reflect error you saw?

$ go test -short -linkshared  reflect
runtime: typeOff 0x289c0 base 0xc4202bbbc0 not in ranges:
        types 0x7fb9f423f3a0 etypes 0x7fb9f4289190
        types 0xaaa020 etypes 0xd83b20
fatal error: runtime: type offset base pointer out of range
        
goroutine 75 [running]:
runtime.throw(0x7fb9f403960b, 0x2e)
        /home/bradfitz/go/src/runtime/panic.go:596 +0x97 fp=0xc420071d18 sp=0xc420071cf8
runtime.resolveTypeOff(0xc4202bbbc0, 0x289c0, 0xc4200f6660)
        /home/bradfitz/go/src/runtime/type.go:223 +0x356 fp=0xc420071d78 sp=0xc420071d18
runtime.(*_type).typeOff(0xc4202bbbc0, 0x289c0, 0xc420071de0)
        /home/bradfitz/go/src/runtime/type.go:239 +0x35 fp=0xc420071da0 sp=0xc420071d78
reflect.resolveTypeOff(0xc4202bbbc0, 0x289c0, 0x0)
        /home/bradfitz/go/src/runtime/runtime1.go:515 +0x35 fp=0xc420071dc8 sp=0xc420071da0
reflect.(*rtype).typeOff(0xc4202bbbc0, 0xc4000289c0, 0xc420071e88)
        /home/bradfitz/go/src/reflect/type.go:679 +0x35 fp=0xc420071df0 sp=0xc420071dc8
reflect.(*rtype).ptrTo(0xc4202bbbc0, 0xc4202bbbc0)
        /home/bradfitz/go/src/reflect/type.go:1430 +0x57b fp=0xc420071ec0 sp=0xc420071df0
reflect.PtrTo(0xe21fc0, 0xc4202bbbc0, 0xe21fc0, 0xc4202bbbc0)
        /home/bradfitz/go/src/reflect/type.go:1425 +0x47 fp=0xc420071ee8 sp=0xc420071ec0
reflect_test.TestPtrTo(0xc4200fbc70)
        /home/bradfitz/go/src/reflect/all_test.go:2485 +0xa1 fp=0xc420071fa8 sp=0xc420071ee8
testing.tRunner(0xc4200fbc70, 0xd80278)
        /home/bradfitz/go/src/testing/testing.go:657 +0xa0 fp=0xc420071fd0 sp=0xc420071fa8
runtime.goexit()
        /home/bradfitz/go/src/runtime/asm_amd64.s:2197 +0x1 fp=0xc420071fd8 sp=0xc420071fd0
created by testing.(*T).Run
        /home/bradfitz/go/src/testing/testing.go:697 +0x2e4
@crawshaw

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

Yes that's the reflect error. It might be real. We could give it its own issue.

The pkg directory gets an extra workout with -buildmode=shared, .so files end up there. I agree it should just work, it's just that these newer build modes don't get enough eyes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.