From: Mark Jamsek <mark@jamsek.com>
Subject: Re: got: gotadmin init -b to specify HEAD + new got import behaviour
To: Game of Trees <gameoftrees@openbsd.org>
Date: Tue, 20 Sep 2022 21:01:48 +1000

On 22-09-19 07:34PM, Stefan Sperling wrote:
> On Mon, Sep 19, 2022 at 11:01:51PM +1000, Mark Jamsek wrote:
> > @@ -843,12 +844,23 @@ cmd_import(int argc, char *argv[])
> >  	 * While technically a valid reference name, this case is usually
> >  	 * an unintended typo.
> >  	 */
> > -	if (branch_name[0] == '-')
> > +	if (branch_name && branch_name[0] == '-')
> >  		return got_error_path(branch_name, GOT_ERR_REF_NAME_MINUS);
> >  
> > -	if (asprintf(&refname, "refs/heads/%s", branch_name) == -1) {
> > -		error = got_error_from_errno("asprintf");
> > +	error = got_ref_open(&head_ref, repo, GOT_REF_HEAD, 0);
> > +	if (error && error->code != GOT_ERR_NOT_REF)
> >  		goto done;
> > +
> > +	if (branch_name)
> > +		n = strlcat(refname, branch_name, sizeof(refname));
> > +	else if (head_ref)
> > +		n = strlcpy(refname, got_ref_get_symref_target(head_ref),
> > +		    sizeof(refname));
> 
> We segfault here is head_ref is not actually a symbolic reference.
> Add an extra check using got_ref_is_symbolic() is avoid this.
> 
> $ cat t/.git/HEAD                              
> a6595b659681118f302825765ca19ec034bd4d49
> $ ls y/
> a  b
> $ got import -r t y
> Segmentation fault (core dumped)
> 
> The above situation was created in a non-bare repository with the
> command: git checkout a6595b659681118f302825765ca19ec034bd4d49
> 

Ah, that was silly of me! We even talked about this case, IIRC, and
I completely forgot to check for it. Thanks, Stefan. Revised diff below
includes a new regress for the detached HEAD case too.

-----------------------------------------------
commit 4209d3aaca8962c082d5164b2881488c50b4c5eb (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Tue Sep 20 10:51:35 2022 UTC
 
 add gotadmin init -b <branch> to specify repo head ref
 
 Similar to `git init -b`. Includes a change to `got import` behaviour such that
 "main" is no longer hardcoded by default; instead, we import to the branch
 resolved via the repository's HEAD reference unless `got import -b` is used,
 and only if HEAD cannot be resolved to a branch do we fallback to "main".
 
 includes fix from stsp@
 
diff 12c07a25b65fee8e7bca0ed2aa35f76e446d1241 4209d3aaca8962c082d5164b2881488c50b4c5eb
commit - 12c07a25b65fee8e7bca0ed2aa35f76e446d1241
commit + 4209d3aaca8962c082d5164b2881488c50b4c5eb
blob - 6cf8ccdbc08a98cb14efa193fceb1eb92fd05f97
blob + 3c54a78702c5a0acbe54c9948029dcd3ebba2ccc
--- got/got.1
+++ got/got.1
@@ -104,12 +104,11 @@ are as follows:
 .Bl -tag -width Ds
 .It Fl b Ar branch
 Create the specified
-.Ar branch
-instead of creating the default branch
-.Dq main .
-Use of this option is required if the
-.Dq main
-branch already exists.
+.Ar branch .
+If this option is not specified, a branch corresponding to the repository's
+HEAD reference will be used.
+Use of this option is required if the branch resolved via the repository's
+HEAD reference already exists.
 .It Fl I Ar pattern
 Ignore files or directories with a name which matches the specified
 .Ar pattern .
blob - 04b01cb72ee7dc2b7005d3926a66b8488cbd5579
blob + 7c95dd13a7945672200a10ba9c507d183e842406
--- got/got.c
+++ got/got.c
@@ -759,12 +759,13 @@ cmd_import(int argc, char *argv[])
 	const struct got_error *error = NULL;
 	char *path_dir = NULL, *repo_path = NULL, *logmsg = NULL;
 	char *gitconfig_path = NULL, *editor = NULL, *author = NULL;
-	const char *branch_name = "main";
-	char *refname = NULL, *id_str = NULL, *logmsg_path = NULL;
+	const char *branch_name = NULL;
+	char *id_str = NULL, *logmsg_path = NULL;
+	char refname[PATH_MAX] = "refs/heads/";
 	struct got_repository *repo = NULL;
 	struct got_reference *branch_ref = NULL, *head_ref = NULL;
 	struct got_object_id *new_commit_id = NULL;
-	int ch;
+	int ch, n = 0;
 	struct got_pathlist_head ignores;
 	struct got_pathlist_entry *pe;
 	int preserve_logmsg = 0;
@@ -843,12 +844,23 @@ cmd_import(int argc, char *argv[])
 	 * While technically a valid reference name, this case is usually
 	 * an unintended typo.
 	 */
-	if (branch_name[0] == '-')
+	if (branch_name && branch_name[0] == '-')
 		return got_error_path(branch_name, GOT_ERR_REF_NAME_MINUS);
 
-	if (asprintf(&refname, "refs/heads/%s", branch_name) == -1) {
-		error = got_error_from_errno("asprintf");
+	error = got_ref_open(&head_ref, repo, GOT_REF_HEAD, 0);
+	if (error && error->code != GOT_ERR_NOT_REF)
 		goto done;
+
+	if (branch_name)
+		n = strlcat(refname, branch_name, sizeof(refname));
+	else if (head_ref && got_ref_is_symbolic(head_ref))
+		n = strlcpy(refname, got_ref_get_symref_target(head_ref),
+		    sizeof(refname));
+	else
+		n = strlcat(refname, "main", sizeof(refname));
+	if (n >= sizeof(refname)) {
+		error = got_error(GOT_ERR_NO_SPACE);
+		goto done;
 	}
 
 	error = got_ref_open(&branch_ref, repo, refname, 0);
@@ -972,7 +984,6 @@ done:
 	free(logmsg_path);
 	free(repo_path);
 	free(editor);
-	free(refname);
 	free(new_commit_id);
 	free(id_str);
 	free(author);
@@ -1689,7 +1700,7 @@ cmd_clone(int argc, char *argv[])
 		goto done;
 
 	if (!list_refs_only) {
-		error = got_repo_init(repo_path);
+		error = got_repo_init(repo_path, NULL);
 		if (error)
 			goto done;
 		error = got_repo_pack_fds_open(&pack_fds);
blob - 80901f0e63c615048925c439ab54ddeb1f70b743
blob + c633b3177963aca3fa00f1a3474129c3b4441c1b
--- gotadmin/gotadmin.1
+++ gotadmin/gotadmin.1
@@ -53,7 +53,7 @@ The commands for
 .Nm
 are as follows:
 .Bl -tag -width checkout
-.It Cm init Ar repository-path
+.It Cm init Oo Fl b Ar branch Oc Ar repository-path
 Create a new empty repository at the specified
 .Ar repository-path .
 .Pp
@@ -64,6 +64,17 @@ the
 command must be used to populate the empty repository before
 .Cm got checkout
 can be used.
+.Pp
+The options for
+.Cm gotadmin init
+are as follows:
+.Bl -tag -width Ds
+.It Fl b Ar branch
+Make the repository's HEAD reference point to the specified
+.Ar branch
+instead of the default branch
+.Dq main .
+.El
 .It Cm info Op Fl r Ar repository-path
 Display information about a repository.
 This includes some configuration settings from
blob - a378b760fc6dade37673aee3debd9a72eddce03a
blob + 075ff5086825e3041fdc58673e8adca7e6016635
--- gotadmin/gotadmin.c
+++ gotadmin/gotadmin.c
@@ -271,7 +271,8 @@ done:
 __dead static void
 usage_init(void)
 {
-	fprintf(stderr, "usage: %s init repository-path\n", getprogname());
+	fprintf(stderr, "usage: %s init [-b branch] repository-path\n",
+	    getprogname());
 	exit(1);
 }
 
@@ -279,11 +280,15 @@ static const struct got_error *
 cmd_init(int argc, char *argv[])
 {
 	const struct got_error *error = NULL;
+	const char *head_name = NULL;
 	char *repo_path = NULL;
 	int ch;
 
-	while ((ch = getopt(argc, argv, "")) != -1) {
+	while ((ch = getopt(argc, argv, "b:")) != -1) {
 		switch (ch) {
+		case 'b':
+			head_name = optarg;
+			break;
 		default:
 			usage_init();
 			/* NOTREACHED */
@@ -315,7 +320,7 @@ cmd_init(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	error = got_repo_init(repo_path);
+	error = got_repo_init(repo_path, head_name);
 done:
 	free(repo_path);
 	return error;
blob - dea6dd81d267dfa92571a33f5c7559726bab8d8b
blob + 4f8b24e0e19f1af4e21707388cf9bbf9bb54cd18
--- include/got_repository.h
+++ include/got_repository.h
@@ -130,8 +130,11 @@ int got_repo_is_bare(struct got_repository *);
 const struct got_error *got_repo_map_path(char **, struct got_repository *,
     const char *);
 
-/* Create a new repository in an empty directory at a specified path. */
-const struct got_error *got_repo_init(const char *);
+/*
+ * Create a new repository with optional specified
+ * HEAD ref in an empty directory at a specified path.
+ */
+const struct got_error *got_repo_init(const char *, const char *);
 
 /* Attempt to find a unique object ID for a given ID string prefix. */
 const struct got_error *got_repo_match_object_id_prefix(struct got_object_id **,
blob - 1e72320442b9c090e0bfb14fb164b3ae9cfe431a
blob + 5b8073b6d323365b34814adee39ff939c26df08e
--- lib/repository.c
+++ lib/repository.c
@@ -1556,7 +1556,7 @@ got_repo_unpin_pack(struct got_repository *repo)
 }
 
 const struct got_error *
-got_repo_init(const char *repo_path)
+got_repo_init(const char *repo_path, const char *head_name)
 {
 	const struct got_error *err = NULL;
 	const char *dirnames[] = {
@@ -1566,12 +1566,12 @@ got_repo_init(const char *repo_path)
 	};
 	const char *description_str = "Unnamed repository; "
 	    "edit this file 'description' to name the repository.";
-	const char *headref_str = "ref: refs/heads/main";
+	const char *headref = "ref: refs/heads/";
 	const char *gitconfig_str = "[core]\n"
 	    "\trepositoryformatversion = 0\n"
 	    "\tfilemode = true\n"
 	    "\tbare = true\n";
-	char *path;
+	char *headref_str, *path;
 	size_t i;
 
 	if (!got_path_dir_is_empty(repo_path))
@@ -1596,7 +1596,13 @@ got_repo_init(const char *repo_path)
 
 	if (asprintf(&path, "%s/%s", repo_path, GOT_HEAD_FILE) == -1)
 		return got_error_from_errno("asprintf");
+	if (asprintf(&headref_str, "%s%s", headref,
+	    head_name ? head_name : "main") == -1) {
+		free(path);
+		return got_error_from_errno("asprintf");
+	}
 	err = got_path_create_file(path, headref_str);
+	free(headref_str);
 	free(path);
 	if (err)
 		return err;
blob - 8d8a754cfb9a23339b32bae0fd1802a079c9370c
blob + 320eaec8470bbe78c68da2fa94637a86fe8832aa
--- regress/cmdline/Makefile
+++ regress/cmdline/Makefile
@@ -54,6 +54,9 @@ backout:
 rebase:
 	./rebase.sh -q -r "$(GOT_TEST_ROOT)"
 
+init:
+	./init.sh -q -r "$(GOT_TEST_ROOT)"
+
 import:
 	./import.sh -q -r "$(GOT_TEST_ROOT)"
 
blob - 71d20c61b9666f8571b8996f1bb477f6e01a3c1f
blob + 9ce21cacfc1fea14fa9289ed4180e0640cfdffdf
--- regress/cmdline/import.sh
+++ regress/cmdline/import.sh
@@ -140,6 +140,199 @@ test_import_basic() {
 	test_done "$testroot" "$ret"
 }
 
+test_import_specified_head() {
+	local testname=import_specified_head
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
+	local headref=trunk
+
+	gotadmin init -b $headref $testroot/repo
+
+	mkdir $testroot/tree
+	make_test_tree $testroot/tree
+
+	got import -m init -r $testroot/repo $testroot/tree > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local head_commit=`git_show_head $testroot/repo`
+	echo "A  $testroot/tree/gamma/delta" > $testroot/stdout.expected
+	echo "A  $testroot/tree/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/tree/alpha" >> $testroot/stdout.expected
+	echo "A  $testroot/tree/beta" >> $testroot/stdout.expected
+	echo "Created branch refs/heads/$headref with commit $head_commit" \
+	    >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/repo && got log -p | grep -v ^date: > $testroot/stdout)
+
+	id_alpha=`get_blob_id $testroot/repo "" alpha`
+	id_beta=`get_blob_id $testroot/repo "" beta`
+	id_zeta=`get_blob_id $testroot/repo epsilon zeta`
+	id_delta=`get_blob_id $testroot/repo gamma delta`
+	tree_id=`(cd $testroot/repo && got cat $head_commit | \
+	    grep ^tree | cut -d ' ' -f 2)`
+
+	echo "-----------------------------------------------" \
+	    > $testroot/stdout.expected
+	echo "commit $head_commit ($headref)" >> $testroot/stdout.expected
+	echo "from: $GOT_AUTHOR" >> $testroot/stdout.expected
+	echo " " >> $testroot/stdout.expected
+	echo " init" >> $testroot/stdout.expected
+	echo " " >> $testroot/stdout.expected
+	echo "diff /dev/null $head_commit" >> $testroot/stdout.expected
+	echo "commit - /dev/null" >> $testroot/stdout.expected
+	echo "commit + $head_commit" >> $testroot/stdout.expected
+	echo "blob - /dev/null" >> $testroot/stdout.expected
+	echo "blob + $id_alpha (mode 644)" >> $testroot/stdout.expected
+	echo "--- /dev/null" >> $testroot/stdout.expected
+	echo "+++ alpha" >> $testroot/stdout.expected
+	echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected
+	echo "+alpha" >> $testroot/stdout.expected
+	echo "blob - /dev/null" >> $testroot/stdout.expected
+	echo "blob + $id_beta (mode 644)" >> $testroot/stdout.expected
+	echo "--- /dev/null" >> $testroot/stdout.expected
+	echo "+++ beta" >> $testroot/stdout.expected
+	echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected
+	echo "+beta" >> $testroot/stdout.expected
+	echo "blob - /dev/null" >> $testroot/stdout.expected
+	echo "blob + $id_zeta (mode 644)" >> $testroot/stdout.expected
+	echo "--- /dev/null" >> $testroot/stdout.expected
+	echo "+++ epsilon/zeta" >> $testroot/stdout.expected
+	echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected
+	echo "+zeta" >> $testroot/stdout.expected
+	echo "blob - /dev/null" >> $testroot/stdout.expected
+	echo "blob + $id_delta (mode 644)" >> $testroot/stdout.expected
+	echo "--- /dev/null" >> $testroot/stdout.expected
+	echo "+++ gamma/delta" >> $testroot/stdout.expected
+	echo "@@ -0,0 +1 @@" >> $testroot/stdout.expected
+	echo "+delta" >> $testroot/stdout.expected
+	echo "" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "A  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "A  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Checked out refs/heads/$headref: $head_commit" \
+	    >> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout $testroot/repo $testroot/wt > $testroot/stdout
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "alpha" > $testroot/content.expected
+	echo "beta" >> $testroot/content.expected
+	echo "zeta" >> $testroot/content.expected
+	echo "delta" >> $testroot/content.expected
+	cat $testroot/wt/alpha $testroot/wt/beta $testroot/wt/epsilon/zeta \
+	    $testroot/wt/gamma/delta > $testroot/content
+
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/content.expected $testroot/content
+	fi
+	test_done "$testroot" "$ret"
+}
+
+test_import_detached_head() {
+	local testroot=`test_init import_detached_head`
+
+	# mute verbose 'detached HEAD' warning
+	(cd $testroot/repo && git config --local advice.detachedHead false)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# enter detached HEAD state
+	local head_commit=`git_show_head $testroot/repo | cut -c1-7`
+	(cd $testroot/repo && \
+	    git checkout $head_commit > $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "HEAD is now at $head_commit adding the test tree" >> \
+	    $testroot/stderr.expected
+
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	mkdir $testroot/import
+	make_test_tree $testroot/import
+
+	# detached HEAD (i.e., not symbolic) so import should fallback to "main"
+	got import -r $testroot/repo -m init $testroot/import > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local main_commit=`(cd $testroot/repo && \
+	    git show-ref main | cut -d ' ' -f 1)`
+	echo "A  $testroot/import/gamma/delta" > $testroot/stdout.expected
+	echo "A  $testroot/import/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/import/alpha" >> $testroot/stdout.expected
+	echo "A  $testroot/import/beta" >> $testroot/stdout.expected
+	echo "Created branch refs/heads/main with commit $main_commit" \
+	    >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_import_requires_new_branch() {
 	local testroot=`test_init import_requires_new_branch`
 
@@ -297,6 +490,8 @@ test_import_symlink() {
 
 test_parseargs "$@"
 run_test test_import_basic
+run_test test_import_specified_head
+run_test test_import_detached_head
 run_test test_import_requires_new_branch
 run_test test_import_ignores
 run_test test_import_empty_dir
blob - /dev/null
blob + c0aa7e8388bf26432184a39d3c0257a016975b76 (mode 755)
--- /dev/null
+++ regress/cmdline/init.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+#
+# Copyright (c) 2022 Mark Jamsek <mark@jamsek.dev>
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+. ./common.sh
+
+test_init_basic() {
+	local testname=init_basic
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
+	local headref=main
+
+	gotadmin init $testroot/repo
+
+	local git_head=`(cd $testroot/repo && git symbolic-ref HEAD)`
+	echo $git_head > $testroot/content
+	echo refs/heads/$headref > $testroot/content.expected
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/content.expected $testroot/content
+	fi
+	test_done "$testroot" "$ret"
+}
+
+test_init_specified_head() {
+	local testname=init_specified_head
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
+	local headref=trunk
+
+	gotadmin init -b $headref $testroot/repo
+
+	local git_head=`(cd $testroot/repo && git symbolic-ref HEAD)`
+	echo refs/heads/$headref > $testroot/content.expected
+	echo $git_head > $testroot/content
+	cmp -s $testroot/content.expected $testroot/content
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "fail"
+		diff -u $testroot/content.expected $testroot/content
+	fi
+	test_done "$testroot" "$ret"
+}
+
+test_parseargs "$@"
+run_test test_init_basic
+run_test test_init_specified_head


-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68