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

Respect env variables in all build environments #6478

Closed
wants to merge 3 commits into from

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Dec 18, 2019

I guess is to get around any security implications.
But modifying the PATH to a local copy of the environments variables of
the host leads to errors.

This was ok when using the local host to run the checkout steps, but now we are running inside the container.

Also, we already set all the READTHEDOCS env variables from

def get_env_vars(self):
"""Get bash environment variables used for all builder commands."""
env = {
'READTHEDOCS': True,
'READTHEDOCS_VERSION': self.version.slug,
'READTHEDOCS_PROJECT': self.project.slug,
'READTHEDOCS_LANGUAGE': self.project.language,
}
if self.config.conda is not None:
env.update({
'CONDA_ENVS_PATH': os.path.join(self.project.doc_path, 'conda'),
'CONDA_DEFAULT_ENV': self.version.slug,
'BIN_PATH': os.path.join(
self.project.doc_path,
'conda',
self.version.slug,
'bin',
),
})
else:
env.update({
'BIN_PATH': os.path.join(
self.project.doc_path,
'envs',
self.version.slug,
'bin',
),
})
# Update environment from Project's specific environment variables,
# avoiding to expose environment variables if the version is external
# for security reasons.
if self.version.type != EXTERNAL:
env.update(self.project.environment_variables)
return env

This is needed for #6436

I guess is to get around any security implications.
But modifying the PATH to a local copy of the environments variables of
the host leads to errors.

This is needed for readthedocs#6436
@stsewd
Copy link
Member Author

stsewd commented Dec 18, 2019

Also, in production the local environment was only used in the checkout step.

@stsewd
Copy link
Member Author

stsewd commented Dec 19, 2019

I split this PR into 3 to see have more idea where the incompatible change is happening.

#6480
#6481

Thos PRs can be merged and it won't break current installations or .com

@stsewd stsewd closed this Dec 19, 2019
@stsewd stsewd deleted the respect-env-var-all-env branch December 19, 2019 14:50
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.

1 participant