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

Enable LTO via cargo profile setting #2145

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Enable LTO via cargo profile setting #2145

merged 1 commit into from
Apr 4, 2024

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Apr 4, 2024

RUSTFLAGS results in
error: lto can only be run for executables, cdylibs and static library outputs

Upstream issue: rust-lang/cargo#6375

Fixes #2144

@link2xt link2xt requested a review from r10s April 4, 2024 16:36
@link2xt link2xt force-pushed the link2xt/cargo-lto branch from 2d303fb to 2a3c6c2 Compare April 4, 2024 16:42
@link2xt
Copy link
Contributor Author

link2xt commented Apr 4, 2024

This breaks the build even for core v1.136.6 somehow:

ld: warning: Could not find or use auto-linked framework 'CoreAudioTypes': framework 'CoreAudioTypes' not found
Undefined symbols for architecture x86_64:
  "_kSCNetworkInterfaceTypeIrDA", referenced from:
      system_configuration::network_configuration::SCNetworkInterfaceType::from_cfstring::hd83cc6f7010dfe05 in libdeltachat.a[x86_64][1433](system_configuration-1164d3f15fd9a555.system_configuration.ffd70cda82b4a369-cgu.0.rcgu.o)
ld: symbol(s) not found for architecture x86_64

EDIT: Enabled LTO in dev profile too, otherwise mullvad/system-configuration-rs#41 is triggered.

@link2xt link2xt marked this pull request as draft April 4, 2024 18:14
@link2xt link2xt force-pushed the link2xt/cargo-lto branch from 2a3c6c2 to 8c20501 Compare April 4, 2024 18:41
Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

thanks a lot.

this PR seems to be necessary but not sufficient to build current core main branch.

at #2144 we got one error further :)

@link2xt: if we stay with rust 1.77, i suggest to merge #2123 as well, this is also necessary but not sufficient :)

@link2xt link2xt marked this pull request as ready for review April 4, 2024 19:02
@link2xt
Copy link
Contributor Author

link2xt commented Apr 4, 2024

I have added CARGO_PROFILE_DEV_LTO=true and for me it works now, see "EDIT" in the post above.

For me main core builds now also with Rust 1.73.0 (until the breaking change that requires 1.77)

Enabling LTO via RUSTFLAGS results in
error: lto can only be run for executables, cdylibs and static library outputs
@link2xt link2xt force-pushed the link2xt/cargo-lto branch from 8c20501 to 476016b Compare April 4, 2024 19:09
@r10s
Copy link
Member

r10s commented Apr 4, 2024

I have added CARGO_PROFILE_DEV_LTO=true and for me it works now, see "EDIT" in the post above.

ah, i will try over

For me main core builds now also with Rust 1.73.0 (until the breaking change that requires 1.77)

what do you mean by that? did you try some special commit?

the original error of #2144 was fixed for me as well

@link2xt
Copy link
Contributor Author

link2xt commented Apr 4, 2024

what do you mean by that? did you try some special commit?

I tried commit 5bda4f0c2640f94cebd44b526cc1be40f94728f2, this is currently main^. Commit deltachat/deltachat-core-rust@2f0f247 (current main) is a merge of PR deltachat/deltachat-core-rust#5369 that requires Rust 1.77.

Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

yip, with core at 5bda4f0c2640f94cebd44b526cc1be40f94728f2 , build succeeds without warnings or errors! (testing on rust 1.77 already, on both, real-device and simulator, all fine)

@r10s
Copy link
Member

r10s commented Apr 4, 2024

@link2xt shall we do a core release at 5bda4f0c2640f94cebd44b526cc1be40f94728f2 ? might be handy for proceeding

@link2xt link2xt merged commit 21d4d08 into main Apr 4, 2024
@link2xt link2xt deleted the link2xt/cargo-lto branch April 4, 2024 19:42
@link2xt
Copy link
Contributor Author

link2xt commented Apr 4, 2024

@link2xt shall we do a core release at 5bda4f0c2640f94cebd44b526cc1be40f94728f2 ? might be handy for proceeding

Why not from the latest commit then, now that we have Rust 1.77?

@r10s
Copy link
Member

r10s commented Apr 4, 2024

Why not from the latest commit then, now that we have Rust 1.77?

that one did not build for me ... but let me double check, with all these changes PR and submodule checkouts one easily get confused. i'll file a new issue if things fail

@r10s
Copy link
Member

r10s commented Apr 4, 2024

i double checked, seems i messed sth up myself on last try, now current core main branch builds - in ci, in simulator and on real device, #2146

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.

cannot compile core main branch
2 participants