pull extra::{serialize, ebml} into a separate libserialize crate #11984

olsonjeffery · 2014-02-02T04:42:03Z

extra::json didn't make the cut, because of extra::json's required
dep on extra::TreeMap. If/when extra::TreeMap moves out of extra,
then extra::json could move into libserialize
libextra, libsyntax and librustc depend on the newly created
libserialize
The extensions to various extra types like DList, RingBuf, TreeMap
and TreeSet for Encodable/Decodable were moved into the respective
modules in extra
There is some trickery, evident in src/libextra/lib.rs where a stub
of extra::serialize is set up (in src/libextra/serialize.rs) for
use in the stage0 build, where the snapshot rustc is still making
deriving for Encodable and Decodable point at extra. Big props to
@huonw for help working out the re-export solution for this
@pcwalton's change in 449a7a8 didn't sneak back in

huonw · 2014-02-02T04:50:50Z

src/libserialize/lib.rs

+pub use self::serialize::{Decoder, Encoder, Decodable, Encodable,
+    DecoderHelpers, EncoderHelpers};
+
+pub mod serialize;


Because of the reexport, I think this should be private (assuming that you're reexporting everything).

sfackler · 2014-02-02T04:59:13Z

Should hex and base64 move here as well?

olsonjeffery · 2014-02-02T05:03:04Z

@sfackler not sure.. i was concentrating on all of the stuff that falls out of Decodable/Encodable, but I guess you could fairly include those in a more expansive defintion of serialization. It also occurred to me that this could be a home for stuff related to string encoding in rust. But maybe that's a bridge too far.

huonw · 2014-02-02T05:04:49Z

src/libserialize/lib.rs

+#[license = "MIT/ASL2"];
+#[allow(missing_doc)];
+#[forbid(non_camel_case_types)];
+#[feature(globs, managed_boxes)];


I think the standard thing for the extra dissolution is to attempt to remove feature(globs); is that possible here?

I'll take a whack at it.. in the morning!

alexcrichton · 2014-02-02T07:08:14Z

Nice work, thanks for doing this!

I also feel as if hex/base64 should be candidates for this crate, and perhaps the crate should be called libencode or libencoding? (just a thought)

omasanori · 2014-02-02T10:50:25Z

Why don't you separate serialize and ebml? I think we don't need to link to EBML or JSON library when we are developing another serialization library.

flaper87 · 2014-02-02T11:07:55Z

@omasanori yours is a good point. However, it also depends on how granular we want to make this extra explosion. It makes sense to group different serialization protocols under the same crate / namespace.

@alexcrichton libencode +1

omasanori · 2014-02-02T11:31:07Z

@flaper87 well, probably we don't want to see hundreds of libraries in mozilla/rust.

I hope the @rust-lang GitHub group will host non-fundamental official libraries (just like what @clojure does) but at this time we should think about the number of libraries in mozilla/rust... hmm...

huonw · 2014-02-02T11:50:31Z

I don't particularly see a reason to separate ebml etc. from libserialize: in a statically linked release build (when the bloat is relevant and at risk of being the worst) the unused code will be removed from the binary.

(Also, this needs a rebase.)

olsonjeffery · 2014-02-02T16:06:17Z

As stated above, I think extra::json should move out as soon as extra::treemap::Treemap leaves extra, as well.

As for the library rename: meh, whatever. I think libencode is loaded because it also implies all of the whatwg encoding ball of spaghetti (and there's already a rust-encoding community library) that we approached but never pulled the trigger on in the past (cc @SimonSapin @lifthrasiir ). So are we prepared to bring that stuff in now, or in the future?

I can bring in hex and base64 if that's the concensus. With that in mind, I would like there to be some boundaries around how long this PR goes on. These big library changes are a PITA to keep rebased.

olsonjeffery · 2014-02-02T16:10:52Z

I still need to remove globs, hopefully will knock that out before the SB.

alexcrichton · 2014-02-02T18:47:42Z

If you don't have the time, then it's perfectly fine to not move hex/base64 right now. I would think that the only thing blocking this from landing is feature(globs) (unless that's too painful to remove).

olsonjeffery · 2014-02-02T19:20:35Z

I have no issues adding them in a later PR. plus would like to start a converastion about data structures like TreeMap, so that I could move json out, too.

alexcrichton · 2014-02-02T19:31:42Z

I would expect libcontainer to come into existence which contains all the fun containers that we've come to have in libextra. "container" is a bit of a long name though...

olsonjeffery · 2014-02-03T14:18:09Z

rebased to deal w/ libterm merge conflict + fix for stage1 libsyntax

olsonjeffery · 2014-02-05T06:19:13Z

this should be ready to go.. r? @alexcrichton

olsonjeffery · 2014-02-05T08:09:52Z

not sure what this is about.. failure on freebsd.

compile_and_link: x86_64-unknown-freebsd/stage2/lib/rustlib/x86_64-unknown-freebsd/lib/librun_pass_stage2.so
warning: ignoring specified output filename for library.
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:16:5: 16:10 error: unresolved import. maybe a missing `extern mod extra`?
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:16 use extra::json;
                                                                                                     ^~~~~
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:16:5: 16:16 error: failed to resolve import `extra::json`
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:16 use extra::json;
                                                                                                     ^~~~~~~~~~~
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:17:5: 17:14 error: unresolved import. maybe a missing `extern mod serialize`?
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:17 use serialize::Decodable;
                                                                                                     ^~~~~~~~~
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:17:5: 17:25 error: failed to resolve import `serialize::Decodable`
/home/rustbuild/src/rust-buildbot/slave/auto-bsd-64-opt/build/src/test/run-pass/issue-4036.rs:17 use serialize::Decodable;
                                                                                                     ^~~~~~~~~~~~~~~~~~~~
error: aborting due to 4 previous errors
gmake: *** [x86_64-unknown-freebsd/stage2/lib/rustlib/x86_64-unknown-freebsd/lib/librun_pass_stage2.so] Error 101
program finished with exit code 2

Those lines are immediately preceeded by the extern mod lines... something in the env/build config?

alexcrichton · 2014-02-05T08:13:34Z

This is likely the check-fast suite, feel free to xfail-fast those tests.

@huonw

- `extra::json` didn't make the cut, because of `extra::json` required dep on `extra::TreeMap`. If/when `extra::TreeMap` moves out of `extra`, then `extra::json` could move into `serialize` - `libextra`, `libsyntax` and `librustc` depend on the newly created `libserialize` - The extensions to various `extra` types like `DList`, `RingBuf`, `TreeMap` and `TreeSet` for `Encodable`/`Decodable` were moved into the respective modules in `extra` - There is some trickery, evident in `src/libextra/lib.rs` where a stub of `extra::serialize` is set up (in `src/libextra/serialize.rs`) for use in the stage0 build, where the snapshot rustc is still making deriving for `Encodable` and `Decodable` point at extra. Big props to @huonw for help working out the re-export solution for this extra: inline extra::serialize stub fix stuff clobbered in rebase + don't reexport serialize::serialize no more globs in libserialize syntax: fix import of libserialize traits librustc: fix bad imports in encoder/decoder add serialize dep to librustdoc fix failing run-pass tests w/ serialize dep adjust uuid dep more rebase de-clobbering for libserialize fixing tests, pushing libextra dep into cfg(test) fix doc code in extra::json adjust index.md links to serialize and uuid library

@huonw

- `extra::json` didn't make the cut, because of `extra::json`'s required dep on `extra::TreeMap`. If/when `extra::TreeMap` moves out of `extra`, then `extra::json` could move into `libserialize` - `libextra`, `libsyntax` and `librustc` depend on the newly created `libserialize` - The extensions to various `extra` types like `DList`, `RingBuf`, `TreeMap` and `TreeSet` for `Encodable`/`Decodable` were moved into the respective modules in `extra` - There is some trickery, evident in `src/libextra/lib.rs` where a stub of `extra::serialize` is set up (in `src/libextra/serialize.rs`) for use in the stage0 build, where the snapshot rustc is still making deriving for `Encodable` and `Decodable` point at extra. Big props to @huonw for help working out the re-export solution for this - @pcwalton's change in 449a7a8 didn't sneak back in

…-pedantic, r=xFrednet Move `uninhabited_references` to `nursery` I think this lint has too many false positives and should be put in pedantic. See rust-lang#11984 and rust-lang#11985 for context. The lint is already in beta and is causing trouble for us, so I would also like this PR to be backported to beta as well. changelog: Moved [`uninhabited_references`] to `nursery` (Now allow-by-default) [rust-lang#11997](rust-lang/rust-clippy#11997) (Check if this has been backported) Fixes rust-lang#11984.

huonw reviewed Feb 2, 2014
View reviewed changes

bors closed this Feb 5, 2014

bors merged commit b8852e8 into rust-lang:master Feb 5, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pull extra::{serialize, ebml} into a separate libserialize crate #11984

pull extra::{serialize, ebml} into a separate libserialize crate #11984

olsonjeffery commented Feb 2, 2014

huonw Feb 2, 2014

sfackler commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

huonw Feb 2, 2014

olsonjeffery Feb 2, 2014

alexcrichton commented Feb 2, 2014

omasanori commented Feb 2, 2014

flaper87 commented Feb 2, 2014

omasanori commented Feb 2, 2014

huonw commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

alexcrichton commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

alexcrichton commented Feb 2, 2014

olsonjeffery commented Feb 3, 2014

olsonjeffery commented Feb 5, 2014

olsonjeffery commented Feb 5, 2014

alexcrichton commented Feb 5, 2014

pull extra::{serialize, ebml} into a separate libserialize crate #11984

pull extra::{serialize, ebml} into a separate libserialize crate #11984

Conversation

olsonjeffery commented Feb 2, 2014

huonw Feb 2, 2014

Choose a reason for hiding this comment

sfackler commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

huonw Feb 2, 2014

Choose a reason for hiding this comment

olsonjeffery Feb 2, 2014

Choose a reason for hiding this comment

alexcrichton commented Feb 2, 2014

omasanori commented Feb 2, 2014

flaper87 commented Feb 2, 2014

omasanori commented Feb 2, 2014

huonw commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

alexcrichton commented Feb 2, 2014

olsonjeffery commented Feb 2, 2014

alexcrichton commented Feb 2, 2014

olsonjeffery commented Feb 3, 2014

olsonjeffery commented Feb 5, 2014

olsonjeffery commented Feb 5, 2014

alexcrichton commented Feb 5, 2014