partial-copy trigger question

12 views
Skip to first unread message

Stephen Morton

unread,
Apr 7, 2022, 5:18:54 PM4/7/22
to gitolite
I've got a couple of minor questions about the partial-copy trigger and vref. They're definitely "non-core" so I hope it's still ok to ask this group about them.

1. Partial-Copy seems to require "option deny-rules" but I don't see that in the docs.
Look at the partial-copy trigger https://github.com/sitaramc/gitolite/blob/master/src/triggers/partial-copy
and the line(s) below in it. You can see that any refs *readable by the person doing a git operation* are slurped into the partial-copy.

```
for ref in `git for-each-ref refs/heads '--format=%(refname)'`
do
    cd $GL_REPO_BASE/$repo.git

    gitolite access -q $repo $user R $ref &&
    git fetch -f $GL_REPO_BASE/$main.git $ref:$ref
done
```

Doesn't this therefore require `option deny-rules = 1` if we're basing this on whether a user has _read_ access? That isn't mentioned in the documentation (afaik) and I suspect it should be there.


2. Why the `update-ref` rather than a `push`

The VREF does this
```
git push -f $GL_REPO_BASE/$main.git $new:refs/partial/br-$rand || die "FATAL: failed to send $new"

cd $GL_REPO_BASE/$main.git
git update-ref -d refs/partial/br-$rand
git update-ref $ref $new $old || die "FATAL: update-ref for $ref failed"
```

Why is that? I'm guessing it's because it therefore bypasses some of the destination repo's hooks, but I don't fully understand.
Would there be a problem if I made a copy of this hook and simply did a `git push $GL_REPO_BASE/$main.git $new:$new || die "FATAL: failed to send $new" ` ? For instance if the upstream repo was not in gitolite at all and if it was actually `git push $REMOTE_SSH_URL/$main.git $new:$new` .

Steve





Sitaram Chamarty

unread,
Apr 8, 2022, 10:12:32 PM4/8/22
to Stephen Morton, gitolite
On Thu, Apr 07, 2022 at 05:18:41PM -0400, Stephen Morton wrote:
> I've got a couple of minor questions about the partial-copy trigger and
> vref. They're definitely "non-core" so I hope it's still ok to ask this
> group about them.

ouch! I didn't realise I was gate-keeping so much. My
apologies!

Discussion doesn't have to be non-core only; core stuff is also
fine -- it just needs a lot more discussion to make *changes* :)

>
> 1. Partial-Copy seems to require "option deny-rules" but I don't see that
> in the docs.
> Look at the partial-copy trigger
> https://github.com/sitaramc/gitolite/blob/master/src/triggers/partial-copy
> and the line(s) below in it. You can see that any refs *readable by the
> person doing a git operation* are slurped into the partial-copy.
>
> ```
> for ref in `git for-each-ref refs/heads '--format=%(refname)'`
> do
> cd $GL_REPO_BASE/$repo.git
>
> gitolite access -q $repo $user R $ref &&

The `access` command automatically respects deny rules if you
supply a specific ref. As `gitolite access -h` says:

The 'any' ref is special -- it ignores deny rules, thus simulating
gitolite's behaviour during the pre-git access check

Although I did not state it explicitly, this does mean that when
an actual ref is used, deny-rules are honored.

As a result, the above test will fail for refs that appear in
a "-" rule before they match an "R" or "RW" etc.

> git fetch -f $GL_REPO_BASE/$main.git $ref:$ref
> done
> ```
>
> Doesn't this therefore require `option deny-rules = 1` if we're basing this
> on whether a user has _read_ access? That isn't mentioned in the
> documentation (afaik) and I suspect it should be there.
>
>
> 2. Why the `update-ref` rather than a `push`
>
> The VREF does this
> ```
> git push -f $GL_REPO_BASE/$main.git $new:refs/partial/br-$rand || die
> "FATAL: failed to send $new"
>
> cd $GL_REPO_BASE/$main.git
> git update-ref -d refs/partial/br-$rand
> git update-ref $ref $new $old || die "FATAL: update-ref for $ref failed"
> ```
>
> Why is that? I'm guessing it's because it therefore bypasses some of the
> destination repo's hooks, but I don't fully understand.
> Would there be a problem if I made a copy of this hook and simply did a
> `git push $GL_REPO_BASE/$main.git $new:$new || die "FATAL: failed to send
> $new" ` ? For instance if the upstream repo was not in gitolite at all and
> if it was actually `git push $REMOTE_SSH_URL/$main.git $new:$new` .

$new is a hash not a ref; you'd need $new:$ref not $new:$new

But other than that, I dare say it should work.

I'm trying to remember why I did this, and I think it was
because I had unspecified concerns about race conditions, and
decided to use something that was atomic to be safe. Very
likely over-engineered, since I am sure git itself will take
care of that. Anyway, it's not a bug per se so I'm going to
leave it as is.

regards
sitaram

Stephen Morton

