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

New CMake ROOT usage / Minor feature in libraries / Pipeline image change #145

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

lobis
Copy link
Member

@lobis lobis commented Mar 5, 2022

Large lobis 511

Hello @rest-for-physics/core_dev,

This pull request only changes the way we include ROOT on the CMake, since I need to link to a new root library and could not do it otherwise.

This change is also present in #111 but since its not yet able to merge I reimplement it here so that my other changes can pass the pipeline.

Same branch pull requests in other libraries:
rest-for-physics/geant4lib#36
rest-for-physics/detectorlib#34
rest-for-physics/restG4#30

I also changed the docker image for all the repositories mentioned for a new one, which has updated ROOT and Garfield. There are also multiple tags of this image which contain more recent versions of Geant4. I think I finally got the image working and running the pipeline correctly. Previously it was throwing very strange errors but I solved them by upgrading Garfield to the latest commit.

@lobis lobis requested review from jgalan and nkx111 March 5, 2022 19:27
@lobis lobis changed the title New CMake ROOT usage New CMake ROOT usage / Minor feature in libraries Mar 6, 2022
@lobis lobis changed the title New CMake ROOT usage / Minor feature in libraries New CMake ROOT usage / Minor feature in libraries / Pipeline image change Mar 7, 2022
@lobis lobis mentioned this pull request Mar 7, 2022
@jgalan
Copy link
Member

jgalan commented Mar 7, 2022

Why #111 is not yet able to merge? The pipeline is green, and there are no conflicts. Looks like the problem was solved there?

Copy link
Member

@jgalan jgalan left a comment

Choose a reason for hiding this comment

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

These updates will update the way cmake compilation system is finding ROOT with the official ROOT macros. Pipeline is passing, so it seems fine to me to merge.

@lobis
Copy link
Member Author

lobis commented Mar 7, 2022

Why #111 is not yet able to merge? The pipeline is green, and there are no conflicts. Looks like the problem was solved there?

The pipeline wasn't passing until I updated the docker image. I will merge this code first since its most recent and then that one in case there are any problems, there will be conflicts for sure.

@lobis
Copy link
Member Author

lobis commented Mar 7, 2022

https://gitlab.cern.ch/rest-for-physics/restg4/-/jobs/20061461 pipeline at restG4 at https://gitlab.cern.ch/rest-for-physics/restg4/-/commit/88ce6267fe0c59e892b98b24f911774faacf3595 is failing because its needs XMLIO root library. @jgalan @nkx111 merging #145 will fix this issue.

@lobis lobis merged commit 2b04de4 into master Mar 8, 2022
@lobis lobis mentioned this pull request Mar 8, 2022
@lobis lobis deleted the lobis-geometry-info branch June 30, 2022 09:25
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.

None yet

3 participants