Commit 1a36436d4d4024fada20498cad3039950c4b2da2

Stefan Sperling 2019-06-10T11:58:32

relax out-of-dateness check for 'got commit'

diff --git a/lib/worktree.c b/lib/worktree.c
index 2d03e99..acc38bf 100644
--- a/lib/worktree.c
+++ b/lib/worktree.c
@@ -3086,36 +3086,103 @@ check_ct_out_of_date(struct got_commitable *ct, struct got_repository *repo,
 	struct got_object_id *head_commit_id)
 {
 	const struct got_error *err = NULL;
-	struct got_object_id *id_in_head;
+	struct got_object_id *id_in_head = NULL, *id = NULL;
+	struct got_commit_object *commit = NULL;
+	char *path = NULL;
+	const char *ct_path = ct->in_repo_path;
+
+	while (ct_path[0] == '/')
+		ct_path++;
 
 	/*
-	 * Require that modified/deleted files are based on the branch head.
-	 * This requirement could be relaxed to force less update operations
-	 * on users but, for now, we want to play it safe and see how it goes.
+	 * Ensure that no modifications were made to files *and their parents*
+	 * in commits between the file's base commit and the branch head.
+	 *
+	 * Checking the parents is important for detecting conflicting tree
+	 * configurations (files or parent folders might have been moved,
+	 * deleted, added again, etc.). Such changes need to be merged with
+	 * local changes before a commit can occur.
 	 *
-	 * If this check is relaxed, it must still ensure that no modifications
-	 * were made to files *and their parents* in commits between the file's
-	 * base commit and the branch head. Otherwise, tree conflicts will not
-	 * be detected reliably.
+	 * The implication is that the file's (parent) entry in the root
+	 * directory must have the same ID in all relevant commits.
 	 */
 	if (ct->status != GOT_STATUS_ADD) {
-		if (got_object_id_cmp(ct->base_commit_id, head_commit_id) != 0)
-			return got_error(GOT_ERR_COMMIT_OUT_OF_DATE);
-		return NULL;
-	}
+		struct got_object_qid *pid;
+		char *slash;
+		struct got_object_id *root_entry_id = NULL;
 
-	/* Require that added files don't exist in the branch head. */
-	err = got_object_id_by_path(&id_in_head, repo, head_commit_id,
-	    ct->in_repo_path);
-	if (err && err->code != GOT_ERR_NO_TREE_ENTRY)
-		return err;
-	if (id_in_head) {
-		free(id_in_head);
-		return got_error(GOT_ERR_COMMIT_OUT_OF_DATE);
-	}
+		/* Trivial case: base commit == head commit */
+		if (got_object_id_cmp(ct->base_commit_id, head_commit_id) == 0)
+			return NULL;
+
+		/* Compute the path to the root directory's entry. */
+		path = strdup(ct_path);
+		if (path == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+		slash = strchr(path, '/');
+		if (slash)
+			*slash = '\0';
+
+		err = got_object_open_as_commit(&commit, repo, head_commit_id);
+		if (err)
+			goto done;
+
+		err = got_object_id_by_path(&root_entry_id, repo,
+		    head_commit_id, path);
+		if (err)
+			goto done;
+
+		pid = SIMPLEQ_FIRST(got_object_commit_get_parent_ids(commit));
+		while (pid) {
+			struct got_commit_object *pcommit;
+
+			err = got_object_id_by_path(&id, repo, pid->id, path);
+			if (err) {
+				if (err->code != GOT_ERR_NO_TREE_ENTRY)
+					goto done;
+				err = NULL;
+				break;
+			}
+
+			err = got_object_id_by_path(&id, repo, pid->id, path);
+			if (err)
+				goto done;
+
+			if (got_object_id_cmp(id, root_entry_id) != 0) {
+				err = got_error(GOT_ERR_COMMIT_OUT_OF_DATE);
+				break;
+			}
+
+			if (got_object_id_cmp(pid->id, ct->base_commit_id) == 0)
+				break; /* all relevant commits scanned */
+
+			err = got_object_open_as_commit(&pcommit, repo,
+			    pid->id);
+			if (err)
+				goto done;
 
+			got_object_commit_close(commit);
+			commit = pcommit;
+			pid = SIMPLEQ_FIRST(got_object_commit_get_parent_ids(
+			    commit));
+		}
+	} else {
+		/* Require that added files don't exist in the branch head. */
+		err = got_object_id_by_path(&id_in_head, repo, head_commit_id,
+		    ct_path);
+		if (err && err->code != GOT_ERR_NO_TREE_ENTRY)
+			goto done;
+		err = id_in_head ? got_error(GOT_ERR_COMMIT_OUT_OF_DATE) : NULL;
+	}
+done:
+	if (commit)
+		got_object_commit_close(commit);
 	free(id_in_head);
-	return NULL;
+	free(id);
+	free(path);
+	return err;
 }
 
 const struct got_error *
