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

Chapel patch and fix for main #3

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Chapel patch and fix for main #3

wants to merge 8 commits into from

Conversation

arezaii
Copy link
Owner

@arezaii arezaii commented Feb 11, 2025

Goal here is to fix bugs with main, incorporate patch for setting the python shared library rpath

@arezaii arezaii changed the title Chapel update prep Chapel patch and fix for main Feb 28, 2025
Copy link
Collaborator

@bonachea bonachea left a comment

Choose a reason for hiding this comment

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

Thanks for getting an early start on this!

Added a few initial comments

@@ -72,8 +72,8 @@ class Chapel(AutotoolsPackage, CudaPackage, ROCmPackage):

patch("fix_spack_cc_wrapper_in_cray_prgenv.patch", when="@2.0.0:")
patch("fix_chpl_shared_lib_path.patch", when="@2.1:2.2 +python-bindings")
patch("fix_chpl_shared_lib_path_2.3.patch", when="@2.2.1: +python-bindings")
patch("fix_chpl_line_length.patch")
patch("fix_chpl_shared_lib_path_2.3.patch", when="@2.2.1:2.3 +python-bindings")
Copy link
Collaborator

Choose a reason for hiding this comment

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

To ease future maintenance, I'd like to request comments here recording where each defect was fixed, as done for the patches below. i.e. something of the form:

Suggested change
patch("fix_chpl_shared_lib_path_2.3.patch", when="@2.2.1:2.3 +python-bindings")
patch("fix_chpl_shared_lib_path_2.3.patch", when="@2.2.1:2.3 +python-bindings") # PR 26388

and similarly for at least the two surrounding patch lines (the prior patch line is the same PR, I have not tried to track down the PR for the subsequent patch line)

@@ -72,8 +72,8 @@ class Chapel(AutotoolsPackage, CudaPackage, ROCmPackage):

patch("fix_spack_cc_wrapper_in_cray_prgenv.patch", when="@2.0.0:")
patch("fix_chpl_shared_lib_path.patch", when="@2.1:2.2 +python-bindings")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tested the updated patch against [email protected]?

Copy link
Owner Author

Choose a reason for hiding this comment

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

the python-bindings variant isn't actually available until 2.2, so maybe this can/should be revised to @2.1.1:2.2?

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this can/should be revised to @2.1.1:2.2?

Sounds reasonable to me.

I was mostly worried about validating the patch applies cleanly.

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