"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

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:
Mon, 19 Sep 2022 23:01:51 +1000

Download raw body.

Thread
On 22-09-09 12:18AM, Mark Jamsek wrote:
> On 22-09-03 03:35PM, Stefan Sperling wrote:
> > On Sat, Sep 03, 2022 at 10:59:28PM +1000, Mark Jamsek wrote:
> > > This is a reworked diff that changes the behaviour of both `gotadmin
> > > init` and `got import` that was briefly discussed with stsp in person.
> > > It enables creating an arbitrary HEAD reference at repo creation with
> > > `gotadmin init`. We also no longer hardcode "main" in `got import` but
> > > instead read HEAD so if a branch is not specified with -b, import
> > > defaults to the branch resolved via the repository's HEAD reference.
> > 
> > This seems fine to me.
> 
> Great! Thanks for reviewing this.
> 
> > There are no changes in regress tests. Is that because they won't be
> > needed or because they have yet to be written?
> 
> No, I think this is needed; however, after discussing this with you last
> week I wanted to get your thoughts on the diff before continuing.
> 
> I got home last night but haven't done much today. I'll finish this
> tomorrow and shoot through the complete diff for review.

Tomorrow == next week :)

I didn't forget about this, I've just been ticking off items from the
backlog as opportunities presented themselves the last week or so.

Three new regress tests were added to cover the new behaviour in
gotadmin init and got import:

  1- gotadmin init
  2- gotadmin init -b
  3- got import for the case where HEAD is not "main" (i.e., specified
    via init -b)

I'm not sure if we should just run the gotadmin init tests inside the
import tests, tbh. We could really run all the got import tests again
against a repo created with `gotadmin init -b` as (3) is just the basic
import with init -b, but I figured it wouldn't hurt to keep them
independent.

-----------------------------------------------
commit 32cb0664669dc660f2c5f92c0f0a6b1798cdeea3 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Mon Sep 19 12:57:17 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 determined do we fallback to "main".
 
diff 34a842a42b1cd020ca656445c0b65e67a97bff1a 32cb0664669dc660f2c5f92c0f0a6b1798cdeea3
commit - 34a842a42b1cd020ca656445c0b65e67a97bff1a
commit + 32cb0664669dc660f2c5f92c0f0a6b1798cdeea3
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 + 022da0fb9398503c8c3e9beaad861d04603cdae4
--- 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)
+		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 + 6c188518510e9e3fe3a4ba4dd43bca8667d10f1e
--- regress/cmdline/import.sh
+++ regress/cmdline/import.sh
@@ -140,6 +140,137 @@ 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_requires_new_branch() {
 	local testroot=`test_init import_requires_new_branch`
 
@@ -297,6 +428,7 @@ test_import_symlink() {
 
 test_parseargs "$@"
 run_test test_import_basic
+run_test test_import_specified_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