Skip to content

Add support for --disable-robot #4781

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions easybuild/base/generaloption.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ class ExtOption(CompleterOption):
- set default to None if no option passed,
- set to default if option without value passed,
- set to value if option with value passed
- store_or_False: Same as store_or_None, but set to False instead of None
- Additionally supports --disable-

Types:
- strlist, strtuple : convert comma-separated string in a list resp. tuple of strings
Expand All @@ -196,14 +198,15 @@ class ExtOption(CompleterOption):

ENABLE = 'enable' # do nothing
DISABLE = 'disable' # inverse action
STORE_OR_FALSE = 'store_or_False'

EXTOPTION_EXTRA_OPTIONS = ('date', 'datetime', 'regex', 'add', 'add_first', 'add_flex',)
EXTOPTION_STORE_OR = ('store_or_None', 'help') # callback type
EXTOPTION_STORE_OR = ('store_or_None', STORE_OR_FALSE, 'help') # callback type
EXTOPTION_LOG = ('store_debuglog', 'store_infolog', 'store_warninglog',)
EXTOPTION_HELP = ('shorthelp', 'confighelp', 'help')

ACTIONS = Option.ACTIONS + EXTOPTION_EXTRA_OPTIONS + EXTOPTION_STORE_OR + EXTOPTION_LOG + EXTOPTION_HELP
STORE_ACTIONS = Option.STORE_ACTIONS + EXTOPTION_EXTRA_OPTIONS + EXTOPTION_LOG + ('store_or_None',)
STORE_ACTIONS = Option.STORE_ACTIONS + EXTOPTION_EXTRA_OPTIONS + EXTOPTION_LOG + ('store_or_None', STORE_OR_FALSE)
TYPED_ACTIONS = Option.TYPED_ACTIONS + EXTOPTION_EXTRA_OPTIONS + EXTOPTION_STORE_OR
ALWAYS_TYPED_ACTIONS = Option.ALWAYS_TYPED_ACTIONS + EXTOPTION_EXTRA_OPTIONS

Expand Down Expand Up @@ -232,7 +235,9 @@ def store_or(option, opt_str, value, parser, *args, **kwargs): # pylint: disabl
"""Callback for supporting options with optional values."""
# see http://stackoverflow.com/questions/1229146/parsing-empty-options-in-python
# ugly code, optparse is crap
if parser.rargs and not parser.rargs[0].startswith('-'):
if option.store_or == self.STORE_OR_FALSE and opt_str.startswith("--%s-" % self.DISABLE):
Copy link
Member

Choose a reason for hiding this comment

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

2nd part of this condition feels very hard-codey...

Is that also how we can for disabling of regular boolean options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is basically copied verbatim from

if opt.startswith("--%s-" % self.ENABLE):
# keep action
pass
elif opt.startswith("--%s-" % self.DISABLE):

val = False
elif parser.rargs and not parser.rargs[0].startswith('-'):
val = option.check_value(opt_str, parser.rargs.pop(0))
else:
val = kwargs.get('orig_default', None)
Expand All @@ -250,11 +255,7 @@ def store_or(option, opt_str, value, parser, *args, **kwargs): # pylint: disabl
'orig_default': copy.deepcopy(self.default),
}
self.action = 'callback' # act as callback

if self.store_or in self.EXTOPTION_STORE_OR:
self.default = None
else:
self.log.raiseException("_set_attrs: unknown store_or %s" % self.store_or, exception=ValueError)
self.default = False if self.action == self.STORE_OR_FALSE else None

def process(self, opt, value, values, parser):
"""Handle option-as-value issues before actually processing option."""
Expand Down Expand Up @@ -1219,6 +1220,8 @@ def add_group_parser(self, opt_dict, description, prefix=None, otherdefaults=Non
if action in self.parser.option_class.BOOLEAN_ACTIONS:
args.append("--%s-%s" % (self.parser.option_class.ENABLE, opt_name))
args.append("--%s-%s" % (self.parser.option_class.DISABLE, opt_name))
elif action == self.parser.option_class.STORE_OR_FALSE:
args.append("--%s-%s" % (self.parser.option_class.DISABLE, opt_name))

# force passed_kwargs as final nameds
nameds.update(passed_kwargs)
Expand Down
6 changes: 4 additions & 2 deletions easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def basic_options(self):
'rebuild': ("Rebuild software, even if module already exists (don't skip OS dependencies checks)",
None, 'store_true', False),
'robot': ("Enable dependency resolution, optionally consider additional paths to search for easyconfigs",
'pathlist', 'store_or_None', [], 'r', {'metavar': '[PATH[%sPATH]]' % os.pathsep}),
'pathlist', 'store_or_False', [], 'r', {'metavar': '[PATH[%sPATH]]' % os.pathsep}),
'robot-paths': ("Additional paths to consider by robot for easyconfigs (--robot paths get priority)",
'pathlist', 'add_flex', self.default_robot_paths, {'metavar': 'PATH[%sPATH]' % os.pathsep}),
'search-paths': ("Additional locations to consider in --search (next to --robot and --robot-paths paths)",
Expand Down Expand Up @@ -1261,7 +1261,9 @@ def _postprocess_config(self):
if self.options.pretend:
self.options.installpath = get_pretend_installpath()

if self.options.robot is not None:
if self.options.robot is False:
self.options.robot = None # Set to None as-if not specified
elif self.options.robot is not None:
# if a single path is specified to --robot/-r, it must be an existing directory;
# this is required since an argument to --robot is optional,
# which makes it susceptible to 'eating' the following argument/option;
Expand Down
19 changes: 17 additions & 2 deletions test/framework/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -3480,9 +3480,24 @@ def test_robot(self):
't/toy/toy-0.0-deps.eb',
'g/gzip/gzip-1.4-GCC-4.6.3.eb',
]
re_template = r'^\s\*\s\[[xF ]\]\s%s'
for ecfile in ecfiles:
ec_regex = re.compile(r'^\s\*\s\[[xF ]\]\s%s' % os.path.join(test_ecs_path, ecfile), re.M)
self.assertTrue(ec_regex.search(outtxt), "Pattern %s found in %s" % (ec_regex.pattern, outtxt))
ec_regex = re.compile(re_template % os.path.join(test_ecs_path, ecfile), re.M)
self.assertRegex(outtxt, ec_regex)

# Check for disabling --robot
args.append('--disable-robot')
# Enabled on cmdline before and via option, but should be disabled
os.environ['EASYBUILD_ROBOT'] = self.test_prefix
with self.mocked_stdout_stderr():
outtxt = self.eb_main(args, raise_error=True)
for ecfile in ecfiles:
ec_regex = re.compile(re_template % os.path.join(test_ecs_path, ecfile), re.M)
# Only the EC passed would be build but not the dependencies
if os.path.basename(ecfile) == os.path.basename(eb_file):
self.assertRegex(outtxt, ec_regex)
else:
self.assertNotRegex(outtxt, ec_regex)

def test_robot_path_check(self):
"""Test path check for --robot"""
Expand Down
Loading