Skip to content

Add support for Charm 7.0.1, switch container to it, and add Charm++ license+copyright #6050

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.

Sign up for GitHub

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

Jump to bottom
Merged
nilsdeppe merged 7 commits into from
Jun 3, 2024
Merged

Add support for Charm 7.0.1, switch container to it, and add Charm++ license+copyright #6050

nilsdeppe merged 7 commits into from
Jun 3, 2024

Conversation

@nilsdeppe
Copy link
Member

@nilsdeppe nilsdeppe commented Jun 1, 2024

Proposed changes

  • This maintains support for 7.0.0 since 7.0.1 is only a license change with no code changes. There's no reason to update the version on any clusters, etc. since the license is only relevant for distributing compiled executables and libraries. @kidder is working on Charm 8.0.0 support, which also has the new license, and we can delay updating Charm++ everywhere until that's released.
  • I have not yet pushed the container but tested it on my fork.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@nilsdeppe nilsdeppe requested a review from kidder June 1, 2024 01:10
kidder
kidder previously approved these changes Jun 1, 2024
View reviewed changes
knelli2
knelli2 previously approved these changes Jun 1, 2024
View reviewed changes
nilsvu
nilsvu previously approved these changes Jun 2, 2024
View reviewed changes
support/Charm/v7.0.1.patch
@@ -0,0 +1,101 @@
diff --git a/cmake/detect-features-cxx.cmake b/cmake/detect-features-cxx.cmake
Copy link
Member

@nilsvu nilsvu Jun 2, 2024

Choose a reason for hiding this comment

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

Are the patches just copies from 7.0.0?

Copy link
Member Author

@nilsdeppe nilsdeppe Jun 3, 2024

Choose a reason for hiding this comment

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

Yes, they are just exact copies, but because of the way we identify things, there's no real way around that. Maybe symlinks? But I don't know how well symlinks work in git repos...

Copy link
Member

@nilsvu nilsvu Jun 3, 2024

Choose a reason for hiding this comment

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

Nah the copy is fine

@nilsdeppe nilsdeppe dismissed stale reviews from nilsvu, knelli2, and kidder via e82b45a June 3, 2024 14:07
nilsvu
nilsvu requested changes Jun 3, 2024
View reviewed changes
.github/workflows/BuildDockerContainer.yaml Outdated
uses: docker/setup-qemu-action@v3
with:
platforms: arm64
build-args: PARALLEL_MAKE_ARG=-j4
Copy link
Member

@nilsvu nilsvu Jun 3, 2024

Choose a reason for hiding this comment

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

Delete this line but add the line twice in DemoContainer.yaml (for the build-push-action). Squash.

Copy link
Member Author

@nilsdeppe nilsdeppe Jun 3, 2024

Choose a reason for hiding this comment

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

Done! Thanks!

@nilsvu nilsvu enabled auto-merge June 3, 2024 15:45
@nilsvu nilsvu disabled auto-merge June 3, 2024 15:46
@nilsvu
Copy link
Member

nilsvu commented Jun 3, 2024 •
edited
Loading

@wthrowe did you want to look at this too? Edit: on vacation

@nilsvu nilsvu enabled auto-merge June 3, 2024 15:52
@nilsdeppe
Copy link
Member Author

nilsdeppe commented Jun 3, 2024

I believe @wthrowe is on vacation until Thursday and we would like this merged for the ETK workshop, which is this week. Kyle's presenting on Thursday. Given the time pressure, I'd like to retroactively address any comments @wthrowe may have.

@knelli2
Copy link
Contributor

knelli2 commented Jun 3, 2024

Yes thank you for the speedy reviews :)

@nilsdeppe nilsdeppe disabled auto-merge June 3, 2024 18:34
@nilsdeppe nilsdeppe merged commit bbfcae2 into sxs-collaboration:develop Jun 3, 2024
@nilsdeppe nilsdeppe deleted the charm_701 branch June 3, 2024 18:34
@nilsdeppe
Copy link
Member Author

nilsdeppe commented Jun 3, 2024

I'll submit a separate PR with the Ubuntu 18.04 support. I have that mostly working now for automated deployment...

@knelli2 knelli2 added ci/cd Continuous integration & deployment build system CMake build system labels Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nilsvu nilsvu nilsvu approved these changes

@kidder kidder kidder left review comments

@knelli2 knelli2 knelli2 left review comments

Assignees

No one assigned

Labels

build system CMake build system ci/cd Continuous integration & deployment

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nilsdeppe @nilsvu @knelli2 @kidder