boost: do not add the context-impl option for version below 1.65.0 by mzeyen1985 · Pull Request #30719 · spack/spack

mzeyen1985 · 2022-05-18T12:18:28Z

#30654 doesn't prevent the addition of the b2 option context-impl when the variant is disabled.
This PR adds a version check to the corresponding if condition.

hainest · 2022-05-18T14:04:28Z

Good catch! I really wish I had better CI for this package. This will be fixed in #30627, but we should merge this as a stop-gap.

tldahlgren · 2022-05-18T16:49:24Z

var/spack/repos/builtin/packages/boost/package.py


        # If we are building context, tell b2 which backend to use
-        if '+context' in spec:
+        if '+context' in spec and spec.version >= Version('1.65.0'):


Based on how versions are checked pretty much everywhere else in the package, I recommend using spec.satisfies() syntax for these changes for consistency. See line 151 for example.

Actually, would this be better or simpler implemented as a conflict?

As far as I can tell, the problem is not +context but context-impl which are tied together.
Making the addition of context a conflict would also break the coroutine variant. See line 221 and 222.

Would the use of spec.satisfies() then look like the following?

if '+context' in spec and spec.satisfies('@1.65.0:'):

Sorry for the late reply.
The latest change makes the fix agnostic of the boost version by simply checking if the spec.variants dict contains context-impl.

In summary:

The context-impl variant is disabled if for boost versions < 1.65.0

The content of the context-impl variant is only read if it is contained within spec.variants

hainest · 2022-05-18T18:52:55Z

This PR does fix the problem, but we're silently hiding the issue. I agree with @tldahlgren, this should be made an explicit conflict.

$ spack install boost
...
==> boost: Successfully installed boost-1.79.0-adpxgb4djyo5jofn22zkbfjdgbyan2ro

$spack install boost+context
...
==> boost: Successfully installed boost-1.79.0-d623p6lo4fqo4cc6ntsnqq3wpkipq6n7

$ spack install boost~context
[+] ROOT/opt/spack/linux-ubuntu20.04-skylake/gcc-11.1.0/boost-1.79.0-adpxgb4djyo5jofn22zkbfjdgbyan2ro


$ spack install [email protected]~context
...
==> boost: Successfully installed boost-1.64.0-wahwvdbc2zoybjrlfl52hs3vrc7f35kw

$ spack install [email protected]+context   <--- THIS SHOULD NOT WORK
...
==> boost: Successfully installed boost-1.64.0-ideggoxhrecvdpz3njijbmqhpiisfhn4

$ spack install [email protected]
[+] ROOT/opt/spack/linux-ubuntu20.04-skylake/gcc-11.1.0/boost-1.64.0-wahwvdbc2zoybjrlfl52hs3vrc7f35kw

tldahlgren · 2022-06-02T16:34:22Z

This PR does fix the problem, but we're silently hiding the issue. I agree with @tldahlgren, this should be made an explicit conflict.

@mzeyen1985 Did you want to add an explicit conflict to this PR?

…onflict Executing spack solve [email protected] +context context-impl=fcontext triggers the following error message: ==> Error: No version for 'boost' satisfies '@1.63.0' and '@1.79.0' With this change, the error message becomes the following: ==> Error: Cannot set variant 'context-impl' for package 'boost' because the variant condition cannot be satisfied for the given spec

mzeyen1985 · 2022-06-27T15:14:59Z

This PR does fix the problem, but we're silently hiding the issue. I agree with @tldahlgren, this should be made an explicit conflict.

@mzeyen1985 Did you want to add an explicit conflict to this PR?

cba17ad adds an explicit conflict for the context-impl variant improving the error message issued by spack.

Adding a conflict for boost@:1.65.0 +context as suggested above would be inconsistent with the following:

spack/var/spack/repos/builtin/packages/boost/package.py

Line 219 in 5698846

conflicts('+context', when='@:1.50') # Context since 1.51.0.
spack/var/spack/repos/builtin/packages/boost/package.py

Line 222 in f09f834

conflicts('~context', when='+coroutine') # Coroutine requires Context.
spack/var/spack/repos/builtin/packages/boost/package.py

Line 225 in 5698846

conflicts('~context', when='+fiber') # Fiber requires Context.

tldahlgren · 2022-06-29T23:29:50Z

@hainest @mzeyen1985 Is this PR ready to merge?

hainest · 2022-07-01T01:52:26Z

@tldahlgren @mzeyen1985 This mostly solves the issue. I think the uncaught cases will be fixed by #30627. For example, it catches

$ spack install [email protected] context-impl=fcontext
  ==> Error: Cannot set variant 'context-impl' for package 'boost' because the variant condition cannot be satisfied for the given spec

