git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [BUG] git push --mirror ignores refs outside head & tags
@ 2021-11-15  6:53 Robin H. Johnson
  2021-11-15 21:04 ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Robin H. Johnson @ 2021-11-15  6:53 UTC (permalink / raw)
  To: Git Mailing List


[-- Attachment #1.1: Type: text/plain, Size: 1703 bytes --]

Hi,

TL;DR: "git push --mirror" does not in fact push all refs, despite
documentation stating it does. It ONLY pushes refs in refs/heads/ and
refs/tags/.

I ran into this while preparing to migrate Gentoo's primary Git server
(which uses Gitolite).

Gitolite ships a post-receive hook, "save-push-signatures" [1] that save
Push certificates into refs/push-certs.

Gitolite mirroring invokes 'git push --mirror' to mirror from the
primary location to replicas.

I was surprised to find, on the new server presently configured as
replica, but intended to take over as the primary, that the
refs/push-certs was missing on every single repo.

```
# git push -h |grep -e mirror -e all
    --all                 push all refs
	--mirror              mirror all refs
```
git-push(1):
```
--mirror
    Instead of naming each ref to push, specifies that all refs under refs/ (which includes
    but is not limited to refs/heads/, refs/remotes/, and refs/tags/) be mirrored to the
    remote repository. ...
```

git --version: 2.32.0

Attached I also include the trace ref & trace packet output as well that shows
it on a sample repo.
```
GIT_TRACE_REFS=/tmp/trace-refs \
GIT_TRACE_PACKET=/tmp/trace-packet \
git push --mirror $REPLICA:$REPO
```

The repo's gitconfig does not contain any remotes, because that's the way gitolite is built.

[1] https://github.com/sitaramc/gitolite/blob/master/contrib/hooks/repo-specific/save-push-signatures

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

[-- Attachment #1.2: trace-packet --]
[-- Type: text/plain, Size: 2832 bytes --]

23:07:49.011947 pkt-line.c:80           packet:         push< bd7bd3a513c8ddc554e211316c990b5f98110982 refs/heads/bug450326\0report-status report-status-v2 delete-refs side-band-64k quiet atomic ofs-delta push-cert=1636931268-40bfd0dc11027620d3ea77024b5aa5aa90ed2342 object-format=sha1 agent=git/2.32.0
23:07:49.011987 pkt-line.c:80           packet:         push< 65c923b603247af68f1b4405363d92c1c19009eb refs/heads/bug504458
23:07:49.011993 pkt-line.c:80           packet:         push< 6e81ff9f85736180516941e630b1db78bd9bf2a2 refs/heads/iw
23:07:49.011998 pkt-line.c:80           packet:         push< e860f40ffd07a41e47a6b49d9400ad20e6a8b7ca refs/heads/master
23:07:49.012002 pkt-line.c:80           packet:         push< 56dd91ee7737dd3d4ebc57185632f407a2cdf3c1 refs/heads/remove-bash-arrays
23:07:49.012007 pkt-line.c:80           packet:         push< 09c6d951fe3c4419317fcd9488df907bd11ac02c refs/tags/0.1
23:07:49.012012 pkt-line.c:80           packet:         push< cb257cea980b74aa7dff4b7bfcfb66b82565bb01 refs/tags/0.2
23:07:49.012016 pkt-line.c:80           packet:         push< 22b3841561b0b686919be466f1caeae6833e30ba refs/tags/0.2.1
23:07:49.012021 pkt-line.c:80           packet:         push< 47d23f5abdac4a2bd95bcbbaadacf994773cbf51 refs/tags/0.2.2
23:07:49.012025 pkt-line.c:80           packet:         push< e2e76bb9c2a7bee77e29faf8f9afb2a89fe19e11 refs/tags/0.2.3
23:07:49.012030 pkt-line.c:80           packet:         push< 986029121ae49984c98a76efa83d4e48d5c9f650 refs/tags/0.3.0
23:07:49.012035 pkt-line.c:80           packet:         push< 904d1b04cb0a147fe39f9952a281563faeb19d48 refs/tags/0.3.1
23:07:49.012039 pkt-line.c:80           packet:         push< a143c1b3a6896cdfe62dd24a0e95ee442c5469e2 refs/tags/0.4.0
23:07:49.012044 pkt-line.c:80           packet:         push< edd52106d7c868c86c396e0c2f486e58a6132d3c refs/tags/0.5.0
23:07:49.012048 pkt-line.c:80           packet:         push< c1b5722fe2dd9aee540cc08b73e7ac59c2f22c4d refs/tags/0.5.1
23:07:49.012055 pkt-line.c:80           packet:         push< 8ec196c7c708340e888573f66e34f9c5171efabf refs/tags/0.6.0
23:07:49.012060 pkt-line.c:80           packet:         push< 7ba81c3198da28101290eed259aef6645d258ebc refs/tags/0.6.1
23:07:49.012064 pkt-line.c:80           packet:         push< 8cdfc30e5cca5da9ea12b7e275f044d452e6894d refs/tags/0.7.0
23:07:49.012069 pkt-line.c:80           packet:         push< 646886f098efddd62162bebb655e413fd99c214e refs/tags/0.7.1
23:07:49.012073 pkt-line.c:80           packet:         push< 93a79ef4b89ee383cdfb5e4be5f07c2ce4e61c90 refs/tags/0.7.2
23:07:49.012078 pkt-line.c:80           packet:         push< 3067e568abbf803e8f47a6e16bc39412a4539b1c refs/tags/0.7.3
23:07:49.012082 pkt-line.c:80           packet:         push< 0000
23:07:49.012126 pkt-line.c:80           packet:         push> 0000

[-- Attachment #1.3: trace-refs --]
[-- Type: text/plain, Size: 2484 bytes --]

23:07:48.501620 refs/debug.c:27         ref_store for .
23:07:48.501670 refs/debug.c:252        read_raw_ref: HEAD: 0000000000000000000000000000000000000000 (=> refs/heads/master) type 1: 0
23:07:48.501687 refs/debug.c:252        read_raw_ref: refs/heads/master: e860f40ffd07a41e47a6b49d9400ad20e6a8b7ca (=> refs/heads/master) type 2: 0
23:07:48.501738 refs/debug.c:235        ref_iterator_begin:  (0x0)
23:07:48.501790 refs/debug.c:191        iterator_advance: refs/heads/bug450326 (0)
23:07:48.501797 refs/debug.c:191        iterator_advance: refs/heads/bug504458 (0)
23:07:48.501801 refs/debug.c:191        iterator_advance: refs/heads/iw (0)
23:07:48.501805 refs/debug.c:191        iterator_advance: refs/heads/master (0)
23:07:48.501809 refs/debug.c:191        iterator_advance: refs/heads/remove-bash-arrays (0)
23:07:48.501812 refs/debug.c:191        iterator_advance: refs/push-certs (0)
23:07:48.501816 refs/debug.c:191        iterator_advance: refs/tags/0.1 (0)
23:07:48.501820 refs/debug.c:191        iterator_advance: refs/tags/0.2 (0)
23:07:48.501823 refs/debug.c:191        iterator_advance: refs/tags/0.2.1 (0)
23:07:48.501827 refs/debug.c:191        iterator_advance: refs/tags/0.2.2 (0)
23:07:48.501831 refs/debug.c:191        iterator_advance: refs/tags/0.2.3 (0)
23:07:48.501834 refs/debug.c:191        iterator_advance: refs/tags/0.3.0 (0)
23:07:48.501838 refs/debug.c:191        iterator_advance: refs/tags/0.3.1 (0)
23:07:48.501841 refs/debug.c:191        iterator_advance: refs/tags/0.4.0 (0)
23:07:48.501845 refs/debug.c:191        iterator_advance: refs/tags/0.5.0 (0)
23:07:48.501848 refs/debug.c:191        iterator_advance: refs/tags/0.5.1 (0)
23:07:48.501852 refs/debug.c:191        iterator_advance: refs/tags/0.6.0 (0)
23:07:48.501855 refs/debug.c:191        iterator_advance: refs/tags/0.6.1 (0)
23:07:48.501861 refs/debug.c:191        iterator_advance: refs/tags/0.7.0 (0)
23:07:48.501865 refs/debug.c:191        iterator_advance: refs/tags/0.7.1 (0)
23:07:48.501868 refs/debug.c:191        iterator_advance: refs/tags/0.7.2 (0)
23:07:48.501872 refs/debug.c:191        iterator_advance: refs/tags/0.7.3 (0)
23:07:48.501875 refs/debug.c:189        iterator_advance: (-1)
23:07:49.042542 refs/debug.c:252        read_raw_ref: HEAD: 0000000000000000000000000000000000000000 (=> refs/heads/master) type 1: 0
23:07:49.042563 refs/debug.c:252        read_raw_ref: refs/heads/master: e860f40ffd07a41e47a6b49d9400ad20e6a8b7ca (=> refs/heads/master) type 2: 0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1113 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] git push --mirror ignores refs outside head & tags
  2021-11-15  6:53 [BUG] git push --mirror ignores refs outside head & tags Robin H. Johnson
@ 2021-11-15 21:04 ` Jeff King
  2021-11-16  6:28   ` Robin H. Johnson
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2021-11-15 21:04 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Mon, Nov 15, 2021 at 06:53:21AM +0000, Robin H. Johnson wrote:

> TL;DR: "git push --mirror" does not in fact push all refs, despite
> documentation stating it does. It ONLY pushes refs in refs/heads/ and
> refs/tags/.

Perhaps there's a bug, but it is meant to and does push all refs in a
simple case:

  git init
  git commit --allow-empty -m foo
  git update-ref refs/heads/other-branch HEAD
  git update-ref refs/foo/bar HEAD

  git init --bare dst.git
  git push --mirror dst.git

produces:

  To dst.git
   * [new reference]   refs/foo/bar -> refs/foo/bar
   * [new branch]      main -> main
   * [new branch]      other-branch -> other-branch

> I was surprised to find, on the new server presently configured as
> replica, but intended to take over as the primary, that the
> refs/push-certs was missing on every single repo.

Ah. You are not likely to have success pushing a single-level ref like
that, as receive-pack rejects it:

  $ git update-ref refs/push-certs HEAD
  $ git push dst.git refs/push-certs
  Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
  remote: error: refusing to create funny ref 'refs/push-certs' remotely
  To dst.git
   ! [remote rejected] refs/push-certs -> refs/push-certs (funny refname)
  error: failed to push some refs to 'dst.git'

There I used an explicit refspec naming it. But if I used "refs/*"
(which is what --mirror is doing under the hood), then it doesn't even
try sending it, as the client filters it out from the wildcard
(otherwise, everyone would get server-side errors from refs/stash).

So you probably want to choose a different refname to store your data.

I do think the status of these single-level refs is not well documented.
The rules in git-check-ref-format(1) imply that they are not valid:

  They must contain at least one /. This enforces the presence of a
  category like heads/, tags/ etc. but the actual names are not
  restricted.

but that rule is not enforced internally, as "refs/" is sufficient for
the internal check_refname_format() to allow it. But receive-pack has,
since 1a7141ff28 (Ignore funny refname sent from remote, 2005-10-13),
implemented the format check after stripping refs/. And then the client
side followed that lead in 30affa1e9a (send-pack: do not send out
single-level refs such as refs/stash, 2008-10-29).

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] git push --mirror ignores refs outside head & tags
  2021-11-15 21:04 ` Jeff King
@ 2021-11-16  6:28   ` Robin H. Johnson
  2021-11-20  0:03     ` [RFC] single-level refs vs push --all/--mirror Robin H. Johnson
  0 siblings, 1 reply; 5+ messages in thread
From: Robin H. Johnson @ 2021-11-16  6:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Robin H. Johnson

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

On Mon, Nov 15, 2021 at 04:04:18PM -0500, Jeff King wrote:
> On Mon, Nov 15, 2021 at 06:53:21AM +0000, Robin H. Johnson wrote:
> 
> > TL;DR: "git push --mirror" does not in fact push all refs, despite
> > documentation stating it does. It ONLY pushes refs in refs/heads/ and
> > refs/tags/.
> 
> Perhaps there's a bug, but it is meant to and does push all refs in a
> simple case:
...
> There I used an explicit refspec naming it. But if I used "refs/*"
> (which is what --mirror is doing under the hood), then it doesn't even
> try sending it, as the client filters it out from the wildcard
> (otherwise, everyone would get server-side errors from refs/stash).
> 
> So you probably want to choose a different refname to store your data.
> 
> I do think the status of these single-level refs is not well documented.
> The rules in git-check-ref-format(1) imply that they are not valid:
Ok, that's great, but this single-level ref have been in existence for
many years at this point. Gitolite added the save-push-signatures hook
in 2014.

- What is a meaningful name for the push-certs ref that follows the
  2-level system?
- What's a safe migration path for Gitolite?
  - How can it atomically fix each existing repo with the problem?
- What's a safe migration path for other consumers?

> 
>   They must contain at least one /. This enforces the presence of a
>   category like heads/, tags/ etc. but the actual names are not
>   restricted.
> 
> but that rule is not enforced internally, as "refs/" is sufficient for
> the internal check_refname_format() to allow it. But receive-pack has,
> since 1a7141ff28 (Ignore funny refname sent from remote, 2005-10-13),
> implemented the format check after stripping refs/. And then the client
> side followed that lead in 30affa1e9a (send-pack: do not send out
> single-level refs such as refs/stash, 2008-10-29).

And the format of the ref wasn't noticed in the hook was previously
posted back in 2014 :-(.
https://www.spinics.net/lists/git/msg244053.html
https://groups.google.com/forum/#!topic/gitolite/7cSrU6JorEY

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1113 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC] single-level refs vs push --all/--mirror
  2021-11-16  6:28   ` Robin H. Johnson
@ 2021-11-20  0:03     ` Robin H. Johnson
  2021-11-20  6:05       ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Robin H. Johnson @ 2021-11-20  0:03 UTC (permalink / raw)
  To: git, robbat2

[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]

On Tue, Nov 16, 2021 at 06:28:28AM +0000, Robin H. Johnson wrote:
> > Perhaps there's a bug, but it is meant to and does push all refs in a
> > simple case:
> ...
> > There I used an explicit refspec naming it. But if I used "refs/*"
> > (which is what --mirror is doing under the hood), then it doesn't even
> > try sending it, as the client filters it out from the wildcard
> > (otherwise, everyone would get server-side errors from refs/stash).
> > 
> > So you probably want to choose a different refname to store your data.
> > 
> > I do think the status of these single-level refs is not well documented.
> > The rules in git-check-ref-format(1) imply that they are not valid:
..
> >   They must contain at least one /. This enforces the presence of a
> >   category like heads/, tags/ etc. but the actual names are not
> >   restricted.
> > 
> > but that rule is not enforced internally, as "refs/" is sufficient for
> > the internal check_refname_format() to allow it. But receive-pack has,
> > since 1a7141ff28 (Ignore funny refname sent from remote, 2005-10-13),
> > implemented the format check after stripping refs/. And then the client
> > side followed that lead in 30affa1e9a (send-pack: do not send out
> > single-level refs such as refs/stash, 2008-10-29).
> 
> And the format of the ref wasn't noticed in the hook was previously
> posted back in 2014 :-(.
> https://www.spinics.net/lists/git/msg244053.html
> https://groups.google.com/forum/#!topic/gitolite/7cSrU6JorEY

Hi,

I didn't see response from Jeff King, so I'm wondering good next steps
here.

I'm esp. surprised that git-stash ends up using single-level refs when
git-check-ref-format(1) says they aren't valid.

I think Git should change git-stash and start issuing warnings to users
for single-level refs.

Therein leads a question:
What should be done with refs/stash?

Being able to work on stashes between multiple systems MIGHT be useful
to some users, so maybe they do want it pushed with --mirror/--all.

Choices (not mutually exclusive):
1. Push the refs anyway
2. Make a configurable way to exclude refs from being pushed w/ --mirror/--all
   e.g. move refs/stash to the exclude list, then push everything not
   that is not excluded.
3. Change the rules to accept single-level refs?
4. Cleanup phases of warnings before errors and good migration
   mechanisms?
5. Something else?

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1113 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] single-level refs vs push --all/--mirror
  2021-11-20  0:03     ` [RFC] single-level refs vs push --all/--mirror Robin H. Johnson
@ 2021-11-20  6:05       ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-11-20  6:05 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: git

"Robin H. Johnson" <robbat2@gentoo.org> writes:

> I didn't see response from Jeff King, so I'm wondering good next steps
> here.
>
> I'm esp. surprised that git-stash ends up using single-level refs when
> git-check-ref-format(1) says they aren't valid.

They aren't valid name to be used as regular branches, tags, etc.,
but as an implementation detail of stash, "refs/stash" is perfectly
good.  There is nothing to "fix".  You cannot and you do not want to
pretend "stash" is yet another ref in the first place.

You can "transfer" refs/stash by pushing the commit to an arbitrary
ref with a regular "three level" name, like so:

   $ git push there refs/stash:refs/my/stash

but it wouldn't be of much use to begin with.

The thing is, the "list of random quick changes stashed away" is not
something stored in the "stash" ref.  These list entries are stored
as reflog entries for the stash ref, which is *never* transferred
over the network.  So, if the higher level issue you want to address
is to allow "stash" to be shared across repositories, none of these
5 choices you listed at the end of your message helps all that much.

The single-level refs are the least of your problems.

Instead, you'd probably want to reimplement "stash" as a set of
normal refs, whose current value only matters, e.g. refs/stash/0
may be the oldest stash, refs/stash/1 is the next one, refs/stash/2
is yet another new one, etc., and have UIs like "git stash list" 
list them in a new way that is different from the current reflog
based implementation, and "git stash pop/apply" take them, e.g.

   $ git stash list
   stash/0: WIP on main: cd3e606211 Git 2.34
   stash/4: WIP on maint: Merge branch 'vd/pthread-setspecific-g11-fix' into maint
   $ git stash apply stash/0

And at that point you'd have refs/stash/* as an intermediate
hierarchy, with another level of real refs hanging there, so you can
transfer them just like refs/tags/* all you want.

> I think Git should change git-stash and start issuing warnings to users
> for single-level refs.

No, single-level refs is perfectly fine, as long as you are using
the current Git and using these refs locally.  The problem arises
only when you start wanting to share stash across repositories, and
it is not from the levels of the refname hierarchy but from the fact
that stash is implemented in terms of reflog mechanism that is not
shared across repositories.



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-20  6:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15  6:53 [BUG] git push --mirror ignores refs outside head & tags Robin H. Johnson
2021-11-15 21:04 ` Jeff King
2021-11-16  6:28   ` Robin H. Johnson
2021-11-20  0:03     ` [RFC] single-level refs vs push --all/--mirror Robin H. Johnson
2021-11-20  6:05       ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).