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

take into account alternate sysroot for /bin/bash used by run_cmd #4646

Merged
merged 5 commits into from
Sep 18, 2024

Conversation

casparvl
Copy link
Contributor

@casparvl casparvl commented Sep 18, 2024

EasyBuild's run_cmd runs commands in a subshell. The shell used is defined in the exec_cmd. However, in context where the use of a sysroot is configured, one would want to use <sysroot>/bin/bash instead. This PR implements that change. Note that since run_cmd is also called a few times before set_up_configuration is called, we need to be able to disable the check for the build_option('sysroot') for those instances (similar to how this is already don for build_option('hooks')).

Solves this issue.

Edit: hmmm, I made this PR, and then realized, this is probably one of the components that is very different in EB 5.0. And it seems to be indeed...

Someone with more EB 5.0 knowledge needs to give me a pointer of how to fix this for EB 5.0 :)

casparvl casparvl and others added 3 commits September 18, 2024 10:20
…se sysroot/bin/bash instead of /bin/bash if sysroot is set
…led. Any command run before that with run_cmd should use the regular /bin/bash
@casparvl casparvl added the EESSI Related to EESSI project label Sep 18, 2024
@casparvl
Copy link
Contributor Author

Ok, this issue actually does not exist for EB 5.0: the whole run_cmd is now replaced by run_shell_cmd, and that has a different way of determining the shell command. See this piece of code:

    # use bash as shell instead of the default /bin/sh used by subprocess.run
    # (which could be dash instead of bash, like on Ubuntu, see https://wiki.ubuntu.com/DashAsBinSh)
    # stick to None (default value) when not running command via a shell
    if use_bash:
        bash = shutil.which('bash')
        _log.info(f"Path to bash that will be used to run shell commands: {bash}")
        executable, shell = bash, True
    else:
        executable, shell = None, False

at least for EESSI, the result is that the bash from the prefix is used. That means our issue no longer exists. I'm closing this PR, as it is not relevant.

@boegel
Copy link
Member

boegel commented Sep 18, 2024

@casparvl As discussed, I think it's worth considering this to be merged into develop, since there's a (small) window of opportunity to get this included in EasyBuild v4.9.4 (which we'll be releasing because of a bug that was introduced in the GCC easyblock, see here).

I'll take a thorough look into this right now...

…cated test for run_cmd using with_sysroot=True
@boegel boegel modified the milestones: 4.x, release after 4.9.3 Sep 18, 2024
@boegel boegel changed the title Exec command with sysroot take into account alternate sysroot for /bin/bash used by run_cmd Sep 18, 2024
@boegel boegel modified the milestones: release after 4.9.4, 4.9.4 Sep 18, 2024
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel boegel merged commit 3b7295f into easybuilders:develop Sep 18, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix EESSI Related to EESSI project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants