Skip to content

Use more robust mechanism for unsetting environment variables#4685

Merged
boegel merged 2 commits intoeasybuilders:5.0.xfrom
Micket:fix_unset
Oct 16, 2024
Merged

Use more robust mechanism for unsetting environment variables#4685
boegel merged 2 commits intoeasybuilders:5.0.xfrom
Micket:fix_unset

Conversation

@Micket
Copy link
Copy Markdown
Contributor

@Micket Micket commented Oct 16, 2024

Parsing the output of "env" can be fragile for certain complex environment.
Using compgen should be safer.

@Micket Micket added the bug fix label Oct 16, 2024
@Micket Micket added this to the 5.0 milestone Oct 16, 2024
@Micket Micket requested a review from boegel October 16, 2024 13:23
Comment thread easybuild/tools/run.py Outdated
# we need to be careful to filter out functions definitions, so first undefine those
fid.write("unset -f $(env | grep '%=' | cut -f1 -d'%' | sed 's/BASH_FUNC_//g')\n")
fid.write("unset $(env | cut -f1 -d=)\n")
fid.write("for var in $(compgen -e); do\n unset \"$var\"\ndone\n")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Micket This is really hard to read, how about this instead? (and then similar below)

        fid.write('\n'.join([
            'for var in $(compgen -e); do',
            '    unset "$var"',
            'done',
        ]) + '\n')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done force pushed

Parsing the output of "env" can be fragile for certain complex
environment.
Using compgen should be safer.
@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 16, 2024

@Micket I've fixed the problem with the failing test in 46102d3

@boegel boegel enabled auto-merge October 16, 2024 15:34
@Micket
Copy link
Copy Markdown
Contributor Author

Micket commented Oct 16, 2024

Did the test just hang and die? There is no output in the CI, just a red mark that it failed the tests for one particular python version.

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 16, 2024

Did the test just hang and die? There is no output in the CI, just a red mark that it failed the tests for one particular python version.

For which commit? For last commit, it's all green...

@boegel boegel merged commit 3716f43 into easybuilders:5.0.x Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants