[v4,0/4] Filter alternate references

Message

Taylor Blau Oct. 2, 2018, 2:23 a.m. UTC

Hi,

Attached is the fourth re-roll of a series to teach
'core.alternateRefsCommand' and 'core.alternateRefsPrefixes' to filter
refs from an alternate from being visible to the fork. This is done in
order to optimize a case described in patch [3/4].

As always, a range-diff is included below, showing that not much has
changed of significance since last round. I mostly focused my efforts on
taking Peff's suggestion towards a more straightforward implementation
of the test setup.

Some extra documentation was written and a couple of commit messages
amended, but no C code has changed since the v2.

Thanks again for all of your review.

Thanks,
Taylor

Jeff King (1):
  transport: drop refnames from for_each_alternate_ref

Taylor Blau (3):
  transport.c: extract 'fill_alternate_refs_command'
  transport.c: introduce core.alternateRefsCommand
  transport.c: introduce core.alternateRefsPrefixes

 Documentation/config.txt           | 23 +++++++++++++++++
 builtin/receive-pack.c             |  3 +--
 fetch-pack.c                       |  3 +--
 t/t5410-receive-pack-alternates.sh | 41 ++++++++++++++++++++++++++++++
 transport.c                        | 38 +++++++++++++++++++++------
 transport.h                        |  2 +-
 6 files changed, 97 insertions(+), 13 deletions(-)
 create mode 100755 t/t5410-receive-pack-alternates.sh

Range-diff against v3:
1:  037273dab0 ! 1:  491f258f50 transport: drop refnames from for_each_alternate_ref
    @@ -14,6 +14,7 @@
         bare minimum.

         Signed-off-by: Jeff King <[e-mail]>
    +    Signed-off-by: Taylor Blau <[e-mail]>

      diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
      --- a/builtin/receive-pack.c
