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

Adjust go111modules detection and fix lint #180

Conversation

zeripath
Copy link
Collaborator

Go changed the way GO111MODULES is interpreted in 1.17+ and therefore xgo needs to recognise this change of behaviour.

Now, we could use the GO_VERSION environment variable but it's not really well defined as the build.sh script has apparently different interpretations within it. Thus this PR actually drops to parsing the result of go version to get the MAJOR and the MINOR version number out. The tests that use GO_VERSION are then changed to heed the results of these and the GO111MODULES behaviour is changed to match that.

Finally there were a number of issues detected with shellcheck in the scripts and this PR fixes the lint for these too.

(This PR could be split in two if necessary to helpfully separate the linting issues from the other fixes.)

Fix #179

Go 1.17+ has a changed default behaviour for GO111MODULES so autodetect if we are running go1.17+
and change the behaviour GO111MODULES to match this.

Signed-off-by: Andrew Thornton <art27@cantab.net>
The old GO_VERSION environment/configuration variable is poorly explained and appears to have
conflicting uses. Switch to the auto-calculated version.

Signed-off-by: Andrew Thornton <art27@cantab.net>
There were multiple issues that shellcheck found. This commit fixes these.

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added bug Something isn't working enhancement New feature or request labels Nov 20, 2022
@techknowlogick techknowlogick merged commit dc59b5d into techknowlogick:main Nov 20, 2022
@zeripath zeripath deleted the adjust-go111modules-detection-and-fix-lint branch November 21, 2022 02:48
zeripath added a commit to zeripath/xgo that referenced this pull request Nov 22, 2022
There was a bug in techknowlogick#180 whereby quoting was messed up - this PR correctly uses arrays to fix this

Fix techknowlogick#182

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Nov 22, 2022
zeripath added a commit to zeripath/xgo that referenced this pull request Nov 22, 2022
There was a bug in techknowlogick#180 whereby quoting was messed up - this PR correctly uses arrays to fix this

Fix techknowlogick#182

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick pushed a commit that referenced this pull request Nov 22, 2022
* Remove ldflags from go get calls

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Fix broken build

There was a bug in #180 whereby quoting was messed up - this PR correctly uses arrays to fix this

Fix #182

Signed-off-by: Andrew Thornton <art27@cantab.net>

* And output images to local registry

Signed-off-by: Andrew Thornton <art27@cantab.net>

* try load instead

Signed-off-by: Andrew Thornton <art27@cantab.net>

* try again

Signed-off-by: Andrew Thornton <art27@cantab.net>

* I give up

I can't get this to update properly

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/xgo that referenced this pull request Jan 8, 2023
There was an unfortunate misunderstanding in techknowlogick#180 which did not properly
take account of how xgo.go handled remote builds as compared to local
builds, and modules.

This PR adjusts the way xgo handles GO111MODULE and the way it sets
GO111MODULE to more closely match the way go 1.16+ handles GO111MODULE -
and it makes remote builds work again (albeit there is a problem here,
see below...)

From go1.16 module aware builds are assumed by default with GO111MODULE.
This means GO111MODULE="" means GO111MODULE="on".  This differs from the
previous state where GO111MODULE="" meant GO111MODULE="auto".

`xgo` will now respect the GO111MODULE environment variable in the
context it run in. If it is empty, a module aware build is performed by
default. If it is "auto" `xgo` will interrogate the repository to
discover it should be "on" or "off" in a similar way to `go`. HOWEVER,
"auto" is not supported for remote builds as source code is not
available to it. (It would be possible to move some of this into the
build.sh or make it a separate command if this was required.)

Next, when doing a module-aware build of a remote repository the bug in
 techknowlogick#180 has been fixed, and because the change in go1.18 that causes `go get`
to not work outside of a module by default - the `go get` command is
explicitly run with `GO111MODULE="off"` to get the old GOPATH `go get`
variant.

Potential issues:

* `GO111MODULE="auto"` is not supported for remote builds - this could
  be made supportable as described above.
* The hack above using `GO111MODULE="off"` to make `go get` work for
  remote builds is probably going to stop working within a few more
  releases of go.

Fix techknowlogick#187

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick pushed a commit that referenced this pull request Jan 10, 2023
There was an unfortunate misunderstanding in #180 which did not properly
take account of how xgo.go handled remote builds as compared to local
builds, and modules.

This PR adjusts the way xgo handles GO111MODULE and the way it sets
GO111MODULE to more closely match the way go 1.16+ handles GO111MODULE -
and it makes remote builds work again (albeit there is a problem here,
see below...)

From go1.16 module aware builds are assumed by default with GO111MODULE.
This means GO111MODULE="" means GO111MODULE="on".  This differs from the
previous state where GO111MODULE="" meant GO111MODULE="auto".

`xgo` will now respect the GO111MODULE environment variable in the
context it run in. If it is empty, a module aware build is performed by
default. If it is "auto" `xgo` will interrogate the repository to
discover it should be "on" or "off" in a similar way to `go`. HOWEVER,
"auto" is not supported for remote builds as source code is not
available to it. (It would be possible to move some of this into the
build.sh or make it a separate command if this was required.)

Next, when doing a module-aware build of a remote repository the bug in
 #180 has been fixed, and because the change in go1.18 that causes `go get`
to not work outside of a module by default - the `go get` command is
explicitly run with `GO111MODULE="off"` to get the old GOPATH `go get`
variant.

Potential issues:

* `GO111MODULE="auto"` is not supported for remote builds - this could
  be made supportable as described above.
* The hack above using `GO111MODULE="off"` to make `go get` work for
  remote builds is probably going to stop working within a few more
  releases of go.

Fix #187

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'go get' is no longer supported outside a module
2 participants