Skip to content

Turn link-args into advanced-linking, add upstream crate linking and static linking #25685

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

Closed
wants to merge 5 commits into from

Conversation

aidanhs
Copy link
Member

@aidanhs aidanhs commented May 21, 2015

This is not complete (the most serious omission being not explaining the 'official' way to statically link (musl)) but I want to check I'm on the right lines for an appropriate format for the rust book.

@steveklabnik @alexcrichton

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@steveklabnik
Copy link
Member

lgtm

languages is important for Rust to achieve seamless interaction with native
libraries.

# Native libraries required by upstream crates
Copy link
Member

Choose a reason for hiding this comment

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

This seems like somewhat of an unusual section, could you elaborate a bit what's going on here? For example I would not expect us to recommend this, but rather contacting the author of the upstream crate to take care of these #[link] dependencies. A failure mode of this is that if the native library is included statically you'll likely receive link errors because the native lib is included before anything depends on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Would the specific examples you gave in the comment make it clearer?

Copy link
Member

Choose a reason for hiding this comment

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

Haha, I see :). That use case is a little flavorful, however, and I still have yet to convince myself that it's 100% correct, so I'd personally prefer to not document it as an "official way to do things". Dealing with native libraries is always an unfortunate burden to place on your downstream consumers, and pooling as much logic as possible in one library is always best practice.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, it's not something that should be done often (hence it being in advanced linking) but if 'normal' (i.e. not manually hacking around with linking) rust development could require it, surely it needs to be documented somewhere?

I could alternatively add it as a couple of sentences to the rust reference to elaborate on the example there - it was just a bit excessive to have to dig through commit logs to realise that #[link on an empty extern block is acceptable.

Copy link
Member

Choose a reason for hiding this comment

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

I think that something like this may best appropriately live in a reference-style piece of documentation rather than tutorial-like docs like the book.

@aidanhs aidanhs force-pushed the aphs-advanced-linking-doc branch from c98f9c4 to 7e334eb Compare June 15, 2015 17:54
@aidanhs
Copy link
Member Author

aidanhs commented Jun 15, 2015

Added extra notes on link-args as requested.
Replaced glibc static linking with musl, including instructions on how to build a musl-enabled rust.

@steveklabnik do you have any feelings on where the 'build-script' style readme for building musl should go? This is the only way of obtaining a musl-enabled rust at the moment and it's not a trivial process. Unfortunately, I'm not sure if it's the right style for the rust book.

Happy to put it in a github repo maintained by me if that's what you'd prefer.

@alexcrichton
Copy link
Member

Yeah I feel like the book may not be the right place for things like build instructions, but our old wiki location also doesn't exist so I'm not quite sure where they'd land up otherwise.

@retep998
Copy link
Member

No sections for advanced linking on Windows? 😞

@aidanhs
Copy link
Member Author

aidanhs commented Jun 21, 2015

Unfortunately I've not (yet) used rust on Windows so I've no idea what details might be useful.

@steveklabnik
Copy link
Member

Sorry about this, I missesd the updates for some reason.

Unfortunately, I'm not sure if it's the right style for the rust book.

Yeah, I'm not sure. Given that this is all in the nightly section, I'm fine with it being a bit odd. Because when this finally lands in stable, it will Just Work, right?

@aidanhs
Copy link
Member Author

aidanhs commented Jun 21, 2015

As soon as nightly linux builds start including musl I'd think we can delete all the build instructions because it's something you'd probably only need to do if you're trying to do dev on rust itself (and hopefully the build scripts would be available in an open repo for people to refer to).

I'm happy to leave this PR open until that happens and then remove the build instructions, I just didn't know how long it'd be and wanted to write it down somewhere.

There's a few more things that'd need to be fleshed out for stable (e.g. "if you're linking against external C libraries, you may need to build them with musl") but yes, pure rust programs should Just Work.

I have no idea if any of that answers your question 😄

required libraries and so don't need libraries installed on every system where
you want to use your compiled project. Rust libraries are statically linked by
default so you can use Rust-created binaries and libraries without installing
the Rust runtime everywhere. By contrast, native libraries are dynamically
Copy link
Member

Choose a reason for hiding this comment

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

This last sentence doesn't quite 100% agree with reality I think because the "Rust runtime" doesn't actually exist. I think this paragraph may want to be re-worded a bit to emphasize that Rust dependencies are statically linked by default but dependencies like libc/librt/libm are all dynamically linked, and this section is specifically about statically linking only the system dependencies.

@alexcrichton
Copy link
Member

It may be a bit to get MUSL nightlies ready to go and smooth integration with distributions in general, so I'm fine landing this ahead of that in the nightly section of the book as well.

@bors
Copy link
Collaborator

bors commented Jul 1, 2015

☔ The latest upstream changes (presumably #26696) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase and my comments addressed!

@aidanhs
Copy link
Member Author

aidanhs commented Jul 21, 2015

@alexcrichton rebased and tweaked based on comments. I've done a minor rephrasing of your comment for the 'justification' of why dynamic linking may not be desirable and removed all mention of the rust runtime.

One note: I'm not 100% clear on the definition of 'system' libraries (e.g. libpython compiled in my home dir?), so I opted for 'native' instead.

@alexcrichton
Copy link
Member

Hm looks like github hasn't picked up the changes, so perhaps a new PR?

@aidanhs
Copy link
Member Author

aidanhs commented Jul 21, 2015

Continued on #27193.

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 21, 2015
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 23, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 24, 2015
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.

6 participants