git_repository and new_git_repository vs http_archive

[Xavier Bonaventura]

Dear Bazel devs,

When teaching Bazel, one of the challenges I find is to explain why there is a new_git_repository but there is no new_http_archive.

http_archive has the option to give the BUILD/WORKSPACE file as optional, in an intuitive way, someone would expect the same for git_repository.

Is there any good reason why to have git_repository and new_git_repository instead of having just a single git_repository where BUILD/WORKSPACE files are optional?

I took a look at the implementation and at a first glance does not look like it should be complicated to do.

Any thoughts on that? Would the Bazel team be in favor of getting rid of new_get_repository and make git_repository consistent with http_archive?

Note: new_http_archive was existing in the past, it just got merged to http_archive. I wonder if the fact that it was not done the same with new_git_repository was intentional or not.

Thanks

Xavi

[Tony Aiuto]

On Sun, Aug 7, 2022 at 4:02 AM Xavier Bonaventura <xavibon...@gmail.com> wrote:

Dear Bazel devs,

When teaching Bazel, one of the challenges I find is to explain why there is a new_git_repository but there is no new_http_archive.

http_archive has the option to give the BUILD/WORKSPACE file as optional, in an intuitive way, someone would expect the same for git_repository.

Is there any good reason why to have git_repository and new_git_repository instead of having just a single git_repository where BUILD/WORKSPACE files are optional?

 

I took a look at the implementation and at a first glance does not look like it should be complicated to do.

Any thoughts on that? Would the Bazel team be in favor of getting rid of new_get_repository and make git_repository consistent with http_archive?

I just looked at the code too.  I don't see any reason why they could not merge into one.

If you are interested in doing the fix, please ask for aiuto@ as one of the reviewers. I am about

to splice some new features into http_archive and I'll probably have to do git as well.

FWIW, the new feature is based on the needs discussed here. https://docs.google.com/document/d/1PkSk1fHcLNYJX9fk8cEMKueMvrpt22QcVIwcTICCEIk/edit#heading=h.axixaba6fll1

It adds the ability to interpose a workspace global rewrite tool after patch and before the repo is finalized. 

For example, you could add a tool that scans all the source files in external repos

and fails out if any of them contain a particular kind of license text.

 

Note: new_http_archive was existing in the past, it just got merged to http_archive. I wonder if the fact that it was not done the same with new_git_repository was intentional or not.

That's what we can figure out in the bug discussion. I did a quick look at the commit log and

I did not see any reasons why not. But, I did not look back to the first PR and find out if there

was a reason we made a new rule name in the first place.

Thanks

Xavi

[Xavier Bonaventura]

Thanks for the feedback Tony. Not sure when I will have time to work on it.
For the moment to not forget and see what the CI says about it, I created a draft PR with only the changes in the .bzl file.

https://github.com/bazelbuild/bazel/pull/16064

If someone wants to take it over, feel free. If not, I will try to make it before Bazel 6.

Bests,

Xavi

[Julian Amann]

BTW: Would be nice if the http_archive would create a "BUILD.bazel" file instead of a "BUILD" file. Would help to prevent name collisions. For instance, Boost contains a non-Bazel-related BUILD file:

maybe(

   http_archive,

   name = "boost",

   build_file = "@com_github_nelhage_rules_boost//:BUILD.boost",

   patch_cmds = ["rm -f doc/pdf/BUILD"],

   patch_cmds_win = ["Remove-Item -Force doc/pdf/BUILD"],

   sha256 = "4b2136f98bdd1f5857f1c3dea9ac2018effe65286cf251534b6ae20cc45e1847",

   strip_prefix = "boost_1_80_0",

   urls = [

     "https://boostorg.jfrog.io/artifactory/main/release/1.80.0/source/boost_1_80_0.tar.gz",

   ],

)

[ai...@google.com]

It would probably be a reasonable thing if we emitted BUILD.bazel for all repository rules. 

But, we should do that in a PR that does it for all the rules at once, rather than in in #16064.

[Xavier Bonaventura]

Tony, now that there is a cut for Bazel 6, should we remove already new_git_repository? The migration is trivial.

The only possible issue I would see, is that in the documentation does not appear that new_git_repository is deprecated. Should we mark it?
Do you think that there is any chance to mark it as deprecated in the documentation for Bazel 6?

Bests,

Xavi

[Tony Aiuto]

Yes. We should remove this. 

https://github.com/bazelbuild/bazel/issues/16823

--
You received this message because you are subscribed to a topic in the Google Groups "bazel-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/bazel-dev/sfeRE3Ucj7E/unsubscribe.
To unsubscribe from this group and all its topics, send an email to bazel-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/63c4df07-b672-485b-90bf-522dfed3907fn%40googlegroups.com.