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

Remove in-tree flate/getopts crates #42664

Merged
merged 2 commits into from
Jun 21, 2017
Merged

Remove in-tree flate/getopts crates #42664

merged 2 commits into from
Jun 21, 2017

Conversation

alexcrichton
Copy link
Member

Remove src/libflate in favor of flate2 on crates.io and src/libgetopts in favor of getopts on crates.io. The replacements have slightly different APIs and the usage in the compiler has been updated to reflect this.

This uncovered an unfortunate limitation of the compiler today to deal with linking everything correctly, and the workaround can be found documented in src/librustc/Cargo.toml.

@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@Mark-Simulacrum
Copy link
Member

We can also remove miniz.c from the COPYRIGHT file, I think.

@aidanhs
Copy link
Member

aidanhs commented Jun 15, 2017

Travis failed:

[00:49:58] make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
[00:49:58] error[E0464]: multiple matching crates for `libc`
[00:49:58]   --> foo.rs:15:1
[00:49:58]    |
[00:49:58] 15 | extern crate libc;
[00:49:58]    | ^^^^^^^^^^^^^^^^^^
[00:49:58]    |
[00:49:58]    = note: candidates:
[00:49:58]    = note: path: /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-6545a7b8b89ca177.rlib
[00:49:58]    = note: crate name: libc
[00:49:58]    = note: path: /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-6ecacccb5bdc4911.rlib
[00:49:58]    = note: crate name: libc
[00:49:58] 
[00:49:58] error[E0463]: can't find crate for `libc`
[00:49:58]   --> foo.rs:15:1
[00:49:58]    |
[00:49:58] 15 | extern crate libc;
[00:49:58]    | ^^^^^^^^^^^^^^^^^^ can't find crate
[00:49:58] 
[00:49:58] error: aborting due to previous error(s)
[00:49:58] 
[00:49:58] make[1]: *** [all] Error 101

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 15, 2017
@alexcrichton
Copy link
Member Author

Should be updated now

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2017
@bors
Copy link
Contributor

bors commented Jun 16, 2017

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

@alexcrichton
Copy link
Member Author

Rebased

@eddyb
Copy link
Member

eddyb commented Jun 16, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2017

📌 Commit 9881c23 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Jun 17, 2017

⌛ Testing commit 9881c23 with merge 21bb1c7...

@bors
Copy link
Contributor

bors commented Jun 17, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

I think we might be trying to put one of these crates into the rust-src component.

[00:02:43] thread 'main' panicked at 'fs::read_dir(src) failed with No such file or directory (os error 2)', /checkout/src/bootstrap/util.rs:82

@frewsxcv frewsxcv added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 17, 2017
@alexcrichton
Copy link
Member Author

@bors: r=eddyb

@bors
Copy link
Contributor

bors commented Jun 17, 2017

📌 Commit f53f838 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Jun 18, 2017

⌛ Testing commit f53f8387ae21bd786194927d1ba9ace1e14855e9 with merge 864ee9a733032b38db6755cb511b7733556b88fd...

@bors
Copy link
Contributor

bors commented Jun 18, 2017

💔 Test failed - status-appveyor

@bors
Copy link
Contributor

bors commented Jun 18, 2017

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

@bors
Copy link
Contributor

bors commented Jun 20, 2017

📌 Commit 2218774 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Jun 20, 2017

⌛ Testing commit 22187747ac777ec7e6554bc0a421560bc5e22495 with merge 67aeea3f6f38497f18bcb11e08166f82a5f7b778...

@bors bors mentioned this pull request Jun 20, 2017
Merged
@bors
Copy link
Contributor

bors commented Jun 20, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Jun 20, 2017 

