easybuilders / easybuild-framework

EasyBuild is a software installation framework in Python that allows you to install software in a structured and robust way.
https://easybuild.io
GNU General Public License v2.0
152 stars 202 forks source link

`cmd.sh` script produced by `run_shell_cmd` does not start a login shell #4616

Closed boegel closed 1 month ago

boegel commented 2 months ago

It would be better if cmd.sh starts a login shell, to avoid this issue:

 $ /tmp/eb-h1q36k1i/run-shell-cmd-output/python-dpgxvppv/cmd.sh
# Shell for the command: '/software/Python/3.11.3-GCCcore-12.3.0/bin/python -m pip install --prefix=/user/gent/400/vsc40023/eb_arcaninescratch/RHEL8-eb5dev-rpath/zen2-ib/software/cutadapt/4.9-GCCcore-12.3.0  --verbose --no-deps --ignore-installed --no-index --no-build-isolation .'
# Use command history, exit to stop

eb-shell> module list
bash: module: command not found

eb-shell> . /etc/profile.d/modules.sh
eb-shell> module --version

Modules based on Lua: Version 8.7.4  2022-06-07 18:23 -05:00
    by Robert McLay mclay@tacc.utexas.edu

It's not as trivial as combining --login with --rcfile though, it seems...

boegel commented 2 months ago

@Micket Any ideas here?

Micket commented 2 months ago

hm, well, it would risk making it not identical environment as to what run_shell_cmd runs as?

boegel commented 2 months ago

hm, well, it would risk making it not identical environment as to what run_shell_cmd runs as?

Hmm right, good point.

It is a bit annoying that the module command is not defined though. I guess we should at least document this, and perhaps print a suggestion on this (I guess sourcing /etc/profile.d/modules.sh should be pretty universal?)...

Micket commented 2 months ago

I really should have just figured out how to solve that from the start, but i guess we just deal with the %% variables, which should be all the functions for bash;

        # excludes bash functions (environment variables ending with %)
        fid.write('\n'.join(f'export {key}={shlex.quote(value)}' for key, value in sorted(env.items())
                            if not key.endswith('%')) + '\n')

These variables "BASH_FUNC_module%%" can't be exported, and i don't think just including them as they are would produce working functions, so instead

        for key, value in env.items():
            if key.startswith('BASH_FUNC_') and key.endswith('%%'):
                func_name = key[10:-2]
                fid.write(f'declare -f {func_name} {value}\n')

which should just define the function again

or maybe something a bit hacky like

        run_subprocess('declare -f >> env.sh', shell=True)  # to lazy to figure out the code here, but the principle is clear

Note: completely untested.

Micket commented 2 months ago

Having tested some things here, i think we should consider just letting bash do the writing of the files. While i haven't found a way to break bash functions, they can be long and complicated and the associated environment variables aren't a 1-to-1 match with the source code. There may also be some complex escaping necessary for special characters.

Maybe we just replace the code that exports the environment variables with just subprocesses that run

declare -x > env.sh  # or keep the current python code
declare -f > func.sh

This lets bash parse them, and it will do the correct job of it always.

boegel commented 2 months ago

@Micket That approach makes sense to me. We now also call a subprocess btw, since the proceduce to collect the contents for env.sh uses the env command (which is provided by coreutils).

With that in mind, declare -x is better, since declare is a bash builtin, no so hidden dependency?

boegel commented 1 month ago

fixed with #4662