but not

$ spack install [email protected]+context   <-- This is broken, but not the subject of this PR
    ==> boost: Successfully installed boost-1.64.0-wgstburpeafwrqwptu5hztcqdqxcjaw7
    [+] ROOT/spack/opt/spack/linux-ubuntu20.04-skylake/gcc-11.1.0/boost-1.64.0-wgstburpeafwrqwptu5hztcqdqxcjaw7

$ spack install [email protected] context-impl=fcontext
    [+] ROOT/spack/opt/spack/linux-ubuntu20.04-skylake/gcc-11.1.0/boost-1.64.0-wgstburpeafwrqwptu5hztcqdqxcjaw7

$ spack/bin/spack install [email protected] context-impl=ucontext
    [+] ROOT/spack/opt/spack/linux-ubuntu20.04-skylake/gcc-11.1.0/boost-1.64.0-wgstburpeafwrqwptu5hztcqdqxcjaw7

I think the reason here is that the concretizer is skipping the context-impl variant because the condition is :1.65.0 +context which isn't satisfied by the last two specs, even though +context was used to build the installed version. I will definitely add a test for this in my PR.

…pack#30719) * boost: do not access the context-impl variant for versions below 1.65.0 * boost: check if spec.variants contains context-impl * boost: improve error message when the context-impl variant causes a conflict Executing spack solve [email protected] +context context-impl=fcontext triggers the following error message: ==> Error: No version for 'boost' satisfies '@1.63.0' and '@1.79.0' With this change, the error message becomes the following: ==> Error: Cannot set variant 'context-impl' for package 'boost' because the variant condition cannot be satisfied for the given spec

spackbot-app bot added the update-package label May 18, 2022

spackbot-app bot requested a review from hainest May 18, 2022 12:18

mzeyen1985 force-pushed the boost/fix_context_impl_option branch from c5beddb to f466cbb Compare May 18, 2022 14:07

tldahlgren reviewed May 18, 2022

View reviewed changes

tldahlgren self-assigned this May 18, 2022

tldahlgren assigned hainest Jun 2, 2022

mzeyen1985 added 2 commits June 27, 2022 15:01

boost: do not access the context-impl variant for versions below 1.65.0

f864aac

boost: check if spec.variants contains context-impl

96f8f9a

mzeyen1985 force-pushed the boost/fix_context_impl_option branch from f466cbb to 96f8f9a Compare June 27, 2022 13:21

spackbot-app bot added the conflicts label Jun 27, 2022

hainest approved these changes Jul 1, 2022

View reviewed changes

hainest mentioned this pull request Jul 1, 2022

Boost: add conditional variants #30627

Closed

4 tasks

tldahlgren approved these changes Jul 1, 2022

View reviewed changes

tldahlgren merged commit 4365407 into spack:develop Jul 1, 2022

hainest mentioned this pull request Aug 12, 2022

Fix hipsycl to build on OSX. Patch boost to be backward-compatible. #31311

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boost: do not add the context-impl option for version below 1.65.0#30719

boost: do not add the context-impl option for version below 1.65.0#30719
tldahlgren merged 3 commits intospack:developfrom
mzeyen1985:boost/fix_context_impl_option

mzeyen1985 commented May 18, 2022

Uh oh!

hainest commented May 18, 2022

Uh oh!

tldahlgren May 18, 2022

Uh oh!

tldahlgren May 18, 2022 •

edited

Loading

Uh oh!

mzeyen1985 May 20, 2022

Uh oh!

tldahlgren May 26, 2022

Uh oh!

mzeyen1985 Jun 27, 2022

Uh oh!

hainest commented May 18, 2022 •

edited

Loading

Uh oh!

tldahlgren commented Jun 2, 2022

Uh oh!

mzeyen1985 commented Jun 27, 2022

Uh oh!

tldahlgren commented Jun 29, 2022

Uh oh!

hainest commented Jul 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mzeyen1985 commented May 18, 2022

Uh oh!

hainest commented May 18, 2022

Uh oh!

tldahlgren May 18, 2022

Choose a reason for hiding this comment

Uh oh!

tldahlgren May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzeyen1985 May 20, 2022

Choose a reason for hiding this comment

Uh oh!

tldahlgren May 26, 2022

Choose a reason for hiding this comment

Uh oh!

mzeyen1985 Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

hainest commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tldahlgren commented Jun 2, 2022

Uh oh!

mzeyen1985 commented Jun 27, 2022

Uh oh!

tldahlgren commented Jun 29, 2022

Uh oh!

hainest commented Jul 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tldahlgren May 18, 2022 •

edited

Loading

hainest commented May 18, 2022 •

edited

Loading