From: Mark Jamsek Subject: improve logmsg ref heuristics To: Game of Trees Date: Sat, 28 Jan 2023 22:17:57 +1100 The below diff fixes a case where we add an unrelated log message to the preopulated editor. If the user cherrypicked or backed-out a commit but also has unrelated local changes in the work tree, and then specifies paths to 'got commit' that are not in the set of paths that were changed in the cherrypicked or backed-out commit, we should not add the log message of the bo/cy commit. This is fixed by checking if the path was indeed passed to 'got commit'. btw, this applies on top of the recent related diff that I've yet to commit. diffstat /home/mark/src/got M got/got.c | 45+ 19- 1 file changed, 45 insertions(+), 19 deletions(-) diff /home/mark/src/got commit - b873aeb71ec4b07c68cc1237fe7d68e5f6ef57c6 path + /home/mark/src/got blob - 0f43972acc8ee969e3863b582c941b718f58b067 file + got/got.c --- got/got.c +++ got/got.c @@ -8370,19 +8370,24 @@ choose_patch(int *choice, void *arg, unsigned char sta return NULL; } +struct wt_commitable_path_arg { + struct got_pathlist_head *commit_paths; + int *has_changes; +}; + /* - * Shortcut work tree status callback to determine if the set of - * paths scanned has at least one versioned path that is modified. - * Set arg and return GOT_ERR_FILE_MODIFIED as soon as a path is - * passed with a status that is neither unchanged nor unversioned. + * Shortcut work tree status callback to determine if the set of paths scanned + * has at least one versioned path that is being modified and, if not NULL, is + * in the arg->commit_paths list. Set arg and return GOT_ERR_FILE_MODIFIED as + * soon as a path is passed with a status that satisfies this criteria. */ static const struct got_error * -worktree_has_changed_path(void *arg, unsigned char status, +worktree_has_commitable_path(void *arg, unsigned char status, unsigned char staged_status, const char *path, struct got_object_id *blob_id, struct got_object_id *staged_blob_id, struct got_object_id *commit_id, int dirfd, const char *de_name) { - int *has_changes = arg; + struct wt_commitable_path_arg *a = arg; if (status == staged_status && (status == GOT_STATUS_DELETE)) status = GOT_STATUS_NO_CHANGE; @@ -8390,8 +8395,18 @@ worktree_has_changed_path(void *arg, unsigned char sta if (!(status == GOT_STATUS_NO_CHANGE || status == GOT_STATUS_UNVERSIONED) || staged_status != GOT_STATUS_NO_CHANGE) { - *has_changes = 1; - return got_error(GOT_ERR_FILE_MODIFIED); + if (a->commit_paths != NULL) { + struct got_pathlist_entry *pe; + + TAILQ_FOREACH(pe, a->commit_paths, entry) { + if (strncmp(path, pe->path, pe->path_len) == 0) + *a->has_changes = 1; + } + } else + *a->has_changes = 1; + + if (*a->has_changes) + return got_error(GOT_ERR_FILE_MODIFIED); } return NULL; @@ -8399,11 +8414,12 @@ worktree_has_changed_path(void *arg, unsigned char sta /* * Check that the changeset of the commit identified by id is - * comprised of at least one path that is modified in the work tree. + * comprised of at least one modified path that is being committed. */ static const struct got_error * -commit_path_changed_in_worktree(int *add_logmsg, struct got_object_id *id, - struct got_worktree *worktree, struct got_repository *repo) +commit_path_changed_in_worktree(struct wt_commitable_path_arg *wcpa, + struct got_object_id *id, struct got_worktree *worktree, + struct got_repository *repo) { const struct got_error *err; struct got_pathlist_head paths; @@ -8440,7 +8456,7 @@ commit_path_changed_in_worktree(int *add_logmsg, struc goto done; err = got_worktree_status(worktree, &paths, repo, 0, - worktree_has_changed_path, add_logmsg, check_cancelled, NULL); + worktree_has_commitable_path, wcpa, check_cancelled, NULL); if (err && err->code == GOT_ERR_FILE_MODIFIED) { /* * At least one changed path in the referenced commit is @@ -8475,6 +8491,7 @@ rm_logmsg_ref(struct got_worktree *worktree, struct go struct got_reflist_entry *re; struct got_commit_object *commit = NULL; struct got_object_id *commit_id = NULL; + struct wt_commitable_path_arg wcpa; char *uuidstr = NULL; TAILQ_INIT(&refs); @@ -8517,7 +8534,10 @@ rm_logmsg_ref(struct got_worktree *worktree, struct go if (err) goto done; - err = commit_path_changed_in_worktree(&has_changes, commit_id, + wcpa.commit_paths = NULL; + wcpa.has_changes = &has_changes; + + err = commit_path_changed_in_worktree(&wcpa, commit_id, worktree, repo); if (err) goto done; @@ -8875,8 +8895,9 @@ lookup_logmsg_ref(char **logmsg, struct got_reflist_he * to be scheduled for removal on successful commit. */ static const struct got_error * -lookup_logmsg_ref(char **logmsg, struct got_reflist_head *matched_refs, - struct got_worktree *worktree, struct got_repository *repo) +lookup_logmsg_ref(char **logmsg, struct got_pathlist_head *paths, + struct got_reflist_head *matched_refs, struct got_worktree *worktree, + struct got_repository *repo) { const struct got_error *err; struct got_commit_object *commit = NULL; @@ -8903,8 +8924,9 @@ lookup_logmsg_ref(char **logmsg, struct got_reflist_he goto done; TAILQ_FOREACH(re, &refs, entry) { - const char *refname, *type; - int add_logmsg = 0; + const char *refname, *type; + struct wt_commitable_path_arg wcpa; + int add_logmsg = 0; refname = got_ref_get_name(re->ref); @@ -8933,7 +8955,10 @@ lookup_logmsg_ref(char **logmsg, struct got_reflist_he if (err) goto done; - err = commit_path_changed_in_worktree(&add_logmsg, id, + wcpa.commit_paths = paths; + wcpa.has_changes = &add_logmsg; + + err = commit_path_changed_in_worktree(&wcpa, id, worktree, repo); if (err) goto done; @@ -9118,7 +9143,8 @@ cmd_commit(int argc, char *argv[]) goto done; if (prepared_logmsg == NULL) { - error = lookup_logmsg_ref(&prepared_logmsg, &refs, + error = lookup_logmsg_ref(&prepared_logmsg, + argc > 0 ? &paths : NULL, &refs, worktree, repo); if (error) goto done; -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68