error: use of unstable library feature 'rustc_private': this crate is being loaded from the sysroot, and unstable location; did you mean to load this crate from crates.io via `Cargo.toml` instead? (see issue #27812)
  --> src\tools\compiletest\src\main.rs:21:1
   |
21 | extern crate getopts;
   | ^^^^^^^^^^^^^^^^^^^^^
   |
   = help: add #![feature(rustc_private)] to the crate attributes to enable

@alexcrichton
Switch to the crates.io getopts crate
5c3d0e6 
This commit deletes the in-tree `getopts` crate in favor of the crates.io-based
`getopts` crate. The main difference here is with a new builder-style API, but
otherwise everything else remains relatively standard.
@alexcrichton
Copy link
Member Author

@bors: r=eddyb

@bors
Copy link
Contributor

bors commented Jun 20, 2017

📌 Commit 5c3d0e6 has been approved by eddyb

@arielb1 arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 20, 2017
@bors
Copy link
Contributor

bors commented Jun 21, 2017

⌛ Testing commit 5c3d0e6 with merge 37a991a...

bors added a commit that referenced this pull request Jun 21, 2017
@bors
Auto merge of #42664 - alexcrichton:moar-crates, r=eddyb
37a991a 
Remove in-tree flate/getopts crates

Remove `src/libflate` in favor of `flate2` on crates.io and `src/libgetopts` in favor of `getopts` on crates.io. The replacements have slightly different APIs and the usage in the compiler has been updated to reflect this.

This uncovered an unfortunate limitation of the compiler today to deal with linking everything correctly, and the workaround can be found documented in `src/librustc/Cargo.toml`.
@bors
Copy link
Contributor

bors commented Jun 21, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 37a991a to master...

@bors bors mentioned this pull request Jun 21, 2017
Closed
2 tasks
@bors bors merged commit 5c3d0e6 into rust-lang:master Jun 21, 2017
This was referenced Jun 21, 2017
Merged
Closed
Merged
@oyvindln
Copy link
Contributor

oyvindln commented Jun 22, 2017 •
edited
Loading

A bit late here, maybe I should have made an issue. Is using the "Default" compression level intended? the flate functions were changed to use a faster compression level some time ago (see #37298), going back to a higher compression setting may cause some performance regressions.

Also, the old functions didn't use a zlib wrapper from what I can see, but the new code seems to. Don't know if this could cause some issues when trying to use libs from different versions, though at least it adds 6 bytes of overhead due to the header and checksum.

@Mark-Simulacrum
Copy link
Member

I'd file a new issue with those concerns (they seem good to address to me) since merged/closed PRs are low-visibility locations.

@oyvindln oyvindln mentioned this pull request Jun 24, 2017
Closed
@alexcrichton alexcrichton deleted the moar-crates branch June 24, 2017 17:26
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Mar 23, 2018
@Mark-Simulacrum
Remove getopts leftover from tree
700fd5a 
This was attempted but left incomplete in PR rust-lang#42664, where only the toml
file was removed.
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Mar 23, 2018
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 23, 2018
@alexcrichton
Rollup merge of rust-lang#49314 - Mark-Simulacrum:remove-getopts, r=a…
b928781 
…lexcrichton

Remove getopts leftover from tree

This was attempted but left incomplete in PR rust-lang#42664, where only the toml
file was removed.

cc @alexcrichton
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
@kennytm
Rollup merge of rust-lang#49314 - Mark-Simulacrum:remove-getopts, r=a…
8d2792e 
…lexcrichton

Remove getopts leftover from tree

This was attempted but left incomplete in PR rust-lang#42664, where only the toml
file was removed.

cc @alexcrichton
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
@kennytm
Rollup merge of rust-lang#49314 - Mark-Simulacrum:remove-getopts, r=a…
3d68959 
…lexcrichton

Remove getopts leftover from tree

This was attempted but left incomplete in PR rust-lang#42664, where only the toml
file was removed.

cc @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@eddyb eddyb

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@alexcrichton @rust-highfive @Mark-Simulacrum @aidanhs @bors @eddyb @kennytm @oyvindln @frewsxcv @arielb1