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

Remote: do not create http repository if it already exists. #50

Merged
merged 1 commit into from
Jun 6, 2018

Conversation

damienmg
Copy link
Member

@damienmg damienmg commented Jun 4, 2018

Using native.existing_rules allow to use several crates.bzl file that
would import the same crates without failing with duplicate rules.

@acmcarther
Copy link
Member

Sync w/ master should fix your build (your other CL was merged).

Can you share a little bit more detail about the use case that motivated this change? I see that, mechanically speaking, it lets you combine several crates.bzl files. This sounds fine to me, but I'm also interested to hear what kind of situation led to needing several crates.bzl files....

Using native.existing_rules allow to use several crates.bzl file that
would import the same crates without failing with duplicate rules.
@damienmg
Copy link
Member Author

damienmg commented Jun 5, 2018

Rebased.

It's not something I encountered with raze yet but it something that happens all the time in other similar tools. The classic use case is I include libA and libB, both use cargo raze and both wrap the repositories in a bzl file exposed to the people. So you call liba_repositories() and llibb_repositories() and you get a collisions.

@acmcarther
Copy link
Member

I'm really nervous thinking about users having multiple cargo-raze resolved crate sets in the same workspace, but in reality this is just fixing a bug that someone will inevitably run into.

I'm thinking that ultimately "library-like" workspaces will need a way to express their third party crate dependencies, and a root workspace will need a mechanism to supply them. The prior art here isn't very encouraging.

@acmcarther acmcarther merged commit aadff8b into google:master Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants