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

preliminary Go modules support #2

Merged
merged 34 commits into from
Oct 3, 2019
Merged

Conversation

thepudds
Copy link
Owner

@thepudds thepudds commented Oct 3, 2019

Summary

The heart of the change is very small -- we stop overwriting the user's value for the GO111MODULE env var in two places. This means we end up respecting whatever the user has set for GO111MODULE, and relying on cmd/go to interpret the meaning of an unset GO111MODULE (which defaults to auto for Go 1.11-1.13, and likely will default to on at some point during Go 1.14 development).

In addition, there are some comments added, as well as an explicit check for GOFLAGS=-mod=vendor, which is not supported.

The tests exercises 'replace' directives, v2+ modules, GO111MODULE set to on|off|auto, running inside GOPATH, running outside GOPATH, the mechanisms by which go-fuzz/go-fuzz-dep is found, as well as vendoring (which is not supported, but it tests that a sensible error is emitted).

Additional Details

Historically, go-fuzz has set up a clean GOPATH environment in a temp directory, instrumented the code for fuzzing coverage, and built the instrumented code in the temp directory.

The current approach is to keep doing that (and not set up a full-blown modules-based environment for building), but use modules to find the code to instrument.

v2+ modules are potentially problematic due to Semantic Import Versioning (with the major version in the import path). However, it turns out to not be too bad to handle it. For a v2+ module created following the 'major branch' approach, the /vN is materialized into a physical directory. For example, the github.com/russross/blackfriday/v2 module ends up in the /tmp/blah/gopath/src/github.com/russross/blackfriday/v2 directory. This should be a valid layout, and people use this layout today (including when manually switching to a 'major subdirectory' approach for a v2+ module, which involves creating a /v2 or /vN directory so that pre-modules toolchains follow a /v2 or /vN import path and find an on-disk layout that they expect without any knowledge of modules).

The approach does not copy go.mod files at all. (Before starting, I was thinking I would need to copy go.mod files to trigger "minimal module awareness", which is a transitional mechanism within cmd/go, but after some experimentation / thinking, I concluded it was not needed based on materializing the /v2).

For tests, I am using the script-driven test facility that the core Go team created for testing cmd/go, which is well tuned to testing the various permutations of modules, including it lets you relatively easily set up go.mod files and source files, run commands and check results. (Some more details of the capability here: https://github.com/golang/go/blob/master/src/cmd/go/testdata/script/README).

@thepudds thepudds merged commit 4ab89a8 into dev-modules-2 Oct 3, 2019
thepudds added a commit that referenced this pull request Dec 1, 2021
* preliminary Go modules support (#2)

#### Summary 

The heart of the change is very small -- we stop overwriting the user's value for the `GO111MODULE` env var in two places. This means we end up respecting whatever the user has set for `GO111MODULE`, and relying on `cmd/go` to interpret the meaning of an unset `GO111MODULE` (which defaults to `auto` for Go 1.11-1.13, and likely will default to `on` at some point during Go 1.14 development).

In addition, there are some comments added, as well as an explicit check for `GOFLAGS=-mod=vendor`, which is not supported.

The tests exercises 'replace' directives, v2+ modules, GO111MODULE set to on|off|auto, running inside GOPATH, running outside GOPATH, the mechanisms by which go-fuzz/go-fuzz-dep is found, as well as vendoring (which is not supported, but it tests that a sensible error is emitted). 

#### Additional Details

Historically, go-fuzz has set up a clean GOPATH environment in a temp directory, instrumented the code for fuzzing coverage, and built the instrumented code in the temp directory.

The current approach is to keep doing that (and not set up a full-blown modules-based environment for building), but use modules to find the code to instrument.

v2+ modules are potentially problematic due to Semantic Import Versioning (with the major version in the import path). However, it turns out to not be too bad to handle it. For a v2+ module created following the 'major branch' approach, the /vN is materialized into a physical directory. For example, the github.com/russross/blackfriday/v2 module ends up in the /tmp/blah/gopath/src/github.com/russross/blackfriday/v2 directory. This should be a valid layout, and people use this layout today (including when manually switching to a 'major subdirectory' approach for a v2+ module, which involves creating a /v2 or /vN directory so that pre-modules toolchains follow a /v2 or /vN import path and find an on-disk layout that they expect without any knowledge of modules).

The approach does not copy go.mod files at all. (Before starting, I was thinking I would need to copy go.mod files to trigger "minimal module awareness", which is a transitional mechanism within cmd/go, but after some experimentation / thinking, I concluded it was not needed based on materializing the /v2).

For tests, I am using the script-driven test facility that the core Go team created for testing cmd/go, which is well tuned to testing the various permutations of modules, including it lets you relatively easily set up go.mod files and source files, run commands and check results. (Some more details of the capability here: https://github.com/golang/go/blob/master/src/cmd/go/testdata/script/README).

* modules support: adjust wording based on first-pass review, and update vendor test (#3)

* travis: attempt to install cmd/testscript

* testing: initial check of 'go-fuzz-build -h' output to see if any testscripts work

* testing: change name of first test script

* travis: check if the 'testscript' cmd seems to function at all.

* testing: use the exec command to invoke 'go-fuzz-build -h'

* testing: create initially failing test case for v2 modules

* travis: run testscript for v2 modules

* testing: remember to use 'exec' to invoke go-fuzz-build

* testing: adjust comments in v2 module testscript

* go-fuzz-build: preliminary module support

* testing: flip  from an expected failure to an expected pass for a module test

* go-fuzz-build: update comments for preliminary modules support

* go-fuzz: preliminary modules support for go-fuzz itself

* travis: go-fuzz-defs sanity check

* travis: additional temp sanity check

* travis: add more comments, some additional sanity checks in case we need to debug in future

* travis: s/go-fuzz-deps/go-fuzz-dep/

* testing: validate behavior of v2 modules

* testing: renamed fuzz_help.txt

* testing: update travis to use the renamed testscripts

* travis: conditionally set GO111MODULE=auto, and test Go 1.13 rather than 1.11

* testing: validate modules outside GOPATH

* testing: validate modules inside GOPATH

* testing: invoke timeout properly

* testing: preliminary test for vendored modules (not yet supported)

* testing: validate how go-fuzz-dep and go-fuzz-defs are found

* travis: run the latest testscripts for modules testing

* testing: fix typo in mod_outside_gopath.txt file name

* travis: fix typo in file name

* go-fuzz-build: detect -mod=vendor

* testing: validate we get a proper error for -mod=vendor

* readme: describe preliminary modules support

* readme: correct typo in modules support

* readme: adjust modules support section

* readme: tweak wording based on PR review

* go-fuzz: tweak comment wording by removing "preliminary modules support"

* go-fuzz-build: tweak comment wording by removing "preliminary modules support"

* mod_vendor.txt: avoid triggering Go 1.14 auto detection of a vendor directory

* readme: update wording based on review

* readme: remove a blank line to re-trigger travis
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.

1 participant