git_repository strip_prefix breaks WORKSPACE load statements

[ghost]

Description of the problem / feature request:

We mirror GitHub repositories internally, so we have to replace http_archive rules with git_repository equivalents.

When cloning bazelbuild/rules_pkg via git_repository with strip_prefix, I encounter the following error:

$ bazel build ...
ERROR: Failed to load Starlark extension '@rules_pkg//:deps.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
The following chain of repository dependencies lead to the missing definition.
 - @rules_pkg
This could either mean you have to add the '@rules_pkg' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
INFO: Call stack for the definition of repository 'rules_pkg' which is a git_repository (rule definition at /home/beasleyr/.cache/bazel/_bazel_beasleyr/86146646415a94b3b4b4be4de036080d/external/bazel_tools/tools/build_defs/repo/git.bzl:181:18):
 - /work/git/git-strip-prefix/WORKSPACE:3:1
ERROR: cycles detected during target parsing
INFO: Elapsed time: 1.789s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "rules_pkg",
    remote = "https://github.com/bazelbuild/rules_pkg.git",
    tag = "0.2.2",
    strip_prefix = "pkg",
)

load("@rules_pkg//:deps.bzl", "rules_pkg_dependencies")

rules_pkg_dependencies()

BUILD:

load("@rules_pkg//:pkg.bzl", "pkg_tar")

If I create a local Git repository rooted at rules_pkg's pkg directory, allowing me to remove the strip_prefix attr, then the WORKSPACE load+rules_pkg_dependencies() calls work just fine.

What operating system are you running Bazel on?

Ubuntu 18.04

What's the output of bazel info release?

release 0.28.1

However, I can also reproduce this with 1.0.0 on CentOS 7.2.

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

Have you found anything relevant by searching the web?

No. :(

Any other information, logs, or outputs that you want to share?

[ghost]

My suspicion is that this is caused by git.bzl reaching outside of ctx.path(".") to create a symlink pointing to ../<repo>-tmp/<prefix>. The following change seems to work, though with a side effect of producing a .tmp directory inside the repository.

--- git.bzl.orig	2019-10-18 09:13:55.720479487 -0400
+++ git.bzl	2019-10-18 09:09:43.118443397 -0400
@@ -31,7 +31,7 @@
     root = ctx.path(".")
     directory = str(root)
     if ctx.attr.strip_prefix:
-        directory = directory + "-tmp"
+        directory = root.get_child(".tmp")
 
     git_ = git_repo(ctx, directory)
 
@@ -39,8 +39,9 @@
         dest_link = "{}/{}".format(directory, ctx.attr.strip_prefix)
         if not ctx.path(dest_link).exists:
             fail("strip_prefix at {} does not exist in repo".format(ctx.attr.strip_prefix))
-        ctx.delete(root)
-        ctx.symlink(dest_link, root)
+
+        for item in ctx.path(dest_link).readdir():
+            ctx.symlink(item, root.get_child(item.basename))
 
     return {"commit": git_.commit, "shallow_since": git_.shallow_since}
 

[clintharrison]

Unrelated to this bug in git.bzl, I think there's a faulty assumption here.

It's not necessarily true that a release .tar.gz consumed as an http_archive can be replaced by git_repository like this. It's probably always fine when the http_archive is using the GitHub-generated archive of the repository, but that is not what's happening here.

The rules_pkg release you're downloading is not the rules_pkg repo with pkg stripped -- it selectively includes the files to include: https://github.com/bazelbuild/rules_pkg/blob/0824299e8211762fa713f531a989b2297e294b20/pkg/distro/BUILD#L10-L23. In the case of rules_pkg, this isn't very complex logic, so it "works" otherwise to just strip the prefix.

But, on the other hand, we have repositories like rules_nodejs where the release process is more involved. You cannot use it as a git_repository, because there are "source-only" dependencies that get stripped out or stubbed during the release process.

Instead of consuming the mirrored repository, it would be much more robust to host the release .tar.gz and use that with an http_archive from an internal mirror.

[smolkaj]

What's the status on this? This is causing a lot of headaches, a fix would be highly appreciated.

[smolkaj]

As a hacky woraround, one can replace

strip_prefix = "foo"

with

patch_cmds = ["mv foo/* ."]

This is likely brittle; a real fix would be highly appreciated :)

[xiay-nv]

Any update on this? This is quite troublesome.

[xiay-nv]

@beasleyr-vmw The first section of your fix is not necessary, the second section alone is enough for it to work. But never the less it breaks patching.

[philwo]

@meteorcloudy Could you please look into this? This seems to affect a few users, maybe we can easily fix this. 😊

[meteorcloudy]

OK, let me take a look~

[meteorcloudy]

I don't think I can figure out a proper easy fix for this in short time, will follow up next week, I have to focus on preparing for BazelCon this week.

[philwo]

No worries and thanks for looking into it!

[meteorcloudy]

I looked into this, Bazel really doesn't like repository rule touching anything outside of <output_base>/external/, I will accept the fix in #10062 (comment), which does the prefix stripping within the repo directory. When users use strip_prefix together with a patch file, the patch file should match the content under the strip_prefix directory, which is also required by the current implementation, therefore patching is actually not broken by the fix.