From: Stefan Sperling Subject: Re: merge chokes, creates bogus conflicts To: Christian Weisgerber , gameoftrees@openbsd.org Date: Thu, 16 Feb 2023 02:11:54 +0100 On Thu, Feb 16, 2023 at 01:17:02AM +0100, Stefan Sperling wrote: > On Wed, Feb 15, 2023 at 04:35:44PM +0100, Christian Weisgerber wrote: > > got merge chokes on the perl-5.36.0 commits to OpenBSD src.git and > > creates bogus conflicts. Here's a script to reproduce this: > > > > git clone --bare https://github.com/openbsd/src.git > > got co src.git > > cd src > > got br -c d122a78 local # before perl-5.36.0 import > > echo dummy >README > > got add README > > got ci -m 'local: dummy commit' > > got up > > got merge master > > got st | grep '^[^MAD]' > > > > C gnu/usr.bin/perl/t/comp/parser.t > > C gnu/usr.bin/perl/t/lib/croak/toke > > The second file also contains conflict markers: > > 423: ######## > 424: # NAME multiple conflict markers > 425: <<<<<<< yours:sample.txt > 426: my $some_code; > 427: ======= > 428: my $some_other_code; > 429: >>>>>>> theirs:sample.txt > 430: EXPECT > 431: Version control conflict marker at - line 1, near "<<<<<<<" > 432: Version control conflict marker at - line 3, near "=======" > 433: Version control conflict marker at - line 5, near ">>>>>>>" > 434: Execution of - aborted due to compilation errors. > 435: ######## 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: ----------------------------------------------- add commit -C option which allows committing conflicted files diff ac51614e087279834159667c6f71fffe546cffa6 6311f9af7da24af59f367128fefe853c2c723d7f commit - ac51614e087279834159667c6f71fffe546cffa6 commit + 6311f9af7da24af59f367128fefe853c2c723d7f blob - d9209e2ae1dd1c1aa01e368bc0016cf86830a9e7 blob + f9fe8e5778a46a240da72b6ab9d01089637a5ba1 --- got/got.1 +++ got/got.1 @@ -1650,6 +1650,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 ... @@ -1736,6 +1737,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 blob - 1ef76ea95b69bf7e45b91f9053a232d241269e7d blob + c99f2b5bd116bc44b3c5e60d7d7495d4f0177acb --- got/got.c +++ got/got.c @@ -8732,7 +8732,7 @@ usage_commit(void) __dead static void usage_commit(void) { - fprintf(stderr, "usage: %s commit [-NS] [-A author] [-F path] " + fprintf(stderr, "usage: %s commit [-CNS] [-A author] [-F path] " "[-m message] [path ...]\n", getprogname()); exit(1); } @@ -9039,7 +9039,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; @@ -9055,7 +9055,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; @@ -9063,6 +9063,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'); @@ -9186,8 +9189,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) blob - 4ea02aaea560daeba058d4b8541f5ea222eb7586 blob + e5f95b72539240162c14b8eee9816c3550d24873 --- 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. */ blob - c4fc2f02d953c285d633a68f2ef9f0cebc512bca blob + 9b8278562fbc8edd1d7d6f3f1e23de62ca1817c0 --- lib/worktree.c +++ lib/worktree.c @@ -4947,6 +4947,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; @@ -5180,13 +5181,14 @@ 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) 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; } @@ -5953,7 +5955,8 @@ commit_worktree(struct got_object_id **new_commit_id, 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 +6107,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 +6161,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"); blob - 0543d4bebab658396a069f968553a9b43594ca74 blob + e0222b3df34ba4dbbae612cb0d90dce8f61fc710 --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -317,6 +317,12 @@ 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 "got: cannot commit file in conflicted status" \ @@ -333,7 +339,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" }