Add opt-in go-fuzz modules support

[yevgenypats]

Hey Team,

I know a complete solution for go-fuzz modules support with comprehensive tests is under the works by @thepudds . In the meantime, a lot of people are blocked by this feature and I suggest adding this opt-in environment (GOFUZZ111MODULES=on) variable that I documented that it will go away.

cc @mvdan @josharian @dvyukov

can you please review and approve if this is ok?

Cheers,
Yevgeny

[josharian]

This is fine, I guess, although I'd prefer something like #272.

I'll leave the decision and merge to Dmitry. I'm ok with either of them going in.

[yevgenypats]

I'm totally fine with #272 as well.

[thepudds]

#274 is now ready as a viable alternative. (It had been waiting on google/oss-fuzz#2878, which is now addressed as of today).

The most relevant code changes in #274 are also fairly small. The primary distinguishing characteristic for #274 compared to the other PRs is that #274 has tests that exercise go-fuzz with modules enabled, disabled, in auto mode, inside GOPATH, outside GOPATH, with v2+ modules, with replace directives, with a vendor directory, ...

Regarding this change here in #271, I suspect as implemented here, it would probably modify the user's go.mod file in the common case when running go-build-fuzz. If the decision is to pursue #271, then at a minimum that should likely be called out in the readme.

[thepudds]

One other nit is that if this PR is pursued, similar logic should be implemented in go-fuzz/main.go as well roughly around here.

Otherwise, I think you can get errors as currently implemented in this PR for some modules based on whether or not the user specifies pkg/func arguments.

[mvdan]

The PR that @thepudds sent includes lots of tests, so I'd feel a lot safer approving and merging that one instead of this one. Essentially, turning on the knob here isn't guaranteed to work well :)

[yevgenypats]

hey @mvdan :) I agree that that @thepudds is a better solution as it's also not opt-in but by default with "auto-detect".

In this PR I didn't just turn-on the knob here I did it as an opt-in so only people who are early adopters and waiting for a long time to try go-fuzz will enable this and they are ok with this feature being deleted later on + having some problems and issues.

https://twitter.com/fuzzitdev/status/1169137825142452225
https://www.reddit.com/r/golang/comments/dc98vv/how_to_fuzz_go_code_with_gofuzz_continuously/
https://twitter.com/search?q=go-fuzz%20modules&src=typed_query

moreover I believe we will find even more small bugs even after @thepudds #274 will be merged or even during review, so there is no bug free solution. I believe if we will merge this it will help many people use go-fuzz as soon as possible + will help us find more bugs and issues faster with go modules support for #274 .

Please consider merging this one:)

cc @dvyukov - @josharian said it's up to you:)

Cheers,
Yevgeny

[dvyukov]

cc @dvyukov - @josharian said it's up to you:)

my verdict: it's up to @josharian :)

[yevgenypats]

@josharian the ball is back in your hands - please:)

[yevgenypats]

@josharian friendly ping:)

[josharian]

OK, I think something like this should go in, as a safety valve. (I intend to review and merge @thepudds's work very soon. But this will provide a useful override mechanism if/when it turns out to be needed.) But I think it should work slightly differently:

I think GOFUZZ111MODULE, if set, should override GO111MODULE. So if you set GOFUZZ111MODULE=auto, then we set GO111MODULE=auto. Use os.LookupEnv to determine whether it is set, and if so, use its value, whatever it happens to be, to set GO111MODULE.

And remove all changes to the README. Let's leave this undocumented, at least for the moment.