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

Improve thisdd4hep.sh setup scripts #745

Merged
merged 4 commits into from
Nov 19, 2020
Merged

Conversation

hansenjo
Copy link
Member

@hansenjo hansenjo commented Nov 13, 2020

BEGINRELEASENOTES

  • Improve path handling in setup scripts, by avoiding duplicates in paths. Also support whitespaces in paths and don't add system paths to library paths on macOS

ENDRELEASENOTES

@hansenjo hansenjo changed the title Improve thisdd4hep.sh setup scripts [WIP] Improve thisdd4hep.sh setup scripts Nov 16, 2020
@hansenjo
Copy link
Member Author

I'm checking why the CI checks are failing with this. The script works fine on my private machines.

@hansenjo hansenjo changed the title [WIP] Improve thisdd4hep.sh setup scripts Improve thisdd4hep.sh setup scripts Nov 17, 2020
@hansenjo
Copy link
Member Author

This seems to be working now in the CI environment as well. The PR is ready to be reviewed.

CMakeLists.txt Outdated
# ----for APPLE scripts have to set the DYLD_LIBRARY_PATH
if(APPLE)
set(USE_DYLD 1)
set(CMAKE_MACOSX_RPATH 1)
Copy link

Choose a reason for hiding this comment

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

set(CMAKE_MACOSX_RPATH 1) is set in cmake/DD4hepBuild.cmake along with all the rpath treatment. And USE_DYLD could be drooped entirely in favor of using the variable APPLE

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, that works. Requested changes made in commit d926c96

@petricm petricm merged commit 0fb59d3 into AIDASoft:master Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants