Skip to content

Add support for whitelisting environment variables to pass through (closes #76)#77

Closed
dflemstr wants to merge 2 commits into
cross-rs:masterfrom
dflemstr:env
Closed

Add support for whitelisting environment variables to pass through (closes #76)#77
dflemstr wants to merge 2 commits into
cross-rs:masterfrom
dflemstr:env

Conversation

@dflemstr
Copy link
Copy Markdown
Contributor

This adds a fairly straight-forward section to Cross.toml like so:

[build.env]
whitelist_all = true
# ...or...
whitelist = ["PATH", "RVM_VERSION", "LD_LIBRARY_PATH"]

@dflemstr dflemstr changed the title Add support for whitelisting environment variables to pass through (fixes #76) Add support for whitelisting environment variables to pass through (closes #76) Mar 11, 2017
@japaric
Copy link
Copy Markdown
Contributor

japaric commented Apr 2, 2017

Thank you for the PR, @dflemstr, and sorry for taking so long to review.

The implementation looks good to me but I'm a bit concerned about whitelist_all as that makes more likely to break the operation of Cross by overriding one of the env variables its docker container internally uses. Ideally, I'd like Cross to warn you if you were to override one of such internal variables but that would make the implementation of this feature way more complicated.

So, how about this: let's implement just whitelist and document this feature on the README noting that Cross requires some env variables to be set to certain values, perhaps also list the common ones in the README, to work correctly and that you should think twice about overriding those.

What do you think?

@kamalmarhubi
Copy link
Copy Markdown
Contributor

I would like this to enable passing TravisCI env vars into the build. Let me know if I can help nudge this along!

kamalmarhubi added a commit to kamalmarhubi/cross that referenced this pull request Apr 18, 2017
This addresses comments on cross-rs#77.
kamalmarhubi added a commit to kamalmarhubi/cross that referenced this pull request Jun 6, 2017
This addresses comments on cross-rs#77.
kamalmarhubi added a commit to kamalmarhubi/cross that referenced this pull request Jun 10, 2017
This addresses comments on cross-rs#77.
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.

3 participants