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

serialize: rename to rustc_serialize. #56323

Closed
wants to merge 1 commit into from

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Nov 28, 2018

In preparation for post-#49219 use of custom derives in rustc, we'll want to stop using Rustc{Encodable,Decodable} derives, and at least adapt the library to suit rustc better, before a full implementation of #36588 can be done.

This will break anyone using extern crate serialize; (on nightly) so let's do a crater run with a rename, just to see what things are like.

EDIT: I was going to test with rustc_serialize (which does work in terms of compiling) but I realized that rustc-serialize from crates.io might make that tricky if it ends up in our sysroot, so for the crater run let's go with something weirder (rustc_ezilaires).

EDIT2: had to remove rustc_save_analysis and rls to get rid of the crates.io rustc-serialize from Cargo.lock. Note that this is just for crater, just like the rustc_ezilaires rename.

EDIT3: cleaned up the PR, after crate run shows only one regression.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2018
@eddyb
Copy link
Member Author

eddyb commented Nov 28, 2018

@bors try

@bors
Copy link
Contributor

bors commented Nov 28, 2018

⌛ Trying commit 494fc3a009984ed6f9b91cd3c665cddcb23804e3 with merge 267cf972c7141a79f125d2b66cecd6ce43b7226a...

@eddyb
Copy link
Member Author

eddyb commented Nov 28, 2018

@rust-lang/infra Whenever nobody else needs crater, a check run would be nice.

@rust-highfive

This comment has been minimized.

@bors bors mentioned this pull request Nov 28, 2018
@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@bors bors 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 Nov 28, 2018
@eddyb
Copy link
Member Author

eddyb commented Nov 28, 2018

@bors try

@bors
Copy link
Contributor

bors commented Nov 28, 2018

⌛ Trying commit 8e10efa8683dfc0c255f0ab2d8efc43a5045dee7 with merge 5bdcd57ba31b39cdfb189318f63a9c55c13cb3b6...

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@eddyb
Copy link
Member Author

eddyb commented Nov 28, 2018

@bors try

@bors
Copy link
Contributor

bors commented Nov 28, 2018

⌛ Trying commit 2325917 with merge 2390276...

bors added a commit that referenced this pull request Nov 28, 2018
[WIP] serialize: rename to rustc_serialize.

In preparation for post-#49219 use of custom derives in rustc, we'll want to stop using `Rustc{Encodable,Decodable}` derives, and *at least* adapt the library to suit `rustc` better, before a full implementation of #36588 can be done.

This will break anyone using `extern crate serialize;` (on nightly) so let's do a crater run with a rename, just to see what things are like.

**EDIT**: I was going to test with `rustc_serialize` (which does work in terms of compiling) but I realized that `rustc-serialize` from crates.io might make that tricky if it ends up in our sysroot, so for the crater run let's go with something weirder (`rustc_ezilaires`).

**EDIT2**: had to remove `rustc_save_analysis` and `rls` to get rid of the crates.io `rustc-serialize` from Cargo.lock. Note that this is just for crater, just like the `rustc_ezilaires` rename.

r? @nikomatsakis
@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@eddyb
Copy link
Member Author

eddyb commented Nov 29, 2018

@bors try

@bors
Copy link
Contributor

bors commented Nov 29, 2018

⌛ Trying commit d84236bf123e1adfc8d8d64a6a0071039b16a554 with merge 32252761328f59ccfadb1bafb8148c685bc830b3...

@craterbot
Copy link
Collaborator

👌 Experiment pr-56323 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 29, 2018
@craterbot
Copy link
Collaborator

🚧 Experiment pr-56323 is now running on agent aws-1.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Nov 29, 2018

In preparation for post-#49219 use of custom derives in rustc,

OMG this is such an exciting concept

@craterbot
Copy link
Collaborator

🎉 Experiment pr-56323 is completed!
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 1, 2018
@eddyb
Copy link
Member Author

eddyb commented Dec 2, 2018

Only one regression from using serialize: mayer_multiple-0.0.1 (log).
Nothing crater checks appears to be using rustc_serialize from the sysroot, which is perfect.

@eddyb eddyb changed the title [WIP] serialize: rename to rustc_serialize. serialize: rename to rustc_serialize. Dec 2, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:08778aef:start=1543749022951712085,finish=1543749025413624378,duration=2461912293
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:58:13] 
[00:58:13] running 119 tests
[00:58:16] i..ii...iii..iiii.....i...i.........i..iii.............i.....i.....ii...i..i.ii..............i...ii. 100/119
[00:58:17] .ii.i.....iiii.....
[00:58:17] 
[00:58:17]  finished in 3.552
[00:58:17] travis_fold:end:test_codegen

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:58:32] 
[00:58:32] running 118 tests
[00:58:58] .iiiii...i.....i..i...i..i.i..i.i..i.....i..i....i..........iiii.........i.i....i...i.......ii.i.i.i 100/118
[00:59:02] ......iii.i.....ii
[00:59:02] 
[00:59:02]  finished in 29.602
[00:59:02] travis_fold:end:test_debuginfo

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@eddyb
Copy link
Member Author

eddyb commented Dec 2, 2018

(I need to either move the test to a run-make one that uses Cargo with the crates.io rustc-serialize dependency, or maybe mock it in the test locally)

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 4, 2018

📌 Commit d76e861 has been approved by nikomatsakis

@bors bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 4, 2018
@nikomatsakis
Copy link
Contributor

Er, @bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 4, 2018
@nikomatsakis
Copy link
Contributor

r=me but I see travis is unhappy?

(To confirm: this affects only nightly users, and we only found 1 of those on the crater run, right?)

@eddyb
Copy link
Member Author

eddyb commented Dec 4, 2018

@nikomatsakis Yeah, everyone else is using the out-of-tree rustc-serialize crate.
And I still need to fix #56323 (comment), so no r+ for me just yet.

@bors
Copy link
Contributor

bors commented Dec 6, 2018

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

@XAMPPRocky
Copy link
Member

Triage; @eddyb Hello, have you been able to get back to this PR?

@TimNN
Copy link
Contributor

TimNN commented Jan 22, 2019

Ping from triage @eddyb: What is the status of this PR?

@Dylan-DPC-zz
Copy link

ping from triage @eddyb Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again!

@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants