Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Sep 18, 2018

(required for easybuilders/easybuild-easyconfigs#6712)

This fixes a problem with --check-conflicts reporting false conflicts when an easyconfig that uses the new ModuleRC easyblock is involved (cfr. easybuilders/easybuild-easyblocks#1503).

For example:

$ eb --check-conflicts Bazel-0.16.0-GCCcore-7.3.0.eb
== temporary log file in case of crash /Users/kehoste/work/TMP/eb-OOgLXG/easybuild-bXUl9c.log
Conflict found for dependencies of Bazel-0.16.0-GCCcore-7.3.0: Java-1.8 vs Java-1.8.0_181
	Java-1.8 as dep of: Bazel-0.16.0-GCCcore-7.3.0
	Java-1.8.0_181 as dep of: Bazel-0.16.0-GCCcore-7.3.0, Java-1.8
Conflict between (dependencies of) easyconfigs: Java-1.8 vs Java-1.8.0_181
	Java-1.8 as dep of: Bazel-0.16.0-GCCcore-7.3.0
	Java-1.8.0_181 as dep of: Bazel-0.16.0-GCCcore-7.3.0, Java-1.8
ERROR: One or more conflicts detected!

Because Java-1.8.eb is a wrapper around Java-1.8.0_181.eb, --check-conflicts detects a conflict where there actually isn't any.

@boegel boegel added the bug fix label Sep 18, 2018
@boegel boegel added this to the 3.7.0 milestone Sep 18, 2018
@boegel boegel requested a review from bartoldeman September 18, 2018 11:55
pass
def install_step(self):
pass
def sanity_check_step(self):

Choose a reason for hiding this comment

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

expected 1 blank line, found 0

pass
def build_step(self):
pass
def install_step(self):

Choose a reason for hiding this comment

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

expected 1 blank line, found 0

"""Dummy implementation of generic easyblock that generates .modulerc files."""
def configure_step(self):
pass
def build_step(self):

Choose a reason for hiding this comment

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

expected 1 blank line, found 0

"""
from easybuild.framework.easyblock import EasyBlock

class ModuleRC(EasyBlock):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Copy link
Contributor

@bartoldeman bartoldeman left a comment

Choose a reason for hiding this comment

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

lgtm, except for the superfluous import (unless I am missing something)

from easybuild.tools.filetools import det_common_path_prefix, search_file
from easybuild.tools.module_naming_scheme.easybuild_mns import EasyBuildMNS
from easybuild.tools.module_naming_scheme.utilities import det_full_ec_version
from easybuild.tools.toolchain import DUMMY_TOOLCHAIN_NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this added but not used later on?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was used during an earlier attempt, but that turned out to be too ugly/hacky. I'll clean this up...

@boegel boegel changed the title Fix check conflicts take into account dependency 'wrappers' in check_conflicts Sep 18, 2018
@bartoldeman
Copy link
Contributor

going in @boegel !

@bartoldeman bartoldeman merged commit 9fb3464 into easybuilders:develop Sep 18, 2018
@boegel boegel deleted the fix_check_conflicts branch September 18, 2018 17:34
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.

3 participants