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

Changed Docker image for GitLab CI pipeline, cmake tweaks #111

Closed
wants to merge 47 commits into from

Conversation

lobis
Copy link
Member

@lobis lobis commented Nov 9, 2021

lobis Large: 548

New image uses more recent build tools such as cmake 3.16 which is needed by #109. It should work as the previous one.

@juanangp
Copy link
Member

juanangp commented Feb 3, 2022

Pipeline is faling, I guess we should fix the cmake to find Gardfield.

@juanangp
Copy link
Member

juanangp commented Feb 3, 2022

I did a work around to avoid the Garfield cmake error, it worked but the pipeline still failing, looks like a compilation error.

@lobis
Copy link
Member Author

lobis commented Feb 3, 2022

I think I have fixed this issue, the problem with Garfield is solved but it breaks backwards compatibility with older versions of Garfield due to classes having been renamed. I think its fair to drop this old version (~5 years old) and stick with the "new" one. I guess we could maintain backwards compatibility with clever use of macros, if someone wants to make a commit.

The way Garfield is linked now is not the prettiest one, in fact, its pretty ugly. I tried to do it properly but couldn't due to the complex structure of our build. I think its time to refactor the cmake code. I will do this down the line.

Also in this PR we changed the CI Docker image to a newer one. This is the image I use for development and so far it has worked pretty well. I have automated the production of new tags so I can get a different C++ standard / ROOT or Geant4 version without much effort.

@lobis
Copy link
Member Author

lobis commented Feb 3, 2022

Now the pipeline has failed due to a dictionary problem. I will try to fix it. The problem seems to be in connectors lib, I can't find a solution so far, perhaps @jgalan has some ideas?

@jgalan
Copy link
Member

jgalan commented Feb 4, 2022

I have seen that problem before, but not 100% sure of what it is the origin. Perhaps it is in fact that Garfield pcm files are not accessible to the installation? They should be placed at $GARFIELD_INSTALL/lib, where $GARFIELD_INSTALL/lib is included at LD_LIBRARY_PATH.

Hope that helps. Perhaps @nkx has also some ideas.

@lobis lobis linked an issue Feb 10, 2022 that may be closed by this pull request
@lobis lobis changed the title Changed Docker image for GitLab CI pipeline Changed Docker image for GitLab CI pipeline, cmake tweaks Feb 10, 2022
…ck in GitLab actions. (but why doesn't this happen elsewhere)
@lobis
Copy link
Member Author

lobis commented Feb 16, 2022

It looks like commit 01f7d13 fixed the pipeline issue. In this commit I added a flag (-q) to quit instead of launching the root REPL. In principle this should not be needed, as the -q flag should be understood without explicitly defining it, and this seems to be the case up until now.

For some reason on the GitLab job this fails, but not when testing it manually on the Dockerfile. I have no idea why this is the case but this commit fixes it. I don't think this has any side-effects.

@@ -16,7 +16,6 @@ int main(int argc, char* argv[]) {
setenv("REST_VERSION", REST_RELEASE, 1);

Int_t loadMacros = 0;
bool quit = false;
Copy link
Member

Choose a reason for hiding this comment

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

why coming back to previous implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

this doesn't work when you run macros :( restRoot -q Macro.C.

Update image version
@lobis lobis self-assigned this Mar 8, 2022
@lobis lobis closed this Nov 10, 2022
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.

Remove FindROOT.cmake
3 participants