Download raw body.
merge chokes, creates bogus conflicts
On 23-02-18 02:47PM, Stefan Sperling wrote:
> On Sat, Feb 18, 2023 at 11:50:16PM +1100, Mark Jamsek wrote:
> > 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!
>
> Oops. The test didn't catch this, so I guess it should be expanded?
Good idea! I've added this to the test.
> > 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?
>
> Yes, it is in line with the new expected behaviour.
Great! I confess I spent way too much time tonight considering whether
it was or wasn't, and in the end decided it was too :)
> > 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.
>
> I would rather avoid suggesting use of -C in error messages.
>
> This could lead to people using -C as a lazy alternative instead
> of resolving conflicts. It is better to bury the existence of -C
> in the man page where it is documented with an appropriate warning.
> It will be discovered by users who have a real need for it and search
> the documentation for clues.
>
> If we are going to adjust the message then I would suggest we change
> the error message to explcitly suggest that conflicts must be resolved.
> This would guide users towards the correct way out.
>
> got: cannot commit file in conflicted status; conflicts must be resolved before changes can be committed
Agree on all counts! I've left the original error message too.
I think it is already clear that conflicts must be resolved before
committing. And we're not going to further advertise the -C workaround.
>
> > +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 .
>
> The sentence above is now wrong and should be deleted.
Oops, I misunderstood this: I thought it applied to the fact that
he/mg/rb won't automatically continue with conflicts unless -C is used
(i.e., -C -c).
I missed the fact this sentence is about already versioned content with
conflicts. Thank you! I've dropped it.
> > 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.
>
> I would drop the above parapraph to avoid promoting use of -C, and
> also drop it from the histedit and merge sections.
>
> It is technically correct to document all possible features and
> behaviours. But the more we advertise this particular feature the
> higher the probability becomes that -C will be misused.
I went back and forth on this myself and ultimately decided to add
it--but I'll defer to your better judgement on this and drop it.
> > +.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.
>
> Let's try to discourage use a bit more:
>
> This option should generally be avoided, and can only be used with the
> .Fl c
> option.
>
> Same for histedit and merge.
Good idea. Done!
> > done:
> > + if (error && error->code == GOT_ERR_COMMIT_CONFLICT)
> > + printf("resolve conflicts or use -C to commit with "
> > + "unresolved conflicts\n");
>
> I would drop the above chunk as explained above.
>
> Two related nits:
>
> Errors should go to stderr.
>
> When amending an error with static information like this, consider
> whether editing the string constant in lib/error.c would do the job.
Good advice; I'll file this away for future reference. Thank you, and
done!
> > -/* 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.
>
> Grammar: Should this say "newly added lines"?
I think I meant "new, added lines" but I actually prefer "newly", which
is what I went with. Thanks!
> > + err = got_diff_blob_file(blob, f1, sz1, NULL, ondisk_file, 1, sb,
> > + path, GOT_DIFF_ALGORITHM_MYERS, 0, 0, 0, NULL, f);
>
> Myers is the right choice in this case indeed. Patience diff would
> imply some additional performane overhead we don't want to pay
> during the status crawl.
That was my thinking :)
> > + 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) {
>
> I suspect using the diff_result structure would be more efficient here.
>
> It should be possible to skip the unidiff output step entirely by exposing
> got_diffreg_result and jumping to relevant lines in the working version of
> the file, available in diffreg_result->diff_result's right side.
> But we can parse the text diff for now and experiment with diff_result later.
Yes, I do agree with this and it's what I wanted to do. Without going
into a long story, we had some drama here today with a storm and
I didn't get to continue work on this till after dinner tonight so
I went for the quicker-to-implement option as I know this impacts the
OpenBSD tree so we want a resolution quickly for release.
But, yes, I will improve this tomorrow by using diff result :)
> > @@ -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)
>
> We do not allow STATUS_CONFLICT for staged files. I see no good reason
> to allow staging of changes with conflict markers, that would just
> complicate things even more.
Thanks for clarifying this: I wasn't at all sure about these additional
GOT_STATUS_CONFLICT changes but thought better to add them and bring it
to your attention so I could both be certain and learn something.
> > @@ -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:
>
> As above, staged_status should only be one of A, D, or M.
> We should not allow files in 'C' status to be staged in any way.
>
> > @@ -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) {
>
> Same.
>
> > @@ -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) {
>
> Same.
>
> > @@ -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.
> > + */
>
> Yes, this is fine. Above XXX comment can be removed.
Great, done!
> > 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)
>
> But the rules for commit of staged files should not be changed.
>
> > 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) {
>
> Same.
>
> > 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) {
>
> Same.
>
> > 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)
>
> Same.
Ditto for the staged_status changes.
Updated diff incorporates all your suggestions. Thanks for the
improvements :)
diffstat refs/remotes/origin/main refs/heads/main
M got/got.1 | 27+ 4-
M got/got.c | 68+ 23-
M include/got_worktree.h | 4+ 4-
M lib/worktree.c | 77+ 16-
M regress/cmdline/commit.sh | 73+ 1-
M regress/cmdline/update.sh | 1+ 1-
6 files changed, 250 insertions(+), 49 deletions(-)
diff refs/remotes/origin/main refs/heads/main
commit - f990756a3987ba6410baf611d561e9b8f285f047
commit + b83449a8ed946f9145945d1089f1b40b88fcc2e1
blob - bb1825eecded3623bbabcf0541bce55367696327
blob + b6cc16e8e5a4b02bf33a4f2af21bc0fd0f58c4bd
--- got/got.1
+++ got/got.1
@@ -1652,7 +1652,7 @@ is a directory.
.Tg ci
.It Xo
.Cm commit
-.Op Fl NnS
+.Op Fl CNnS
.Op Fl A Ar author
.Op Fl F Ar path
.Op Fl m Ar message
@@ -1740,6 +1740,14 @@ 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.
.It Fl F Ar path
Use the prepared log message stored in the file found at
.Ar path
@@ -2209,7 +2217,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
@@ -2359,6 +2367,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 should generally be avoided, and 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 +2435,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
@@ -2597,6 +2610,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 should generally be avoided, and 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 +2767,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
@@ -2864,6 +2882,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 should generally be avoided, and 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 + 438e15cb2486d96840625d284c94c90018d7917b
--- 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)
@@ -10368,7 +10371,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 +10495,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 +10508,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 +11004,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 +11022,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 +11051,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 +11062,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 +11194,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 +11373,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 +11444,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 +12184,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 +12203,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 +12288,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 +12309,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 +12350,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 +12386,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 +12401,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 +12422,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 +12440,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 +12758,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 +12834,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 +13067,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 +13083,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 +13097,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 +13119,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 +13313,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 + 055da929b846e6b27b801620463ae896ca9faf0a
--- 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 newly 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,46 @@ get_modified_file_content_status(unsigned char *status
GOT_DIFF_CONFLICT_MARKER_SEP,
GOT_DIFF_CONFLICT_MARKER_END
};
+ FILE *f = NULL, *f1 = NULL;
int i = 0;
char *line = NULL;
size_t linesize = 0;
ssize_t linelen;
+ off_t sz1 = 0;
+ if (*status == GOT_STATUS_MODIFY) {
+ 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 +1577,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 +1586,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 +1833,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 +4995,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;
@@ -5023,7 +5072,8 @@ append_ct_diff(struct got_commitable *ct, int *diff_he
} 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;
}
@@ -5180,13 +5230,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;
}
@@ -5536,7 +5589,8 @@ match_deleted_or_modified_ct(struct got_commitable **c
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 &&
@@ -5744,6 +5798,7 @@ 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->status == GOT_STATUS_CONFLICT ||
ct->staged_status == GOT_STATUS_MODIFY) {
err = alloc_modified_blob_tree_entry(
&new_te, te, ct);
@@ -5953,7 +6008,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 +6160,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 +6214,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 +6771,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 +6795,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 +6891,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 +6916,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 +6931,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 +6947,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 +7912,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 +7958,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 + 488cdaf04a738a2e6a11df99d0d5df6b7a604c83
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -294,6 +294,7 @@ test_commit_rejects_conflicted_file() {
echo "modified alpha" > $testroot/wt/alpha
(cd $testroot/wt && got commit -m "modified alpha" >/dev/null)
+ local commit_id1=`git_show_head $testroot/repo`
(cd $testroot/wt && got update -c $initial_rev > /dev/null)
@@ -317,8 +318,14 @@ 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 "got: cannot commit file in conflicted status" \
> $testroot/stderr.expected
@@ -333,7 +340,72 @@ 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
+
+ # make sure the conflicted commit produces a diff
+ local conflict_commit=`git_show_head $testroot/repo`
+ local blob_minus=`got tree -r $testroot/repo -c $commit_id1 -i | \
+ grep 'alpha$' | cut -d' ' -f1`
+ local blob_plus=`got tree -r $testroot/repo -c $conflict_commit -i | \
+ grep 'alpha$' | cut -d' ' -f1`
+
+ 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 diff -c master > $testroot/stdout)
+
+ echo -n > $testroot/stdout.expected
+ cat > $testroot/stdout.expected <<EOF
+diff $commit_id1 refs/heads/master
+commit - $commit_id1
+commit + $conflict_commit
+blob - $blob_minus
+blob + $blob_plus
+--- alpha
++++ alpha
+@@ -1 +1,7 @@
++<<<<<<< merged change: commit $commit_id1
+ modified alpha
++||||||| 3-way merge base: commit $initial_rev
++alpha
++=======
++modified alpha, too
++>>>>>>>
+EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ 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
merge chokes, creates bogus conflicts