diff --git a/regress/cmdline/commit.sh b/regress/cmdline/commit.sh
index abae3cb..085d355 100755
--- a/regress/cmdline/commit.sh
+++ b/regress/cmdline/commit.sh
@@ -120,7 +120,7 @@ function test_commit_single_file {
 	echo "modified alpha" > $testroot/wt/alpha
 	echo "modified zeta" > $testroot/wt/epsilon/zeta
 
-	(cd $testroot/wt && got commit -m 'test commit_subdir' epsilon/zeta \
+	(cd $testroot/wt && got commit -m 'changed zeta' epsilon/zeta \
 		> $testroot/stdout)
 
 	local head_rev=`git_show_head $testroot/repo`
@@ -269,6 +269,38 @@ function test_commit_rejects_conflicted_file {
 	test_done "$testroot" "$ret"
 }
 
+function test_commit_single_file_multiple {
+	local testroot=`test_init commit_single_file_multiple`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	for i in 1 2 3 4; do
+		echo "modified alpha" >> $testroot/wt/alpha
+
+		(cd $testroot/wt && \
+			got commit -m "changed alpha" > $testroot/stdout)
+
+		local head_rev=`git_show_head $testroot/repo`
+		echo "M  alpha" > $testroot/stdout.expected
+		echo "Created commit $head_rev" >> $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
+	done
+
+	test_done "$testroot" "0"
+}
+
 run_test test_commit_basic
 run_test test_commit_new_subdir
 run_test test_commit_subdir
@@ -276,3 +308,4 @@ run_test test_commit_single_file
 run_test test_commit_out_of_date
 run_test test_commit_added_subdirs
 run_test test_commit_rejects_conflicted_file
+run_test test_commit_single_file_multiple
diff --git a/regress/cmdline/update.sh b/regress/cmdline/update.sh
index 1df093a..07a2d33 100755
--- a/regress/cmdline/update.sh
+++ b/regress/cmdline/update.sh
@@ -1429,6 +1429,10 @@ function test_update_to_commit_on_wrong_branch {
 function test_update_bumps_base_commit_id {
 	local testroot=`test_init update_bumps_base_commit_id`
 
+	echo "psi" > $testroot/repo/epsilon/psi
+	(cd $testroot/repo && git add .)
+	git_commit $testroot/repo -m "adding another file"
+
 	got checkout $testroot/repo $testroot/wt > /dev/null
 	ret="$?"
 	if [ "$ret" != "0" ]; then
@@ -1436,11 +1440,11 @@ function test_update_bumps_base_commit_id {
 		return 1
 	fi
 
-	echo "modified alpha" > $testroot/wt/alpha
-	(cd $testroot/wt && got commit -m "changed alpha" > $testroot/stdout)
+	echo "modified psi" > $testroot/wt/epsilon/psi
+	(cd $testroot/wt && got commit -m "changed psi" > $testroot/stdout)
 
 	local head_rev=`git_show_head $testroot/repo`
-	echo "M  alpha" > $testroot/stdout.expected
+	echo "M  epsilon/psi" > $testroot/stdout.expected
 	echo "Created commit $head_rev" >> $testroot/stdout.expected
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
@@ -1450,8 +1454,8 @@ function test_update_bumps_base_commit_id {
 		return 1
 	fi
 
-	echo "modified beta" > $testroot/wt/beta
-	(cd $testroot/wt && got commit -m "changed beta" > $testroot/stdout \
+	echo "modified zeta" > $testroot/wt/epsilon/zeta
+	(cd $testroot/wt && got commit -m "changed zeta" > $testroot/stdout \
 		2> $testroot/stderr)
 
 	echo -n "" > $testroot/stdout.expected
@@ -1464,7 +1468,6 @@ function test_update_bumps_base_commit_id {
 		return 1
 	fi
 
-	# XXX At present, got requires users to run 'update' after 'commit'.
 	(cd $testroot/wt && got update > $testroot/stdout)
 
 	echo "Already up-to-date" > $testroot/stdout.expected
@@ -1476,10 +1479,10 @@ function test_update_bumps_base_commit_id {
 		return 1
 	fi
 
-	(cd $testroot/wt && got commit -m "changed beta" > $testroot/stdout)
+	(cd $testroot/wt && got commit -m "changed zeta" > $testroot/stdout)
 
 	local head_rev=`git_show_head $testroot/repo`
-	echo "M  beta" > $testroot/stdout.expected
+	echo "M  epsilon/zeta" > $testroot/stdout.expected
 	echo "Created commit $head_rev" >> $testroot/stdout.expected
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"