2:  9479470cb1 = 2:  6119de15f2 transport.c: extract 'fill_alternate_refs_command'
3:  2dbcd54190 ! 3:  aadb27c010 transport.c: introduce core.alternateRefsCommand
    @@ -24,9 +24,7 @@

         Let the repository that has alternates configure this command to avoid
         trusting the alternate to provide us a safe command to run in the shell.
    -    To behave differently on each alternate (e.g., only list tags from
    -    alternate A, only heads from B) provide the path of the alternate as the
    -    first argument.
    +    To find the alternate, pass its absolute path as the first argument.

         Signed-off-by: Taylor Blau <[e-mail]>

    @@ -40,51 +38,41 @@
     +core.alternateRefsCommand::
     +	When advertising tips of available history from an alternate, use the shell to
     +	execute the specified command instead of linkgit:git-for-each-ref[1]. The
    -+	first argument is the absolute path of the alternate. Output must be of the
    -+	form: `%(objectname)`, where multiple tips are separated by newlines.
    ++	first argument is the absolute path of the alternate. Output must contain one
    ++	hex object id per line (i.e., the same as produce by `git for-each-ref
    ++	--format='%(objectname)'`).
     ++
     +This is useful when a repository only wishes to advertise some of its
    -+alternate's references as ".have"'s. For example, to only advertise branch
    ++alternate's references as `.have`'s. For example, to only advertise branch
     +heads, configure `core.alternateRefsCommand` to the path of a script which runs
     +`git --git-dir="$1" for-each-ref --format='%(objectname)' refs/heads`.
    +++
    ++Note that the configured value is executed in a shell, and thus
    ++linkgit:git-for-each-ref[1] by itself does not work, as scripts have to handle
    ++the path argument specially.
     +
      core.bare::
      	If true this repository is assumed to be 'bare' and has no
      	working directory associated with it.  If this is the case a

    - diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
    + diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh
      new file mode 100755
      --- /dev/null
    - +++ b/t/t5410-receive-pack.sh
    + +++ b/t/t5410-receive-pack-alternates.sh
     @@
     +#!/bin/sh
     +
    -+test_description='git receive-pack test'
    ++test_description='git receive-pack with alternate ref filtering'
     +
     +. ./test-lib.sh
     +
     +test_expect_success 'setup' '
    -+	test_commit one &&
    -+	git update-ref refs/heads/a HEAD &&
    -+	test_commit two &&
    -+	git update-ref refs/heads/b HEAD &&
    -+	test_commit three &&
    -+	git update-ref refs/heads/c HEAD &&
    -+	git clone --bare . fork &&
    -+	git clone fork pusher &&
    -+	(
    -+		cd fork &&
    -+		git update-ref --stdin <<-\EOF &&
    -+		delete refs/heads/a
    -+		delete refs/heads/b
    -+		delete refs/heads/c
    -+		delete refs/heads/master
    -+		delete refs/tags/one
    -+		delete refs/tags/two
    -+		delete refs/tags/three
    -+		EOF
    -+		echo "../../.git/objects" >objects/info/alternates
    -+	)
    ++	test_commit base &&
    ++	git clone -s --bare . fork &&
    ++	git checkout -b public/branch master &&
    ++	test_commit public &&
    ++	git checkout -b private/branch master &&
    ++	test_commit private
     +'
     +
     +extract_haves () {
    @@ -95,11 +83,10 @@
     +	write_script fork/alternate-refs <<-\EOF &&
     +		git --git-dir="$1" for-each-ref \
     +			--format="%(objectname)" \
    -+			refs/heads/a \
    -+			refs/heads/c
    ++			refs/heads/public/
     +	EOF
     +	test_config -C fork core.alternateRefsCommand alternate-refs &&
    -+	git rev-parse a c >expect &&
    ++	git rev-parse public/branch >expect &&
     +	printf "0000" | git receive-pack fork >actual &&
     +	extract_haves <actual >actual.haves &&
     +	test_cmp expect actual.haves
4:  48eb774c9e ! 4:  0d3521e92a transport.c: introduce core.alternateRefsPrefixes
    @@ -12,7 +12,8 @@
         'core.alternateRefsCommand' would have to do:

           $ git config core.alternateRefsCommand ' \
    -          git -C "$1" for-each-ref refs/tags --format="%(objectname)"'
    +          f() { git -C "$1" for-each-ref \
    +                  refs/tags --format="%(objectname)" }; f "$@"'

         The above is cumbersome to write, so let's introduce a
         "core.alternateRefsPrefixes" to address this common case. Instead, the
    @@ -38,8 +39,8 @@
      --- a/Documentation/config.txt
      +++ b/Documentation/config.txt
     @@
    - heads, configure `core.alternateRefsCommand` to the path of a script which runs
    - `git --git-dir="$1" for-each-ref --format='%(objectname)' refs/heads`.
    + linkgit:git-for-each-ref[1] by itself does not work, as scripts have to handle
    + the path argument specially.

     +core.alternateRefsPrefixes::
     +	When listing references from an alternate, list only references that begin
    @@ -52,16 +53,16 @@
      	If true this repository is assumed to be 'bare' and has no
      	working directory associated with it.  If this is the case a

    - diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh
    - --- a/t/t5410-receive-pack.sh
    - +++ b/t/t5410-receive-pack.sh
    + diff --git a/t/t5410-receive-pack-alternates.sh b/t/t5410-receive-pack-alternates.sh
    + --- a/t/t5410-receive-pack-alternates.sh
    + +++ b/t/t5410-receive-pack-alternates.sh
     @@
      	test_cmp expect actual.haves
      '

     +test_expect_success 'with core.alternateRefsPrefixes' '
    -+	test_config -C fork core.alternateRefsPrefixes "refs/tags" &&
    -+	git rev-parse one three two >expect &&
    ++	test_config -C fork core.alternateRefsPrefixes "refs/heads/private" &&
    ++	git rev-parse private/branch expect &&
     +	printf "0000" | git receive-pack fork >actual &&
     +	extract_haves <actual >actual.haves &&
     +	test_cmp expect actual.haves
--
2.19.0.221.g150f307af