From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: got add recursion ignore ignores
To: gameoftrees@openbsd.org
Date: Thu, 5 Dec 2019 12:44:18 -0700

On Thu, Dec 05, 2019 at 06:57:12PM +0100, Stefan Sperling wrote:
> On Thu, Dec 05, 2019 at 10:22:43AM -0700, Tracey Emery wrote:
> > Hello,
> > 
> > The diff below adds -I to disregard ignores. Adding flags to the
> > got_worktree struct seemed like the simplest approach. If there is a
> > better alternative, let me know what you think.
> 
> Would it work to add a new 'int' argument to worktree_status() instead?
> It seems no_ignores could be passed from got_worktree_schedule_add()
> to worktree_status(). All other callers of worktree_status() would pass 0.
> 
> Adding this information to a new flags field in struct got_worktree
> essentially creates a global variable. I'd prefer to pass arguments so
> that it becomes easier to see where the values get used further down
> the call chain. And should use of the variable change later, auditing for
> unused function arguments is easier than auditing for unnecessary checks
> of a global flag.

Diff below removes global flag and passes int argument instead.

Ok?

-- 

Tracey Emery

diff 22694bb8ff1cf412bbdfc251e2dce27baca160b6 /home/basepr1me/Documents/got/got
blob - 044aadf30b3ae798d851660a58e8dddd57a6b746
file + TODO
--- TODO
+++ TODO
@@ -11,7 +11,6 @@ lib:
 
 got:
 - 'histedit -c' prompts for log message even if there are no changes to commit
-- recursive addition: got add -R
 - recursive removal: got rm -R
 
 tog:
blob - 57af02b4edfdbb7d0e6764d05c516a435f823c56
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -625,7 +625,7 @@ If a tag must be deleted, the
 command may be used to delete a tag's reference.
 This should only be done if the tag has not already been copied to
 another repository.
-.It Cm add Oo Fl R Oc Ar path ...
+.It Cm add Oo Fl R Oc Oo Fl I Oc Ar path ...
 Schedule unversioned files in a work tree for addition to the
 repository in the next commit.
 .Pp
@@ -640,6 +640,10 @@ If this option is not specified,
 will refuse to run if a specified
 .Ar path
 is a directory.
+.It Fl I
+With -R, add files even if they match a
+.Cm got status
+ignore pattern.
 .El
 .It Cm remove Ar file-path ...
 Remove versioned files from a work tree and schedule them for deletion
blob - 756eaa2ea3216dabe5aaf7080a2c652fdbfd0f74
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -4146,7 +4146,8 @@ done:
 __dead static void
 usage_add(void)
 {
-	fprintf(stderr, "usage: %s add file-path ...\n", getprogname());
+	fprintf(stderr, "usage: %s add [-R] [-I] file-path ...\n",
+	    getprogname());
 	exit(1);
 }
 
@@ -4168,12 +4169,15 @@ cmd_add(int argc, char *argv[])
 	char *cwd = NULL;
 	struct got_pathlist_head paths;
 	struct got_pathlist_entry *pe;
-	int ch, can_recurse = 0;
+	int ch, can_recurse = 0, no_ignores = 0;
 
 	TAILQ_INIT(&paths);
 
-	while ((ch = getopt(argc, argv, "R")) != -1) {
+	while ((ch = getopt(argc, argv, "IR")) != -1) {
 		switch (ch) {
+		case 'I':
+			no_ignores = 1;
+			break;
 		case 'R':
 			can_recurse = 1;
 			break;
@@ -4218,6 +4222,13 @@ cmd_add(int argc, char *argv[])
 	if (error)
 		goto done;
 
+	if (!can_recurse && no_ignores) {
+		error = got_error_msg(GOT_ERR_BAD_PATH,
+		    "disregarding ignores requires -R option");
+			goto done;
+
+	}
+
 	if (!can_recurse) {
 		char *ondisk_path;
 		struct stat sb;
@@ -4246,8 +4257,9 @@ cmd_add(int argc, char *argv[])
 			}
 		}
 	}
+
 	error = got_worktree_schedule_add(worktree, &paths, add_progress,
-	    NULL, repo);
+	    NULL, repo, no_ignores);
 done:
 	if (repo)
 		got_repo_close(repo);
