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

Fix: Remove serialbox as permanent dep of ICON #1031

Merged
merged 3 commits into from
Nov 19, 2024
Merged

Conversation

jonasjucker
Copy link
Contributor

Serialbox has been a dependency of ICON even if serialization=none.

This was caused by the following lines that enforce same compilers for serialbox as for ICON:

    for x in serialization_values:
        depends_on('serialbox+fortran', when='serialization={0}'.format(x))
        # WORKAROUND: A build and link dependency should imply that the same compiler is used. This enforces it.
        depends_on('serialbox %nvhpc', when='%nvhpc')
        depends_on('serialbox %gcc', when='%gcc')

The two last lines falsely added serialbox as a dep, no matter the values the serialbox variant had.

The PR reorganises these lines to not add serialbox as a dep to ICON in case of serialization=none

@jonasjucker jonasjucker added the bug Something isn't working label Nov 18, 2024
@jonasjucker
Copy link
Contributor Author

launch jenkins

Copy link
Contributor

github-actions bot commented Nov 18, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-11-19 14:49 UTC

@jonasjucker
Copy link
Contributor Author

jonasjucker commented Nov 18, 2024

spack spec icon%nvhpc serialization=create
==> Warning: Missing dependency not in database: zlib/suuzrnc needs gmake-5slczkk
==> Error: Spack concretizer internal error. Please submit a bug report and include the command, environment if applicable and the following error message.
    icon%nvhpc serialization=create is unsatisfiable

EDIT: seemed to be a cache problem

@jonasjucker jonasjucker requested a review from huppd November 19, 2024 07:30
Copy link
Contributor

@huppd huppd left a comment

Choose a reason for hiding this comment

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

Looks good to me. But is this still a workaround?

@jonasjucker
Copy link
Contributor Author

Looks good to me. But is this still a workaround?

No this would be permanent. The package builds as it should.

@huppd
Copy link
Contributor

huppd commented Nov 19, 2024

Looks good to me. But is this still a workaround?

No this would be permanent. The package builds as it should.

But would it make sense to remove or adapt the workaround comment?

@jonasjucker
Copy link
Contributor Author

Ah now I get it.

Yes the workaround for the concretizer is still there.
It is needed, otherwise we might try to link a gcc serialbox with nvhpc icon

@huppd
Copy link
Contributor

huppd commented Nov 19, 2024

Ah now I get it.

Yes the workaround for the concretizer is still there. It is needed, otherwise we might try to link a gcc serialbox with nvhpc icon

OK, got it. thanks for the explanation.

@jonasjucker
Copy link
Contributor Author

launch jenkins

1 similar comment
@jonasjucker
Copy link
Contributor Author

launch jenkins

@jonasjucker jonasjucker merged commit 666babb into main Nov 19, 2024
4 checks passed
@jonasjucker jonasjucker deleted the fix_serialbox branch November 19, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants