Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to rollback to the condition we had in RB-2.2 and do the same for CompilerFlags.
if (UNIX AND NOT CMAKE_SKIP_RPATH)
I originally was trying to have the Python build only ship with the required RPATH and nothing more but it turned out it created this bug and I also later realised that the wheel have a "repair" step at the end, which for macOS simply replace the RPATH entirely (as can be seen in GHA logs; I assume Linux is similar but the log is quieter) so these were probably not really needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Having a dummy change in the .github/workflows/wheel_workflow.yml might be nice to trigger the wheel build and make sure this doesn't have any negative impact).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KevinJW What do you think about simplifying the above as suggested in the earlier comment? Would be great to have this in the upcoming release @doug-walker is planning for this week.