Skip to content
This repository has been archived by the owner on Jun 21, 2019. It is now read-only.

Vendor sources into separate directories #96

Merged
merged 1 commit into from
Sep 3, 2018

Conversation

ljli
Copy link
Contributor

@ljli ljli commented Aug 12, 2018

I thought #59 could be addressed by keeping sources in separate directories and replacing them 1-to-1, instead of replacing all sources with a single merged source.
It seems to work so far. Currently it breaks most of the tests, if this is the way to go I'll fix the tests, incorporate any feedback and update the PR.

@alexcrichton
Copy link
Owner

Thanks for the PR! I wonder if perhaps we could still put everything into a vendor folder, but subfolders could be used for non-crates-io sources? Something like vendor/git-$host-$hash/$krate or something like that?

@ljli
Copy link
Contributor Author

ljli commented Aug 14, 2018

The scheme of this PR is vendor/${source-hash}/$krate. You propose to remove the ${source-hash} component for crates.io crates and to make it less opaque for non-crates.io crates by adding the source type and the host part of the url, right? I think that should work, too. The $host part will probably be git{hub,lab,...}.com most of the time, but still more info than before.

@alexcrichton
Copy link
Owner

Ah ok, I just think we by default want to stick with mostly the same layout as we have today to avoid lots of churn in vendored repos. This could also be behind a flag though to prevent that!

@ljli ljli changed the title [RFC] Vendor sources into separate directories Vendor sources into separate directories Aug 25, 2018
@ljli
Copy link
Contributor Author

ljli commented Aug 25, 2018

I updated the PR. It is possible to get the old behavior with the flag --merge-sources. The vendor folder structure looks like this vendor/{git,registry}-${source-hash}/$crate. It is no problem to switch between having --merge-sources and not having it, but nothing clever happens, the vendor folder is just deleted and repopulated.
The .cargo/config file that gets printed looks like this now:

[source.crates-io]
replace-with = "vendor+crates-io"

[source."https://github.com/rust-lang/libc"]
git = "https://github.com/rust-lang/libc"
rev = "add1a320b4e1b454794a034e3f4218f877c393fc"
replace-with = "vendor+https://github.com/rust-lang/libc"

[source."vendor+crates-io"]
directory = "/home/me/devel/cargo-vendor/target/tmp/test0/vendor/registry-40351f815f426200"

[source."vendor+https://github.com/rust-lang/libc"]
directory = "/home/me/devel/cargo-vendor/target/tmp/test0/vendor/git-4d8ca19d6a651200"

I made the new behavior the default for now, but I don't have a strong opinion on this and no insight into how many users checked vendor folders into version control, if you want another default I'll change it.

@alexcrichton
Copy link
Owner

Thanks! Could the old behavior be left as the default though? Perhaps a --no-merge-sources option instead? Could the README also document the flag and why it would be used?

@ljli ljli force-pushed the multi-sources branch 2 times, most recently from 8ec0e6a to f8ffcf6 Compare August 30, 2018 18:49
The flag --no-merge-sources activates the new behavior.
Vendoring Cargo projects with [replace] sections can
fail without this flag.
@ljli
Copy link
Contributor Author

ljli commented Aug 30, 2018

Could the old behavior be left as the default though? Perhaps a --no-merge-sources option instead?

Done

Could the README also document the flag and why it would be used?

Done

@alexcrichton alexcrichton merged commit 65a9e46 into alexcrichton:master Sep 3, 2018
@alexcrichton
Copy link
Owner

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants