Download raw body.
merge chokes, creates bogus conflicts
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"
}
merge chokes, creates bogus conflicts