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

Spack-solvable versions, add tests to Spack workflow #2156

Merged
merged 50 commits into from
Aug 7, 2023

Conversation

brenthuisman
Copy link
Contributor

@brenthuisman brenthuisman commented Jul 19, 2023

Changes

  1. Spack interprets version info consisting of 2-tuples as ranges, leading to v0.8 being uninstallable if there has been a point release (v0.8.1).
  2. Change Spack CI to use latest release and develop branch, because the former is how Arbor is rolled out in various centers, and thus should be verified to work, and the latter lets us know of upcoming issues with dependencies.
  3. Changed Spack CI to not use build_spack_package.sh. This script does everything in /tmp, and Github Actions appear to be wiping /tmp between steps, which means breaking up the run into steps is difficult. Breaking up is desirable, because then one can see whether building or testing breaks.
  4. Run Python tests after building the Spack package.
  5. Also use Spack-provided deps when Spack is instructed to build master, not submodules.
  6. Tiny fix for the ciwheelbuild failure.

Future issues

Out of scope for now, but identified as problems:

  1. Currently, Spack cannot execute the unit tests, because the Google-Test package isn't built and Arbor doesn't look for it. @boeschf is looking at the latter: making it possible to build with an external Google Test.
  2. The former is another problem: Spack does not or not always (it's not clear) concretize test dependencies. This is an open issue with low priority: spack install --test root not working properly in environment spack/spack#29447
    This mean that despite oru package.py properly including depends_on("[email protected]", type="test", when="@0.7.1:"), Spack does not build it, at least not in the dev-build workflow as we currently do. And it isn't clear what an always working workaround is, see issue. (Spack can't display the deps for the test variant: spack spec/solve include option for --test dependencies spack/spack#23586 spack spec -I arbor doesn't show Google Test, and neither does spack dependencies --deptype test arbor.)
    • Blocked by upstream.

@brenthuisman brenthuisman marked this pull request as ready for review July 28, 2023 13:16
@brenthuisman brenthuisman requested a review from a team July 28, 2023 13:56
@brenthuisman brenthuisman added this to the v0.9 milestone Jul 31, 2023
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

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

Great, some wording though (and I am scared of curl | bash)

.github/workflows/spack.yml Outdated Show resolved Hide resolved
doc/contrib/release.rst Outdated Show resolved Hide resolved
thorstenhater
thorstenhater previously approved these changes Jul 31, 2023
doc/contrib/release.rst Outdated Show resolved Hide resolved
thorstenhater
thorstenhater previously approved these changes Aug 2, 2023
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

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

🎉

Merge remote-tracking branch 'upstream/master' into spack_improvements
Copy link
Contributor

@thorstenhater thorstenhater left a comment

Choose a reason for hiding this comment

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

👍

@brenthuisman brenthuisman merged commit 7d36beb into arbor-sim:master Aug 7, 2023
26 checks passed
@brenthuisman brenthuisman deleted the spack_improvements branch August 7, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants