commit b416585cf047654c6eb266874b5465a6f67861a7
from: Stefan Sperling <stsp@stsp.name>
date: Mon May 13 22:19:59 2019 UTC

store file's base commit ID in got_commitable and use it for OOD check

commit - 016a88dd43b7cb2c25db43168377a501de53b1d9
commit + b416585cf047654c6eb266874b5465a6f67861a7
blob - 37547bfa3b92c8e142fb8293c30bb06a1e202a95
blob + 204c0b97c37f8f8c73bf17fbfc12436c416b62a3
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -37,6 +37,7 @@ struct got_commitable {
 	unsigned char status;
 	struct got_object_id *blob_id;
 	struct got_object_id *base_blob_id;
+	struct got_object_id *base_commit_id;
 	mode_t mode;
 };
 
blob - 562647c3bd8bf993798cedfc890acb049eeb70c3
blob + c6e5bb6cbe9b53419ce886ccea0a6899b78ca368
--- lib/worktree.c
+++ lib/worktree.c
@@ -2237,6 +2237,7 @@ free_commitable(struct got_commitable *ct)
 	free(ct->ondisk_path);
 	free(ct->blob_id);
 	free(ct->base_blob_id);
+	free(ct->base_commit_id);
 	free(ct);
 }
 
@@ -2311,6 +2312,11 @@ collect_commitables(void *arg, unsigned char status, c
 	if (ct->status != GOT_STATUS_ADD) {
 		ct->base_blob_id = got_object_id_dup(blob_id);
 		if (ct->base_blob_id == NULL) {
+			err = got_error_from_errno("got_object_id_dup");
+			goto done;
+		}
+		ct->base_commit_id = got_object_id_dup(commit_id);
+		if (ct->base_commit_id == NULL) {
 			err = got_error_from_errno("got_object_id_dup");
 			goto done;
 		}
@@ -2680,7 +2686,8 @@ write_tree(struct got_object_id **new_tree_id,
 					if (err)
 						goto done;
 				}
-				err = report_ct_status(ct, status_cb, status_arg);
+				err = report_ct_status(ct, status_cb,
+				    status_arg);
 				if (err)
 					goto done;
 			} else {
@@ -2785,29 +2792,33 @@ check_ct_out_of_date(struct got_commitable *ct, struct
 	struct got_object_id *id_in_head;
 
 	/*
-	 * XXX This should probably be checking each commit from
-	 * worktree's base_commit -> head and verify that the
-	 * same blob exists in each of these commits.
-	 * Removals+additions within this line of history could mean
-	 * that renames have occured in which case we should really
-	 * be forcing the user to run an update...
-	 */
-	err = got_object_id_by_path(&id_in_head, repo,
-	    head_commit_id, ct->in_repo_path);
-	if (err) {
-		if (err->code != GOT_ERR_NO_TREE_ENTRY)
-			return err;
-		if (ct->status != GOT_STATUS_ADD)
-			return got_error(GOT_ERR_COMMIT_OUT_OF_DATE);
-		err = NULL;
-		id_in_head = NULL;
+	 * 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.
+	 *
+	 * 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.
+	 */
+	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;
 	}
 
-	if (id_in_head && got_object_id_cmp(id_in_head, ct->base_blob_id) != 0)
-		err = got_error(GOT_ERR_COMMIT_OUT_OF_DATE);
+	/* 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);
+	}
 
 	free(id_in_head);
-	return err;
+	return NULL;
 }
 
 const struct got_error *