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

Potential issues with setting environment variables #3

Open
disconnect3d opened this issue Mar 20, 2023 · 1 comment
Open

Potential issues with setting environment variables #3

disconnect3d opened this issue Mar 20, 2023 · 1 comment

Comments

@disconnect3d
Copy link
Contributor

Hi,

Currently, the LoadPackage function adds environment variables this way:

goat/pkgutil/load.go

Lines 51 to 54 in e7b9eed

config.Env = append(os.Environ(), "GOPATH="+gopath, "GO111MODULE=on")
} else {
// Load packages according to the legacy "module-unaware" mode (GO111MODULE=off).
config.Env = append(os.Environ(), "GOPATH="+gopath, "GO111MODULE=off")

However, this may not be the best approach since:

  1. It uses all old/other environment variables that may be set in the environment and may somehow influence the build system, like CGO_FLAGS
  2. It literally adds new environment variables to the list and all the old ones are preserved. What if someone had GO111MODULES=off and now it sets it to on? Well, it will then all depend on the underlying code that processes environment variables: if the code iterates over the slice and gets the value from the first variable that matches the key, then we will use incorrect value.
  3. It may also be good to document the GO111MODULES stuff better in the project README or/and inform the user with an info log about us setting it for the package load.
@disconnect3d disconnect3d changed the title Potential issue with environment variables Potential issues with setting environment variables Mar 20, 2023
@BarrensZeppelin
Copy link
Collaborator

  1. I agree. It would probably be better if the command line flags that the user wants to pass on to the build system are provided explicitly.
  2. The documentation for packages.Load specifies that later entries take priority.
  3. Yes, it would be good to improve the documentation. There's a small piece of documentation in the help text for the -modulepath flag, but it doesn't tell you that GO111MODULE will be set to off when the flag is not passed.

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

No branches or pull requests

2 participants