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

add support for filtering dependencies by using False as version #3506

Merged

Conversation

edmondac
Copy link
Contributor

Useful for architecture-specific deps, e.g.:

dependencies = [
    ('VTK', {'arch=x86_64': '8.2.0', 'arch=POWER': False}, local_python_versionsuffix),
]

So in this example, on POWER don't include a VTK dependency at all

@edmondac edmondac changed the title Specifying False for a version removes the dep WIP: Specifying False for a version removes the dep Nov 27, 2020
@akesandgren
Copy link
Contributor

akesandgren commented Nov 27, 2020

Hmm.... wouldn't it be enough to write

dependencies = [
    ('VTK', {'arch=x86_64': '8.2.0'}, local_python_versionsuffix),
]

to get that effect?
I.e., default being False?

And then we could perhaps do,

dependencies = [
    ('VTK', {'arch=default': '8.2.0', 'arch=POWER': False}, local_python_versionsuffix),
]

To only turn it off for POWER, or something, replace "default" with something better...

@edmondac
Copy link
Contributor Author

Hmm.... wouldn't it be enough to write

dependencies = [
    ('VTK', {'arch=x86_64': '8.2.0'}, local_python_versionsuffix),
]

to get that effect?
I.e., default being False?

And then we could perhaps do,

dependencies = [
    ('VTK', {'arch=default': '8.2.0', 'arch=POWER': False}, local_python_versionsuffix),
]

To only turn it off for POWER, or something, replace "default" with something better...

I do agree with that, but those changes would be in a different part of the codebase, so I'd prefer not to do it in this PR if possible. The syntax for arch-specific deps exists already - I'm just adding support for specifying version=False (for any dep) so I can say "this arch doesn't want this dep at all".

@akesandgren
Copy link
Contributor

Yes, I was just throwing out ideas...

@edmondac edmondac changed the title WIP: Specifying False for a version removes the dep Specifying False for a version removes the dep Nov 27, 2020
@easybuilders easybuilders deleted a comment from boegelbot Nov 28, 2020
@boegel boegel modified the milestones: 4.3.2 (next release), 4.4.0 Nov 28, 2020
@Flamefire
Copy link
Contributor

I do like the arch=default idea however I'm strongly against the "implicit default" idea for the version: This will lead to skipping the dependency for an arch not yet added. So IMO this is good as-is and should be followed-up by the arch=default which allows the same but being explicit: {'arch=x86_64': '8.2.0', 'arch=default': False}

Co-authored-by: Alexander Grund <Flamefire@users.noreply.github.com>
Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Contributor

Going in, thanks @edmondac!

@akesandgren akesandgren merged commit 95442b4 into easybuilders:develop Feb 22, 2021
@boegel boegel changed the title Specifying False for a version removes the dep add support for filtering dependencies by using False as version Apr 7, 2021
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