Skip to content

Conversation

@casparvl
Copy link
Contributor

@casparvl casparvl commented Aug 1, 2018

Change proposed in this feature request: #2543

This is actually my first contribution to the framework. I'm not sure how to properly test the framework just from a clone of its repository (I guess I need to 'build' or 'install' the framework in soem way?). I tested these changes by performing them in an actual installation (installed through eb EasyBuild/3.6.0). I then simply performed the exact same changes to my own fork of the framework to create this pull request...

@ocaisa
Copy link
Member

ocaisa commented Aug 2, 2018

This is passing the tests because you would need explicit tests for this build option.

It looks like the best template/place to do that would be near https://github.com/easybuilders/easybuild-framework/blob/develop/test/framework/easyconfig.py#L1362 on, for example, the test easyconfig https://github.com/easybuilders/easybuild-framework/blob/develop/test/framework/easyconfigs/test_ecs/b/bzip2/bzip2-1.0.6-GCC-4.9.2.eb where you can check that the build dependency on gzip is returned as a hidden module.

@damianam
Copy link
Member

As @ocaisa said, tests are missing. But besides that, I think we need to make this PR a bit smarter. This PR will force to rebuild all the dependencies, even if they are already installed (but not hidden). This means risking changes in third party software. Let's say software A has dependencies X and Y already installed with a given option. Then software B has also X and Y as dependencies, but hides all of them. So they are reinstalled potentially with different options, overwriting X and Y, and changing the behaviour of software A for no reason.

@boegel boegel added this to the next release milestone Sep 4, 2018
@ocaisa
Copy link
Member

ocaisa commented Sep 12, 2018

I wonder if there is a role here for the new .modulerc features (see #2575)? You could add an additional option like --check-for-and-expose-hidden-modules which would just create the .modulerc to expose hidden module(s)

@boegel boegel modified the milestones: 3.7.1, next release Oct 4, 2018
@boegel boegel changed the title 2543 hide all deps add support for --hide-all-deps Nov 20, 2018
@boegel boegel modified the milestones: 3.8.0, 3.x Nov 20, 2018
@boegel
Copy link
Member

boegel commented Nov 20, 2018

@casparvl Any plans for working on this further?

@casparvl
Copy link
Contributor Author

casparvl commented Jan 2, 2019

@boegel Hm, not really.

I must admit, it seemed very useful since with older toolchains, I'd sometimes get a myriad of versions for the same software as dependencies. However, it seems we (as a community) are now more consistent in picking (for dependencies) one version per software per toolchain, limiting the usefulness of this 'hide-all-deps' kind of thing.

Out of curiosity, I guess we are now more selective because this also avoids conflicts higher up in the hierarchy? E.g. problems that occur if there are deps like

lib C -> lib A 1.1
lib B -> lib A 1.0

and later, some lib

lib D -> lib B + lib C

is added. Is the one version per software per toolchain (for dependencies) a policy that you actively pursue now?

@boegel
Copy link
Member

boegel commented Jan 6, 2019

@casparvl We have a test in place in the easyconfigs repository that ensures we only have one "dependency variant" (basically version + versionsuffix) per toolchain generation (current is 2018b). The reason for this is indeed to avoid creating conflicts when suddenly two things come together in the same dependency graph, but also to allow for more easily combining multiple tools that are disjoint w.r.t. dependencies in the same workflow.

There are a couple of exceptions to the rule though, the most obvious one being Python where we do allow a single 2.x and 3.x version. See the check_dep_vars method in https://github.com/easybuilders/easybuild-easyconfigs/blob/master/test/easyconfigs/easyconfigs.py#L172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants