Skip to content

Implement infrastructure for the souped-up build command#792

Merged
bors merged 28 commits into
rust-lang:masterfrom
alexcrichton:build-cmd
Nov 5, 2014
Merged

Implement infrastructure for the souped-up build command#792
bors merged 28 commits into
rust-lang:masterfrom
alexcrichton:build-cmd

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

@alexcrichton

Copy link
Copy Markdown
Member Author

I've now added some hopefully extensive documentation about build scripts.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do the stability markers not work here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For now these are actually just markers of what to remove once we remove the old system of build cmd, not necessarily for deprecated functionality itself.

@steveklabnik

Copy link
Copy Markdown
Contributor

@alexcrichton doc review conducted as asked ❤️ Looks really great, just a couple of nits

Comment thread src/doc/build-script.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Side note: do we have any docs anywhere on target triples?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, not that I know of actually. That is... a bit sad though. I'll see if I can find some internet docs to link to.

@alexcrichton alexcrichton force-pushed the build-cmd branch 2 times, most recently from a4b5fb1 to ce7c253 Compare November 4, 2014 07:22
@alexcrichton

Copy link
Copy Markdown
Member Author

Updated with a re-worded code generation example and with lots of nits fixed (thanks @tomaka, @steveklabnik, and @huonw!)

I agree @steveklabnik that the last section is lacking. I think I ran out of steam once I reached that point, so I think I'll wait until morning to update it :)

@alexcrichton

Copy link
Copy Markdown
Member Author

Updated with an expanded "linking to system libraries" section. I'm still not super thrilled about it as I feel like it's just too overwhelming to show tons of code, but it's not super useful without a couple of code examples. It's at the end though and it's a case study so maybe it's not too bad?

@alexcrichton alexcrichton force-pushed the build-cmd branch 2 times, most recently from d259798 to c05442a Compare November 5, 2014 19:40
bors added a commit that referenced this pull request Nov 5, 2014
@bors bors merged commit 5e29a8b into rust-lang:master Nov 5, 2014
@alexcrichton alexcrichton deleted the build-cmd branch November 5, 2014 21:30
///
/// This is currently used to drive some entries which are added to the
/// LD_LIBRARY_PATH as appropriate.
// TODO: deprecated, remove

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this TODO correct, @alexcrichton? I.e. should I PR the removal of Compilation.native_dirs, or PR the removal of the TODO?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh dear that's a good question, this PR is... quite old though! I vaguely remember placing this on a few things throughout Cargo I've wished I deleted at the time and then never got to, but removing this would be good if it's not actually used in Cargo today! (or if it's actually used just remove the comment)

bors added a commit that referenced this pull request Oct 24, 2018
…=dwijnand

Remove the remote TODO on Compilation.native_dirs

It's still used, see tests:

* build_script::doctest_receives_build_link_args
* build_script::rename_with_link_search_path
* run::run_with_library_paths
* run::library_paths_sorted_alphabetically

Originally I was trying to action #792 (comment) with this PR
@ehuss ehuss mentioned this pull request Aug 4, 2019
bors added a commit that referenced this pull request Aug 5, 2019
Fix an old test.

This test was added in f888b4b, part of #792, in a commented state. Might as well make it work. It is a bit surprising that passing `-l nonexistinglib` doesn't cause an error, but seems to be fine.
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