merge chokes, creates bogus conflicts

On 23-02-16 01:27PM, Stefan Sperling wrote: > On Thu, Feb 16, 2023 at 11:14:54PM +1100, Mark Jamsek wrote: > > On 23-02-16 12:45PM, Stefan Sperling wrote: > > > On Thu, Feb 16, 2023 at 02:11:54AM +0100, Stefan Sperling wrote: > > > > If we want to allow users to commit such files in spite of the > > > > obvious downsides, here is a patch which would make it possible: > > > > I agree that, while such conflict markers should not be embedded > > verbatim into versioned files, users should be able to override this > > check and commit unresolved conflicts for such cases as demonstrated by > > this thread. IIRC, git just lets you commit conflicts, but we don't in > > Fossil, so a flag is provided to allow committing with unresolved > > conflicts. However, I might even suggest we only allow such commits > > interactively, by prompting the user when the -C flag is used and > > conflicts indeed do exist in the commitables to confirm they want to > > commit despite the unresolved conflicts. > > > > In any case, I'll have a go at implementing your below proposal. > > Great, thanks! Please feel free to use the diff I sent as a starting point. The below diff extends your original diff to only look for conflict markers in new added lines, and adds the -C flag to histedit, merge, and rebase. This is done by using your suggestion such that we parse a zero-context diff in get_modified_file_content_status() and only check added lines for conflict markers. There was a problem I found in the original -C diff where after forcing a commit with unresolved conflicts using 'got commit -C', the diff of that commit with 'got diff -c', 'got log -p', or tog, presented a completely empty diff! This was fixed by adding GOT_STATUS_CONFLICT to the check in match_deleted_or_modified_ct() so that a new tree entry for the file is added (see XXX comment). This made me look at other similar checks where I have also added GOT_STATUS_CONFLICT; however, I'm not as certain about these so I want to bring them to your attention. Another artifact of this conflict management tweak that I'm not certain about is the change in update.sh where a binary file that already had conflict markers is now reported as "M foo" instead of "C foo", but I think this comports with the desired behaviour? Otherwise, I think the diff implements your suggestion for the commit, histedit, merge, and rebase commands, and regress is passing. I also took the liberty to tweak the output slightly from: got: cannot commit file in conflicted status to something admittedly more verbose, but also potentially more accurate and informing as we now can commit files in conflicted status, albeit with an extra flag: C conflicted/file/path resolve conflicts or use -C to commit with unresolved conflicts got: cannot commit file in conflicted status That said, I really don't mind whether we keep or nix it, or report something else. diffstat refs/remotes/origin/main refs/heads/main M got/got.1 | 55+ 3- M got/got.c | 71+ 23- M include/got_worktree.h | 4+ 4- M lib/worktree.c | 94+ 23- M regress/cmdline/commit.sh | 38+ 1- M regress/cmdline/update.sh | 1+ 1- 6 files changed, 263 insertions(+), 55 deletions(-) diff refs/remotes/origin/main refs/heads/main commit - bf1c78e5100932aa445b8ef07ebf9b712500c67e commit + 8611c0fdc8e4f70d86c91cca9e87eff7a064c420 blob - bb1825eecded3623bbabcf0541bce55367696327 blob + 3018e9193697fa4789c04d11e99299a0740c241e --- got/got.1 +++ got/got.1 @@ -1654,6 +1654,7 @@ is a directory. .Cm commit .Op Fl NnS .Op Fl A Ar author +.Op Fl C .Op Fl F Ar path .Op Fl m Ar message .Op Ar path ... @@ -1740,6 +1741,27 @@ or Git configuration settings. environment variable, or .Xr got.conf 5 , or Git configuration settings. +.It Fl C +Allow committing files in conflicted status. +.Pp +Committing files with conflict markers should generally be avoided. +Cases where conflict markers must be stored in the repository for +some legitimate reason should be very rare. +There are usually ways to avoid storing conflict markers verbatim by +applying appropriate programming tricks. +.Pp +Conflicted files committed with +.Fl C +will always appear to be in conflicted status once modified in a work tree. +This prevents automatic merging of changes to such files during +.Cm got update , +.Cm got rebase , +.Cm got histedit , +.Cm got merge , +.Cm got cherrypick , +.Cm got backout , +and +.Cm got patch . .It Fl F Ar path Use the prepared log message stored in the file found at .Ar path @@ -2209,7 +2231,7 @@ This option cannot be used with .Tg rb .It Xo .Cm rebase -.Op Fl aclX +.Op Fl aCclX .Op Ar branch .Xc .Dl Pq alias: Cm rb @@ -2292,6 +2314,11 @@ If any files with destined changes are found to be mis .Pp If merge conflicts occur, the rebase operation is interrupted and may be continued once conflicts have been resolved. +If for some legitimate reason the conflicts cannot be resolved, the +.Fl C +option can be used to allow the rebase operation to continue despite +unresolved conflicts. +Such cases are exceedingly rare and should only be used as a last resort. If any files with destined changes are found to be missing or unversioned, or if files could not be deleted due to differences in deleted content, the rebase operation will be interrupted to prevent potentially incomplete @@ -2359,6 +2386,11 @@ If this option is used, no other command-line argument .It Fl a Abort an interrupted rebase operation. If this option is used, no other command-line arguments are allowed. +.It Fl C +Allow a rebase operation to continue with files in conflicted status. +This option can only be used with the +.Fl c +option. .It Fl c Continue an interrupted rebase operation. If this option is used, no other command-line arguments are allowed. @@ -2422,7 +2454,7 @@ None of the other options can be used together with .Tg he .It Xo .Cm histedit -.Op Fl acdeflmX +.Op Fl aCcdeflmX .Op Fl F Ar histedit-script .Op Ar branch .Xc @@ -2543,6 +2575,11 @@ If any files with destined changes are found to be mis .Pp If merge conflicts occur, the histedit operation is interrupted and may be continued once conflicts have been resolved. +If for some legitimate reason the conflicts cannot be resolved, the +.Fl C +option can be used to allow the histedit operation to continue despite +unresolved conflicts. +Such cases are exceedingly rare and should only be used as a last resort. If any files with destined changes are found to be missing or unversioned, or if files could not be deleted due to differences in deleted content, the histedit operation will be interrupted to prevent potentially incomplete @@ -2597,6 +2634,11 @@ If this option is used, no other command-line argument .It Fl a Abort an interrupted histedit operation. If this option is used, no other command-line arguments are allowed. +.It Fl C +Allow a histedit operation to continue with files in conflicted status. +This option can only be used with the +.Fl c +option. .It Fl c Continue an interrupted histedit operation. If this option is used, no other command-line arguments are allowed. @@ -2749,7 +2791,7 @@ or reverted with .Tg mg .It Xo .Cm merge -.Op Fl acn +.Op Fl aCcn .Op Ar branch .Xc .Dl Pq alias: Cm mg @@ -2800,6 +2842,11 @@ If any files with destined changes are found to be mis .Pp If merge conflicts occur, the merge operation is interrupted and conflicts must be resolved before the merge operation can continue. +If for some legitimate reason the conflicts cannot be resolved, the +.Fl C +option can be used to allow the merge operation to continue despite +unresolved conflicts. +Such cases are exceedingly rare and should only be used as a last resort. If any files with destined changes are found to be missing or unversioned, or if files could not be deleted due to differences in deleted content, the merge operation will be interrupted to prevent potentially incomplete @@ -2864,6 +2911,11 @@ If this option is used, no other command-line argument .It Fl a Abort an interrupted merge operation. If this option is used, no other command-line arguments are allowed. +.It Fl C +Allow a merge operation to continue with files in conflicted status. +This option can only be used with the +.Fl c +option. .It Fl c Continue an interrupted merge operation. If this option is used, no other command-line arguments are allowed. blob - 97e8fcfb43b02a9fb38a2a41964a9a6249b31c2b blob + 2d4c6fb05bb78023a9bc3e3fcd3df0be3e01b723 --- got/got.c +++ got/got.c @@ -8776,7 +8776,7 @@ usage_commit(void) __dead static void usage_commit(void) { - fprintf(stderr, "usage: %s commit [-NnS] [-A author] [-F path] " + fprintf(stderr, "usage: %s commit [-CNnS] [-A author] [-F path] " "[-m message] [path ...]\n", getprogname()); exit(1); } @@ -9083,7 +9083,7 @@ cmd_commit(int argc, char *argv[]) char *gitconfig_path = NULL, *editor = NULL, *committer = NULL; int ch, rebase_in_progress, histedit_in_progress, preserve_logmsg = 0; int allow_bad_symlinks = 0, non_interactive = 0, merge_in_progress = 0; - int show_diff = 1; + int show_diff = 1, commit_conflicts = 0; struct got_pathlist_head paths; struct got_reflist_head refs; struct got_reflist_entry *re; @@ -9099,7 +9099,7 @@ cmd_commit(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "A:F:m:NnS")) != -1) { + while ((ch = getopt(argc, argv, "A:CF:m:NnS")) != -1) { switch (ch) { case 'A': author = optarg; @@ -9107,6 +9107,9 @@ cmd_commit(int argc, char *argv[]) if (error) return error; break; + case 'C': + commit_conflicts = 1; + break; case 'F': if (logmsg != NULL) option_conflict('F', 'm'); @@ -9230,8 +9233,8 @@ cmd_commit(int argc, char *argv[]) } cl_arg.repo_path = got_repo_get_path(repo); error = got_worktree_commit(&id, worktree, &paths, author, committer, - allow_bad_symlinks, show_diff, collect_commit_logmsg, &cl_arg, - print_status, NULL, repo); + allow_bad_symlinks, show_diff, commit_conflicts, + collect_commit_logmsg, &cl_arg, print_status, NULL, repo); if (error) { if (error->code != GOT_ERR_COMMIT_MSG_EMPTY && cl_arg.logmsg_path != NULL) @@ -9251,6 +9254,9 @@ done: } done: + if (error && error->code == GOT_ERR_COMMIT_CONFLICT) + printf("resolve conflicts or use -C to commit with " + "unresolved conflicts\n"); if (preserve_logmsg) { fprintf(stderr, "%s: log message preserved in %s\n", getprogname(), cl_arg.logmsg_path); @@ -10368,7 +10374,7 @@ usage_rebase(void) __dead static void usage_rebase(void) { - fprintf(stderr, "usage: %s rebase [-aclX] [branch]\n", getprogname()); + fprintf(stderr, "usage: %s rebase [-aCclX] [branch]\n", getprogname()); exit(1); } @@ -10492,7 +10498,8 @@ rebase_commit(struct got_pathlist_head *merged_paths, rebase_commit(struct got_pathlist_head *merged_paths, struct got_worktree *worktree, struct got_fileindex *fileindex, struct got_reference *tmp_branch, const char *committer, - struct got_object_id *commit_id, struct got_repository *repo) + struct got_object_id *commit_id, int allow_conflict, + struct got_repository *repo) { const struct got_error *error; struct got_commit_object *commit; @@ -10504,7 +10511,7 @@ rebase_commit(struct got_pathlist_head *merged_paths, error = got_worktree_rebase_commit(&new_commit_id, merged_paths, worktree, fileindex, tmp_branch, committer, commit, commit_id, - repo); + allow_conflict, repo); if (error) { if (error->code != GOT_ERR_COMMIT_NO_CHANGES) goto done; @@ -11000,6 +11007,7 @@ cmd_rebase(int argc, char *argv[]) int ch, rebase_in_progress = 0, abort_rebase = 0, continue_rebase = 0; int histedit_in_progress = 0, merge_in_progress = 0; int create_backup = 1, list_backups = 0, delete_backups = 0; + int allow_conflict = 0; struct got_object_id_queue commits; struct got_pathlist_head merged_paths; const struct got_object_id_queue *parent_ids; @@ -11017,11 +11025,14 @@ cmd_rebase(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "aclX")) != -1) { + while ((ch = getopt(argc, argv, "aCclX")) != -1) { switch (ch) { case 'a': abort_rebase = 1; break; + case 'C': + allow_conflict = 1; + break; case 'c': continue_rebase = 1; break; @@ -11043,6 +11054,8 @@ cmd_rebase(int argc, char *argv[]) if (list_backups) { if (abort_rebase) option_conflict('l', 'a'); + if (allow_conflict) + option_conflict('l', 'C'); if (continue_rebase) option_conflict('l', 'c'); if (delete_backups) @@ -11052,12 +11065,19 @@ cmd_rebase(int argc, char *argv[]) } else if (delete_backups) { if (abort_rebase) option_conflict('X', 'a'); + if (allow_conflict) + option_conflict('X', 'C'); if (continue_rebase) option_conflict('X', 'c'); if (list_backups) option_conflict('l', 'X'); if (argc != 0 && argc != 1) usage_rebase(); + } else if (allow_conflict) { + if (abort_rebase) + option_conflict('C', 'a'); + if (!continue_rebase) + errx(1, "-C option requires -c"); } else { if (abort_rebase && continue_rebase) usage_rebase(); @@ -11177,7 +11197,7 @@ cmd_rebase(int argc, char *argv[]) goto done; error = rebase_commit(NULL, worktree, fileindex, tmp_branch, - committer, resume_commit_id, repo); + committer, resume_commit_id, allow_conflict, repo); if (error) goto done; @@ -11356,7 +11376,7 @@ cmd_rebase(int argc, char *argv[]) } error = rebase_commit(&merged_paths, worktree, fileindex, - tmp_branch, committer, commit_id, repo); + tmp_branch, committer, commit_id, 0, repo); got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH); if (error) goto done; @@ -11427,7 +11447,7 @@ usage_histedit(void) __dead static void usage_histedit(void) { - fprintf(stderr, "usage: %s histedit [-acdeflmX] [-F histedit-script] " + fprintf(stderr, "usage: %s histedit [-aCcdeflmX] [-F histedit-script] " "[branch]\n", getprogname()); exit(1); } @@ -12167,7 +12187,7 @@ histedit_commit(struct got_pathlist_head *merged_paths histedit_commit(struct got_pathlist_head *merged_paths, struct got_worktree *worktree, struct got_fileindex *fileindex, struct got_reference *tmp_branch, struct got_histedit_list_entry *hle, - const char *committer, struct got_repository *repo) + const char *committer, int allow_conflict, struct got_repository *repo) { const struct got_error *err; struct got_commit_object *commit; @@ -12186,7 +12206,7 @@ histedit_commit(struct got_pathlist_head *merged_paths err = got_worktree_histedit_commit(&new_commit_id, merged_paths, worktree, fileindex, tmp_branch, committer, commit, hle->commit_id, - hle->logmsg, repo); + hle->logmsg, allow_conflict, repo); if (err) { if (err->code != GOT_ERR_COMMIT_NO_CHANGES) goto done; @@ -12271,7 +12291,7 @@ cmd_histedit(int argc, char *argv[]) struct got_update_progress_arg upa; int edit_in_progress = 0, abort_edit = 0, continue_edit = 0; int drop_only = 0, edit_logmsg_only = 0, fold_only = 0, edit_only = 0; - int list_backups = 0, delete_backups = 0; + int allow_conflict = 0, list_backups = 0, delete_backups = 0; const char *edit_script_path = NULL; struct got_object_id_queue commits; struct got_pathlist_head merged_paths; @@ -12292,11 +12312,14 @@ cmd_histedit(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "acdeF:flmX")) != -1) { + while ((ch = getopt(argc, argv, "aCcdeF:flmX")) != -1) { switch (ch) { case 'a': abort_edit = 1; break; + case 'C': + allow_conflict = 1; + break; case 'c': continue_edit = 1; break; @@ -12330,16 +12353,24 @@ cmd_histedit(int argc, char *argv[]) argc -= optind; argv += optind; + if (abort_edit && allow_conflict) + option_conflict('a', 'C'); if (abort_edit && continue_edit) option_conflict('a', 'c'); + if (edit_script_path && allow_conflict) + option_conflict('F', 'C'); if (edit_script_path && edit_logmsg_only) option_conflict('F', 'm'); if (abort_edit && edit_logmsg_only) option_conflict('a', 'm'); + if (edit_logmsg_only && allow_conflict) + option_conflict('m', 'C'); if (continue_edit && edit_logmsg_only) option_conflict('c', 'm'); if (abort_edit && fold_only) option_conflict('a', 'f'); + if (fold_only && allow_conflict) + option_conflict('f', 'C'); if (continue_edit && fold_only) option_conflict('c', 'f'); if (fold_only && edit_logmsg_only) @@ -12358,6 +12389,8 @@ cmd_histedit(int argc, char *argv[]) option_conflict('f', 'e'); if (drop_only && abort_edit) option_conflict('d', 'a'); + if (drop_only && allow_conflict) + option_conflict('d', 'C'); if (drop_only && continue_edit) option_conflict('d', 'c'); if (drop_only && edit_logmsg_only) @@ -12371,6 +12404,8 @@ cmd_histedit(int argc, char *argv[]) if (list_backups) { if (abort_edit) option_conflict('l', 'a'); + if (allow_conflict) + option_conflict('l', 'C'); if (continue_edit) option_conflict('l', 'c'); if (edit_script_path) @@ -12390,6 +12425,8 @@ cmd_histedit(int argc, char *argv[]) } else if (delete_backups) { if (abort_edit) option_conflict('X', 'a'); + if (allow_conflict) + option_conflict('X', 'C'); if (continue_edit) option_conflict('X', 'c'); if (drop_only) @@ -12406,7 +12443,9 @@ cmd_histedit(int argc, char *argv[]) option_conflict('X', 'l'); if (argc != 0 && argc != 1) usage_histedit(); - } else if (argc != 0) + } else if (allow_conflict && !continue_edit) + errx(1, "-C option requires -c"); + else if (argc != 0) usage_histedit(); /* @@ -12722,7 +12761,7 @@ cmd_histedit(int argc, char *argv[]) if (have_changes) { error = histedit_commit(NULL, worktree, fileindex, tmp_branch, hle, - committer, repo); + committer, allow_conflict, repo); if (error) goto done; } else { @@ -12798,7 +12837,7 @@ cmd_histedit(int argc, char *argv[]) } error = histedit_commit(&merged_paths, worktree, fileindex, - tmp_branch, hle, committer, repo); + tmp_branch, hle, committer, allow_conflict, repo); got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH); if (error) goto done; @@ -13031,7 +13070,7 @@ usage_merge(void) __dead static void usage_merge(void) { - fprintf(stderr, "usage: %s merge [-acn] [branch]\n", getprogname()); + fprintf(stderr, "usage: %s merge [-aCcn] [branch]\n", getprogname()); exit(1); } @@ -13047,7 +13086,7 @@ cmd_merge(int argc, char *argv[]) struct got_object_id *branch_tip = NULL, *yca_id = NULL; struct got_object_id *wt_branch_tip = NULL; int ch, merge_in_progress = 0, abort_merge = 0, continue_merge = 0; - int interrupt_merge = 0; + int allow_conflict = 0, interrupt_merge = 0; struct got_update_progress_arg upa; struct got_object_id *merge_commit_id = NULL; char *branch_name = NULL; @@ -13061,11 +13100,13 @@ cmd_merge(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "acn")) != -1) { + while ((ch = getopt(argc, argv, "aCcn")) != -1) { switch (ch) { case 'a': abort_merge = 1; break; + case 'C': + allow_conflict = 1; case 'c': continue_merge = 1; break; @@ -13081,6 +13122,12 @@ cmd_merge(int argc, char *argv[]) argc -= optind; argv += optind; + if (allow_conflict) { + if (abort_merge) + option_conflict('a', 'C'); + if (!continue_merge) + errx(1, "-C option requires -c"); + } if (abort_merge && continue_merge) option_conflict('a', 'c'); if (abort_merge || continue_merge) { @@ -13269,7 +13316,8 @@ cmd_merge(int argc, char *argv[]) } else { error = got_worktree_merge_commit(&merge_commit_id, worktree, fileindex, author, NULL, 1, branch_tip, branch_name, - repo, continue_merge ? print_status : NULL, NULL); + allow_conflict, repo, continue_merge ? print_status : NULL, + NULL); if (error) goto done; error = got_worktree_merge_complete(worktree, fileindex, repo); blob - 4ea02aaea560daeba058d4b8541f5ea222eb7586 blob + f7de090bfb2bd10312bdd0c389b5eabe41a33fdd --- include/got_worktree.h +++ include/got_worktree.h @@ -257,7 +257,7 @@ const struct got_error *got_worktree_commit(struct got */ const struct got_error *got_worktree_commit(struct got_object_id **, struct got_worktree *, struct got_pathlist_head *, const char *, - const char *, int, int, got_worktree_commit_msg_cb, void *, + const char *, int, int, int, got_worktree_commit_msg_cb, void *, got_worktree_status_cb, void *, struct got_repository *); /* Get the path of a commitable worktree item. */ @@ -318,7 +318,7 @@ const struct got_error *got_worktree_rebase_commit(str const struct got_error *got_worktree_rebase_commit(struct got_object_id **, struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *, struct got_reference *, const char *, struct got_commit_object *, - struct got_object_id *, struct got_repository *); + struct got_object_id *, int, struct got_repository *); /* Postpone the rebase operation. Should be called after a merge conflict. */ const struct got_error *got_worktree_rebase_postpone(struct got_worktree *, @@ -392,7 +392,7 @@ const struct got_error *got_worktree_histedit_commit(s const struct got_error *got_worktree_histedit_commit(struct got_object_id **, struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *, struct got_reference *, const char *, struct got_commit_object *, - struct got_object_id *, const char *, struct got_repository *); + struct got_object_id *, const char *, int, struct got_repository *); /* * Record the specified commit as skipped during histedit. @@ -469,7 +469,7 @@ got_worktree_merge_commit(struct got_object_id **new_c struct got_worktree *worktree, struct got_fileindex *fileindex, const char *author, const char *committer, int allow_bad_symlinks, struct got_object_id *branch_tip, const char *branch_name, - struct got_repository *repo, + int allow_conflict, struct got_repository *repo, got_worktree_status_cb status_cb, void *status_arg); /* blob - c4fc2f02d953c285d633a68f2ef9f0cebc512bca blob + 2950ab1bcf81fb9cf5688b824c81b36a01c47ebc --- lib/worktree.c +++ lib/worktree.c @@ -1513,9 +1513,14 @@ done: return err; } -/* Upgrade STATUS_MODIFY to STATUS_CONFLICT if a conflict marker is found. */ +/* + * Upgrade STATUS_MODIFY to STATUS_CONFLICT if a + * conflict marker is found in new added lines only. + */ static const struct got_error * -get_modified_file_content_status(unsigned char *status, FILE *f) +get_modified_file_content_status(unsigned char *status, + struct got_blob_object *blob, const char *path, struct stat *sb, + FILE *ondisk_file) { const struct got_error *err = NULL; const char *markers[3] = { @@ -1523,11 +1528,43 @@ get_modified_file_content_status(unsigned char *status GOT_DIFF_CONFLICT_MARKER_SEP, GOT_DIFF_CONFLICT_MARKER_END }; + FILE *f, *f1 = NULL; int i = 0; char *line = NULL; size_t linesize = 0; ssize_t linelen; + off_t sz1 = 0; + f = got_opentemp(); + if (f == NULL) + return got_error_from_errno("got_opentemp"); + + f1 = got_opentemp(); + if (f1 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + + if (blob) { + err = got_object_blob_dump_to_file(&sz1, NULL, NULL, f1, blob); + if (err) + goto done; + } + + err = got_diff_blob_file(blob, f1, sz1, NULL, ondisk_file, 1, sb, + path, GOT_DIFF_ALGORITHM_MYERS, 0, 0, 0, NULL, f); + if (err) + goto done; + + if (fflush(f) == EOF) { + err = got_error_from_errno("fflush"); + goto done; + } + if (fseeko(f, 0L, SEEK_SET) == -1) { + err = got_error_from_errno("fseek"); + goto done; + } + while (*status == GOT_STATUS_MODIFY) { linelen = getline(&line, &linesize, f); if (linelen == -1) { @@ -1537,7 +1574,8 @@ get_modified_file_content_status(unsigned char *status break; } - if (strncmp(line, markers[i], strlen(markers[i])) == 0) { + if (*line == '+' && + strncmp(line + 1, markers[i], strlen(markers[i])) == 0) { if (strcmp(markers[i], GOT_DIFF_CONFLICT_MARKER_END) == 0) *status = GOT_STATUS_CONFLICT; @@ -1545,7 +1583,13 @@ get_modified_file_content_status(unsigned char *status i++; } } + +done: free(line); + if (f != NULL && fclose(f) == EOF && err == NULL) + err = got_error_from_errno("fclose"); + if (f1 != NULL && fclose(f1) == EOF && err == NULL) + err = got_error_from_errno("fclose"); return err; } @@ -1786,7 +1830,8 @@ get_file_status(unsigned char *status, struct stat *sb if (*status == GOT_STATUS_MODIFY) { rewind(f); - err = get_modified_file_content_status(status, f); + err = get_modified_file_content_status(status, blob, ie->path, + sb, f); } else if (xbit_differs(ie, sb->st_mode)) *status = GOT_STATUS_MODE_CHANGE; done: @@ -4947,6 +4992,7 @@ struct collect_commitables_arg { int have_staged_files; int allow_bad_symlinks; int diff_header_shown; + int commit_conflicts; FILE *diff_outfile; FILE *f1; FILE *f2; @@ -5018,12 +5064,14 @@ append_ct_diff(struct got_commitable *ct, int *diff_he if (diff_staged) { if (ct->staged_status != GOT_STATUS_MODIFY && ct->staged_status != GOT_STATUS_ADD && - ct->staged_status != GOT_STATUS_DELETE) + ct->staged_status != GOT_STATUS_DELETE && + ct->staged_status != GOT_STATUS_CONFLICT) return NULL; } else { if (ct->status != GOT_STATUS_MODIFY && ct->status != GOT_STATUS_ADD && - ct->status != GOT_STATUS_DELETE) + ct->status != GOT_STATUS_DELETE && + ct->status != GOT_STATUS_CONFLICT) return NULL; } @@ -5051,6 +5099,7 @@ append_ct_diff(struct got_commitable *ct, int *diff_he const char *label1 = NULL, *label2 = NULL; switch (ct->staged_status) { case GOT_STATUS_MODIFY: + case GOT_STATUS_CONFLICT: label1 = ct->path; label2 = ct->path; break; @@ -5180,13 +5229,16 @@ collect_commitables(void *arg, unsigned char status, staged_status != GOT_STATUS_DELETE) return NULL; } else { - if (status == GOT_STATUS_CONFLICT) + if (status == GOT_STATUS_CONFLICT && !a->commit_conflicts) { + printf("C %s\n", relpath); return got_error(GOT_ERR_COMMIT_CONFLICT); + } if (status != GOT_STATUS_MODIFY && status != GOT_STATUS_MODE_CHANGE && status != GOT_STATUS_ADD && - status != GOT_STATUS_DELETE) + status != GOT_STATUS_DELETE && + status != GOT_STATUS_CONFLICT) return NULL; } @@ -5217,7 +5269,8 @@ collect_commitables(void *arg, unsigned char status, } if (staged_status == GOT_STATUS_ADD || - staged_status == GOT_STATUS_MODIFY) { + staged_status == GOT_STATUS_MODIFY || + staged_status == GOT_STATUS_CONFLICT) { struct got_fileindex_entry *ie; ie = got_fileindex_entry_get(a->fileindex, path, strlen(path)); switch (got_fileindex_entry_staged_filetype_get(ie)) { @@ -5297,7 +5350,8 @@ collect_commitables(void *arg, unsigned char status, } } if (ct->staged_status == GOT_STATUS_ADD || - ct->staged_status == GOT_STATUS_MODIFY) { + ct->staged_status == GOT_STATUS_MODIFY || + ct->staged_status == GOT_STATUS_CONFLICT) { ct->staged_blob_id = got_object_id_dup(staged_blob_id); if (ct->staged_blob_id == NULL) { err = got_error_from_errno("got_object_id_dup"); @@ -5533,14 +5587,21 @@ match_deleted_or_modified_ct(struct got_commitable **c char *ct_name = NULL; int path_matches; + /* + * XXX Files with GOT_STATUS_CONFLICT must be allowed else + * the caller write_tree() will fail to add a new tree entry + * and subsequent diffs of the file will show up empty. + */ if (ct->staged_status == GOT_STATUS_NO_CHANGE) { if (ct->status != GOT_STATUS_MODIFY && ct->status != GOT_STATUS_MODE_CHANGE && - ct->status != GOT_STATUS_DELETE) + ct->status != GOT_STATUS_DELETE && + ct->status != GOT_STATUS_CONFLICT) continue; } else { if (ct->staged_status != GOT_STATUS_MODIFY && - ct->staged_status != GOT_STATUS_DELETE) + ct->staged_status != GOT_STATUS_DELETE && + ct->staged_status != GOT_STATUS_CONFLICT) continue; } @@ -5744,7 +5805,9 @@ write_tree(struct got_object_id **new_tree_id, int *ne /* NB: Deleted entries get dropped here. */ if (ct->status == GOT_STATUS_MODIFY || ct->status == GOT_STATUS_MODE_CHANGE || - ct->staged_status == GOT_STATUS_MODIFY) { + ct->status == GOT_STATUS_CONFLICT || + ct->staged_status == GOT_STATUS_MODIFY || + ct->staged_status == GOT_STATUS_CONFLICT) { err = alloc_modified_blob_tree_entry( &new_te, te, ct); if (err) @@ -5804,7 +5867,8 @@ update_fileindex_after_commit(struct got_worktree *wor ct->staged_status == GOT_STATUS_DELETE) { got_fileindex_entry_remove(fileindex, ie); } else if (ct->staged_status == GOT_STATUS_ADD || - ct->staged_status == GOT_STATUS_MODIFY) { + ct->staged_status == GOT_STATUS_MODIFY || + ct->staged_status == GOT_STATUS_CONFLICT) { got_fileindex_entry_stage_set(ie, GOT_FILEIDX_STAGE_NONE); got_fileindex_entry_staged_filetype_set(ie, 0); @@ -5948,12 +6012,14 @@ commit_worktree(struct got_object_id **new_commit_id, /* Blobs for staged files already exist. */ if (ct->staged_status == GOT_STATUS_ADD || - ct->staged_status == GOT_STATUS_MODIFY) + ct->staged_status == GOT_STATUS_MODIFY || + ct->staged_status == GOT_STATUS_CONFLICT) continue; if (ct->status != GOT_STATUS_ADD && ct->status != GOT_STATUS_MODIFY && - ct->status != GOT_STATUS_MODE_CHANGE) + ct->status != GOT_STATUS_MODE_CHANGE && + ct->status != GOT_STATUS_CONFLICT) continue; if (asprintf(&ondisk_path, "%s/%s", @@ -6104,7 +6170,8 @@ got_worktree_commit(struct got_object_id **new_commit_ got_worktree_commit(struct got_object_id **new_commit_id, struct got_worktree *worktree, struct got_pathlist_head *paths, const char *author, const char *committer, int allow_bad_symlinks, - int show_diff, got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg, + int show_diff, int commit_conflicts, + got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg, got_worktree_status_cb status_cb, void *status_arg, struct got_repository *repo) { @@ -6157,6 +6224,7 @@ got_worktree_commit(struct got_object_id **new_commit_ cc_arg.have_staged_files = have_staged_files; cc_arg.allow_bad_symlinks = allow_bad_symlinks; cc_arg.diff_header_shown = 0; + cc_arg.commit_conflicts = commit_conflicts; if (show_diff) { err = got_opentemp_named(&diff_path, &cc_arg.diff_outfile, GOT_TMPDIR_STR "/got", ".diff"); @@ -6713,7 +6781,7 @@ rebase_commit(struct got_object_id **new_commit_id, struct got_worktree *worktree, struct got_fileindex *fileindex, struct got_reference *tmp_branch, const char *committer, struct got_commit_object *orig_commit, const char *new_logmsg, - struct got_repository *repo) + int allow_conflict, struct got_repository *repo) { const struct got_error *err, *sync_err; struct got_pathlist_head commitable_paths; @@ -6737,6 +6805,7 @@ rebase_commit(struct got_object_id **new_commit_id, cc_arg.worktree = worktree; cc_arg.repo = repo; cc_arg.have_staged_files = 0; + cc_arg.commit_conflicts = allow_conflict; /* * If possible get the status of individual files directly to * avoid crawling the entire work tree once per rebased commit. @@ -6832,7 +6901,8 @@ got_worktree_rebase_commit(struct got_object_id **new_ struct got_pathlist_head *merged_paths, struct got_worktree *worktree, struct got_fileindex *fileindex, struct got_reference *tmp_branch, const char *committer, struct got_commit_object *orig_commit, - struct got_object_id *orig_commit_id, struct got_repository *repo) + struct got_object_id *orig_commit_id, int allow_conflict, + struct got_repository *repo) { const struct got_error *err; char *commit_ref_name; @@ -6856,7 +6926,7 @@ got_worktree_rebase_commit(struct got_object_id **new_ err = rebase_commit(new_commit_id, merged_paths, commit_ref, worktree, fileindex, tmp_branch, committer, orig_commit, - NULL, repo); + NULL, allow_conflict, repo); done: if (commit_ref) got_ref_close(commit_ref); @@ -6871,7 +6941,7 @@ got_worktree_histedit_commit(struct got_object_id **ne struct got_fileindex *fileindex, struct got_reference *tmp_branch, const char *committer, struct got_commit_object *orig_commit, struct got_object_id *orig_commit_id, const char *new_logmsg, - struct got_repository *repo) + int allow_conflict, struct got_repository *repo) { const struct got_error *err; char *commit_ref_name; @@ -6887,7 +6957,7 @@ got_worktree_histedit_commit(struct got_object_id **ne err = rebase_commit(new_commit_id, merged_paths, commit_ref, worktree, fileindex, tmp_branch, committer, orig_commit, - new_logmsg, repo); + new_logmsg, allow_conflict, repo); done: if (commit_ref) got_ref_close(commit_ref); @@ -7852,7 +7922,7 @@ got_worktree_merge_commit(struct got_object_id **new_c struct got_worktree *worktree, struct got_fileindex *fileindex, const char *author, const char *committer, int allow_bad_symlinks, struct got_object_id *branch_tip, const char *branch_name, - struct got_repository *repo, + int allow_conflict, struct got_repository *repo, got_worktree_status_cb status_cb, void *status_arg) { @@ -7898,6 +7968,7 @@ got_worktree_merge_commit(struct got_object_id **new_c cc_arg.repo = repo; cc_arg.have_staged_files = have_staged_files; cc_arg.allow_bad_symlinks = allow_bad_symlinks; + cc_arg.commit_conflicts = allow_conflict; err = worktree_status(worktree, "", fileindex, repo, collect_commitables, &cc_arg, NULL, NULL, 1, 0); if (err) blob - 0543d4bebab658396a069f968553a9b43594ca74 blob + 4bcb90babe877aac525887fb4eabeb49a4a2827d --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -317,8 +317,16 @@ test_commit_rejects_conflicted_file() { (cd $testroot/wt && got commit -m 'commit it' > $testroot/stdout \ 2> $testroot/stderr) + ret=$? + if [ $ret -eq 0 ]; then + echo "got commit succeeded unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi - echo -n > $testroot/stdout.expected + echo "C alpha" > $testroot/stdout.expected + echo "resolve conflicts or use -C to commit with unresolved" \ + "conflicts" >> $testroot/stdout.expected echo "got: cannot commit file in conflicted status" \ > $testroot/stderr.expected @@ -333,7 +341,36 @@ test_commit_rejects_conflicted_file() { ret=$? if [ $ret -ne 0 ]; then diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 fi + + (cd $testroot/wt && got commit -C -m 'commit it' > $testroot/stdout \ + 2> $testroot/stderr) + ret=$? + if [ $ret -ne 0 ]; then + echo "got commit failed unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got status > $testroot/stdout) + + echo -n > $testroot/stdout.expected + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + fi test_done "$testroot" "$ret" } blob - afb98ebb0b2de07c90dfa887d9d20fe0cb5f0db4 blob + 1ab6bfe75bae74735e4040c6d98ea1dea7cd9dc3 --- regress/cmdline/update.sh +++ regress/cmdline/update.sh @@ -2970,7 +2970,7 @@ test_update_binary_file() { fi (cd $testroot/wt && got status > $testroot/stdout) - echo 'C foo' > $testroot/stdout.expected + echo 'M foo' > $testroot/stdout.expected echo -n '? ' >> $testroot/stdout.expected (cd $testroot/wt && ls foo-1-* >> $testroot/stdout.expected) echo -n '? ' >> $testroot/stdout.expected -- Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org> GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68

2023-02-16 12:14 Mark Jamsek:
merge chokes, creates bogus conflicts
- 2023-02-16 12:27 Stefan Sperling:
  merge chokes, creates bogus conflicts
- - 2023-02-18 12:50 Mark Jamsek:
    merge chokes, creates bogus conflicts
  - - 2023-02-18 13:30 Mark Jamsek:
      merge chokes, creates bogus conflicts
    - - 2023-02-18 13:34 Mark Jamsek:
        merge chokes, creates bogus conflicts