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

Fix build with LLD #332

Merged
merged 1 commit into from Oct 19, 2023
Merged

Fix build with LLD #332

merged 1 commit into from Oct 19, 2023

Conversation

vimproved
Copy link
Contributor

@vimproved vimproved commented Sep 21, 2023

LLVM's LLD handles the -retain-symbols-file option (used by -export-symbols-regex in libtool) differently from GNU ld, causing undefined references during link. This commit removes the -export-symbols-regex option from libcalf_la_LDFLAGS since by default libtool exports all symbols anyway, so it should not be necessary.

Fixes #156.

vimproved added a commit to vimproved/gentoo that referenced this pull request Sep 22, 2023
Upstream-PR: calf-studio-gear/calf#332
Closes: https://bugs.gentoo.org/740158
Signed-off-by: Violet Purcell <vimproved@inventati.org>
vimproved added a commit to vimproved/gentoo that referenced this pull request Sep 22, 2023
Upstream-PR: calf-studio-gear/calf#332
Closes: https://bugs.gentoo.org/740158
Signed-off-by: Violet Purcell <vimproved@inventati.org>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Sep 22, 2023
Upstream-PR: calf-studio-gear/calf#332
Closes: https://bugs.gentoo.org/740158
Signed-off-by: Violet Purcell <vimproved@inventati.org>
Signed-off-by: Sam James <sam@gentoo.org>
LLVM's LLD handles the -retain-symbols-file option (used by
-export-symbols-regex in libtool) differently from GNU ld, causing
undefined references during link. This commit removes the
-export-symbols-regex option from libcalf_la_LDFLAGS since by default
libtool exports all symbols anyway, so it should not be necessary.

Fixes calf-studio-gear#156.

Signed-off-by: Violet Purcell <vimproved@inventati.org>
@JohannesLorenz
Copy link
Collaborator

Thanks. I will probably set up a new CI for the project, and as soon as it works with your PR, I will merge it.

@JohannesLorenz
Copy link
Collaborator

Hi again @vimproved . I am starting to maintain CALF, and before I merge PRs, I would like to set up a CI. I have it working for GCC already (branch github-actions). What do I need to do to test your PR? Only add CC=clang CXX=clang++ to the configure step? Because I did this and it compiled on master, which somehow contradicts the reason for this PR.

@vimproved
Copy link
Contributor Author

Hi again @vimproved . I am starting to maintain CALF, and before I merge PRs, I would like to set up a CI. I have it working for GCC already (branch github-actions). What do I need to do to test your PR? Only add CC=clang CXX=clang++ to the configure step? Because I did this and it compiled on master, which somehow contradicts the reason for this PR.

If your distro doesn't ship clang compiled to use LLD by default, then you'll need to also set LDFLAGS="-fuse-ld=lld"

@vimproved
Copy link
Contributor Author

(also, if you're looking at merging some PRs could you potentially make a new release? it would be helpful for gentoo's packaging to not have lingering backports.)

@JohannesLorenz
Copy link
Collaborator

If your distro doesn't ship clang compiled to use LLD by default, then you'll need to also set LDFLAGS="-fuse-ld=lld"

Thanks, that indeed made it fail and if I used the CI like this, your PR indeed fixed the issue. I will make some more CI tests and then soon merge your PR.

(also, if you're looking at merging some PRs could you potentially make a new release? it would be helpful for gentoo's packaging to not have lingering backports.)

I can look after it. FWIR, ebuilds are inside the gentoo repo, so there are no artifacts we need to produce for gentoo? If you have any experience with artifacts for distros, it would be cool if you could name them in #335 .

@JohannesLorenz JohannesLorenz merged commit f6c6aae into calf-studio-gear:master Oct 19, 2023
@JohannesLorenz
Copy link
Collaborator

@vimproved I found another occurrence of -export-symbols-regex in src/Makefile.am for the UI descriptor. The new cmake based CI seems to work without it (Linux Gcc, Linux Clang, Mac, Windows), but I am not sure if the CI covers all potential issues. Can you please judge if this is safe to remove, and if yes, submit another PR for src/Makefile.am?

@vimproved
Copy link
Contributor Author

@vimproved I found another occurrence of -export-symbols-regex in src/Makefile.am for the UI descriptor. The new cmake based CI seems to work without it (Linux Gcc, Linux Clang, Mac, Windows), but I am not sure if the CI covers all potential issues. Can you please judge if this is safe to remove, and if yes, submit another PR for src/Makefile.am?

Sure.

@JohannesLorenz
Copy link
Collaborator

@vimproved I just pushed #343 to remove that flag. Please feel free to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning: Linking the executable calfmakerdf against the loadable module
2 participants