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.

Sign up for GitHub

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

Jump to bottom

Clarify documentation regarding +bundled #12238

Closed
wants to merge 3 commits into from
Closed

Clarify documentation regarding +bundled #12238

wants to merge 3 commits into from

Conversation

In-line
Copy link
Contributor

@In-line In-line commented Jun 7, 2023

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Jun 7, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 7, 2023
@In-line In-line changed the title Clarify documentation regarding bundled Clarify documentation regarding +bundled Jun 7, 2023
weihanglo
weihanglo reviewed Jun 7, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the pull request!

Should we put this in Rustc Book instead? From my understanding Cargo passes it as arg value of -l flag to rustc. Cargo itself doesn't nothing to the value you set.

cargo/src/cargo/core/compiler/mod.rs

Lines 492 to 494 in e07efd8

for name in output.library_links.iter() {
rustc.arg("-l").arg(name);
}

@In-line
Copy link
Contributor Author

In-line commented Jun 7, 2023

I think ideally it should be duplicated in both places, just because it's very confusing point. We had to debug for few weeks a linking issue at the company I'm currently working, just because someone forgot static in one place 😅 .

weihanglo
weihanglo reviewed Jun 7, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification!

Since it's more like a behavior of rustc, I'd like to see a PR landed in rustc first. Then we can confirm the behavior is correctly documented.

src/doc/src/reference/build-scripts.md
@@ -183,7 +183,11 @@ through the library target's public API.
The optional `KIND` may be one of `dylib`, `static`, or `framework`. See the
[rustc book][option-link] for more detail.

Note that if you don't specify the `KIND` when using `rustc-link-lib`, the default linkage modifiers won't be applied.
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could these lines respect line wraps in this markdown file?

@In-line
Copy link
Contributor Author

In-line commented Jun 7, 2023

I opened a PR in rustc: rust-lang/rust#112384

@weihanglo weihanglo added S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 7, 2023
sanmai-NL
sanmai-NL reviewed Dec 30, 2023
View reviewed changes
src/doc/src/reference/build-scripts.md
@@ -183,7 +183,11 @@ through the library target's public API.
The optional `KIND` may be one of `dylib`, `static`, or `framework`. See the
[rustc book][option-link] for more detail.

Note that if you don't specify the `KIND` when using `rustc-link-lib`, the default linkage modifiers won't be applied.
For instance, [`+bundled` linkage modifier][bundled-link] won't be used in conjunction with `static` by default.
Copy link

@sanmai-NL sanmai-NL Dec 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

..., the [+bundled ...

@weihanglo
Copy link
Member

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks

@weihanglo weihanglo closed this Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanmai-NL sanmai-NL sanmai-NL left review comments

@weihanglo weihanglo weihanglo left review comments

Assignees

@ehuss ehuss

Labels
A-documenting-cargo-itself Area: Cargo's documentation S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@In-line @rustbot @weihanglo @sanmai-NL @ehuss