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