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

Don't use run to run k4run #123

Merged
merged 1 commit into from
Jul 19, 2023
Merged

Don't use run to run k4run #123

merged 1 commit into from
Jul 19, 2023

Conversation

jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented Jul 19, 2023

BEGINRELEASENOTES

  • Remove run (autogenerated script to set up the environment) from some tests that use it together with k4run; we should set up the environment either in CMakeLists.txt or externally.

ENDRELEASENOTES

Either we set the environment externally or in CMakeLists.txt but don't use run and force some relative paths that may or may not be how it is built. I tested this and both test work fine

@andresailer
Copy link
Collaborator

Can you please elaborate more on the release notes?

@jmcarcell
Copy link
Member Author

jmcarcell commented Jul 19, 2023

Done. CI is failing but the two tests that this is changing succeeded using the key4hep nightlies

@tmadlener
Copy link
Contributor

who or what generates the run script? If it is no longer necessary can it be removed?

@jmcarcell
Copy link
Member Author

That comes from Gaudi: https://gitlab.cern.ch/gaudi/Gaudi/-/blob/master/cmake/GaudiToolbox.cmake#L1546
Probably people rely on this so it's not easy to have it removed. It doesn't hurt either, it's just there if we don't use it.
This is the only issue I found about this: https://gitlab.cern.ch/gaudi/Gaudi/-/issues/183

@jmcarcell jmcarcell merged commit e0a300b into master Jul 19, 2023
@jmcarcell jmcarcell deleted the remove-run branch July 19, 2023 19:53
@jmcarcell
Copy link
Member Author

Ahah now one the tests that succeeded in the checks in this PR and also after #114 fails, of course

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