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

source env_vars.sh in the test docker container #198

Closed
wants to merge 1 commit into from

Conversation

anthrotype
Copy link
Contributor

... similarly to how it's done in the build phase.

Fixes https://github.com/matthew-brett/multibuild/issues/197

@matthew-brett
Copy link
Collaborator

Again - so sorry to be slow to get back to you. Would you mind updating the README to say that env_vars.sh also applies to the test phase?

@native-api
Copy link
Contributor

@anthrotype Are you sure it's going to be specifically env_vars.sh? The variables for test are probably going to be different than the variables for build.
It could be shared if there was a way for the code to distinguish which phase is currently running...

Then, for the advertized TOXENV use case, you'll need to somehow "know" about the outside variables anyway since you're going to set TOXENV conditionally.

So, I suspect that using the functionality as suggested is going to be really awkward.

It would very much help if you override some of Multibuild's functionality privately in your repo (by sourceing additional code after them or appending to its files on the fly) to demonstrate how you envision the use cases.

@anthrotype
Copy link
Contributor Author

Hi! I'm afraid I won't have time to work in this, but feel free to use/discard/continue as you see fit. Sorry, thanks!

@matthew-brett
Copy link
Collaborator

I guess we could just add a test_vars.sh equivalent. Then it would be good to allow a build_vars.sh file instead of / as well as an env_vars.sh.

@native-api
Copy link
Contributor

Well, yes, but I don't see a use case for a separate script file here.

The purpose of env_vars.sh as a separate script is to customize input to the stock Multibuild logic that runs in the container before the user's script gets control.

But in the case of testing, there's no stock logic to speak of: all it does is source a library script which doesn't make any changes.

@native-api
Copy link
Contributor

native-api commented Aug 9, 2019

So there's nothing it can do that config.sh or install_run() cannot.
Besides, being run before the stock logic, it can't take advantage of Multibuild's utils library.

So AFAICS at this point, it's going to confuse users and do more harm than good.
Maybe later, if/when the testing container picks up some stock machinery as Python's build system evolves...

@matthew-brett
Copy link
Collaborator

Ok to close this one?

@mattip
Copy link
Collaborator

mattip commented Feb 17, 2020

Closing. Please reopen if a use case arises.

@mattip mattip closed this Feb 17, 2020
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.

Allow to configure run_tests via environment variables
4 participants