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

Reordered linking - plase don't merge blindly #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kkauder
Copy link
Collaborator

@kkauder kkauder commented Feb 28, 2023

I found while hacking up PEPSI by hand
https://gitlab.com/eic/mceg/pepsi/-/commit/84ca2fd52e8e0ff3a84da73c768a2eb12e1efdde
that repetition isn't enough.

It may actually not even be needed. If that's the case, l. 39,

set(NANOCERNLIB_LIBRARIES ${NANOCERNLIB_LIBRARIES} ${NANOCERNLIB_LIBRARIES})

can be deleted too.

But I don't have enough test cases to be comfortable just pulling this change in. Opinions?

@kkauder kkauder requested a review from sly2j February 28, 2023 19:01
@kkauder
Copy link
Collaborator Author

kkauder commented Feb 28, 2023

There's more to it, actually. I built BeAGLE using all four libraries and it quietly overrode my LHAPDF linking. I need to specifically only use -lnanocernlib_mathlib -lnanocernlib_packlib. Or changing the order with lhapdf probably. But either way, it should be a bit more fine-grained than cmake delivering all four in one lump.

@sly2j
Copy link
Owner

sly2j commented Mar 5, 2023

Maybe we could export some extra targets for those cases? I can't really test your changes (no time right now) but it does seem reasonable to me at first glance

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.

2 participants