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

Remove link attribute generation #1600

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Remove link attribute generation #1600

merged 2 commits into from
Sep 19, 2024

Conversation

eerii
Copy link
Contributor

@eerii eerii commented Sep 15, 2024

Follow up to #1475. Motivated by work at gtk-rs-core and gstreamer-rs to override library linking.

At the moment a #[link] attribute is being generated for every extern "C" code block. This is not generally needed since system-deps already handles linking libraries, and it makes overriding dependencies difficult from higher up crates.

The only exception is one edge case when using Windows and static constants, but that is a limitation of the compiler. To handle this case, allow to use custom build.rs scripts by not overriding the package field.

All of the context for this change and all of the alternatives we considered are explained in gtk-rs/gtk-rs-core#1508.

Changes

  • Remove #[link] attribute generation.
  • Allow custom build scripts. If the package.build key doesn't exist in Cargo.toml, generate it to be build.rs. Otherwise keep the existing value.

Caveats

Something to consider is whether removing the #[link] generation could affect other crates. From an overview of other crates relying on gir, it seems that they are not linking static constants, just functions, which do not explicitly need dllimport. If this is expected to be a problem, maybe a command line flag like --link could be added to the generator script that allows customizing this behaviour.

This reverts commit a8dc213.

Linking flags should be provided by the system-deps crate. Specifying
linking with the attribute creates issue when statically linking
system dependencies.
@pbor
Copy link
Contributor

pbor commented Sep 15, 2024

See also #1207

@eerii
Copy link
Contributor Author

eerii commented Sep 16, 2024

See also #1207

I missed that, but yes, a toggle like that would be very useful indeed, thank you for pointing it out! The custom build script part is still relevant because in some very specific cases you need to do some work to make Windows happy.

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.

4 participants