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

travis: also test go-fuzz with Go tip (including because oss-fuzz is using tip) #239

Merged
merged 6 commits into from
May 6, 2019
Merged

Conversation

thepudds
Copy link
Collaborator

@thepudds thepudds commented May 3, 2019

In travis, add tests for Go tip on linux and osx.

In tip and Go 1.13, GO111MODULE defaults to on (whereas it defaults to auto in 1.11 and 1.12).

However, travis explicitly sets GO111MODULE=auto. Therefore, when testing tip, we explicitly set GO111MODULE=on to give normal tip / 1.13 behavior.

Note: it does not work just to use env: GO111MODULE=on because then go get stops working (perhaps because go-fuzz is not yet a module). Instead, we use SET_GO111MODULE in the travis configuration, and then check that to know whether or not to do export GO111MODULE=on just before invoking go-fuzz-build and go-fuzz.

Separately, pull request #237 was merged, but that current fix is probably incomplete for disabling modules. With these changes, travis passes with 1.11 and 1.12, but currently fails on tip with:

$ go-fuzz-build
could not resolve package ".": go [list -e -json -compiled=false -test=false -export=false -deps=false -find=true -tags gofuzz -- .]: exit status 1: go: cannot find main module, but found .git/config in /home/travis/gopath/src/github.com/dvyukov/go-fuzz
	to create a module there, run:
	cd .. && go mod init

Updates #237
Updates google/oss-fuzz#2188

thepudds added 5 commits May 3, 2019 07:49
Travis exports GO111MODULE=auto. Go tip (1.13) defaults to GO111MODULE=on. 

To match the tip default, explicitly set GO111MODULE=on for tip in .travis.yml
@josharian
Copy link
Collaborator

Thanks, @thepudds. This LGTM, but I'll let @dvyukov take a look before merging.

Out of curiosity, does this pass with #240 included?

(Also, out of curiosity, why don't you dive into the code yourself? You have useful and insightful comments.)

@thepudds
Copy link
Collaborator Author

thepudds commented May 3, 2019

Out of curiosity, does this pass with #240 included?

Hi @josharian, I need to run, but I can try that later or maybe tomorrow. (I was going to say I thought go-fuzz would need to be updated too... but you were too fast and now looks like you updated go-fuzz too ;-)

@thepudds
Copy link
Collaborator Author

thepudds commented May 3, 2019

And sorry, in my fork I was juggling branches trying to see if os.Setenv would work to disable modules (and trying to test it with travis), but I see now that I messed up this PR by later accidentally adding one of the os.Setenv changes. My mistake.

I can try to correct that tomorrow as well.

@thepudds
Copy link
Collaborator Author

thepudds commented May 4, 2019

OK, this PR is back to just the one expected changed file and ready for any additional review. Sorry about that.

And good news: as expected or hoped, with #240 now merged to master, this PR now passes travis (whereas prior to #240 being merged, travis would fail this PR with Go tip due to modules being enabled by default on tip / 1.13).

@dvyukov dvyukov merged commit 8416b0d into dvyukov:master May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants