Refactor git_repository and new_git_repository rules implementations … #8264
Conversation
…so that they can be used with Bazel on Windows without MSYS - do not use shell scripting, call git binary and use repository_context methods - add black box tests for new_git_repository, so that they run on Windows without MSYS - prefer fetch to clone, as in that case less content is fetched (with fetch, we specify both the reference and depth in the same command; on contrary, clone will fetch all branches)
|
NB probably Buildkite Windows image should be updated, the git fails with a weird error. |
| if ctx.attr.shallow_since: | ||
| shallow = "--shallow-since=%s" % ctx.attr.shallow_since | ||
| if ctx.attr.commit: | ||
| shallow = "" |
There was a problem hiding this comment.
This seems wrong. The reproducible case is that we specify a commit and a shallow_since value to allow shallow cloning. But your code seems to erase the value of shallow in this case.
There was a problem hiding this comment.
I will have another look tomorrow. (Of course the contract should stay).
Also, maybe you could point to a test for this specific combination?
NB this PR is mostly for passing my already-prepared changes to Yun, it's gonna be some work with this stuff before it gets submitted.
There was a problem hiding this comment.
The tests are now passing on Windows. @irengrig Can you address Klaus's comment or should I look into it? Is there anything else I should do before getting this merged?
There was a problem hiding this comment.
The current code works exactly the same way as the previous variant:
bazel/tools/build_defs/repo/git.bzl
Line 25 in 220a899
Before refactoring, if the commit was specified, and no shallow_since, the commit value was not used for shallow variable.
Please note that we can not call "fetch" with the commit number, only with tag/branch/HEAD (isaacs/github#436), but maybe we could indeed use the --shallow_since attribute in this case.
|
I'm investigating the test failure on Windows. Seems like a conflict between Git on Windows and MSYS2, still trying to reproduce locally. |
- do not try to pass empty shallow argument, and do not quote it - when user specified commit hash, use it as shallow_since argument
There was a problem hiding this comment.
Thanks for doing this refactor! @aehlig Can you also take a look?
|
@irengrig Can you import this PR? |
Sure, will do that |
|
This was submitted in the other PR: #8677 |
philwo
added
the
team-OSS
…so that they can be used with Bazel on Windows without MSYS