Testing, dependencies via path, multiple repos

[dhardy]

#623 fails tests:

$ cargo test --package rand_core
error: There are multiple `rand_core` packages in your project, and the specification `rand_core` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  file:///home/dhardy/projects/rand/rand/rand_core#0.3.0                                                                               
  https://github.com/rust-lang/crates.io-index#rand_core:0.3.0                                                                         

@vks @newpavlov I seem to recall that you were advocates of multiple crates, though @newpavlov not of multiple repos... anyway, now we seem to have the following problem:

• Rand now depends on rand_xorshift and rand_pcg, housed in the small-rngs repo
• these crates cannot depend on rand_core via path since it is in a different repo
• in order to have a single version of rand_core while testing Rand (in order to only have a single copy of traits like SeedableRng) we need to depend on the release version of rand_core here too
• now we have multiple copies of rand_core in the tree, so cargo test --package rand_core is ambiguous

So, what's the best solution?

• we obviously can't use an absolute path for <spec> like above and I can't find an unambiguous relative version
• we could try making AppVeyor and Travis cd to the sub-crate directory before testing
• we could move rand_core to a new repo
• we could pull those crates which Rand depends on back into this repo

[newpavlov]

In RustCrypto I use roughly the following approach:

• Crates in master branches depend on published versions (i.e. usually I don't use path or git)
• When introducing a breaking change (which usually originate from traits repo) I create a new branch with [patch] section in a workspace Cargo.toml, so upstream dependencies use yet unpublished git version. (usually in a separate branch)
• When code with breaking changes is ready to be published, I first publish upstream dependencies, then remove [patch] section for downstream crates, if tests pass I merge PR and publish those downstream crates.
• Non-breaking changes are developed independently.

This is why I recommended to use repositories with multiple crates, as with one-repo-per-crate approach it will be significantly more burdensome to do the same.

[dhardy]

Okay, I just switched the dependencies to use published versions in any case.

If rand_core has breaking changes, probably we will publish soon, and can stick with published versions in other crates. I don't believe we will have to use unpublished versions (aside from local testing). In other crates we use as dependencies (like rand_pcg) I'm really not worried about breaking changes being an issue; we'd have no need to update dependent crates simultaneously.

So how do you propose we fix the tests? I see RustCrypto/hashes uses cargo test --verbose --all --release but we need to specify features (which doesn't work correctly with --all). See the error quoted above.

[newpavlov]

You can use a script similar to build_std.sh, but instead of the loop you could use explicit list instead. (BTW you of course do not need --release, in my case some hashes are too slow while having large test suits)

[dhardy]

Yes, a build script could be used, but it's not a tidy solution and needs a different implementation for AppVeyor (Windows tests; very important for StdRng at least).

I think the better option would be to pull rand_core out into a new repo.

[dhardy]

Well, extracting history via git subtree isn't hard: https://github.com/rust-random/rand_core
(This still needs CI setting up and a few links correcting.)

But, should we?

• advantage: we don't need confusing sub-branches like core-0.2 and tag names can be simplified
• disadvantage: we will no longer have all Rustdoc in one place
• test configuration becomes a little simpler, though more redundant
• tests will use release versions of dependencies everywhere, which is a little closer to published packages but makes coordinated changes across packages harder (should be rare anyway)

The alternative is that we scrap the small-rngs repo and keep everything here, or at least move rand_pcg (which is a dependency) here.

[newpavlov]

As I was talking before I prefer several repositories which contain one or several crates. (though instead of small-rngs I would prefer one repo for all PRNG impls) I think rand_core deserves its own repo.

disadvantage: we will no longer have all Rustdoc in one place

Personally I don't see much value in it (e.g. what is wrong with having docs per repo or even per crate?), but I think it should be possible to work around it.

test configuration becomes a little simpler, though more redundant

Additionally on a plus side, CI runs will be a bit faster.

[burdges]

I think rustdoc being disjointed makes rand significantly harder to use. Why not keep all crates used by rand inside one repo? Is it that you want a small-rngs that contains both stuff you do not maintain and stuff you want to depend on?

[dhardy]

I think rustdoc being disjointed makes rand significantly harder to use.

This is why I'm still undecided. There isn't a strong argument for using multiple crates, but in some ways it's cleaner.

[coltfred]

FWIW I think the rustdoc being all together is a big positive and if there isn't a strong argument to combat it, I'd vote for moving smallrng in.

[newpavlov]

@burdges @coltfred
Can you specify use-cases and why it's important for you to have rustdoc build docs for all rand crates? Considering that we can use docs.rs and tables like this. AFAIK many multi-crate projects do not bother with it and simply use virtual manifests. (e.g. see serde, vulkano, gfx-rs and many others)

[dhardy]

@newpavlov your comments are applicable to RNG crates like those in the small-rngs repo, but less so to rand_core which is an integral part of Rand. That said, since rand re-exports all key parts of rand_core, most of the relevant docs still are available in one place.

[burdges]

We're talking about html where cross links should be ubiquitous. We should not throw people into making guesses about paths from a table of contents like some printed book.

Ideally, a user viewing docs could click src, observe an item name like BlockRng, click back, enter the item name into the search bar, and then read about the item they just discovered. In particular, users need not do a manual binary search through some forest of use ..::* statements to figure out where an item originates.

[newpavlov]

Ideally, a user viewing docs could click src, observe an item name like BlockRng, click back, enter the item name into the search bar, and then read about the item they just discovered.

docs.rs already links traits and items from other crates. Lets take for example rand_pcg::Mcg128Xsl64, you can click on SeedableRng and RngCore and you immediately will get the relevant documentation. Absolutely no need for the dance around search bar.

[coltfred]

I agree with @burdges that being able to use the search bar for things someone has just discovered is a better experience. You're right that the links do help, but seeing all the out of the box SeedableRng implementations in this section is a better user experience than clicking around IMHO.

The desire to split them to lessen pain of the maintainers is completely understandable, but I was just making this point: The experience for the user is better if they're all in one doc location, if the downsides for the maintainers aren't too onerous.

[burdges]

I think the Implementations on Foreign Types section is compelling. It'd disappear if we split repos but it's essential to understanding the public interface.

There are valid concerns with exposing algorithm details from rand itself of course, but one can achieve this with individual algorithm crates inside the rand repository, and one or two opaque wrapper types.

We could RFC some "codependency" relationship among crates, but it's considerably easier for new contributors if all the code lives in one repository, so maybe only std needs fancy things like that.

[dhardy]

Very well. Lets bring rand_pcg into this repo and not move any of the other PRNG crates out (at least, not while they are still tightly linked to Rand, as now many are used for deprecations).

TODO: decide the fate of the rand_core and small-rngs repos (which are non-trivial to recreate though might not be used again).

[newpavlov]

Well, if you'll bring rand_pcg into this repo, then it will be logical to keep all other crates in the same repo as well. Though personally I don't like such organization and think that it will be more clunky.

[newpavlov]

BTW it's possible to use a separate repo for doc generation, see e.g. dalek-cryptography.

[dhardy]

That sounds like more maintenance overhead than help. Lets keep things simple...

[dhardy]

rand_core has been deleted (there was nothing there other than my rebase; I've still got this locally though it's not that hard to generate).

small-rngs has been archived.