Repository: bazelbuild/bazel
Issue: new_git_repository doesn't have a strip_prefix option
Labels: type: feature request
P4


Issue comments:
@aehlig this is done, right?
It was addressed in https://github.com/bazelbuild/bazel/pull/4368 in the skylark rules only (not the built-in).

Pull request review comments:
I know you didn't start it, but I would have preferred email adresses in the rfc2606-reserved domains for fictional characters. (This also applies to other parts of that change.)
  
I shall enter a new PR to fix those (since it will be a big diff but mostly noise). Thanks!

Patch that resolved the issue:
diff --git a/tools/build_defs/repo/git.bzl b/tools/build_defs/repo/git.bzl
index 2375b21f71f886..0f735528d6d2d5 100644
--- a/tools/build_defs/repo/git.bzl
+++ b/tools/build_defs/repo/git.bzl
@@ -15,7 +15,8 @@
 
 def _clone_or_update(ctx):
   if (ctx.attr.verbose):
-    print('git.bzl: Cloning or updating repository %s' % ctx.name)
+    print('git.bzl: Cloning or updating repository %s using strip_prefix of [%s]' %
+    (ctx.name, ctx.attr.strip_prefix if ctx.attr.strip_prefix else 'None'))
   if ((not ctx.attr.tag and not ctx.attr.commit) or
       (ctx.attr.tag and ctx.attr.commit)):
     fail('Exactly one of commit and tag must be provided')
@@ -23,12 +24,15 @@ def _clone_or_update(ctx):
     ref = ctx.attr.commit
   else:
     ref = 'tags/' + ctx.attr.tag
+  dir=str(ctx.path('.'))
+  if ctx.attr.strip_prefix:
+    dir = dir + "-tmp"
 
   st = ctx.execute(['bash', '-c', """
 set -ex
 ( cd {working_dir} &&
-    if ! ( cd '{dir}' && git rev-parse --git-dir ) >/dev/null 2>&1; then
-      rm -rf '{dir}'
+    if ! ( cd '{dir_link}' && git rev-parse --git-dir ) >/dev/null 2>&1; then
+      rm -rf '{dir}' '{dir_link}'
       git clone '{remote}' '{dir}'
     fi
     cd '{dir}'
@@ -36,12 +40,21 @@ set -ex
     git clean -xdf )
   """.format(
       working_dir=ctx.path('.').dirname,
-      dir=ctx.path('.'),
+      dir_link=ctx.path('.'),
+      dir=dir,
       remote=ctx.attr.remote,
       ref=ref,
   )])
+
   if st.return_code:
     fail('error cloning %s:\n%s' % (ctx.name, st.stderr))
