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

replace run_cmd with run_shell_cmd in custom easyblock for Perl (perl.py) #3162

Merged
merged 2 commits into from Feb 5, 2024

Conversation

bartoldeman
Copy link
Contributor

(created using eb --new-pr)

@bartoldeman bartoldeman added this to the 5.0 milestone Feb 5, 2024
@branfosj
Copy link
Member

branfosj commented Feb 5, 2024

Test report by @branfosj

Overview of tested easyconfigs (in order)

  • SUCCESS Perl-5.32.0-GCCcore-10.2.0.eb
  • SUCCESS Perl-5.38.0-GCCcore-13.2.0.eb
  • SUCCESS Perl-5.34.1-GCCcore-11.3.0-minimal.eb
  • SUCCESS Perl-5.34.0-GCCcore-11.2.0.eb

Build succeeded for 4 out of 4 (4 easyconfigs in total)
bear-pg0105u03a - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Platinum 8360Y CPU @ 2.40GHz (icelake), Python 3.6.8
See https://gist.github.com/branfosj/f8dfb469c1d38406c0a9729b0f9d1b02 for a full test report.

@branfosj
Copy link
Member

branfosj commented Feb 5, 2024

Going in, thanks @bartoldeman!

@branfosj branfosj merged commit e699d91 into easybuilders:5.0.x Feb 5, 2024
17 checks passed
@@ -139,7 +139,7 @@ def test_step(self):
# specify locale to be used, to avoid that a handful of tests fail
cmd = "export LC_ALL=C && %s" % cmd

run_cmd(cmd, log_all=False, log_ok=False, simple=False)
run_shell_cmd(cmd, fail_on_error=False)
Copy link
Member

Choose a reason for hiding this comment

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

Why are we ignoring non-zero exit codes here? That seems wrong...

@bartoldeman I understand that you're just translating from run_cmd to run_shell_cmd without changing behavior here, but this looks like a bug we should fix, no?

Copy link
Member

@branfosj branfosj Feb 7, 2024

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants