sync: push current branch first
sync: Push the current branch first, rather than a synced branch, to better support git forges (gitlab, gitea, forgejo, etc.) which use push-to-create with the first pushed branch becoming the default branch. With considerable complication to filter out warning message about receive.denyCurrentBranch when pushing to a non-bare repository. Localization may break it in the future, but it seems like the best way to handle this. See my comments for the gory details.
This commit is contained in:
Joey Hess
parent
48e7497f83
commit
9f4e956346
7 changed files with 288 additions and 38 deletions
|
|
@ -14,6 +14,10 @@ git-annex (10.20250521) UNRELEASED; urgency=medium
|
|||
When set, this allows the copy_file_range syscall to be used, which
|
||||
can eg allow for server-side copies on NFS. (For fastest copying,
|
||||
also disable annex.verify or remote.name.annex-verify.)
|
||||
* sync: Push the current branch first, rather than a synced branch,
|
||||
to better support git forges (gitlab, gitea, forgejo, etc.) which
|
||||
use push-to-create with the first pushed branch becoming the default
|
||||
branch.
|
||||
|
||||
-- Joey Hess <id@joeyh.name> Thu, 22 May 2025 12:43:38 -0400
|
||||
|
||||
|
|
|
|||
|
|
@ -83,7 +83,6 @@ import Types.Availability
|
|||
import qualified Database.Export as Export
|
||||
import Utility.Bloom
|
||||
import Utility.OptParse
|
||||
import Utility.Process.Transcript
|
||||
import Utility.Tuple
|
||||
import Utility.Matcher
|
||||
|
||||
|
|
@ -706,20 +705,13 @@ pushRemote o remote (Just branch, _) = do
|
|||
- Git offers no way to tell if a remote is bare or not, so both methods
|
||||
- are tried.
|
||||
-
|
||||
- The direct push is likely to spew an ugly error message, so its stderr is
|
||||
- often elided. Since git progress display goes to stderr too, the
|
||||
- sync push is done first, and actually sends the data. Then the
|
||||
- direct push is tried, with stderr discarded, to update the branch ref
|
||||
- on the remote.
|
||||
- The direct push is done first, because some hosting providers like
|
||||
- github may treat the first branch pushed to a new repository as the
|
||||
- default branch for that repository.
|
||||
-
|
||||
- The sync push first sends the synced/master branch,
|
||||
- and then forces the update of the remote synced/git-annex branch.
|
||||
-
|
||||
- Since some providers like github may treat the first branch sent
|
||||
- as the default branch, it's better to make that be synced/master than
|
||||
- synced/git-annex. (Although neither is ideal, it's the best that
|
||||
- can be managed given the constraints on order.)
|
||||
-
|
||||
- The forcing is necessary if a transition has rewritten the git-annex branch.
|
||||
- Normally any changes to the git-annex branch get pulled and merged before
|
||||
- this push, so this forcing is unlikely to overwrite new data pushed
|
||||
|
|
@ -728,34 +720,59 @@ pushRemote o remote (Just branch, _) = do
|
|||
- But overwriting of data on synced/git-annex can happen, in a race.
|
||||
- The only difference caused by using a forced push in that case is that
|
||||
- the last repository to push wins the race, rather than the first to push.
|
||||
-
|
||||
- The git-annex branch is pushed last. This push may fail if the remote
|
||||
- has other changes in the git-annex branch, and that is not treated as an
|
||||
- error, since the synced/git-annex branch has been sent already. Since no
|
||||
- new data is usually sent in this push (due to synced/git-annex already
|
||||
- having been pushed), it's ok to hide git's output to avoid displaying
|
||||
- a push error.
|
||||
-}
|
||||
pushBranch :: Remote -> Maybe Git.Branch -> MessageState -> Git.Repo -> IO Bool
|
||||
pushBranch remote mbranch ms g = directpush `after` annexpush `after` syncpush
|
||||
pushBranch remote mbranch ms g = do
|
||||
directpush
|
||||
annexpush `after` syncpush
|
||||
where
|
||||
syncpush = flip Git.Command.runBool g $ pushparams $ catMaybes
|
||||
[ (refspec . origBranch) <$> mbranch
|
||||
, Just $ Git.Branch.forcePush $ refspec Annex.Branch.name
|
||||
]
|
||||
annexpush = void $ tryIO $ flip Git.Command.runQuiet g $ pushparams
|
||||
[ Git.fromRef $ Git.Ref.base $ Annex.Branch.name ]
|
||||
directpush = case mbranch of
|
||||
Nothing -> noop
|
||||
-- Git prints out an error message when this fails.
|
||||
-- In the default configuration of receive.denyCurrentBranch,
|
||||
-- the error message mentions that config setting
|
||||
-- (and should even if it is localized), and is quite long,
|
||||
-- and the user was not intending to update the checked out
|
||||
-- branch, so in that case, avoid displaying the error
|
||||
-- message. Do display other error messages though,
|
||||
-- including the error displayed when
|
||||
-- receive.denyCurrentBranch=updateInstead -- the user
|
||||
-- will want to see that one.
|
||||
Just branch -> do
|
||||
let p = flip Git.Command.gitCreateProcess g $ pushparams
|
||||
[ Git.fromRef $ Git.Ref.base $ origBranch branch ]
|
||||
(transcript, ok) <- processTranscript' p Nothing
|
||||
when (not ok && not ("denyCurrentBranch" `isInfixOf` transcript)) $
|
||||
hPutStr stderr transcript
|
||||
let p' = p { std_err = CreatePipe }
|
||||
bracket (createProcess p') cleanupProcess $ \h -> do
|
||||
filterstderr [] (stderrHandle h) (processHandle h)
|
||||
void $ waitForProcess (processHandle h)
|
||||
Nothing -> noop
|
||||
|
||||
syncpush = flip Git.Command.runBool g $ pushparams $ catMaybes
|
||||
[ (syncrefspec . origBranch) <$> mbranch
|
||||
, Just $ Git.Branch.forcePush $ syncrefspec Annex.Branch.name
|
||||
]
|
||||
|
||||
annexpush = void $ tryIO $ flip Git.Command.runQuiet g $ pushparams
|
||||
[ Git.fromRef $ Git.Ref.base $ Annex.Branch.name ]
|
||||
|
||||
-- In the default configuration of receive.denyCurrentBranch,
|
||||
-- git's stderr message mentions that config setting
|
||||
-- (and should even if it is localized), and is quite long,
|
||||
-- and the user was not intending to update the checked out
|
||||
-- branch, so in that case, avoid displaying the error
|
||||
-- message. Do display other error messages though,
|
||||
-- including the error displayed when
|
||||
-- receive.denyCurrentBranch=updateInstead; the user
|
||||
-- will want to see that one. Also display progress messages.
|
||||
filterstderr buf herr pid = hGetLineUntilExitOrEOF pid herr >>= \case
|
||||
Just l
|
||||
| "remote: " `isPrefixOf` l || not (null buf)->
|
||||
filterstderr (l:buf) herr pid
|
||||
| otherwise -> do
|
||||
hPutStrLn stderr l
|
||||
filterstderr [] herr pid
|
||||
Nothing -> displaybuf
|
||||
where
|
||||
displaybuf =
|
||||
unless (any ("receive.denyCurrentBranch" `isInfixOf`) buf) $
|
||||
mapM_ (hPutStrLn stderr) (reverse buf)
|
||||
|
||||
pushparams branches = catMaybes
|
||||
[ Just $ Param "push"
|
||||
, if commandProgressDisabled' ms
|
||||
|
|
@ -763,7 +780,8 @@ pushBranch remote mbranch ms g = directpush `after` annexpush `after` syncpush
|
|||
else Nothing
|
||||
, Just $ Param $ Remote.name remote
|
||||
] ++ map Param branches
|
||||
refspec b = concat
|
||||
|
||||
syncrefspec b = concat
|
||||
[ Git.fromRef $ Git.Ref.base b
|
||||
, ":"
|
||||
, Git.fromRef $ Git.Ref.base $ syncBranch b
|
||||
|
|
|
|||
|
|
@ -12,3 +12,5 @@ This is very useful as it enables quick creation of repos without going through
|
|||
However, `git annex assist|sync|push` seem to push `git-annex`, `synced/git-annex`, or `synced/<currentbranch>` (in a seemingly random order? 🤔) **before** pushing `<currentbranch>` itself, causing this first pushed branch to become the repository's default branch. A `git clone ssh://me@myserver.com/me/myrepo` will then result in a local repo with e.g. `synced/main` checked out - or worse - `synced/git-annex`, causing a lot of confusion. Accidentally running `git annex assist` again will produce another level of `synced/synced/main` branches and all that fun stuff. (Very fun time during that summer school where I established git-annex + forgejo as data exchange 😉).
|
||||
|
||||
Of course the solution is to just `git push` manually before `git annex assist`. But `git annex assist` is already such a brilliant command that does it all, and telling people to just run that to "do the git stuff" is very comfortable and easily accepted. Could the current branch be pushed first? Or is there a reason for pushing all the meta-branches first?
|
||||
|
||||
> [[fixed|done]] --[[Joey]]
|
||||
|
|
|
|||
|
|
@ -8,14 +8,19 @@ Basically:
|
|||
|
||||
* We don't know if the remote is bare or non-bare. git does not generally
|
||||
provide a way to tell.
|
||||
* Pushing to the checked out branch of a non-bare repo will complain on stderr.
|
||||
* Pushing to the checked out branch of a non-bare repo will complain on
|
||||
stderr, and the overall git push will fail even if other branches were
|
||||
successfully pushed.
|
||||
But this is a fairly common use case for `git-annex sync`, and that
|
||||
complaint would be unwanted noise. git progress output also goes to stderr,
|
||||
so /dev/null of stderr is not desirable.
|
||||
* So instead push the synced branches, which doesn't have that problem, and lets
|
||||
git display progress for the main data transfer.
|
||||
* Then the current branch is pushed, with stderr collected and displayed
|
||||
after filtering out denyCurrentBranch error messages.
|
||||
git display progress for the main data transfer. As long as the
|
||||
synced/master branch is pushed, the overall push part of sync can be
|
||||
considered to succeed.
|
||||
* Then the current branch is pushed, with stderr collected and displayed,
|
||||
unless it contains the denyCurrentBranch warning message. A failure of this
|
||||
push is not treated as an error.
|
||||
|
||||
Also this was previously considered and partly addressed in
|
||||
[[!commit 1cc7b2661e5ec60f73f04dbe91940d2602df6246]] which made it push
|
||||
|
|
@ -25,6 +30,7 @@ using a version from before that change. At that point I thought this was a
|
|||
github specific problem, mind.
|
||||
|
||||
I think that to improve this, git-annex would need to run git push of master
|
||||
with stderr intercepted and the denyCurrentBranch error message filtered out.
|
||||
with stderr intercepted and the denyCurrentBranch error message filtered
|
||||
out, but the rest of stderr (progress, etc) still displayed.
|
||||
Which does seem doable.
|
||||
"""]]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,164 @@
|
|||
[[!comment format=mdwn
|
||||
username="joey"
|
||||
subject="""comment 2"""
|
||||
date="2025-06-04T15:11:21Z"
|
||||
content="""
|
||||
Doable, but not easy! The message that would need to be filtered:
|
||||
|
||||
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
|
||||
remote: error: refusing to update checked out branch: refs/heads/master
|
||||
remote: error: By default, updating the current branch in a non-bare repository
|
||||
remote: is denied, because it will make the index and work tree inconsistent
|
||||
remote: with what you pushed, and will require 'git reset --hard' to match
|
||||
remote: the work tree to HEAD.
|
||||
remote:
|
||||
remote: You can set the 'receive.denyCurrentBranch' configuration variable
|
||||
remote: to 'ignore' or 'warn' in the remote repository to allow pushing into
|
||||
remote: its current branch; however, this is not recommended unless you
|
||||
remote: arranged to update its work tree to match what you pushed in some
|
||||
remote: other way.
|
||||
remote:
|
||||
remote: To squelch this message and still keep the default behaviour, set
|
||||
remote: 'receive.denyCurrentBranch' configuration variable to 'refuse'.
|
||||
To localhost:tmp/bench2/a
|
||||
! [remote rejected] master -> master (branch is currently checked out)
|
||||
error: failed to push some refs to 'localhost:tmp/bench2/a'
|
||||
|
||||
Filtering out the error part of that while keeping the rest, and without
|
||||
assuming too much about git's output, would be rather hard. Especially
|
||||
since progress output is displayed too, so it has to decide whether or not
|
||||
to display the first line of that error message, before it has necessarily
|
||||
received the rest of it. So it can't look for "receive.denyCurrentBranch"
|
||||
to know when to filter out that error message.
|
||||
|
||||
Hmm, the 'refuse' setting seems to have been added after git-annex dealt
|
||||
with this, and setting that does avoid the ugly message, and users could
|
||||
just be left to set it if they dislike it. But that would be disruptive for
|
||||
existing users, and a lot of bother for what is a pretty common use case
|
||||
for git-annex.
|
||||
|
||||
I came up with the following patch before realizing this difficulty. So its
|
||||
filterstderr is wrong.
|
||||
|
||||
diff --git a/Command/Sync.hs b/Command/Sync.hs
|
||||
index 2892768b73..0ea472624a 100644
|
||||
--- a/Command/Sync.hs
|
||||
+++ b/Command/Sync.hs
|
||||
@@ -83,7 +83,6 @@ import Types.Availability
|
||||
import qualified Database.Export as Export
|
||||
import Utility.Bloom
|
||||
import Utility.OptParse
|
||||
-import Utility.Process.Transcript
|
||||
import Utility.Tuple
|
||||
import Utility.Matcher
|
||||
|
||||
@@ -706,20 +705,13 @@ pushRemote o remote (Just branch, _) = do
|
||||
- Git offers no way to tell if a remote is bare or not, so both methods
|
||||
- are tried.
|
||||
-
|
||||
- - The direct push is likely to spew an ugly error message, so its stderr is
|
||||
- - often elided. Since git progress display goes to stderr too, the
|
||||
- - sync push is done first, and actually sends the data. Then the
|
||||
- - direct push is tried, with stderr discarded, to update the branch ref
|
||||
- - on the remote.
|
||||
+ - The direct push is done first, because some hosting providers like
|
||||
+ - github may treat the first branch pushed to a new repository as the
|
||||
+ - default branch for that repository.
|
||||
-
|
||||
- The sync push first sends the synced/master branch,
|
||||
- and then forces the update of the remote synced/git-annex branch.
|
||||
-
|
||||
- - Since some providers like github may treat the first branch sent
|
||||
- - as the default branch, it's better to make that be synced/master than
|
||||
- - synced/git-annex. (Although neither is ideal, it's the best that
|
||||
- - can be managed given the constraints on order.)
|
||||
- -
|
||||
- The forcing is necessary if a transition has rewritten the git-annex branch.
|
||||
- Normally any changes to the git-annex branch get pulled and merged before
|
||||
- this push, so this forcing is unlikely to overwrite new data pushed
|
||||
@@ -728,34 +720,53 @@ pushRemote o remote (Just branch, _) = do
|
||||
- But overwriting of data on synced/git-annex can happen, in a race.
|
||||
- The only difference caused by using a forced push in that case is that
|
||||
- the last repository to push wins the race, rather than the first to push.
|
||||
+ -
|
||||
+ - The git-annex branch is pushed last. This push may fail if the remote
|
||||
+ - has other changes in the git-annex branch, and that is not treated as an
|
||||
+ - error, since the synced/git-annex branch has been sent already. Since no
|
||||
+ - new data is usually sent in this push (due to synced/git-annex already
|
||||
+ - having been pushed), it's ok to hide git's output to avoid displaying
|
||||
+ - a push error.
|
||||
-}
|
||||
pushBranch :: Remote -> Maybe Git.Branch -> MessageState -> Git.Repo -> IO Bool
|
||||
-pushBranch remote mbranch ms g = directpush `after` annexpush `after` syncpush
|
||||
+pushBranch remote mbranch ms g = do
|
||||
+ directpush
|
||||
+ annexpush `after` syncpush
|
||||
where
|
||||
- syncpush = flip Git.Command.runBool g $ pushparams $ catMaybes
|
||||
- [ (refspec . origBranch) <$> mbranch
|
||||
- , Just $ Git.Branch.forcePush $ refspec Annex.Branch.name
|
||||
- ]
|
||||
- annexpush = void $ tryIO $ flip Git.Command.runQuiet g $ pushparams
|
||||
- [ Git.fromRef $ Git.Ref.base $ Annex.Branch.name ]
|
||||
directpush = case mbranch of
|
||||
- Nothing -> noop
|
||||
- -- Git prints out an error message when this fails.
|
||||
- -- In the default configuration of receive.denyCurrentBranch,
|
||||
- -- the error message mentions that config setting
|
||||
- -- (and should even if it is localized), and is quite long,
|
||||
- -- and the user was not intending to update the checked out
|
||||
- -- branch, so in that case, avoid displaying the error
|
||||
- -- message. Do display other error messages though,
|
||||
- -- including the error displayed when
|
||||
- -- receive.denyCurrentBranch=updateInstead -- the user
|
||||
- -- will want to see that one.
|
||||
Just branch -> do
|
||||
let p = flip Git.Command.gitCreateProcess g $ pushparams
|
||||
[ Git.fromRef $ Git.Ref.base $ origBranch branch ]
|
||||
- (transcript, ok) <- processTranscript' p Nothing
|
||||
- when (not ok && not ("denyCurrentBranch" `isInfixOf` transcript)) $
|
||||
- hPutStr stderr transcript
|
||||
+ let p' = p { std_err = CreatePipe }
|
||||
+ bracket (createProcess p') cleanupProcess $ \h -> do
|
||||
+ filterstderr (stderrHandle h) (processHandle h)
|
||||
+ void $ waitForProcess (processHandle h)
|
||||
+ Nothing -> noop
|
||||
+
|
||||
+ syncpush = flip Git.Command.runBool g $ pushparams $ catMaybes
|
||||
+ [ (syncrefspec . origBranch) <$> mbranch
|
||||
+ , Just $ Git.Branch.forcePush $ syncrefspec Annex.Branch.name
|
||||
+ ]
|
||||
+
|
||||
+ annexpush = void $ tryIO $ flip Git.Command.runQuiet g $ pushparams
|
||||
+ [ Git.fromRef $ Git.Ref.base $ Annex.Branch.name ]
|
||||
+
|
||||
+ -- In the default configuration of receive.denyCurrentBranch,
|
||||
+ -- git's stderr message mentions that config setting
|
||||
+ -- (and should even if it is localized), and is quite long,
|
||||
+ -- and the user was not intending to update the checked out
|
||||
+ -- branch, so in that case, avoid displaying the error
|
||||
+ -- message. Do display other error messages though,
|
||||
+ -- including the error displayed when
|
||||
+ -- receive.denyCurrentBranch=updateInstead; the user
|
||||
+ -- will want to see that one.
|
||||
+ filterstderr herr pid = hGetLineUntilExitOrEOF pid herr >>= \case
|
||||
+ Just l -> do
|
||||
+ unless ("denyCurrentBranch" `isInfixOf` l) $
|
||||
+ hPutStrLn stderr l
|
||||
+ filterstderr herr pid
|
||||
+ Nothing -> return ()
|
||||
+
|
||||
pushparams branches = catMaybes
|
||||
[ Just $ Param "push"
|
||||
, if commandProgressDisabled' ms
|
||||
@@ -763,7 +774,8 @@ pushBranch remote mbranch ms g = directpush `after` annexpush `after` syncpush
|
||||
else Nothing
|
||||
, Just $ Param $ Remote.name remote
|
||||
] ++ map Param branches
|
||||
- refspec b = concat
|
||||
+
|
||||
+ syncrefspec b = concat
|
||||
[ Git.fromRef $ Git.Ref.base b
|
||||
, ":"
|
||||
, Git.fromRef $ Git.Ref.base $ syncBranch b
|
||||
"""]]
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
[[!comment format=mdwn
|
||||
username="joey"
|
||||
subject="""comment 3"""
|
||||
date="2025-06-04T15:26:18Z"
|
||||
content="""
|
||||
If the remote is brand-new, there will be no remote tracking refs.
|
||||
git-annex could detect that as a special case, and push the
|
||||
current branch first then. Since that would only be done one time,
|
||||
the user would only see any receive.denyCurrentBranch message once.
|
||||
So it wouldn't be appreciable clutter.
|
||||
|
||||
And in the common case where a user has cloned a repo to another drive,
|
||||
there would already be a tracking remote ref, so the special case wouldn't
|
||||
happen. And that's the kind of case where a clone of a non-bare repository
|
||||
is typically done.
|
||||
|
||||
This approach does have an edge case where the wrong
|
||||
thing is done: If the user had origin pointing to one repository, but then
|
||||
changed the url to a new, empty repository, sync would do the usual push, not
|
||||
the special case. And so if the new origin url was github or whatever has
|
||||
this problem, they would still end up pushing the "wrong" branch first.
|
||||
|
||||
That is an unlikely edge case, but it's also the kind of edge case that
|
||||
makes it hard to reason about software, IMHO.
|
||||
"""]]
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
[[!comment format=mdwn
|
||||
username="joey"
|
||||
subject="""comment 4"""
|
||||
date="2025-06-04T15:40:46Z"
|
||||
content="""
|
||||
For filtering, the best approach I can see to do that is to look for lines
|
||||
starting with "remote:", and buffer blocks of those lines. At the end of a
|
||||
"remote:" block, look for "receive.denyCurrentBranch" in it to decide
|
||||
whether to filter it out or display it. Pass other lines through.
|
||||
|
||||
Note that the remote may choose to display anything at all to stderr, and
|
||||
it would probably all come in the same block, so that might filter out
|
||||
messages that the user legitimatly would expect to see. But I suppose in
|
||||
that case, the other git push that is run for the sync branches, which does
|
||||
not get filtered, would still let such messages be displayed.
|
||||
|
||||
Currently, git does not localize the "remote:" even when the message itself
|
||||
is localized. This might just be bad/missing localization; some other parts
|
||||
also don't get localized:
|
||||
|
||||
remote: error: refusing to update checked out branch: refs/heads/master
|
||||
remote: error: Standardmäßig wird die Aktualisierung des aktuellen Branches in einem
|
||||
...
|
||||
! [remote rejected] master -> master (branch is currently checked out)
|
||||
Fehler: Fehler beim Versenden einiger Referenzen nach '../c'
|
||||
|
||||
I suppose if git localizes "remote:" later, it will only prevent the
|
||||
filtering, so not the end of the world.
|
||||
|
||||
Implemented this.
|
||||
"""]]
|
||||
Loading…
Add table
Add a link
Reference in a new issue