+
+  if ctx.attr.strip_prefix:
+    dest_link="{}/{}".format(dir, 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.symlink(dest_link, ctx.path('.'))
   if ctx.attr.init_submodules:
     st = ctx.execute(['bash', '-c', """
 set -ex
@@ -75,6 +88,7 @@ _common_attrs = {
     'tag': attr.string(default=''),
     'init_submodules': attr.bool(default=False),
     'verbose': attr.bool(default=False),
+    'strip_prefix': attr.string(default='')
 }
 
 
@@ -107,6 +121,8 @@ Args:
   init_submodules: Whether to clone submodules in the repository.
 
   remote: The URI of the remote Git repository.
+
+  strip_prefix: A directory prefix to strip from the extracted files.
 """
 
 git_repository = repository_rule(
@@ -124,4 +140,6 @@ Args:
   init_submodules: Whether to clone submodules in the repository.
 
   remote: The URI of the remote Git repository.
+
+  strip_prefix: A directory prefix to strip from the extracted files.
 """


Test patch:
diff --git a/src/test/shell/bazel/skylark_git_repository_test.sh b/src/test/shell/bazel/skylark_git_repository_test.sh
index 1c7231f8efe2a0..2e683bf5dd2279 100755
--- a/src/test/shell/bazel/skylark_git_repository_test.sh
+++ b/src/test/shell/bazel/skylark_git_repository_test.sh
@@ -58,7 +58,16 @@ function set_up() {
 #   BUILD
 #   info
 #
-# Then, set up workspace with the following files:
+# Followed by a test at 2-subdir which contains the following files to test
+# the strip_prefix functionality:
+#
+# pluto/
+#   pluto/
+#     WORKSPACE
+#     BUILD
+#     info
+#
+# In each case, set up workspace with the following files:
 #
 # $WORKSPACE_DIR/
 #   WORKSPACE
@@ -67,11 +76,12 @@ function set_up() {
 #     planet_info.sh
 #
 # //planets has a dependency on a target in the pluto Git repository.
-function test_git_repository() {
+function do_git_repository_test() {
   local pluto_repo_dir=$TEST_TMPDIR/repos/pluto
-  # Commit 85b8224 corresponds to tag 1-build. See testdata/pluto.git_log.
-  local commit_hash="b87de93"
-
+  # Commit corresponds to tag 1-build. See testdata/pluto.git_log.
+  local commit_hash="$1"
+  local strip_prefix=""
+  [ $# -eq 2 ] && strip_prefix="strip_prefix=\"$2\","
   # Create a workspace that clones the repository at the first commit.
   cd $WORKSPACE_DIR
   cat > WORKSPACE <<EOF
@@ -80,6 +90,7 @@ git_repository(
     name = "pluto",
     remote = "$pluto_repo_dir",
     commit = "$commit_hash",
+    $strip_prefix
 )
 EOF
   mkdir -p planets
@@ -102,12 +113,30 @@ EOF
   expect_log "Pluto is a dwarf planet"
 }
 
+function test_git_repository() {
+  do_git_repository_test "b87de93"
+}
+
+function test_git_repository_strip_prefix() {
+  # This commit has the files in a subdirectory named 'pluto'
+  # so we strip_prefix and the build should still work.
+  do_git_repository_test "ceab34f" "pluto"
+}
+
 function test_new_git_repository_with_build_file() {
-  do_new_git_repository_test "build_file"
+  do_new_git_repository_test "0-initial" "build_file"
+}
+
+function test_new_git_repository_with_build_file_strip_prefix() {
+  do_new_git_repository_test "3-subdir-bare" "build_file" "pluto"
 }
 
 function test_new_git_repository_with_build_file_content() {
-  do_new_git_repository_test "build_file_content"
+  do_new_git_repository_test "0-initial" "build_file_content"
+}
+
+function test_new_git_repository_with_build_file_content_strip_prefix() {
+  do_new_git_repository_test "3-subdir-bare" "build_file_content" "pluto"
 }
 
 # Test cloning a Git repository using the new_git_repository rule.
@@ -118,6 +147,12 @@ function test_new_git_repository_with_build_file_content() {
 # pluto/
 #   info
 #
+# Then it uses the pluto Git repository at tag 3-subdir-bare, which contains the
+# following files:
+# pluto/
+#   pluto/
+#     info
+#
 # Set up workspace with the following files:
 #
 # $WORKSPACE_DIR/
@@ -131,18 +166,21 @@ function test_new_git_repository_with_build_file_content() {
 # repository.
 function do_new_git_repository_test() {
   local pluto_repo_dir=$TEST_TMPDIR/repos/pluto
+  local strip_prefix=""
+  [ $# -eq 3 ] && strip_prefix="strip_prefix=\"$3\","
 
   # Create a workspace that clones the repository at the first commit.
   cd $WORKSPACE_DIR
 
-  if [ "$1" == "build_file" ] ; then
+  if [ "$2" == "build_file" ] ; then
     cat > WORKSPACE <<EOF
 load('@bazel_tools//tools/build_defs/repo:git.bzl', 'new_git_repository')
 new_git_repository(
     name = "pluto",
     remote = "$pluto_repo_dir",
-    tag = "0-initial",
+    tag = "$1",
     build_file = "//:pluto.BUILD",
+    $strip_prefix
 )
 EOF
 
@@ -162,7 +200,8 @@ load('@bazel_tools//tools/build_defs/repo:git.bzl', 'new_git_repository')
 new_git_repository(
     name = "pluto",
     remote = "$pluto_repo_dir",
-    tag = "0-initial",
+    tag = "$1",
+    $strip_prefix
     build_file_content = """
 filegroup(
     name = "pluto",
@@ -189,8 +228,12 @@ EOF
   chmod +x planets/planet_info.sh
 
   bazel run //planets:planet-info >& $TEST_log \
-    || echo "Expected build/run to succeed"
-  expect_log "Pluto is a planet"
+      || echo "Expected build/run to succeed"
+  if [ "$1" == "0-initial" ]; then
+      expect_log "Pluto is a planet"
+  else
+      expect_log "Pluto is a dwarf planet"
+  fi
 }
 
 # Test cloning a Git repository that has a submodule using the
@@ -316,6 +359,26 @@ EOF
   assert_contains "GIT 2" bazel-genfiles/external/g/go
 }
 
+function test_git_repository_not_refetched_on_server_restart_strip_prefix() {
+  local repo_dir=$TEST_TMPDIR/repos/refetch
+  # Change the strip_prefix which should cause a new checkout
+  cat > WORKSPACE <<EOF
+load('@bazel_tools//tools/build_defs/repo:git.bzl', 'git_repository')
+git_repository(name='g', remote='$repo_dir', commit='b0a2ada', verbose=True)
+EOF
+  bazel --batch build @g//gdir:g >& $TEST_log || fail "Build failed"
+  expect_log "Cloning"
+  assert_contains "GIT 2" bazel-genfiles/external/g/gdir/go
+
+  cat > WORKSPACE <<EOF
+load('@bazel_tools//tools/build_defs/repo:git.bzl', 'git_repository')
+git_repository(name='g', remote='$repo_dir', commit='b0a2ada', verbose=True, strip_prefix="gdir")
+EOF
+  bazel --batch build @g//:g >& $TEST_log || fail "Build failed"
+  expect_log "Cloning"
+  assert_contains "GIT 2" bazel-genfiles/external/g/go
+}
+
 
 function test_git_repository_refetched_when_commit_changes() {
   local repo_dir=$TEST_TMPDIR/repos/refetch
@@ -366,6 +429,17 @@ EOF
   assert_contains "GIT 1" bazel-genfiles/external/g/go
   bazel build  @g//:g >& $TEST_log || fail "Build failed"
   assert_contains "GIT 2" bazel-genfiles/external/g/go
+
+  cat > WORKSPACE <<EOF
+load('@bazel_tools//tools/build_defs/repo:git.bzl', 'git_repository')
+git_repository(name='g', remote='$repo_dir', commit='b0a2ada', strip_prefix="gdir")
+EOF
+
+  bazel build --nofetch @g//:g >& $TEST_log || fail "Build failed"
+  expect_log "External repository 'g' is not up-to-date"
+  bazel build  @g//:g >& $TEST_log || fail "Build failed"
+  assert_contains "GIT 2" bazel-genfiles/external/g/go
+
 }
 
 # Helper function for setting up the workspace as follows
@@ -403,7 +477,7 @@ EOF
 #   info
 function test_git_repository_both_commit_tag_error() {
   setup_error_test
-  local pluto_repo_dir=$TEST_TMPDIR/pluto
+  local pluto_repo_dir=$TEST_TMPDIR/repos/pluto
   # Commit 85b8224 corresponds to tag 1-build. See testdata/pluto.git_log.
   local commit_hash="b87de93"
 
@@ -434,7 +508,7 @@ EOF
 #   info
 function test_git_repository_no_commit_tag_error() {
   setup_error_test
-  local pluto_repo_dir=$TEST_TMPDIR/pluto
+  local pluto_repo_dir=$TEST_TMPDIR/repos/pluto
 
   cd $WORKSPACE_DIR
   cat > WORKSPACE <<EOF
@@ -450,4 +524,26 @@ EOF
   expect_log "Exactly one of commit and tag must be provided"
 }
 
+# Verifies that if a non-existent subdirectory is supplied, then strip_prefix
+# throws an error.
+function test_invalid_strip_prefix_error() {
+  setup_error_test
+  local pluto_repo_dir=$TEST_TMPDIR/repos/pluto
+
+  cd $WORKSPACE_DIR
+  cat > WORKSPACE <<EOF
+load('@bazel_tools//tools/build_defs/repo:git.bzl', 'git_repository')
+git_repository(
+    name = "pluto",
+    remote = "$pluto_repo_dir",
+    tag = "1-build",
+    strip_prefix = "dir_does_not_exist"
+)
+EOF
+
+  bazel fetch //planets:planet-info >& $TEST_log \
+    || echo "Expect run to fail."
+  expect_log "strip_prefix at dir_does_not_exist does not exist in repo"
+}
+
 run_suite "skylark git_repository tests"
diff --git a/src/test/shell/bazel/testdata/pluto.git_log b/src/test/shell/bazel/testdata/pluto.git_log
index fb98d77cb060b1..28a31640032322 100644
--- a/src/test/shell/bazel/testdata/pluto.git_log
+++ b/src/test/shell/bazel/testdata/pluto.git_log
@@ -1,4 +1,40 @@
-commit b87de9346bb1a4a3d4d2ab1a567b07c5d11a486a (HEAD -> master, tag: 1-build)
+commit a367b7e7f8f362697a079b7d23935bd079058af8 (HEAD -> master, tag: 3-subdir-bare)
+Author: John Doe <john@foo.com>
+Date:   Fri Dec 29 20:33:35 2017 -0500
+
+    Remove BUILD and WORKSPACE files
+
+diff --git a/WORKSPACE b/WORKSPACE
+deleted file mode 100644
+index e69de29..0000000
+diff --git a/pluto/BUILD b/pluto/BUILD
+deleted file mode 100644
+index 874e03f..0000000
+--- a/pluto/BUILD
++++ /dev/null
+@@ -1,5 +0,0 @@
+-filegroup(
+-    name = "pluto",
+-    srcs = ["info"],
+-    visibility = ["//visibility:public"],
+-)
+
+commit ceab34fb9b4b9370d52dd5153074089a51bfb50e (tag: 2-subdir)
+Author: John Doe <john@foo.com>
+Date:   Wed Dec 27 20:54:31 2017 +0000
+
+    Move pluto files into pluto subdirectory
+
+diff --git a/BUILD b/pluto/BUILD
+similarity index 100%
+rename from BUILD
+rename to pluto/BUILD
+diff --git a/info b/pluto/info
+similarity index 100%
+rename from info
+rename to pluto/info
+
+commit b87de9346bb1a4a3d4d2ab1a567b07c5d11a486a (tag: 1-build)
 Author: John Doe <john@foo.com>
 Date:   Thu Jul 16 04:50:53 2015 -0700
 
diff --git a/src/test/shell/bazel/testdata/refetch.git_log b/src/test/shell/bazel/testdata/refetch.git_log
index 1d42f6979b1960..e0ef8294bc0fa3 100644
--- a/src/test/shell/bazel/testdata/refetch.git_log
+++ b/src/test/shell/bazel/testdata/refetch.git_log
@@ -1,4 +1,15 @@
-commit 62777acc140a240a3ec8fa1adecaa6bc9e25ccdd (HEAD -> master)
+commit b0a2adafea74d36e0374a119eab6623dbe5ba418 (HEAD -> master)
+Author: John Doe <john@foo.com>
+Date:   Fri Dec 29 18:36:06 2017 -0500
+
+    Move BUILD into a subdirectory
+
+diff --git a/BUILD b/gdir/BUILD
+similarity index 100%
+rename from BUILD
+rename to gdir/BUILD
+
+commit 62777acc140a240a3ec8fa1adecaa6bc9e25ccdd
 Author: John Doe <john@foo.com>
 Date:   Wed Nov 25 13:20:07 2015 +0100