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.

Sign up for GitHub

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
Merged

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be3667c
replace `run_cmd` with `run_shell_cmd` in custom easyblock for Perl (…
bartoldeman Feb 5, 2024
bc3e271
remove accidental import
bartoldeman Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions easybuild/easyblocks/p/perl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
from easybuild.tools.filetools import adjust_permissions
from easybuild.tools.environment import setvar, unset_env_vars
from easybuild.tools.modules import get_software_root
from easybuild.tools.run import run_cmd
from easybuild.tools.run import run_shell_cmd

# perldoc -lm seems to be the safest way to test if a module is available, based on exit code
EXTS_FILTER_PERL_MODULES = ("perldoc -lm %(ext_name)s ", "")
Expand Down Expand Up @@ -116,7 +116,7 @@ def configure_step(self):
unset_env_vars(['COLUMNS'])

cmd = '%s ./Configure -de %s' % (self.cfg['preconfigopts'], configopts)
run_cmd(cmd, log_all=True, simple=True)
run_shell_cmd(cmd)

def test_step(self):
"""Test Perl build via 'make test'."""
Expand All @@ -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 •
edited by boegel
Loading

Choose a reason for hiding this comment

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

see:


def prepare_for_extensions(self):
"""
Expand Down Expand Up @@ -198,8 +198,8 @@ def get_major_perl_version():
Returns the major verson of the perl binary in the current path
"""
cmd = "perl -MConfig -e 'print $Config::Config{PERL_API_REVISION}'"
(perlmajver, _) = run_cmd(cmd, log_all=True, log_output=True, simple=False)
return perlmajver
res = run_shell_cmd(cmd)
return res.output


def get_site_suffix(tag):
Expand All @@ -212,6 +212,7 @@ def get_site_suffix(tag):
"""
perl_cmd = 'my $a = $Config::Config{"%s"}; $a =~ s/($Config::Config{"siteprefix"})//; print $a' % tag
cmd = "perl -MConfig -e '%s'" % perl_cmd
(sitesuffix, _) = run_cmd(cmd, log_all=True, log_output=True, simple=False)
res = run_shell_cmd(cmd)
sitesuffix = res.output
# obtained value usually contains leading '/', so strip it off
return sitesuffix.lstrip(os.path.sep)
Loading