From: Stefan Sperling Subject: Re: improve got fetch error reporting To: Mark Jamsek Cc: gameoftrees@openbsd.org Date: Sun, 5 Feb 2023 23:12:23 +0100 On Sun, Feb 05, 2023 at 05:34:11PM +1100, Mark Jamsek wrote: > On the topic of got's new fetch behaviour, do you think it would be a > good idea to fallback to the branch resolved by the remote's HEAD ref > if 'got fetch' is run in a work tree without -b? > > So, 'got fetch' will first attempt to fetch the work tree's current > branch, but if not found it will fetch the remote repository's HEAD. Yes, I think that is indeed better than just erroring out. Below is a new patch which includes your suggestion. However, while working on this I realized that our assumptions about the intended default behaviour have been wrong. Please check the log message and diff below. This now seems a lot more consistent to me. ----------------------------------------------- improve 'got fetch' behaviour when work tree's branch is not on server Only fetch the work tree's branch if the -b option is not specified. This keeps -b functional as an override when invoked in a work tree. Our previous changes did not consider that got.conf is also a source of lists of branches to fetch, and that -b is supposed to work as an override of any default behaviour. We were implicitly appending the work tree's branch as if it was mentioned as an override on the command line, which was wrong and based on a misunderstanding of the intended behaviour. Without -b on the command line we obtain a list of branches to fetch from got.conf and use this list if it is not empty. The repository's HEAD will be fetched only if neither the -b option, nor got.conf, nor a work tree tell us what to fetch. Make the man page more clear by moving the explanation of the default behaviour into the main section of 'got fetch', leaving the -a and -b option descriptions free of such details. diff 27749ea2ddbc482ad434ed865e0f855313db0a27 d3b02654263618fd1e1aa95b172c3f877ffe6e9c commit - 27749ea2ddbc482ad434ed865e0f855313db0a27 commit + d3b02654263618fd1e1aa95b172c3f877ffe6e9c blob - 2f2d51be710c9d1ff0291d1284b33f43eb85e145 blob + 2dcb63267e36d94e9cdff8cf736147e2ded5f199 --- got/got.1 +++ got/got.1 @@ -342,6 +342,25 @@ New changes will be stored in a separate pack file dow file of the local repository, as created by .Cm got clone . .Pp +By default, any branches configured in +.Xr got.conf 5 +for the +.Ar remote-repository +will be fetched. +If +.Cm got fetch +is invoked in a work tree then this work tree's current branch will be +fetched, too, provided it is present on the server. +This default behaviour can be overridden with the +.Fl a +and +.Fl b +options. +If no branches to fetch can be found in +.Xr got.conf 5 , +on the command line, or via a work tree, a branch resolved via the remote +repository's HEAD reference will be fetched. +.Pp New changes will be stored in a separate pack file downloaded from the server. Optionally, separate pack files stored in the repository can be combined with .Xr git-repack 1 . @@ -383,10 +402,6 @@ If this option is not specified, the work tree's curre reference namespace. This option can be enabled by default for specific repositories in .Xr got.conf 5 . -If this option is not specified, the work tree's current branch -will be fetched if invoked within a work tree, -otherwise a branch resolved via the remote repository's HEAD reference -will be fetched. Cannot be used together with the .Fl b option. @@ -398,10 +413,6 @@ If this option is not specified, the work tree's curre reference namespace. This option may be specified multiple times to build a list of branches to fetch. -If this option is not specified, the work tree's current branch -will be fetched if invoked within a work tree, -otherwise a branch resolved via the remote repository's HEAD reference -will be fetched. Cannot be used together with the .Fl a option. blob - fe66f5fc26c7e236053a98d519915b574179a529 blob + a32137a402ce476d4300983fe5e9e17d680c8ac0 --- got/got.c +++ got/got.c @@ -2443,12 +2443,6 @@ cmd_fetch(int argc, char *argv[]) } } } - if (TAILQ_EMPTY(&wanted_branches)) { - error = got_pathlist_append(&wanted_branches, - got_worktree_get_head_ref_name(worktree), NULL); - if (error) - goto done; - } } if (remote == NULL) { repo_conf = got_repo_get_gotconfig(repo); @@ -2486,6 +2480,12 @@ cmd_fetch(int argc, char *argv[]) if (error) goto done; } + if (worktree) { + error = got_pathlist_append(&wanted_branches, + got_worktree_get_head_ref_name(worktree), NULL); + if (error) + goto done; + } } if (TAILQ_EMPTY(&wanted_refs)) { for (i = 0; i < remote->nfetch_refs; i++) { blob - 0211785dcc6e7adad10d1a2d594bcda72bf2efa7 blob + 0f381607476a2ed62e00ff9cb319557a26e8d633 --- regress/cmdline/fetch.sh +++ regress/cmdline/fetch.sh @@ -188,6 +188,8 @@ test_fetch_branch() { git_commit $testroot/repo -m "modified alpha" local commit_id3=`git_show_head $testroot/repo` + # foo is now the default HEAD branch in $testroot/repo + # but got.conf still says to fetch "master" got fetch -q -r $testroot/repo-clone -b foo > $testroot/stdout ret=$? if [ $ret -ne 0 ]; then @@ -228,7 +230,8 @@ test_fetch_branch() { return 1 fi - got fetch -q -r $testroot/repo-clone -b master > $testroot/stdout + # got.conf tells us to fetch the 'master' branch by default + got fetch -q -r $testroot/repo-clone > $testroot/stdout ret=$? if [ $ret -ne 0 ]; then echo "got fetch command failed unexpectedly" >&2 @@ -271,6 +274,9 @@ test_fetch_branch() { git_commit $testroot/repo -m "modified beta" local commit_id4=`git_show_head $testroot/repo` + # set the default HEAD branch back to master + (cd $testroot/repo && git checkout -q master) + got checkout -b foo $testroot/repo-clone $testroot/wt > /dev/null # fetch new commits on branch 'foo', implicitly obtaining the @@ -297,7 +303,7 @@ test_fetch_branch() { >> $testroot/stdout.expected echo "refs/heads/foo: $commit_id3" >> $testroot/stdout.expected echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected - echo "refs/remotes/origin/HEAD: refs/remotes/origin/foo" \ + echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \ >> $testroot/stdout.expected echo "refs/remotes/origin/foo: $commit_id4" >> $testroot/stdout.expected echo "refs/remotes/origin/master: $commit_id2" \