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

Declare repository dependencies in child WORKSPACE files #420

Closed
wants to merge 1 commit into from

Conversation

kchodorow
Copy link
Contributor

Bazel is moving to a model where child repository WORKSPACE files will be
parsed (see bazelbuild/bazel#1943). This means
that top-level WORKSPACE files won't have to declare every dependency
(yay!) but, as a part of this, child repositories will have to start
declaring the repositories they use.

This should be a no-op, though, since the top-level WORKSPACE file
overrides all of these repository definitions. So it doesn't really
matter what they are (hence the dummy paths).

Bazel is moving to a model where child repository WORKSPACE files will be
parsed (see bazelbuild/bazel#1943). This means
that top-level WORKSPACE files won't have to declare every dependency
(yay!) but, as a part of this, child repositories will have to start
declaring the repositories they use.

This should be a no-op, though, since the top-level WORKSPACE file
overrides all of these repository definitions.  So it doesn't really
matter what they are (hence the dummy paths).
@pmbethe09
Copy link
Contributor

So for every new_go_repository, you have to know what repos it depends on?

@kchodorow
Copy link
Contributor Author

Yes. Don't you have to now, anyway? They repository definitions just go in the main WORKSPACE file.

@pmbethe09
Copy link
Contributor

maintaining this 2-way list manually sounds hard

@pmbethe09
Copy link
Contributor

LGTM

@kchodorow
Copy link
Contributor Author

How are people doing this now? Just building, seeing what deps are missing, adding a new_go_repo, and rebuilding?

@pmbethe09
Copy link
Contributor

pretty much. there is a bare-bones tool in this repo 'wtool', which I use to add a new_go_repository, then build, then call wtool again for whatever was missing, etc.

@jayconrod
Copy link
Contributor

We're thinking of giving Gazelle more capabilities for managing the the WORKSPACE file. See #388 and #389. Even if it doesn't end up writing new dependencies to WORKSPACE, it at least needs to be able to read them. If they're hidden in other WORKSPACE files, I don't know how we'll do that. Bazel doesn't really provide any capabilities for querying external repositories, as far as I know.

Personally, I'd much rather have a flat WORKSPACE file.

{repo_deps}
""".format(
workspace_name = ctx.name,
repo_deps="\n".join(["local_repository(name='%s', path='/dummy')" % r for r in ctx.attr.repo_deps])))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using local_repository here doesn't make sense to me. Shouldn't Bazel report a conflict if you declare a local_repository in one place and, for example, a git_repository in another place? Seems like that should count as different versions.

Maybe there should be a special use_repository rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add a built-in "dummy" repository rule, if that works better for you guys.

@jayconrod
Copy link
Contributor

When are these declarations going to be mandatory in Bazel? I think we're going to have to improve our tooling to support this.

@kchodorow
Copy link
Contributor Author

If this is going to cause you guys a lot of issues, I might be able to get away with not having per-repo declarations as long as the main repo declares them. Let me think about it and see if I can implement that tomorrow...

@kchodorow
Copy link
Contributor Author

Seems like that will be do-able.

@kchodorow kchodorow closed this Apr 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants