From: Omar Polo Subject: Re: got patch: diff3 merge for git diffs To: Omar Polo Cc: gameoftrees@openbsd.org Date: Mon, 27 Jun 2022 15:01:51 +0200 Omar Polo wrote: > the idea is to parse also the > > index oldblobid...newblobid > > field in git diffs for the diff3 merge machinery. Yep, those are blob > ids and not commit nor tree ids, I got it wrong initially too. > > The diff seems to work (only tested thru the added regress) but could > use some help to tidy it a bit. One annoying thing is that git uses > abbreviated blob ids, unlike got diff which prints the full blob id, at > least with a default config. The change in `blobid' (got-read-patch.c) > is to accomodate for it, got_parse_sha1_digest fails if the given blob > id is abbreviated. I forgot to mention, there's another place I'm not so sure about: > [...] > - if (!got_parse_sha1_digest(id.sha1, blobid)) > - return got_error(GOT_ERR_BAD_OBJ_ID_STR); > + if (strlen(blobid) != 40) { looking at the length of the blobid to judge if it's abbreviated or not is fine? > + err = got_repo_match_object_id(&matched_id, NULL, blobid, > + GOT_OBJ_TYPE_BLOB, NULL /* do not resolve tags */, > + repo); > + if (err) > + return err; > + idptr = matched_id; > + } else { > + if (!got_parse_sha1_digest(id.sha1, blobid)) > + return got_error(GOT_ERR_BAD_OBJ_ID_STR); > + idptr = &id; > + } slightly revised diff with a more "git-looking" diff in test_patch_merge_unknown_blob diff refs/heads/main refs/heads/gitdiff commit - 3d589bee0bbbe812bb91f3b0284fbf2596304132 commit + 943bdc96d99adddfe53e7cb9b793c495ff6645eb blob - 965f8606f5dc0febfa47fceeba356519258f1021 blob + efe7f1a611a60b4528c47d210297d944f7e1fd38 --- lib/patch.c +++ lib/patch.c @@ -192,10 +192,11 @@ recv_patch(struct imsgbuf *ibuf, int *done, struct got goto done; } - if (*patch.cid != '\0' && *patch.blob != '\0') { + if (*patch.cid != '\0') strlcpy(p->cid, patch.cid, sizeof(p->cid)); + + if (*patch.blob != '\0') strlcpy(p->blob, patch.blob, sizeof(p->blob)); - } /* automatically set strip=1 for git-style diffs */ if (strip == -1 && patch.git && @@ -586,15 +587,25 @@ open_blob(char **path, FILE **fp, const char *blobid, { const struct got_error *err = NULL; struct got_blob_object *blob = NULL; - struct got_object_id id; + struct got_object_id id, *idptr, *matched_id = NULL; *fp = NULL; *path = NULL; - if (!got_parse_sha1_digest(id.sha1, blobid)) - return got_error(GOT_ERR_BAD_OBJ_ID_STR); + if (strlen(blobid) != 40) { + err = got_repo_match_object_id(&matched_id, NULL, blobid, + GOT_OBJ_TYPE_BLOB, NULL /* do not resolve tags */, + repo); + if (err) + return err; + idptr = matched_id; + } else { + if (!got_parse_sha1_digest(id.sha1, blobid)) + return got_error(GOT_ERR_BAD_OBJ_ID_STR); + idptr = &id; + } - err = got_object_open_as_blob(&blob, repo, &id, 8192); + err = got_object_open_as_blob(&blob, repo, idptr, 8192); if (err) goto done; @@ -609,6 +620,8 @@ open_blob(char **path, FILE **fp, const char *blobid, done: if (blob) got_object_blob_close(blob); + if (matched_id != NULL) + free(matched_id); if (err) { if (*fp != NULL) fclose(*fp); @@ -647,7 +660,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work * ignore failures to open this blob, we might have * parsed gibberish. */ - if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT)) + if (err && !(err->code == GOT_ERR_ERRNO && errno == ENOENT) && + err->code != GOT_ERR_NO_OBJ) return err; else if (err == NULL) do_merge = 1; @@ -689,6 +703,8 @@ apply_patch(int *overlapcnt, struct got_worktree *work goto done; if (do_merge) { + const char *type, *id; + if (fseeko(afile, 0, SEEK_SET) == -1 || fseeko(oldfile, 0, SEEK_SET) == -1 || fseeko(tmpfile, 0, SEEK_SET) == -1) { @@ -708,7 +724,15 @@ apply_patch(int *overlapcnt, struct got_worktree *work goto done; } - if (asprintf(&anclabel, "commit %s", p->cid) == -1) { + if (*p->cid != '\0') { + type = "commit"; + id = p->cid; + } else { + type = "blob"; + id = p->blob; + } + + if (asprintf(&anclabel, "%s %s", type, id) == -1) { err = got_error_from_errno("asprintf"); anclabel = NULL; goto done; blob - 05415ddde579ebfde446284ba961406bc37a4255 blob + d6eea51f8d3203ae05b110adaac04ee330bd4d28 --- libexec/got-read-patch/got-read-patch.c +++ libexec/got-read-patch/got-read-patch.c @@ -74,10 +74,11 @@ send_patch(const char *oldname, const char *newname, c if (newname != NULL) strlcpy(p.new, newname, sizeof(p.new)); - if (commitid != NULL && blob != NULL) { + if (commitid != NULL) strlcpy(p.cid, commitid, sizeof(p.cid)); + + if (blob != NULL) strlcpy(p.blob, blob, sizeof(p.blob)); - } p.git = git; if (imsg_compose(&ibuf, GOT_IMSG_PATCH, 0, 0, -1, &p, sizeof(p)) == -1) @@ -129,7 +130,7 @@ filename(const char *at, char **name) } static const struct got_error * -blobid(const char *line, char **blob) +blobid(const char *line, char **blob, int git) { uint8_t digest[SHA1_DIGEST_LENGTH]; size_t len; @@ -140,7 +141,7 @@ blobid(const char *line, char **blob) if ((*blob = strndup(line, len)) == NULL) return got_error_from_errno("strndup"); - if (!got_parse_sha1_digest(digest, *blob)) { + if (!git && !got_parse_sha1_digest(digest, *blob)) { /* silently ignore invalid blob ids */ free(*blob); *blob = NULL; @@ -176,13 +177,16 @@ find_patch(int *done, FILE *fp) err = filename(line+4, &new); } else if (!git && !strncmp(line, "blob - ", 7)) { free(blob); - err = blobid(line + 7, &blob); + err = blobid(line + 7, &blob, git); } else if (rename && !strncmp(line, "rename to ", 10)) { free(new); err = filename(line + 10, &new); } else if (git && !strncmp(line, "similarity index 100%", 21)) rename = 1; - else if (!strncmp(line, "diff --git a/", 13)) { + else if (git && !strncmp(line, "index ", 6)) { + free(blob); + err = blobid(line + 6, &blob, git); + } else if (!strncmp(line, "diff --git a/", 13)) { git = 1; free(commitid); commitid = NULL; @@ -190,10 +194,10 @@ find_patch(int *done, FILE *fp) blob = NULL; } else if (!git && !strncmp(line, "diff ", 5)) { free(commitid); - err = blobid(line + 5, &commitid); + err = blobid(line + 5, &commitid, git); } else if (!git && !strncmp(line, "commit - ", 9)) { free(commitid); - err = blobid(line + 9, &commitid); + err = blobid(line + 9, &commitid, git); } if (err) blob - 9fb1dd39304f1cd716283dd5684a1162ee8f745c blob + 51903218ccbe2f14d00ba02be9bbc8ead7c2f1e4 --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -1507,6 +1507,62 @@ test_patch_merge_simple() { test_done $testroot $ret } +test_patch_merge_gitdiff() { + local testroot=`test_init patch_merge_gitdiff` + + jot 10 > $testroot/repo/numbers + (cd $testroot/repo && git add numbers && \ + git_commit $testroot/repo -m "nums") + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + jot 10 | sed 's/4/four/g' > $testroot/repo/numbers + (cd $testroot/repo && git diff > $testroot/old.diff) + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + # restore numbers + jot 10 > $testroot/repo/numbers + + jot 10 | sed 's/6/six/g' > $testroot/repo/numbers + (cd $testroot/repo && git add numbers && \ + git_commit $testroot/repo -m "edit") + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + # now work with got: + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + (cd $testroot/wt && got patch $testroot/old.diff) > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + echo 'G numbers' > $testroot/stdout.expected + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout $testroot/stdout.expected + fi + test_done $testroot $ret +} + test_patch_merge_conflict() { local testroot=`test_init patch_merge_conflict` @@ -1665,7 +1721,36 @@ EOF ret=$? if [ $ret -ne 0 ]; then diff -u $testroot/stdout.expected $testroot/stdout + test_done $testroot $ret + return 1 fi + + # try again with a git-style diff + + cat < $testroot/wt/patch +diff --git a/alpha b/alpha +index 0123456789ab..abcdef012345 100644 +--- a/alpha ++++ b/alpha +@@ -1 +1 @@ +-alpha ++ALPHA +EOF + + (cd $testroot/wt && got revert alpha > /dev/null && got patch patch) \ + > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + test_done $testroot $ret + return 1 + fi + + echo 'M alpha' > $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 } @@ -1695,5 +1780,6 @@ run_test test_patch_with_path_prefix run_test test_patch_relpath_with_path_prefix run_test test_patch_reverse run_test test_patch_merge_simple +run_test test_patch_merge_gitdiff run_test test_patch_merge_conflict run_test test_patch_merge_unknown_blob