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

Add link-flags-ld emit option. #31875

Closed
wants to merge 3 commits into from
Closed

Add link-flags-ld emit option. #31875

wants to merge 3 commits into from

Conversation

rillian
Copy link
Contributor

@rillian rillian commented Feb 25, 2016

Attempt to address #31471. r? @alexcrichton

  • Assumes unix-style ldflags.
  • Doesn't propagate OutputFilenames, so the default output filename doesn't work.
  • Wasn't sure where/how to add a test for this run-make and check if a file is produced?

@bors
Copy link
Contributor

bors commented Feb 25, 2016

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

@rillian
Move native artifact notification to a helper.
8308b0f
@rillian
Use an early return.
a6a9194 
If there's nothing to do, returning early saves an indentation
level and reduces the amount of state one needs to track reading
the code.
@rillian
Add link-flags-ld emit switch.
cdcc1a4 
Generate an unix-style ldflags line when building with
--crate-type staticlib --emit link-flags-ld.

If a filename is given, write the link line there for
later use by an external build system in linking the
static library. If none is given, just modify the
normal informative message to print the flags line
instead of the library list.

Issue rust-lang#31471.
@alexcrichton
Copy link
Member

Thanks @rillian! I think we may want to document this as well somewhere. I guess the best place may be... the man page? I don't think we have exhaustive docs in the reference for this right now...

Also in terms of tests, yeah we'll want to add some tests for this along the lines of:

  • Make sure the file is emitted
  • Make sure -o is respected
  • Make sure --emit foo=bar is respected
  • Ensure warnings aren't printed if this emit type is specified

etc

You should be able to add a test by just creating a directory in src/test/run-make, adding a Makefile, and then just doing whatever shell commands you need to run the tests. There should be plenty of examples in sibling tests.

cc @rust-lang/tools, this is also adding what would essentially be a new insta-stable emission type of the compiler.

@rillian
Copy link
Contributor Author

rillian commented Feb 26, 2016

Thanks for the feedback, @alexcrichton. I've rebased around the bit-rot. Will add tests and documentation.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with the tests + docs!

@BartMassey BartMassey mentioned this pull request Jul 20, 2016
Open
@rillian rillian mentioned this pull request Feb 27, 2017
Closed
@pnkfelix pnkfelix mentioned this pull request Jul 10, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rillian @bors @alexcrichton