Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Set environment variables for go tools #932

Merged
merged 2 commits into from
May 1, 2017
Merged

Conversation

bancek
Copy link
Contributor

@bancek bancek commented Apr 19, 2017

This will let users set env variables for go build (e.g. CGO_CFLAGS, CGO_LDFLAGS and LD_LIBRARY_PATH).

@msftclas
Copy link

@bancek,
Thanks for having already signed the Contribution License Agreement. Your agreement has not been validated yet. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@ramya-rao-a
Copy link
Contributor

@bancek Why only build/lint/vet? Wouldn't other tools/features benefit from this as well?

If yes, then it would make more sense to update process.env with these env variables once during extension activations, and it will get used everywhere.

@hickeng This can help your GOOS scenario also I believe. #632

@hickeng
Copy link

hickeng commented Apr 21, 2017

@ramya-rao-a Yes! Looking forward to the merge.

@bancek
Copy link
Contributor Author

bancek commented Apr 21, 2017

@ramya-rao-a env variables are mostly needed for building.

I don't think it's a good idea to change process.env because it would effect other extensions and especially when config can be changed while VS Code is running.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Apr 23, 2017

@bancek See #632 (comment) for scenario where the env vars need to be used by some of the Go tools that we use the Go extension.

About your concerns on changes to process.env affecting other extensions:

VS Code runs each extension in a separate process, and so any changes we do to the process.env is reflected only in the operations carried out by the Go extension.

See https://code.visualstudio.com/docs/extensionAPI/patterns-and-principles#_stability-extension-isolation

Edit: I was wrong, VS Code runs all extensions in the same process which is separate from the process used by the core VS Code.

Will merge this PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants