From: Stefan Sperling <stsp@stsp.name>
Subject: Re: allow checkout into non-empty directory
To: gameoftrees@openbsd.org
Date: Mon, 13 Jan 2020 11:00:02 +0100

On Mon, Jan 13, 2020 at 09:19:25AM +0100, Klemens Nanni wrote:
> On Sun, Jan 12, 2020 at 11:44:24PM +0100, Stefan Sperling wrote:
> > kn@ has a use case for running 'got checkout' on top of existing files.
> >
> > At present 'got checkout' refuses to operate on a non-empty directory.
> > This adds a new 'got checkout -E' operation mode which ignores existing
> > files and proceeds with the checkout operation anyway.
> > See the new regress test for behaviour details.
> OK kn with S_IF* fixed, rest are just comments.
> 
> > @@ -136,8 +136,14 @@ follows the globbing rules documented in
> >  .It Cm im
> >  Short alias for
> >  .Cm import .
> > -.It Cm checkout Oo Fl b Ar branch Oc Oo Fl c Ar commit Oc Oo Fl p Ar path-prefix Oc Ar repository-path Op Ar work-tree-path
> > +.It Cm checkout Oo Fl b Ar branch Oc Oo Fl c Ar commit Oc Oo Fl E Oc Oo Fl p Ar path-prefix Oc Ar repository-path Op Ar work-tree-path
> Flags without options usually go first, .e.g. 
> 
> 	man [-acfhklw] [-C file] [-M path] [-m path] [-S subsection] [[-s] section]
> 	    name ...

The current ordering of options in usage strings and man pages is rather
adh-hoc all over the place I'm afraid. I was hoping someone would eventually
go through and fix it up.

