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 the remove TODO on Compilation.native_dirs #6194

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Oct 20, 2018

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

@rust-highfive
Copy link

r? @matklad

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

@alexcrichton
Copy link
Member

Hm it looks like there's travis failure?

@bors
Copy link
Contributor

bors commented Oct 20, 2018

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

@alexcrichton
Copy link
Member

@bors: delegate+

@bors
Copy link
Contributor

bors commented Oct 21, 2018

✌️ @dwijnand can now approve this pull request

@Eh2406
Copy link
Contributor

Eh2406 commented Oct 21, 2018

Is filter_dynamic_search_path now ded code?

@dwijnand
Copy link
Member Author

No, there's another usage.

@dwijnand dwijnand force-pushed the todo-remove-Compilation.native_dirs branch from 87d4afc to ab1bd24 Compare October 22, 2018 01:33
@dwijnand
Copy link
Member Author

The test failures are legitimate. How do we remove Compilation.native_dirs but not break the expected -L native=... args?

@alexcrichton
Copy link
Member

Ah we may just not be able to remove it at this time (as it seems to have lingering usage!)

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
@dwijnand dwijnand force-pushed the todo-remove-Compilation.native_dirs branch from ab1bd24 to b9f864e Compare October 23, 2018 08:57
@dwijnand dwijnand changed the title Remove Compilation.native_dirs Remove the remote TODO on Compilation.native_dirs Oct 23, 2018
@dwijnand
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Oct 24, 2018

📌 Commit b9f864e has been approved by dwijnand

@bors
Copy link
Contributor

bors commented Oct 24, 2018

⌛ Testing commit b9f864e with merge efb7972...

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
@dwijnand dwijnand changed the title Remove the remote TODO on Compilation.native_dirs Remove the remove TODO on Compilation.native_dirs Oct 24, 2018
@bors
Copy link
Contributor

bors commented Oct 24, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: dwijnand
Pushing efb7972 to master...

@bors bors merged commit b9f864e into rust-lang:master Oct 24, 2018
@dwijnand dwijnand deleted the todo-remove-Compilation.native_dirs branch October 24, 2018 13:43
@ehuss ehuss added this to the 1.31.0 milestone Feb 6, 2022
weihanglo added a commit to weihanglo/cargo that referenced this pull request Mar 23, 2024
`native_dirs` is still in use.

See< rust-lang#6194>
weihanglo added a commit to weihanglo/cargo that referenced this pull request Mar 23, 2024
`native_dirs` is still in use.

See <rust-lang#6194>
charmitro pushed a commit to charmitro/cargo that referenced this pull request Sep 13, 2024
`native_dirs` is still in use.

See <rust-lang#6194>
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.

7 participants