unread,
Apr 8, 2022, 10:56:06 PM4/8/22
to Sitaram Chamarty, gitolite
Thank you.

Responses inline.


On Fri, 8 Apr 2022 at 22:12, Sitaram Chamarty <sita...@gmail.com> wrote:
On Thu, Apr 07, 2022 at 05:18:41PM -0400, Stephen Morton wrote:
> I've got a couple of minor questions about the partial-copy trigger and
> vref. They're definitely "non-core" so I hope it's still ok to ask this
> group about them.

ouch!  I didn't realise I was gate-keeping so much.  My
apologies!

No, no you're not gate-keeping so much. I just prefer to err on the side of being deferential, than barging in with demands. :-)
 
Discussion doesn't have to be non-core only; core stuff is also
fine -- it just needs a lot more discussion to make *changes* :)

>
> 1. Partial-Copy seems to require "option deny-rules" but I don't see that
> in the docs.
> Look at the partial-copy trigger
> https://github.com/sitaramc/gitolite/blob/master/src/triggers/partial-copy
> and the line(s) below in it. You can see that any refs *readable by the
> person doing a git operation* are slurped into the partial-copy.
>
> ```
> for ref in `git for-each-ref refs/heads '--format=%(refname)'`
> do
>     cd $GL_REPO_BASE/$repo.git
>
>     gitolite access -q $repo $user R $ref &&

The `access` command automatically respects deny rules if you
supply a specific ref.  As `gitolite access -h` says:

    The 'any' ref is special -- it ignores deny rules, thus simulating
    gitolite's behaviour during the pre-git access check

Although I did not state it explicitly, this does mean that when
an actual ref is used, deny-rules are honored.

Aha, interesting. I was having some problems at first and I thought that this fixed it, but I also fixed several other things too.

One thing that I did discover is that having a "super admin" who has RW+ access on all repos is not particularly compatible with the partial-copy trigger. Their "super admin" powers should ideally be defined after the partial-copy repo is defined. Otherwise, the super-admin just needs to look sideways at the partial-copy and all of a sudden it is populated with all tags and branches. (Yes, the next person's git operation will undo all that, but that all results in a lot of thrashing on the poor repo if there are thousands of branches/tags being added/removed.)
 

As a result, the above test will fail for refs that appear in
a "-" rule before they match an "R" or "RW" etc.

>     git fetch -f $GL_REPO_BASE/$main.git $ref:$ref
> done
> ```
>
> Doesn't this therefore require `option deny-rules = 1` if we're basing this
> on whether a user has _read_ access? That isn't mentioned in the
> documentation (afaik) and I suspect it should be there.
>
>
> 2. Why the `update-ref` rather than a `push`
>
> The VREF does this
> ```
> git push -f $GL_REPO_BASE/$main.git $new:refs/partial/br-$rand || die
> "FATAL: failed to send $new"
>
> cd $GL_REPO_BASE/$main.git
> git update-ref -d refs/partial/br-$rand
> git update-ref $ref $new $old || die "FATAL: update-ref for $ref failed"
> ```
>
> Why is that? I'm guessing it's because it therefore bypasses some of the
> destination repo's hooks, but I don't fully understand.
> Would there be a problem if I made a copy of this hook and simply did a
> `git push $GL_REPO_BASE/$main.git $new:$new || die "FATAL: failed to send
> $new" ` ? For instance if the upstream repo was not in gitolite at all and
> if it was actually `git push $REMOTE_SSH_URL/$main.git $new:$new` .

$new is a hash not a ref; you'd need $new:$ref not $new:$new

Yes, right, thanks.
 

But other than that, I dare say it should work.

I'm trying to remember why I did this, and I think it was
because I had unspecified concerns about race conditions, and
decided to use something that was atomic to be safe.  Very
likely over-engineered, since I am sure git itself will take
care of that.  Anyway, it's not a bug per se so I'm going to
leave it as is.

Ok, that's good to know. I've been given the "reasonable" request to have a read/write partial-copy repo. It's conceptually reasonable, anyway, it's just not really how git works so ingenuity is required. Thankfully you already supplied that. Right now it will all be in gitolite, but I suspect eventually there will be a gitolite partial-copy whose upstream repo is in GitLab. It's at this point that I'll have to push rather than directly manipulate a gitolite repo on the same server/filesystem.

One thing I have found is that for a repo with lots of refs (a few hundred branches, a few tens of thousands of tags) all the checking that the partial-copy trigger does is too much, so I created my own copy of it that is optimized both for not running all those access commands and also not looping through as many refs (I basically pulled the intelligence out of the conf file and hard-coded it into the trigger), and, knowing that the branches/tags allowed are not going to change frequently, I skip the whole looping over them to see what needs to be deleted steps. As it is it still takes an extra ~20s to pull and up to an extra ~1min20s if the pull decides that it should make sure all tags are updated as well.

Stephen


Reply all
Reply to author
Forward
0 new messages