> > @@ -166,6 +172,11 @@ An abbreviated hash argument will be expanded to a ful
> >  automatically, provided the abbreviation is unique.
> >  If this option is not specified, the most recent commit on the selected
> >  branch will be used.
> > +.It Fl E
> > +Proceed with the checkout operation even if the directory at
> > +.Ar work-tree-path
> > +is not empty.
> > +Existing file contents will be left intact.
> I'd omit "contents", you're leaving permissions alone as well of course.
> The simpler the better.
> 
> > @@ -800,7 +800,7 @@ __dead static void
> >  usage_checkout(void)
> >  {
> >  	fprintf(stderr, "usage: %s checkout [-b branch] [-c commit] "
> > -	    "[-p prefix] repository-path [worktree-path]\n", getprogname());
> > +	    "[-E] [-p prefix] repository-path [worktree-path]\n", getprogname());
> Move `[-E]' after checkout as above.
> 
> > @@ -16,8 +16,10 @@
> >  
> >  /* Utilities for dealing with filesystem paths. */
> >  
> > -#define GOT_DEFAULT_FILE_MODE	(S_IRUSR|S_IWUSR | S_IRGRP | S_IROTH)
> > -#define GOT_DEFAULT_DIR_MODE	(S_IRWXU | S_IRGRP|S_IXGRP | S_IROTH|S_IXOTH)
> > +#define GOT_DEFAULT_FILE_MODE	S_IFREG | \
> > +	(S_IRUSR|S_IWUSR | S_IRGRP | S_IROTH)
> > +#define GOT_DEFAULT_DIR_MODE	S_IFDIR | \
> > +	(S_IRWXU | S_IRGRP|S_IXGRP | S_IROTH|S_IXOTH)
> S_IF* must be guarded inside the parantheses.

Good catch, thanks!

New diff:

diff c5b7833452086e7e8e0866b16a5932430cfe9a79 /home/stsp/src/got
blob - 4099167c32d0a15e7689e9a2b02ba26dcf17d90f
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -136,8 +136,14 @@ follows the globbing rules documented in
 .It Cm im
 Short alias for
 .Cm import .
-.It Cm checkout Oo Fl b Ar branch Oc Oo Fl c Ar commit Oc Oo Fl p Ar path-prefix Oc Ar repository-path Op Ar work-tree-path
+.It Cm checkout  Oo Fl E Oc Oo Fl b Ar branch Oc Oo Fl c Ar commit OcOo Fl p Ar path-prefix Oc Ar repository-path Op Ar work-tree-path
 Copy files from a repository into a new work tree.
+Show the status of each affected file, using the following status codes:
+.Bl -column YXZ description
+.It A Ta new file was added
+.It E Ta file already exists in work tree's meta-data
+.El
+.Pp
 If the
 .Ar work tree path
 is not specified, either use the last component of
@@ -151,6 +157,11 @@ The options for
 .Cm got checkout
 are as follows:
 .Bl -tag -width Ds
+.It Fl E
+Proceed with the checkout operation even if the directory at
+.Ar work-tree-path
+is not empty.
+Existing files will be left intact.
 .It Fl b Ar branch
 Check out files from a commit on the specified
 .Ar branch .
blob - 5490ed3772d0e6daa9cd31a902f0f3129bb779f5
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -799,7 +799,7 @@ done:
 __dead static void
 usage_checkout(void)
 {
-	fprintf(stderr, "usage: %s checkout [-b branch] [-c commit] "
+	fprintf(stderr, "usage: %s checkout [-E] [-b branch] [-c commit] "
 	    "[-p prefix] repository-path [worktree-path]\n", getprogname());
 	exit(1);
 }
@@ -998,13 +998,13 @@ cmd_checkout(int argc, char *argv[])
 	const char *path_prefix = "";
 	const char *branch_name = GOT_REF_HEAD;
 	char *commit_id_str = NULL;
-	int ch, same_path_prefix;
+	int ch, same_path_prefix, allow_nonempty = 0;
 	struct got_pathlist_head paths;
 	struct got_checkout_progress_arg cpa;
 
 	TAILQ_INIT(&paths);
 
-	while ((ch = getopt(argc, argv, "b:c:p:")) != -1) {
+	while ((ch = getopt(argc, argv, "b:c:Ep:")) != -1) {
 		switch (ch) {
 		case 'b':
 			branch_name = optarg;
@@ -1014,6 +1014,9 @@ cmd_checkout(int argc, char *argv[])
 			if (commit_id_str == NULL)
 				return got_error_from_errno("strdup");
 			break;
+		case 'E':
+			allow_nonempty = 1;
+			break;
 		case 'p':
 			path_prefix = optarg;
 			break;
@@ -1100,7 +1103,8 @@ cmd_checkout(int argc, char *argv[])
 		if (!(error->code == GOT_ERR_ERRNO && errno == EISDIR) &&
 		    !(error->code == GOT_ERR_ERRNO && errno == EEXIST))
 			goto done;
-		if (!got_path_dir_is_empty(worktree_path)) {
+		if (!allow_nonempty &&
+		    !got_path_dir_is_empty(worktree_path)) {
 			error = got_error_path(worktree_path,
 			    GOT_ERR_DIR_NOT_EMPTY);
 			goto done;
blob - 756faf8e9b273a128bad4efb6aedef83e1d5426c
file + include/got_path.h
--- include/got_path.h
+++ include/got_path.h
@@ -16,8 +16,10 @@
 
 /* Utilities for dealing with filesystem paths. */
 
-#define GOT_DEFAULT_FILE_MODE	(S_IRUSR|S_IWUSR | S_IRGRP | S_IROTH)
-#define GOT_DEFAULT_DIR_MODE	(S_IRWXU | S_IRGRP|S_IXGRP | S_IROTH|S_IXOTH)
+#define GOT_DEFAULT_FILE_MODE	(S_IFREG | \
+	S_IRUSR|S_IWUSR | S_IRGRP | S_IROTH)
+#define GOT_DEFAULT_DIR_MODE	(S_IFDIR | \
+	S_IRWXU | S_IRGRP|S_IXGRP | S_IROTH|S_IXOTH)
 
 /* Determine whether a path is an absolute path. */
 int got_path_is_absolute(const char *);
blob - fcbeadbc09c83ae8ca419b0dd909c62dc50d4d99
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -937,7 +937,7 @@ get_ondisk_perms(int executable, mode_t st_mode)
 
 static const struct got_error *
 install_blob(struct got_worktree *worktree, const char *ondisk_path,
-    const char *path, uint16_t te_mode, uint16_t st_mode,
+    const char *path, mode_t te_mode, mode_t st_mode,
     struct got_blob_object *blob, int restoring_missing_file,
     int reverting_versioned_file, struct got_repository *repo,
     got_worktree_checkout_cb progress_cb, void *progress_arg)
blob - 34747be3c1c93a3c475b7e42583dca7f4ae844a4
file + regress/cmdline/checkout.sh
--- regress/cmdline/checkout.sh
+++ regress/cmdline/checkout.sh
@@ -356,6 +356,146 @@ function test_checkout_read_only {
 	test_done "$testroot" "$ret"
 }
 
+function test_checkout_into_nonempty_dir {
+	local testroot=`test_init checkout_into_nonempty_dir`
+
+	mkdir -p $testroot/wt
+	make_test_tree $testroot/wt
+
+	got checkout $testroot/repo $testroot/wt > $testroot/stdout \
+		2> $testroot/stderr
+	ret="$?"
+	if [ "$ret" == "0" ]; then
+		echo "checkout succeeded unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "got: $testroot/wt: directory exists and is not empty" \
+		> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "U  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "U  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "U  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "U  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout -E $testroot/repo $testroot/wt > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "E  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "E  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "E  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "E  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout -E $testroot/repo $testroot/wt > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		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" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "modified alpha" > $testroot/wt/alpha
+
+	echo "E  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "E  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "E  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "E  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
+
+	got checkout -E $testroot/repo $testroot/wt > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "modified 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" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return
+	fi
+
+	echo 'M  alpha' > $testroot/stdout.expected
+	(cd $testroot/wt && got status > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 run_test test_checkout_basic
 run_test test_checkout_dir_exists
 run_test test_checkout_dir_not_empty
@@ -364,3 +504,4 @@ run_test test_checkout_commit_from_wrong_branch
 run_test test_checkout_tag
 run_test test_checkout_ignores_submodules
 run_test test_checkout_read_only
+run_test test_checkout_into_nonempty_dir