Download raw body.
merge chokes, creates bogus conflicts
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. It'll
be trivial to add a confirmation prompt after that's done if we think an
extra layer is needed. It' just a lot easier to resolve conflicts as
they arise rather than sometime later and perhaps even by someone other
than the conflict creator.
> Thinking some more about this, I would like to extend this patch further.
> The change is not useful enough to be committed as it is.
>
> I don't have time to keep working on this right now. But I already want to
> my rough plan. And given that this affects the OpenBSD-current source tree
> I would like to get this done relatively quickly and ship a fix in the
> next Got release.
>
> We should make get_modified_file_content_status() in worktree.c look for
> conflict markers on newly added lines only, by driving the file through
> the diff engine and looking for conflict markers in the diff result.
> This should work similarly to how 'got stage' and 'got unstage' are driven.
> Conflict markers which were already part of the base blob would then be
> ignored during the status walk, allowing such files to show up as 'M' after
> regular edits, as long as no new genuine conflicts appear.
>
> (Before we call get_modified_file_content_status() we already scan the
> file once to detect modifications. This scan should be faster than diffing
> the file. For performance reasons, diffing should only be used if file
> modifications have already been detected the fast way, such that we pay
> the additional performance penalty of diff on modified files only.)
>
> The -C option added to the commit command in the patch below should also
> be added to rebase, histedit, and merge commands. It would only be accepted
> in combination with the -c (lower case) options which trigger creation of
> new commits. naddy's merge would still detect a conflict. But upon realizing
> that these conflict markers do not represent actual conflicts, merge -c -C
> could be used to continue the merge. In combination with the status walk
> changes above, existing conflict markers in affected files would no longer
> interfere with future merge operations.
>
> > -----------------------------------------------
> >
> > 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"
> > }
> >
> >
> >
> >
> >
>
--
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
merge chokes, creates bogus conflicts