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" \