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

Use the cmake ExternalData module to manage test data #508

Merged
merged 5 commits into from
Nov 2, 2023

Conversation

jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented Oct 30, 2023

BEGINRELEASENOTES

  • Use the cmake ExternalData module to manage test data. This lets cmake take care of downloading the tests by hash so they can be version controlled. In addition, it's possible to set up a local store using -DExternalData_OBJECT_STORES=/path/to/store and it will download the test files there if they are not there but otherwise use them from there, so building from scratch won't download the test files again.

ENDRELEASENOTES

The downside is that the test files are now hashes since we have a very simple web server to retrieve them. And there is now a file for each test file with its hash, so changing a file means also the hash should be changed.

I wanted to have a look at this because I didn't like the fact that test files are always downloaded at configuration time and I'm happy now with the local store and will add this to the other key4hep repos with test files.

@jmcarcell
Copy link
Member Author

It also seems to be quite fast; since it downloads files as cmake targets I think it may be also doing downloads in parallel

@hegner
Copy link
Collaborator

hegner commented Nov 1, 2023

@jmcarcell - what's the compilation problem in the sanitizer?

@jmcarcell
Copy link
Member Author

jmcarcell commented Nov 1, 2023

I have no idea, there isn't an error message and it seems to be able to build everything. Maybe something transient? Nothing changes for how C++ files are built so it also doesn't make sense that it would fail with this PR. Maybe rerun the workflow?

@tmadlener
Copy link
Collaborator

@jmcarcell
Copy link
Member Author

If you click the link now it's fine and in my tests I have not yet seen that problem 🤷
I hashed every file that was on EOS so it has to work. By the way the SIO test files are not used in tests I think so they will never be downloaded.

@tmadlener
Copy link
Collaborator

Seems to have been transient.

Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks :)

@jmcarcell
Copy link
Member Author

Wait a bit before merging please because I may be able to change a bit how files are named on the EOS side... (so maybe there is a different path at the end)

@jmcarcell
Copy link
Member Author

Ok I think this is ready. The best we can seem to get is to have symbolic links in the same directory as the files (that is, everything in the same directory):

lrwxrwxrwx. 1 sftkeyhep def-cg   22 Nov  1 20:21 153ca8cc2f6110076c16319f425e214d -> v00-16-02-example.root
lrwxrwxrwx. 1 sftkeyhep def-cg   27 Nov  1 20:21 22504d7cd722cde81202c1bdfbe44308 -> v00-16-06-example_frame.sio
lrwxrwxrwx. 1 sftkeyhep def-cg   21 Nov  1 20:21 2b38723420c408517ab34449efdab8f6 -> v00-16-02-example.sio
-rw-r--r--. 1 sftkeyhep def-cg 3.4M Nov  1 20:19 v00-16-02-example.root
-rw-r--r--. 1 sftkeyhep def-cg  34K Nov  1 20:19 v00-16-06-example_frame.sio
-rw-r--r--. 1 sftkeyhep def-cg 3.4M Nov  1 20:19 v00-16-02-example.root

@tmadlener
Copy link
Collaborator

I think that is a pretty nice solution, especially because the symbolic links don't seem to show up in the web interface:
image

@tmadlener tmadlener merged commit 01e1d05 into AIDASoft:master Nov 2, 2023
17 checks passed
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.

3 participants