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

Extensions can should not be allowed to impact global environment during builds #4706

Open
Micket opened this issue Nov 13, 2024 · 2 comments
Labels

Comments

@Micket
Copy link
Contributor

Micket commented Nov 13, 2024

Example of this happening: The PyTorch bundle has torchvision and torchaudio.
https://github.com/easybuilders/easybuild-easyconfigs/blob/df6a3afb8ad03f0bc72c2652a3f3781c56af0b6b/easybuild/easyconfigs/p/PyTorch-bundle/PyTorch-bundle-2.1.2-foss-2023a-CUDA-12.1.1.eb#L104

torchvision sets the environment variable BUILD_VERSION globally, and torchaudio happens to have a setup.py that does

def _get_version(sha):
    with open(ROOT_DIR / "version.txt", "r") as f:
        version = f.read().strip()
    if os.getenv("BUILD_VERSION"):                                                                                       
        version = os.getenv("BUILD_VERSION")
     elif sha is not None:
        version += "+" + sha[:7]
    return version

which introduces the bug that the version is misreported for torchaudio as 0.16.2, despite it being 2.1.2

  1. We should avoid setting environment variables globally, and going forward always stick to using run_cmd(..., env=custom_env).
  2. Framework should record and restore the environment before and after each extension.

This goes to os.environ as well as the cwd.

I don't know if we have any situations where we rely on leaky behavior like this, though we really should avoid it.

@boegel
Copy link
Member

boegel commented Jan 7, 2025

Framework should record and restore the environment before and after each extension.

@Micket This is the correct fix, extensions shouldn't be influencing each others build environment, that's a bug...

@boegel
Copy link
Member

boegel commented Jan 7, 2025

@pavelToman Are you up for taking a look at this?

It probably boils down to recorded and restoring the environment before/after installing an extension, in install_extensions_sequential, you can use the restore_env function for that.

Not sure what needs to happen to also make that work correctly with install_extensions_parallel, since environment variables are global...

@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Jan 22, 2025
@boegel boegel moved this to Breaking changes in EasyBuild v5.0 Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Breaking changes
Development

No branches or pull requests

2 participants