blob - a18fc5a59d6c2c165d6f449c5accb262eb450485
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -159,7 +159,7 @@ const struct got_error *got_worktree_resolve_path(char
 /* Schedule files at on-disk paths for addition in the next commit. */
 const struct got_error *got_worktree_schedule_add(struct got_worktree *,
     struct got_pathlist_head *, got_worktree_checkout_cb, void *,
-    struct got_repository *);
+    struct got_repository *, int);
 
 /*
  * Remove files from disk and schedule them to be deleted in the next commit.
blob - c56c307e7fad0d579e6eb3fa799150a8b8d31558
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -2637,7 +2637,7 @@ static const struct got_error *
 worktree_status(struct got_worktree *worktree, const char *path,
     struct got_fileindex *fileindex, struct got_repository *repo,
     got_worktree_status_cb status_cb, void *status_arg,
-    got_cancel_cb cancel_cb, void *cancel_arg)
+    got_cancel_cb cancel_cb, void *cancel_arg, int no_ignores)
 {
 	const struct got_error *err = NULL;
 	DIR *workdir = NULL;
@@ -2670,11 +2670,13 @@ worktree_status(struct got_worktree *worktree, const c
 		arg.cancel_cb = cancel_cb;
 		arg.cancel_arg = cancel_arg;
 		TAILQ_INIT(&arg.ignores);
-		err = add_ignores(&arg.ignores, worktree->root_path, path,
-		    ".cvsignore");
-		if (err == NULL)
+		if (!no_ignores) {
 			err = add_ignores(&arg.ignores, worktree->root_path,
-			    path, ".gitignore");
+			    path, ".cvsignore");
+			if (err == NULL)
+				err = add_ignores(&arg.ignores,
+				    worktree->root_path, path, ".gitignore");
+		}
 		if (err == NULL)
 			err = got_fileindex_diff_dir(fileindex, workdir,
 			    worktree->root_path, path, repo, &fdiff_cb, &arg);
@@ -2704,7 +2706,7 @@ got_worktree_status(struct got_worktree *worktree,
 
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-			status_cb, status_arg, cancel_cb, cancel_arg);
+			status_cb, status_arg, cancel_cb, cancel_arg, 0);
 		if (err)
 			break;
 	}
@@ -2840,7 +2842,7 @@ const struct got_error *
 got_worktree_schedule_add(struct got_worktree *worktree,
     struct got_pathlist_head *paths,
     got_worktree_checkout_cb progress_cb, void *progress_arg,
-    struct got_repository *repo)
+    struct got_repository *repo, int no_ignores)
 {
 	struct got_fileindex *fileindex = NULL;
 	char *fileindex_path = NULL;
@@ -2869,7 +2871,7 @@ got_worktree_schedule_add(struct got_worktree *worktre
 			return got_error_from_errno("asprintf");
 		saa.ondisk_path = ondisk_path;
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-			schedule_addition, &saa, NULL, NULL);
+			schedule_addition, &saa, NULL, NULL, no_ignores);
 		free(ondisk_path);
 		if (err)
 			break;
@@ -3517,7 +3519,7 @@ got_worktree_revert(struct got_worktree *worktree,
 	rfa.repo = repo;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    revert_file, &rfa, NULL, NULL);
+		    revert_file, &rfa, NULL, NULL, 0);
 		if (err)
 			break;
 	}
@@ -4430,7 +4432,7 @@ got_worktree_commit(struct got_object_id **new_commit_
 	cc_arg.have_staged_files = have_staged_files;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    collect_commitables, &cc_arg, NULL, NULL);
+		    collect_commitables, &cc_arg, NULL, NULL, 0);
 		if (err)
 			goto done;
 	}
@@ -4988,13 +4990,13 @@ rebase_commit(struct got_object_id **new_commit_id,
 		}
 		TAILQ_FOREACH(pe, merged_paths, entry) {
 			err = worktree_status(worktree, pe->path, fileindex,
-			    repo, collect_commitables, &cc_arg, NULL, NULL);
+			    repo, collect_commitables, &cc_arg, NULL, NULL, 0);
 			if (err)
 				goto done;
 		}
 	} else {
 		err = worktree_status(worktree, "", fileindex, repo,
-		    collect_commitables, &cc_arg, NULL, NULL);
+		    collect_commitables, &cc_arg, NULL, NULL, 0);
 		if (err)
 			goto done;
 	}
@@ -5302,7 +5304,7 @@ got_worktree_rebase_abort(struct got_worktree *worktre
 	rfa.patch_arg = NULL;
 	rfa.repo = repo;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    revert_file, &rfa, NULL, NULL);
+	    revert_file, &rfa, NULL, NULL, 0);
 	if (err)
 		goto sync;
 
@@ -5655,7 +5657,7 @@ got_worktree_histedit_abort(struct got_worktree *workt
 	rfa.patch_arg = NULL;
 	rfa.repo = repo;
 	err = worktree_status(worktree, "", fileindex, repo,
-	    revert_file, &rfa, NULL, NULL);
+	    revert_file, &rfa, NULL, NULL, 0);
 	if (err)
 		goto sync;
 
@@ -6111,7 +6113,7 @@ got_worktree_stage(struct got_worktree *worktree,
 	oka.have_changes = 0;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    check_stage_ok, &oka, NULL, NULL);
+		    check_stage_ok, &oka, NULL, NULL, 0);
 		if (err)
 			goto done;
 	}
@@ -6130,7 +6132,7 @@ got_worktree_stage(struct got_worktree *worktree,
 	spa.staged_something = 0;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    stage_path, &spa, NULL, NULL);
+		    stage_path, &spa, NULL, NULL, 0);
 		if (err)
 			goto done;
 	}
@@ -6481,7 +6483,7 @@ got_worktree_unstage(struct got_worktree *worktree,
 	upa.patch_arg = patch_arg;
 	TAILQ_FOREACH(pe, paths, entry) {
 		err = worktree_status(worktree, pe->path, fileindex, repo,
-		    unstage_path, &upa, NULL, NULL);
+		    unstage_path, &upa, NULL, NULL, 0);
 		if (err)
 			goto done;
 	}
blob - 9260016060fa594ef14a31a90e815737bb487ca1
file + regress/cmdline/add.sh
--- regress/cmdline/add.sh
+++ regress/cmdline/add.sh
@@ -163,12 +163,19 @@ function test_add_directory {
 
 	(cd $testroot/wt && got add . > $testroot/stdout 2> $testroot/stderr)
 	ret="$?"
-	if [ "$ret" == "0" ]; then
-		echo "got add command succeeded unexpectedly" >&2
-		test_done "$testroot" "1"
+	echo "got: adding directories requires -R option" \
+		> $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 "got: adding directories requires -R option" \
+
+	(cd $testroot/wt && got add -I . > $testroot/stdout 2> $testroot/stderr)
+	ret="$?"
+	echo "got: disregarding ignores requires -R option" \
 		> $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret="$?"
@@ -187,11 +194,19 @@ function test_add_directory {
 		return 1
 	fi
 
-	(touch $testroot/wt/epsilon/zeta1 && touch $testroot/wt/epsilon/zeta2)
+	mkdir -p $testroot/wt/tree1
+	mkdir -p $testroot/wt/tree2
+	echo "tree1/**" > $testroot/wt/.gitignore
+	echo "tree2/**" >> $testroot/wt/.gitignore
+	echo -n > $testroot/wt/tree1/foo
+	echo -n > $testroot/wt/tree2/foo
+	echo -n > $testroot/wt/epsilon/zeta1
+	echo -n > $testroot/wt/epsilon/zeta2
 
 	(cd $testroot/wt && got add -R . > $testroot/stdout)
 
-	echo 'A  epsilon/zeta1' > $testroot/stdout.expected
+	echo 'A  .gitignore' > $testroot/stdout.expected
+	echo 'A  epsilon/zeta1' >> $testroot/stdout.expected
 	echo 'A  epsilon/zeta2' >> $testroot/stdout.expected
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
@@ -202,13 +217,28 @@ function test_add_directory {
 		return 1
 	fi
 
-	echo "zeta" > $testroot/content.expected
-	cat $testroot/wt/epsilon/zeta > $testroot/content
+	(cd $testroot/wt && got add -RI tree1 > $testroot/stdout)
 
-	cmp -s $testroot/content.expected $testroot/content
+	echo 'A  tree1/foo' > $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
-		diff -u $testroot/content.expected $testroot/content
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got add tree2/foo > $testroot/stdout)
+
+	echo 'A  tree2/foo' > $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
 	test_done "$testroot" "$ret"
 }