{"thread":{"id":"60423","subject":"[PATCH 1/1] merge-file: add an option to process object IDs","startedAt":"2023-10-24T19:58:05Z","lastAt":"2023-11-02T16:28:53Z","messageCount":34,"participants":["brian m. carlson","Eric Sunshine","Elijah Newren","Phillip Wood","Taylor Blau","Junio C Hamano","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"483811","messageId":"20231024195655.2413191-2-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231024195655.2413191-1-sandals@crustytoothpaste.net","subject":"[PATCH 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-24T19:56:55Z","receivedAt":"2023-10-24T19:58:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" <bk2204@github.com>\n\ngit merge-file knows how to merge files on the file system already.  It\nwould be helpful, however, to allow it to also merge single blobs.\nTeach it an `--object-id` option which means that its arguments are\nobject IDs and not files to allow it to do so.\n\nSince we obviously won't be writing the data to the first argument,\neither write to the object store and print the object ID, or honor the\n-p argument and print it to standard out.\n\nWe handle the empty blob specially since read_mmblob doesn't read it\ndirectly, instead throwing an error, and otherwise users cannot specify\nan empty ancestor.\n\nSigned-off-by: brian m. carlson <bk2204@github.com>\n---\n Documentation/git-merge-file.txt | 20 +++++++++++\n builtin/merge-file.c             | 58 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++++\n 3 files changed, 120 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex 7e9093fab6..45460f3916 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -12,6 +12,9 @@ SYNOPSIS\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n \t[--[no-]diff3] <current-file> <base-file> <other-file>\n+'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n+\t[--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]\n+\t[--[no-]diff3] <current-oid> <base-oid> <other-oid>\n \n \n DESCRIPTION\n@@ -40,6 +43,10 @@ however, these conflicts are resolved favouring lines from `<current-file>`,\n lines from `<other-file>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n+If `--object-id` is specified, exactly the same behavior occurs, except that\n+instead of specifying what to merge as files, it is specified as a list of\n+object IDs referring to blobs.\n+\n The exit value of this program is negative on error, and the number of\n conflicts otherwise (truncated to 127 if there are more than that many\n conflicts). If the merge was clean, the exit value is 0.\n@@ -52,6 +59,14 @@ linkgit:git[1].\n OPTIONS\n -------\n \n+--object-id::\n+\tSpecify the contents to merge as blobs in the current repository instead of\n+\tfiles.  In this case, the operation must take place within a valid repository.\n++\n+If the `-p` option is specified, the merged file (including conflicts, if any)\n+goes to standard output as normal; otherwise, the merged file is written to the\n+object store and the object ID of its blob is written to standard output.\n+\n -L <label>::\n \tThis option may be given up to three times, and\n \tspecifies labels to be used in place of the\n@@ -93,6 +108,11 @@ EXAMPLES\n \tmerges tmp/a123 and tmp/c345 with the base tmp/b234, but uses labels\n \t`a` and `c` instead of `tmp/a123` and `tmp/c345`.\n \n+`git merge-file -p --object-id abc1234 def567 890abcd`::\n+\n+\tcombines the changes of the blob abc1234 and 890abcd since def567,\n+\ttries to merge them and writes the result to standard output\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex d7eb4c6540..d308434b8e 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -1,5 +1,8 @@\n #include \"builtin.h\"\n #include \"abspath.h\"\n+#include \"hex.h\"\n+#include \"object-name.h\"\n+#include \"object-store.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n #include \"setup.h\"\n@@ -31,10 +34,11 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tmmfile_t mmfs[3] = { 0 };\n \tmmbuffer_t result = { 0 };\n \txmparam_t xmp = { 0 };\n-\tint ret = 0, i = 0, to_stdout = 0;\n+\tint ret = 0, i = 0, to_stdout = 0, object_id = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL('p', \"stdout\", &to_stdout, N_(\"send results to standard output\")),\n+\t\tOPT_BOOL(0,   \"object-id\", &object_id, N_(\"use object IDs instead of filenames\")),\n \t\tOPT_SET_INT(0, \"diff3\", &xmp.style, N_(\"use a diff3 based merge\"), XDL_MERGE_DIFF3),\n \t\tOPT_SET_INT(0, \"zdiff3\", &xmp.style, N_(\"use a zealous diff3 based merge\"),\n \t\t\t\tXDL_MERGE_ZEALOUS_DIFF3),\n@@ -71,8 +75,12 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\t\treturn error_errno(\"failed to redirect stderr to /dev/null\");\n \t}\n \n+\tif (object_id)\n+\t\tsetup_git_directory();\n+\n \tfor (i = 0; i < 3; i++) {\n \t\tchar *fname;\n+\t\tstruct object_id oid;\n \t\tmmfile_t *mmf = mmfs + i;\n \n \t\tif (!names[i])\n@@ -80,12 +88,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \t\tfname = prefix_filename(prefix, argv[i]);\n \n-\t\tif (read_mmfile(mmf, fname))\n+\t\tif (object_id) {\n+\t\t\tif (repo_get_oid(the_repository, argv[i], &oid))\n+\t\t\t\tret = -1;\n+\t\t\telse if (!oideq(&oid, the_hash_algo->empty_blob))\n+\t\t\t\tread_mmblob(mmf, &oid);\n+\t\t\telse\n+\t\t\t\tread_mmfile(mmf, \"/dev/null\");\n+\t\t} else if (read_mmfile(mmf, fname)) {\n \t\t\tret = -1;\n-\t\telse if (mmf->size > MAX_XDIFF_SIZE ||\n-\t\t\t buffer_is_binary(mmf->ptr, mmf->size))\n+\t\t}\n+\t\tif (ret != -1 && (mmf->size > MAX_XDIFF_SIZE ||\n+\t\t    buffer_is_binary(mmf->ptr, mmf->size))) {\n \t\t\tret = error(\"Cannot merge binary files: %s\",\n \t\t\t\t    argv[i]);\n+\t\t}\n \n \t\tfree(fname);\n \t\tif (ret)\n@@ -99,20 +116,29 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);\n \n \tif (ret >= 0) {\n-\t\tconst char *filename = argv[0];\n-\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n-\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n+\t\tif (object_id && !to_stdout) {\n+\t\t\tstruct object_id oid;\n+\t\t\tif (result.size)\n+\t\t\t\twrite_object_file(result.ptr, result.size, OBJ_BLOB, &oid);\n+\t\t\telse\n+\t\t\t\toidcpy(&oid, the_hash_algo->empty_blob);\n+\t\t\tprintf(\"%s\\n\", oid_to_hex(&oid));\n+\t\t} else {\n+\t\t\tconst char *filename = argv[0];\n+\t\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n+\t\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n \n-\t\tif (!f)\n-\t\t\tret = error_errno(\"Could not open %s for writing\",\n-\t\t\t\t\t  filename);\n-\t\telse if (result.size &&\n-\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n-\t\t\tret = error_errno(\"Could not write to %s\", filename);\n-\t\telse if (fclose(f))\n-\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tif (!f)\n+\t\t\t\tret = error_errno(\"Could not open %s for writing\",\n+\t\t\t\t\t\t  filename);\n+\t\t\telse if (result.size &&\n+\t\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n+\t\t\t\tret = error_errno(\"Could not write to %s\", filename);\n+\t\t\telse if (fclose(f))\n+\t\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tfree(fpath);\n+\t\t}\n \t\tfree(result.ptr);\n-\t\tfree(fpath);\n \t}\n \n \tif (ret > 127)\ndiff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh\nindex 1a7082323d..2c92209eca 100755\n--- a/t/t6403-merge-file.sh\n+++ b/t/t6403-merge-file.sh\n@@ -65,11 +65,30 @@ test_expect_success 'merge with no changes' '\n \ttest_cmp test.txt orig.txt\n '\n \n+test_expect_success 'merge with no changes with --object-id' '\n+\tgit add orig.txt &&\n+\tgit merge-file -p --object-id :orig.txt :orig.txt :orig.txt >actual &&\n+\ttest_cmp actual orig.txt\n+'\n+\n test_expect_success \"merge without conflict\" '\n \tcp new1.txt test.txt &&\n \tgit merge-file test.txt orig.txt new2.txt\n '\n \n+test_expect_success 'merge without conflict with --object-id' '\n+\tgit add orig.txt new2.txt &&\n+\tgit merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n+test_expect_success 'can accept object ID with --object-id' '\n+\tgit merge-file --object-id $(test_oid empty_blob) $(test_oid empty_blob) :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n test_expect_success 'works in subdirectory' '\n \tmkdir dir &&\n \tcp new1.txt dir/a.txt &&\n@@ -138,6 +157,31 @@ test_expect_success \"expected conflict markers\" '\n \ttest_cmp expect.txt test.txt\n '\n \n+test_expect_success \"merge with conflicts with --object-id\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id :backup.txt :orig.txt :new3.txt >actual &&\n+\tsed -e \"s/<< test.txt/<< :backup.txt/\" \\\n+\t    -e \"s/>> new3.txt/>> :new3.txt/\" \\\n+\t    expect.txt >expect &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git merge-file --object-id :backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success \"merge with conflicts with --object-id with labels\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >actual &&\n+\ttest_cmp expect.txt actual &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect.txt actual\n+'\n+\n test_expect_success \"merge conflicting with --ours\" '\n \tcp backup.txt test.txt &&\n \n@@ -256,6 +300,14 @@ test_expect_success 'binary files cannot be merged' '\n \tgrep \"Cannot merge binary files\" merge.err\n '\n \n+test_expect_success 'binary files cannot be merged with --object-id' '\n+\tcp \"$TEST_DIRECTORY\"/test-binary-1.png . &&\n+\tgit add orig.txt new1.txt test-binary-1.png &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t:orig.txt :test-binary-1.png :new1.txt 2> merge.err &&\n+\tgrep \"Cannot merge binary files\" merge.err\n+'\n+\n test_expect_success 'MERGE_ZEALOUS simplifies non-conflicts' '\n \tsed -e \"s/deerit.\\$/deerit;/\" -e \"s/me;\\$/me./\" <new5.txt >new6.txt &&\n \tsed -e \"s/deerit.\\$/deerit,/\" -e \"s/me;\\$/me,/\" <new5.txt >new7.txt &&\n@@ -389,4 +441,10 @@ test_expect_success 'conflict sections match existing line endings' '\n \ttest $(tr \"\\015\" Q <nolf.txt | grep \"^[<=>].*Q$\" | wc -l) = 0\n '\n \n+test_expect_success '--object-id fails without repository' '\n+\tempty=\"$(test_oid empty_blob)\" &&\n+\tnongit test_must_fail git merge-file --object-id $empty $empty $empty 2>err &&\n+\tgrep \"not a git repository\" err\n+'\n+\n test_done\n"},{"id":"483812","messageId":"20231024195655.2413191-1-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":null,"subject":"[PATCH 0/1] Object ID support for git merge-file","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-24T19:56:54Z","receivedAt":"2023-10-24T19:58:06Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces an --object-id option to git merge-file such\nthat, instead of reading and writing from files on the system, it reads\nfrom and writes to the object store using blobs.  This is in use at\nGitHub to produce conflict diffs when a merge fails, and it seems\ngenerally useful, so I'm sending it here.\n\nThe only tricky piece is the fact that we have to special-case the empty\nblob since otherwise it isn't handled correctly.\n\nbrian m. carlson (1):\n  merge-file: add an option to process object IDs\n\n Documentation/git-merge-file.txt | 20 +++++++++++\n builtin/merge-file.c             | 58 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++++\n 3 files changed, 120 insertions(+), 16 deletions(-)\n\n"},{"id":"483814","messageId":"CAPig+cT_yq-ke4RTpTdTTLsnJFxCtyGAP2K0mQ_S23jJYtUp=w@mail.gmail.com","threadId":"60423","inReplyTo":"20231024195655.2413191-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-10-24T20:12:52Z","receivedAt":"2023-10-24T20:13:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Oct 24, 2023 at 3:58 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> git merge-file knows how to merge files on the file system already.  It\n> would be helpful, however, to allow it to also merge single blobs.\n> Teach it an `--object-id` option which means that its arguments are\n> object IDs and not files to allow it to do so.\n>\n> Since we obviously won't be writing the data to the first argument,\n> either write to the object store and print the object ID, or honor the\n> -p argument and print it to standard out.\n>\n> We handle the empty blob specially since read_mmblob doesn't read it\n> directly, instead throwing an error, and otherwise users cannot specify\n> an empty ancestor.\n>\n> Signed-off-by: brian m. carlson <bk2204@github.com>\n> ---\n> diff --git a/builtin/merge-file.c b/builtin/merge-file.c\n> @@ -99,20 +116,29 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n>         if (ret >= 0) {\n> -               const char *filename = argv[0];\n> -               char *fpath = prefix_filename(prefix, argv[0]);\n> -               FILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n> +               if (object_id && !to_stdout) {\n> +                       struct object_id oid;\n> +                       if (result.size)\n> +                               write_object_file(result.ptr, result.size, OBJ_BLOB, &oid);\n\nShould this be caring about errors by checking the return value of\nwrite_object_file()?\n\n> +               } else {\n> +                       const char *filename = argv[0];\n> +                       char *fpath = prefix_filename(prefix, argv[0]);\n> +                       FILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n> +                       if (!f)\n> +                               ret = error_errno(\"Could not open %s for writing\",\n> +                                                 filename);\n> +                       else if (result.size &&\n> +                                fwrite(result.ptr, result.size, 1, f) != 1)\n> +                               ret = error_errno(\"Could not write to %s\", filename);\n> +                       else if (fclose(f))\n> +                               ret = error_errno(\"Could not close %s\", filename);\n> +                       free(fpath);\n\n The non-\"object-id\" case cares about errors.\n"},{"id":"483822","messageId":"ZTg1t33cxzvmvAVK@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"CAPig+cT_yq-ke4RTpTdTTLsnJFxCtyGAP2K0mQ_S23jJYtUp=w@mail.gmail.com","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-24T21:23:03Z","receivedAt":"2023-10-24T21:23:08Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-10-24 at 20:12:52, Eric Sunshine wrote:\n> On Tue, Oct 24, 2023 at 3:58 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > git merge-file knows how to merge files on the file system already.  It\n> > would be helpful, however, to allow it to also merge single blobs.\n> > Teach it an `--object-id` option which means that its arguments are\n> > object IDs and not files to allow it to do so.\n> >\n> > Since we obviously won't be writing the data to the first argument,\n> > either write to the object store and print the object ID, or honor the\n> > -p argument and print it to standard out.\n> >\n> > We handle the empty blob specially since read_mmblob doesn't read it\n> > directly, instead throwing an error, and otherwise users cannot specify\n> > an empty ancestor.\n> >\n> > Signed-off-by: brian m. carlson <bk2204@github.com>\n> > ---\n> > diff --git a/builtin/merge-file.c b/builtin/merge-file.c\n> > @@ -99,20 +116,29 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n> >         if (ret >= 0) {\n> > -               const char *filename = argv[0];\n> > -               char *fpath = prefix_filename(prefix, argv[0]);\n> > -               FILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n> > +               if (object_id && !to_stdout) {\n> > +                       struct object_id oid;\n> > +                       if (result.size)\n> > +                               write_object_file(result.ptr, result.size, OBJ_BLOB, &oid);\n> \n> Should this be caring about errors by checking the return value of\n> write_object_file()?\n\nProbably so.  I think I saw write_object_file_prepare returned void and\nmisinterpreted that as write_object_file returning void.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484042","messageId":"CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com","threadId":"60423","inReplyTo":"20231024195655.2413191-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-10-29T06:17:09Z","receivedAt":"2023-10-29T06:17:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOverall, looks good.  Just a couple questions...\n\nOn Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> From: \"brian m. carlson\" <bk2204@github.com>\n>\n[...]\n> --- a/Documentation/git-merge-file.txt\n> +++ b/Documentation/git-merge-file.txt\n> @@ -12,6 +12,9 @@ SYNOPSIS\n>  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n>         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n>         [--[no-]diff3] <current-file> <base-file> <other-file>\n> +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> +       [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]\n> +       [--[no-]diff3] <current-oid> <base-oid> <other-oid>\n\nWhy was the `[-p|--stdout]` option removed in the second synopsis?\nElsewhere you explicitly call it out as a possibility to be used with\n--object-id.\n\nAlso, why the extra synopsis instead of just adding a `[--object-id]`\noption to the previous one?\n\n[...]\n> @@ -80,12 +88,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n>\n>                 fname = prefix_filename(prefix, argv[i]);\n>\n> -               if (read_mmfile(mmf, fname))\n> +               if (object_id) {\n> +                       if (repo_get_oid(the_repository, argv[i], &oid))\n> +                               ret = -1;\n> +                       else if (!oideq(&oid, the_hash_algo->empty_blob))\n> +                               read_mmblob(mmf, &oid);\n> +                       else\n> +                               read_mmfile(mmf, \"/dev/null\");\n\nDoes \"/dev/null\" have any portability considerations?  (I really don't\nknow; just curious.)\n"},{"id":"484043","messageId":"CABPp-BFJn5NsSaaxEMpicqND_-8CBw370kyBQuKHGOX-3PRyMw@mail.gmail.com","threadId":"60423","inReplyTo":"20231024195655.2413191-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 0/1] Object ID support for git merge-file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-10-29T06:24:06Z","receivedAt":"2023-10-29T06:24:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> This series introduces an --object-id option to git merge-file such\n> that, instead of reading and writing from files on the system, it reads\n> from and writes to the object store using blobs.\n\nThis seems like a reasonable capability to want from such a plumbing command.\n\n> This is in use at\n> GitHub to produce conflict diffs when a merge fails, and it seems\n> generally useful, so I'm sending it here.\n\nBut...wouldn't you already have the conflicts generated when doing the\nmerge and learning that it fails?  Why would you need to generate them\nagain?\n\n(Also, generating them again may risk getting names munged for\nconflict markers in edge cases involving renames.)\n\nThat said, even if I have questions about your particular usecase, I\nthink the feature you are submitting here makes sense independently.\n\n\nI left a few minor questions on the patch itself, but overall it looks\ngood to me.\n"},{"id":"484046","messageId":"fec21bbe-46da-4f1c-a9b8-6be44403d68f@gmail.com","threadId":"60423","inReplyTo":"CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-10-29T10:12:57Z","receivedAt":"2023-10-29T10:13:04Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi brian\n\nI agree with everything Elijah said, I just have one more comment\n\n>> @@ -80,12 +88,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n>>\n>>                  fname = prefix_filename(prefix, argv[i]);\n>>\n>> -               if (read_mmfile(mmf, fname))\n>> +               if (object_id) {\n>> +                       if (repo_get_oid(the_repository, argv[i], &oid))\n>> +                               ret = -1;\n\nIt would be nice to print an error message here\n\n\tret = error(\"object '%s' does not exist\", argv[i]);\n\nnone of the existing error messages are marked for translation so I've \nleft this untranslated as well.\n\nBest Wishes\n\nPhillip\n"},{"id":"484047","messageId":"80c6b0d9-7d6c-4a1c-ba2e-330786b13cf0@gmail.com","threadId":"60423","inReplyTo":"CABPp-BFJn5NsSaaxEMpicqND_-8CBw370kyBQuKHGOX-3PRyMw@mail.gmail.com","subject":"Re: [PATCH 0/1] Object ID support for git merge-file","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-10-29T10:15:02Z","receivedAt":"2023-10-29T10:15:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 29/10/2023 06:24, Elijah Newren wrote:\n> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n>> This is in use at\n>> GitHub to produce conflict diffs when a merge fails, and it seems\n>> generally useful, so I'm sending it here.\n> \n> But...wouldn't you already have the conflicts generated when doing the\n> merge and learning that it fails?  Why would you need to generate them\n> again?\n\nI was surprised by this as well, but as you say this seems like a \nuseful addition independent of any specific use at GitHub.\n\nBest Wishes\n\nPhillip\n"},{"id":"484049","messageId":"ZT5p0WUw7EeaY8vW@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-29T14:18:57Z","receivedAt":"2023-10-29T14:19:01Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-10-29 at 06:17:09, Elijah Newren wrote:\n> Hi,\n> \n> Overall, looks good.  Just a couple questions...\n> \n> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > From: \"brian m. carlson\" <bk2204@github.com>\n> >\n> [...]\n> > --- a/Documentation/git-merge-file.txt\n> > +++ b/Documentation/git-merge-file.txt\n> > @@ -12,6 +12,9 @@ SYNOPSIS\n> >  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> >         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n> >         [--[no-]diff3] <current-file> <base-file> <other-file>\n> > +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> > +       [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]\n> > +       [--[no-]diff3] <current-oid> <base-oid> <other-oid>\n> \n> Why was the `[-p|--stdout]` option removed in the second synopsis?\n> Elsewhere you explicitly call it out as a possibility to be used with\n> --object-id.\n\nOriginally because it implied `-p`, but I changed that to write into the\nobject store.  I'll restore it.\n\n> Also, why the extra synopsis instead of just adding a `[--object-id]`\n> option to the previous one?\n\nBecause there's a relevant difference: the former has <current-file>,\n<base-file>, and <other-file>, and the latter has the -oid versions.\n\n> Does \"/dev/null\" have any portability considerations?  (I really don't\n> know; just curious.)\n\nWe already use it elsewhere in the codebase, so I assume it works.  We\nalso have a test for that case and it worked in CI, so it's probably\nfine.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484119","messageId":"ZT/RpqvfQyx+uzxa@nand.local","threadId":"60423","inReplyTo":"CABPp-BFJn5NsSaaxEMpicqND_-8CBw370kyBQuKHGOX-3PRyMw@mail.gmail.com","subject":"Re: [PATCH 0/1] Object ID support for git merge-file","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-10-30T15:54:14Z","receivedAt":"2023-10-30T15:54:18Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:\n> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > This series introduces an --object-id option to git merge-file such\n> > that, instead of reading and writing from files on the system, it reads\n> > from and writes to the object store using blobs.\n>\n> This seems like a reasonable capability to want from such a plumbing command.\n\nAgreed.\n\n> > This is in use at\n> > GitHub to produce conflict diffs when a merge fails, and it seems\n> > generally useful, so I'm sending it here.\n>\n> But...wouldn't you already have the conflicts generated when doing the\n> merge and learning that it fails?  Why would you need to generate them\n> again?\n\nbrian would know better than I do, but I believe the reason is because\nthe \"attempt this merge\" RPC is handled separately from the \"show me the\nmerge conflict(s) at xyz path\". Those probably could be combined\n(obviating the need for this patch), but doing so is probably rather\ncomplicated.\n\nSince this feature is generally useful for callers that haven't already\ncompleted a tree-level merge and really just care about the result of\nmerging a single path, I don't have any objections here.\n\nThanks,\nTaylor\n"},{"id":"484123","messageId":"ZT_Wcm8kwT4uWzHc@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"fec21bbe-46da-4f1c-a9b8-6be44403d68f@gmail.com","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-30T16:14:42Z","receivedAt":"2023-10-30T16:14:46Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-10-29 at 10:12:57, Phillip Wood wrote:\n> It would be nice to print an error message here\n> \n> \tret = error(\"object '%s' does not exist\", argv[i]);\n\nGood point.  I'll include that in my reroll.\n\n> none of the existing error messages are marked for translation so I've left\n> this untranslated as well.\n\nI've opted to make them translatable here because I think that's for the\nbest.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484124","messageId":"ZT_YuF4g-8P9fc4t@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"ZT/RpqvfQyx+uzxa@nand.local","subject":"Re: [PATCH 0/1] Object ID support for git merge-file","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-30T16:24:24Z","receivedAt":"2023-10-30T16:24:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-10-30 at 15:54:14, Taylor Blau wrote:\n> On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:\n> > But...wouldn't you already have the conflicts generated when doing the\n> > merge and learning that it fails?  Why would you need to generate them\n> > again?\n> \n> brian would know better than I do, but I believe the reason is because\n> the \"attempt this merge\" RPC is handled separately from the \"show me the\n> merge conflict(s) at xyz path\". Those probably could be combined\n> (obviating the need for this patch), but doing so is probably rather\n> complicated.\n\nThat's correct.  They could in theory happen at different times, which\nis why they're not linked.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484125","messageId":"20231030162658.567523-1-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231024195655.2413191-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 0/1] Object ID support for git merge-file","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-30T16:26:57Z","receivedAt":"2023-10-30T16:27:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces an --object-id option to git merge-file such\nthat, instead of reading and writing from files on the system, it reads\nfrom and writes to the object store using blobs.\n\nChanges from v1:\n* Improve error handling\n* Re-add `-p` argument for documentation\n\nbrian m. carlson (1):\n  merge-file: add an option to process object IDs\n\n Documentation/git-merge-file.txt | 20 +++++++++++\n builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n 3 files changed, 124 insertions(+), 16 deletions(-)\n\n"},{"id":"484126","messageId":"20231030162658.567523-2-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231030162658.567523-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-30T16:26:58Z","receivedAt":"2023-10-30T16:27:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" <bk2204@github.com>\n\ngit merge-file knows how to merge files on the file system already.  It\nwould be helpful, however, to allow it to also merge single blobs.\nTeach it an `--object-id` option which means that its arguments are\nobject IDs and not files to allow it to do so.\n\nSince we obviously won't be writing the data to the first argument,\nimply the -p option so we write to standard output.\n\nWe handle the empty blob specially since read_mmblob doesn't read it\ndirectly and otherwise users cannot specify an empty ancestor.\n\nSigned-off-by: brian m. carlson <bk2204@github.com>\n---\n Documentation/git-merge-file.txt | 20 +++++++++++\n builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n 3 files changed, 124 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex 7e9093fab6..a4de2bd297 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -12,6 +12,9 @@ SYNOPSIS\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n \t[--[no-]diff3] <current-file> <base-file> <other-file>\n+'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n+\t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n+\t[--[no-]diff3] <current-oid> <base-oid> <other-oid>\n \n \n DESCRIPTION\n@@ -40,6 +43,10 @@ however, these conflicts are resolved favouring lines from `<current-file>`,\n lines from `<other-file>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n+If `--object-id` is specified, exactly the same behavior occurs, except that\n+instead of specifying what to merge as files, it is specified as a list of\n+object IDs referring to blobs.\n+\n The exit value of this program is negative on error, and the number of\n conflicts otherwise (truncated to 127 if there are more than that many\n conflicts). If the merge was clean, the exit value is 0.\n@@ -52,6 +59,14 @@ linkgit:git[1].\n OPTIONS\n -------\n \n+--object-id::\n+\tSpecify the contents to merge as blobs in the current repository instead of\n+\tfiles.  In this case, the operation must take place within a valid repository.\n++\n+If the `-p` option is specified, the merged file (including conflicts, if any)\n+goes to standard output as normal; otherwise, the merged file is written to the\n+object store and the object ID of its blob is written to standard output.\n+\n -L <label>::\n \tThis option may be given up to three times, and\n \tspecifies labels to be used in place of the\n@@ -93,6 +108,11 @@ EXAMPLES\n \tmerges tmp/a123 and tmp/c345 with the base tmp/b234, but uses labels\n \t`a` and `c` instead of `tmp/a123` and `tmp/c345`.\n \n+`git merge-file -p --object-id abc1234 def567 890abcd`::\n+\n+\tcombines the changes of the blob abc1234 and 890abcd since def567,\n+\ttries to merge them and writes the result to standard output\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex d7eb4c6540..832c93d8d5 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -1,5 +1,8 @@\n #include \"builtin.h\"\n #include \"abspath.h\"\n+#include \"hex.h\"\n+#include \"object-name.h\"\n+#include \"object-store.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n #include \"setup.h\"\n@@ -31,10 +34,11 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tmmfile_t mmfs[3] = { 0 };\n \tmmbuffer_t result = { 0 };\n \txmparam_t xmp = { 0 };\n-\tint ret = 0, i = 0, to_stdout = 0;\n+\tint ret = 0, i = 0, to_stdout = 0, object_id = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL('p', \"stdout\", &to_stdout, N_(\"send results to standard output\")),\n+\t\tOPT_BOOL(0,   \"object-id\", &object_id, N_(\"use object IDs instead of filenames\")),\n \t\tOPT_SET_INT(0, \"diff3\", &xmp.style, N_(\"use a diff3 based merge\"), XDL_MERGE_DIFF3),\n \t\tOPT_SET_INT(0, \"zdiff3\", &xmp.style, N_(\"use a zealous diff3 based merge\"),\n \t\t\t\tXDL_MERGE_ZEALOUS_DIFF3),\n@@ -71,8 +75,12 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\t\treturn error_errno(\"failed to redirect stderr to /dev/null\");\n \t}\n \n+\tif (object_id)\n+\t\tsetup_git_directory();\n+\n \tfor (i = 0; i < 3; i++) {\n \t\tchar *fname;\n+\t\tstruct object_id oid;\n \t\tmmfile_t *mmf = mmfs + i;\n \n \t\tif (!names[i])\n@@ -80,12 +88,22 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \t\tfname = prefix_filename(prefix, argv[i]);\n \n-\t\tif (read_mmfile(mmf, fname))\n+\t\tif (object_id) {\n+\t\t\tif (repo_get_oid(the_repository, argv[i], &oid))\n+\t\t\t\tret = error(_(\"object '%s' does not exist\"),\n+\t\t\t\t\t      argv[i]);\n+\t\t\telse if (!oideq(&oid, the_hash_algo->empty_blob))\n+\t\t\t\tread_mmblob(mmf, &oid);\n+\t\t\telse\n+\t\t\t\tread_mmfile(mmf, \"/dev/null\");\n+\t\t} else if (read_mmfile(mmf, fname)) {\n \t\t\tret = -1;\n-\t\telse if (mmf->size > MAX_XDIFF_SIZE ||\n-\t\t\t buffer_is_binary(mmf->ptr, mmf->size))\n+\t\t}\n+\t\tif (ret != -1 && (mmf->size > MAX_XDIFF_SIZE ||\n+\t\t    buffer_is_binary(mmf->ptr, mmf->size))) {\n \t\t\tret = error(\"Cannot merge binary files: %s\",\n \t\t\t\t    argv[i]);\n+\t\t}\n \n \t\tfree(fname);\n \t\tif (ret)\n@@ -99,20 +117,32 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);\n \n \tif (ret >= 0) {\n-\t\tconst char *filename = argv[0];\n-\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n-\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n+\t\tif (object_id && !to_stdout) {\n+\t\t\tstruct object_id oid;\n+\t\t\tif (result.size) {\n+\t\t\t\tif (write_object_file(result.ptr, result.size, OBJ_BLOB, &oid) < 0)\n+\t\t\t\t\tret = error(_(\"Could not write object file\"));\n+\t\t\t} else {\n+\t\t\t\toidcpy(&oid, the_hash_algo->empty_blob);\n+\t\t\t}\n+\t\t\tif (ret >= 0)\n+\t\t\t\tprintf(\"%s\\n\", oid_to_hex(&oid));\n+\t\t} else {\n+\t\t\tconst char *filename = argv[0];\n+\t\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n+\t\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n \n-\t\tif (!f)\n-\t\t\tret = error_errno(\"Could not open %s for writing\",\n-\t\t\t\t\t  filename);\n-\t\telse if (result.size &&\n-\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n-\t\t\tret = error_errno(\"Could not write to %s\", filename);\n-\t\telse if (fclose(f))\n-\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tif (!f)\n+\t\t\t\tret = error_errno(\"Could not open %s for writing\",\n+\t\t\t\t\t\t  filename);\n+\t\t\telse if (result.size &&\n+\t\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n+\t\t\t\tret = error_errno(\"Could not write to %s\", filename);\n+\t\t\telse if (fclose(f))\n+\t\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tfree(fpath);\n+\t\t}\n \t\tfree(result.ptr);\n-\t\tfree(fpath);\n \t}\n \n \tif (ret > 127)\ndiff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh\nindex 1a7082323d..2c92209eca 100755\n--- a/t/t6403-merge-file.sh\n+++ b/t/t6403-merge-file.sh\n@@ -65,11 +65,30 @@ test_expect_success 'merge with no changes' '\n \ttest_cmp test.txt orig.txt\n '\n \n+test_expect_success 'merge with no changes with --object-id' '\n+\tgit add orig.txt &&\n+\tgit merge-file -p --object-id :orig.txt :orig.txt :orig.txt >actual &&\n+\ttest_cmp actual orig.txt\n+'\n+\n test_expect_success \"merge without conflict\" '\n \tcp new1.txt test.txt &&\n \tgit merge-file test.txt orig.txt new2.txt\n '\n \n+test_expect_success 'merge without conflict with --object-id' '\n+\tgit add orig.txt new2.txt &&\n+\tgit merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n+test_expect_success 'can accept object ID with --object-id' '\n+\tgit merge-file --object-id $(test_oid empty_blob) $(test_oid empty_blob) :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n test_expect_success 'works in subdirectory' '\n \tmkdir dir &&\n \tcp new1.txt dir/a.txt &&\n@@ -138,6 +157,31 @@ test_expect_success \"expected conflict markers\" '\n \ttest_cmp expect.txt test.txt\n '\n \n+test_expect_success \"merge with conflicts with --object-id\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id :backup.txt :orig.txt :new3.txt >actual &&\n+\tsed -e \"s/<< test.txt/<< :backup.txt/\" \\\n+\t    -e \"s/>> new3.txt/>> :new3.txt/\" \\\n+\t    expect.txt >expect &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git merge-file --object-id :backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success \"merge with conflicts with --object-id with labels\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >actual &&\n+\ttest_cmp expect.txt actual &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect.txt actual\n+'\n+\n test_expect_success \"merge conflicting with --ours\" '\n \tcp backup.txt test.txt &&\n \n@@ -256,6 +300,14 @@ test_expect_success 'binary files cannot be merged' '\n \tgrep \"Cannot merge binary files\" merge.err\n '\n \n+test_expect_success 'binary files cannot be merged with --object-id' '\n+\tcp \"$TEST_DIRECTORY\"/test-binary-1.png . &&\n+\tgit add orig.txt new1.txt test-binary-1.png &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t:orig.txt :test-binary-1.png :new1.txt 2> merge.err &&\n+\tgrep \"Cannot merge binary files\" merge.err\n+'\n+\n test_expect_success 'MERGE_ZEALOUS simplifies non-conflicts' '\n \tsed -e \"s/deerit.\\$/deerit;/\" -e \"s/me;\\$/me./\" <new5.txt >new6.txt &&\n \tsed -e \"s/deerit.\\$/deerit,/\" -e \"s/me;\\$/me,/\" <new5.txt >new7.txt &&\n@@ -389,4 +441,10 @@ test_expect_success 'conflict sections match existing line endings' '\n \ttest $(tr \"\\015\" Q <nolf.txt | grep \"^[<=>].*Q$\" | wc -l) = 0\n '\n \n+test_expect_success '--object-id fails without repository' '\n+\tempty=\"$(test_oid empty_blob)\" &&\n+\tnongit test_must_fail git merge-file --object-id $empty $empty $empty 2>err &&\n+\tgrep \"not a git repository\" err\n+'\n+\n test_done\n"},{"id":"484127","messageId":"CABPp-BHWjyaj9Z0Oj5UQdeSSf1hX+4DGCtYgMF+3hpWH7j40AQ@mail.gmail.com","threadId":"60423","inReplyTo":"ZT5p0WUw7EeaY8vW@tapette.crustytoothpaste.net","subject":"Re: [PATCH 1/1] merge-file: add an option to process object IDs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-10-30T16:39:07Z","receivedAt":"2023-10-30T16:39:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Sun, Oct 29, 2023 at 7:18 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2023-10-29 at 06:17:09, Elijah Newren wrote:\n> > Hi,\n> >\n> > Overall, looks good.  Just a couple questions...\n> >\n> > On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> > >\n> > > From: \"brian m. carlson\" <bk2204@github.com>\n> > >\n> > [...]\n> > > --- a/Documentation/git-merge-file.txt\n> > > +++ b/Documentation/git-merge-file.txt\n> > > @@ -12,6 +12,9 @@ SYNOPSIS\n> > >  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> > >         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n> > >         [--[no-]diff3] <current-file> <base-file> <other-file>\n> > > +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> > > +       [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]\n> > > +       [--[no-]diff3] <current-oid> <base-oid> <other-oid>\n> >\n> > Why was the `[-p|--stdout]` option removed in the second synopsis?\n> > Elsewhere you explicitly call it out as a possibility to be used with\n> > --object-id.\n>\n> Originally because it implied `-p`, but I changed that to write into the\n> object store.  I'll restore it.\n>\n> > Also, why the extra synopsis instead of just adding a `[--object-id]`\n> > option to the previous one?\n>\n> Because there's a relevant difference: the former has <current-file>,\n> <base-file>, and <other-file>, and the latter has the -oid versions.\n\nAh, I looked over it a couple times and just kept missing that\ndifference.  Thanks for pointing it out.\n\n>\n> > Does \"/dev/null\" have any portability considerations?  (I really don't\n> > know; just curious.)\n>\n> We already use it elsewhere in the codebase, so I assume it works.  We\n> also have a test for that case and it worked in CI, so it's probably\n> fine.\n\nSeems reasonable to me; thanks.\n"},{"id":"484129","messageId":"CABPp-BG6HqX9CzjjJrPYyS=AvmKF0gMOQT7f+jXDHOGFTXFU0A@mail.gmail.com","threadId":"60423","inReplyTo":"ZT_YuF4g-8P9fc4t@tapette.crustytoothpaste.net","subject":"Re: [PATCH 0/1] Object ID support for git merge-file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-10-30T17:14:42Z","receivedAt":"2023-10-30T17:14:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Oct 30, 2023 at 9:24 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2023-10-30 at 15:54:14, Taylor Blau wrote:\n> > On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:\n> > > But...wouldn't you already have the conflicts generated when doing the\n> > > merge and learning that it fails?  Why would you need to generate them\n> > > again?\n> >\n> > brian would know better than I do, but I believe the reason is because\n> > the \"attempt this merge\" RPC is handled separately from the \"show me the\n> > merge conflict(s) at xyz path\". Those probably could be combined\n> > (obviating the need for this patch), but doing so is probably rather\n> > complicated.\n>\n> That's correct.  They could in theory happen at different times, which\n> is why they're not linked.\n\nMaybe this is digging a little into \"historical reasons\" too much, but\nthis still seems a little funny.  If they happen at different times,\nyou still need multiple pieces of information remembered from the\nmerge operation in order for git-merge-file to be able to regenerate\nthe conflict correctly in general.  In particular, you need the OIDs\nand the filenames.  Trying to regenerate a conflict without\nremembering those from the merge step would only work for common\ncases, but would be problematic in the face of either renames being\ninvolved or recursive merges or both.  And if you need to remember\ninformation from the merge step, then why not remember the actual\nconflicts (or at least the tree OID generated by the merge operation,\nwhich has the conflicts embedded within it)?\n\nI know, I know, there's probably just historical cruft that needs\ncleaning up, and I don't think any of this matters to the patch at\nhand since it's independently useful.  It just sounds like a system\nhas been set up that has some rough edge cases caused by a poor\nsplitting.\n\n\n> --\n> brian m. carlson (he/him or they/them)\n> Toronto, Ontario, CA\n"},{"id":"484130","messageId":"CABPp-BF6vxU5x5VLGbRhtcTBqDu3x31=vMOd2bimZNg2mkkvuA@mail.gmail.com","threadId":"60423","inReplyTo":"20231030162658.567523-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 0/1] Object ID support for git merge-file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-10-30T17:15:10Z","receivedAt":"2023-10-30T17:15:27Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Oct 30, 2023 at 9:27 AM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> This series introduces an --object-id option to git merge-file such\n> that, instead of reading and writing from files on the system, it reads\n> from and writes to the object store using blobs.\n>\n> Changes from v1:\n> * Improve error handling\n> * Re-add `-p` argument for documentation\n>\n> brian m. carlson (1):\n>   merge-file: add an option to process object IDs\n>\n>  Documentation/git-merge-file.txt | 20 +++++++++++\n>  builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n>  t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n>  3 files changed, 124 insertions(+), 16 deletions(-)\n\nThanks, this version looks good to me.\n"},{"id":"484167","messageId":"xmqqttq78xpr.fsf@gitster.g","threadId":"60423","inReplyTo":"CABPp-BF6vxU5x5VLGbRhtcTBqDu3x31=vMOd2bimZNg2mkkvuA@mail.gmail.com","subject":"Re: [PATCH v2 0/1] Object ID support for git merge-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-31T00:03:28Z","receivedAt":"2023-10-31T00:03:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> On Mon, Oct 30, 2023 at 9:27 AM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>>\n>> This series introduces an --object-id option to git merge-file such\n>> that, instead of reading and writing from files on the system, it reads\n>> from and writes to the object store using blobs.\n>>\n>> Changes from v1:\n>> * Improve error handling\n>> * Re-add `-p` argument for documentation\n>>\n>> brian m. carlson (1):\n>>   merge-file: add an option to process object IDs\n>>\n>>  Documentation/git-merge-file.txt | 20 +++++++++++\n>>  builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n>>  t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n>>  3 files changed, 124 insertions(+), 16 deletions(-)\n>\n> Thanks, this version looks good to me.\n\nThanks, both.  Will queue.\n"},{"id":"484233","messageId":"6a8f3a6c-c3b0-4aff-ab36-be980ec6bbe0@gmail.com","threadId":"60423","inReplyTo":"20231030162658.567523-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 0/1] Object ID support for git merge-file","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-10-31T11:05:12Z","receivedAt":"2023-10-31T11:05:16Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi brian\n\nThanks for the re-roll, this version looks good to me\n\nPhillip\n\nOn 30/10/2023 16:26, brian m. carlson wrote:\n> This series introduces an --object-id option to git merge-file such\n> that, instead of reading and writing from files on the system, it reads\n> from and writes to the object store using blobs.\n> \n> Changes from v1:\n> * Improve error handling\n> * Re-add `-p` argument for documentation\n> \n> brian m. carlson (1):\n>    merge-file: add an option to process object IDs\n> \n>   Documentation/git-merge-file.txt | 20 +++++++++++\n>   builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n>   t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n>   3 files changed, 124 insertions(+), 16 deletions(-)\n> \n> \n\n"},{"id":"484256","messageId":"20231031214859.25293-1-martin.agren@gmail.com","threadId":"60423","inReplyTo":"20231030162658.567523-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 1/1] merge-file: add an option to process object IDs","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-10-31T21:48:57Z","receivedAt":"2023-10-31T21:49:48Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"\nOn Mon, 30 Oct 2023 at 17:37, brian m. carlson <sandals@crustytoothpaste.net> wrote:\n>\n> Since we obviously won't be writing the data to the first argument,\n> imply the -p option so we write to standard output.\n\nThis paragraph changed from v1, but this doesn't match the actual\nbehavior, from what I can tell: `-p` is not implied.\n\n>  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n>         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n>         [--[no-]diff3] <current-file> <base-file> <other-file>\n> +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n> +       [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n> +       [--[no-]diff3] <current-oid> <base-oid> <other-oid>\n\nI see this duplicated synopsis was discussed on v1, and that the\ndifference here is \"file\" vs \"oid\". It seems we could avoid this\nredundancy and risk of going out of sync with no downside that I can see\nby instead dropping all these \"-file\". See below for a patch that could\ngo in as a preparatory step.\n\n> +If `--object-id` is specified, exactly the same behavior occurs, except that\n> +instead of specifying what to merge as files, it is specified as a list of\n> +object IDs referring to blobs.\n\nMakes sense.\n\n> +If the `-p` option is specified, the merged file (including conflicts, if any)\n> +goes to standard output as normal; otherwise, the merged file is written to the\n> +object store and the object ID of its blob is written to standard output.\n\n(Here, `-p` is not implied.)\n\n> +test_expect_success 'merge without conflict with --object-id' '\n> +       git add orig.txt new2.txt &&\n> +       git merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&\n> +       git rev-parse :new2.txt >expected &&\n> +       test_cmp actual expected\n> +'\n\n(Here, `-p` is not implied.)\n\nMartin\n\n-- >8 --\nSubject: [PATCH] git-merge-file doc: drop \"-file\" from argument placeholders\n\n`git merge-file` takes three positional arguments. Each of them is\ndocumented as `<foo-file>`. In preparation for teaching this command to\nalternatively take three object IDs, make these placeholders a bit more\ngeneric by dropping the \"-file\" parts. Instead, clarify early that the\nthree arguments are filenames. Even after the next commit, we can afford\nto present this file-centric view up front and in the general\ndiscussion, since it will remain the default one.\n\nSigned-off-by: Martin Ågren <martin.agren@gmail.com>\n---\n Documentation/git-merge-file.txt | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex 7e9093fab6..bf0a18cf02 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -11,19 +11,20 @@ SYNOPSIS\n [verse]\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n-\t[--[no-]diff3] <current-file> <base-file> <other-file>\n+\t[--[no-]diff3] <current> <base> <other>\n \n \n DESCRIPTION\n -----------\n-'git merge-file' incorporates all changes that lead from the `<base-file>`\n-to `<other-file>` into `<current-file>`. The result ordinarily goes into\n-`<current-file>`. 'git merge-file' is useful for combining separate changes\n-to an original. Suppose `<base-file>` is the original, and both\n-`<current-file>` and `<other-file>` are modifications of `<base-file>`,\n+Given three files `<current>`, `<base>` and `<other>`,\n+'git merge-file' incorporates all changes that lead from `<base>`\n+to `<other>` into `<current>`. The result ordinarily goes into\n+`<current>`. 'git merge-file' is useful for combining separate changes\n+to an original. Suppose `<base>` is the original, and both\n+`<current>` and `<other>` are modifications of `<base>`,\n then 'git merge-file' combines both changes.\n \n-A conflict occurs if both `<current-file>` and `<other-file>` have changes\n+A conflict occurs if both `<current>` and `<other>` have changes\n in a common segment of lines. If a conflict is found, 'git merge-file'\n normally outputs a warning and brackets the conflict with lines containing\n <<<<<<< and >>>>>>> markers. A typical conflict will look like this:\n@@ -36,8 +37,8 @@ normally outputs a warning and brackets the conflict with lines containing\n \n If there are conflicts, the user should edit the result and delete one of\n the alternatives.  When `--ours`, `--theirs`, or `--union` option is in effect,\n-however, these conflicts are resolved favouring lines from `<current-file>`,\n-lines from `<other-file>`, or lines from both respectively.  The length of the\n+however, these conflicts are resolved favouring lines from `<current>`,\n+lines from `<other>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n The exit value of this program is negative on error, and the number of\n@@ -62,7 +63,7 @@ OPTIONS\n \n -p::\n \tSend results to standard output instead of overwriting\n-\t`<current-file>`.\n+\t`<current>`.\n \n -q::\n \tQuiet; do not warn about conflicts.\n-- \n2.42.0.899.gfd14d11e2b\n\n"},{"id":"484263","messageId":"ZUGASkMgoAbe7RjR@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"20231031214859.25293-1-martin.agren@gmail.com","subject":"Re: [PATCH v2 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-10-31T22:31:38Z","receivedAt":"2023-10-31T22:31:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-10-31 at 21:48:57, Martin Ågren wrote:\n> \n> On Mon, 30 Oct 2023 at 17:37, brian m. carlson <sandals@crustytoothpaste.net> wrote:\n> >\n> > Since we obviously won't be writing the data to the first argument,\n> > imply the -p option so we write to standard output.\n> \n> This paragraph changed from v1, but this doesn't match the actual\n> behavior, from what I can tell: `-p` is not implied.\n\nYes, apparently that commit message snuck back in after it having been\nedited out.\n\n> -- >8 --\n> Subject: [PATCH] git-merge-file doc: drop \"-file\" from argument placeholders\n> \n> `git merge-file` takes three positional arguments. Each of them is\n> documented as `<foo-file>`. In preparation for teaching this command to\n> alternatively take three object IDs, make these placeholders a bit more\n> generic by dropping the \"-file\" parts. Instead, clarify early that the\n> three arguments are filenames. Even after the next commit, we can afford\n> to present this file-centric view up front and in the general\n> discussion, since it will remain the default one.\n\nThis seems reasonable.  Junio, do you want to sneak this in and fix the\ncommit message above, or do you want me to do a v3?\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484265","messageId":"xmqqo7ge2xzl.fsf@gitster.g","threadId":"60423","inReplyTo":"6a8f3a6c-c3b0-4aff-ab36-be980ec6bbe0@gmail.com","subject":"Re: [PATCH v2 0/1] Object ID support for git merge-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-10-31T23:06:22Z","receivedAt":"2023-10-31T23:06:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi brian\n>\n> Thanks for the re-roll, this version looks good to me\n>\n> Phillip\n\nYup, competently done.\n\nI briefly thought about suggesting --blob-id instead of --object-id\nsimply because you'd never want to feed it trees and commits, but\nthe error message from read_mmblob() the users would get mentions\n'blob' to signal that non-blob objects are unwelcome, so the name of\nthe optionwould be OK as-is.\n\nQueued.  Let's mark it for 'next'.  Thanks.\n"},{"id":"484280","messageId":"xmqqedhaw31d.fsf@gitster.g","threadId":"60423","inReplyTo":"ZUGASkMgoAbe7RjR@tapette.crustytoothpaste.net","subject":"Re: [PATCH v2 1/1] merge-file: add an option to process object IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-01T03:44:30Z","receivedAt":"2023-11-01T03:44:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> This seems reasonable.  Junio, do you want to sneak this in and fix the\n> commit message above, or do you want me to do a v3?\n\nAs it hasn't hit 'next' on my end, I'd prefer to see a version I can\nblindly apply without having to care all the details of what was\ndiscussed.  Thanks.\n\n"},{"id":"484310","messageId":"ZUKkAueca1jzrQ5A@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"xmqqedhaw31d.fsf@gitster.g","subject":"Re: [PATCH v2 1/1] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-11-01T19:16:18Z","receivedAt":"2023-11-01T19:16:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-11-01 at 03:44:30, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> \n> > This seems reasonable.  Junio, do you want to sneak this in and fix the\n> > commit message above, or do you want me to do a v3?\n> \n> As it hasn't hit 'next' on my end, I'd prefer to see a version I can\n> blindly apply without having to care all the details of what was\n> discussed.  Thanks.\n\nI'll send out a v3 shortly.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484311","messageId":"20231101192419.794162-2-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231101192419.794162-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-11-01T19:24:18Z","receivedAt":"2023-11-01T19:24:31Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: Martin Ågren <martin.agren@gmail.com>\n\n`git merge-file` takes three positional arguments. Each of them is\ndocumented as `<foo-file>`. In preparation for teaching this command to\nalternatively take three object IDs, make these placeholders a bit more\ngeneric by dropping the \"-file\" parts. Instead, clarify early that the\nthree arguments are filenames. Even after the next commit, we can afford\nto present this file-centric view up front and in the general\ndiscussion, since it will remain the default one.\n\nSigned-off-by: Martin Ågren <martin.agren@gmail.com>\nSigned-off-by: brian m. carlson <bk2204@github.com>\n---\n Documentation/git-merge-file.txt | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex 7e9093fab6..bf0a18cf02 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -11,19 +11,20 @@ SYNOPSIS\n [verse]\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n-\t[--[no-]diff3] <current-file> <base-file> <other-file>\n+\t[--[no-]diff3] <current> <base> <other>\n \n \n DESCRIPTION\n -----------\n-'git merge-file' incorporates all changes that lead from the `<base-file>`\n-to `<other-file>` into `<current-file>`. The result ordinarily goes into\n-`<current-file>`. 'git merge-file' is useful for combining separate changes\n-to an original. Suppose `<base-file>` is the original, and both\n-`<current-file>` and `<other-file>` are modifications of `<base-file>`,\n+Given three files `<current>`, `<base>` and `<other>`,\n+'git merge-file' incorporates all changes that lead from `<base>`\n+to `<other>` into `<current>`. The result ordinarily goes into\n+`<current>`. 'git merge-file' is useful for combining separate changes\n+to an original. Suppose `<base>` is the original, and both\n+`<current>` and `<other>` are modifications of `<base>`,\n then 'git merge-file' combines both changes.\n \n-A conflict occurs if both `<current-file>` and `<other-file>` have changes\n+A conflict occurs if both `<current>` and `<other>` have changes\n in a common segment of lines. If a conflict is found, 'git merge-file'\n normally outputs a warning and brackets the conflict with lines containing\n <<<<<<< and >>>>>>> markers. A typical conflict will look like this:\n@@ -36,8 +37,8 @@ normally outputs a warning and brackets the conflict with lines containing\n \n If there are conflicts, the user should edit the result and delete one of\n the alternatives.  When `--ours`, `--theirs`, or `--union` option is in effect,\n-however, these conflicts are resolved favouring lines from `<current-file>`,\n-lines from `<other-file>`, or lines from both respectively.  The length of the\n+however, these conflicts are resolved favouring lines from `<current>`,\n+lines from `<other>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n The exit value of this program is negative on error, and the number of\n@@ -62,7 +63,7 @@ OPTIONS\n \n -p::\n \tSend results to standard output instead of overwriting\n-\t`<current-file>`.\n+\t`<current>`.\n \n -q::\n \tQuiet; do not warn about conflicts.\n"},{"id":"484312","messageId":"20231101192419.794162-3-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231101192419.794162-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 2/2] merge-file: add an option to process object IDs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-11-01T19:24:19Z","receivedAt":"2023-11-01T19:24:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" <bk2204@github.com>\n\ngit merge-file knows how to merge files on the file system already.  It\nwould be helpful, however, to allow it to also merge single blobs.\nTeach it an `--object-id` option which means that its arguments are\nobject IDs and not files to allow it to do so.\n\nWe handle the empty blob specially since read_mmblob doesn't read it\ndirectly and otherwise users cannot specify an empty ancestor.\n\nSigned-off-by: brian m. carlson <bk2204@github.com>\n---\n Documentation/git-merge-file.txt | 19 +++++++++-\n builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n 3 files changed, 122 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex bf0a18cf02..6a081eacb7 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n-\t[--[no-]diff3] <current> <base> <other>\n+\t[--[no-]diff3] [--object-id] <current> <base> <other>\n \n \n DESCRIPTION\n@@ -41,6 +41,10 @@ however, these conflicts are resolved favouring lines from `<current>`,\n lines from `<other>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n+If `--object-id` is specified, exactly the same behavior occurs, except that\n+instead of specifying what to merge as files, it is specified as a list of\n+object IDs referring to blobs.\n+\n The exit value of this program is negative on error, and the number of\n conflicts otherwise (truncated to 127 if there are more than that many\n conflicts). If the merge was clean, the exit value is 0.\n@@ -53,6 +57,14 @@ linkgit:git[1].\n OPTIONS\n -------\n \n+--object-id::\n+\tSpecify the contents to merge as blobs in the current repository instead of\n+\tfiles.  In this case, the operation must take place within a valid repository.\n++\n+If the `-p` option is specified, the merged file (including conflicts, if any)\n+goes to standard output as normal; otherwise, the merged file is written to the\n+object store and the object ID of its blob is written to standard output.\n+\n -L <label>::\n \tThis option may be given up to three times, and\n \tspecifies labels to be used in place of the\n@@ -94,6 +106,11 @@ EXAMPLES\n \tmerges tmp/a123 and tmp/c345 with the base tmp/b234, but uses labels\n \t`a` and `c` instead of `tmp/a123` and `tmp/c345`.\n \n+`git merge-file -p --object-id abc1234 def567 890abcd`::\n+\n+\tcombines the changes of the blob abc1234 and 890abcd since def567,\n+\ttries to merge them and writes the result to standard output\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex d7eb4c6540..832c93d8d5 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -1,5 +1,8 @@\n #include \"builtin.h\"\n #include \"abspath.h\"\n+#include \"hex.h\"\n+#include \"object-name.h\"\n+#include \"object-store.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n #include \"setup.h\"\n@@ -31,10 +34,11 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tmmfile_t mmfs[3] = { 0 };\n \tmmbuffer_t result = { 0 };\n \txmparam_t xmp = { 0 };\n-\tint ret = 0, i = 0, to_stdout = 0;\n+\tint ret = 0, i = 0, to_stdout = 0, object_id = 0;\n \tint quiet = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL('p', \"stdout\", &to_stdout, N_(\"send results to standard output\")),\n+\t\tOPT_BOOL(0,   \"object-id\", &object_id, N_(\"use object IDs instead of filenames\")),\n \t\tOPT_SET_INT(0, \"diff3\", &xmp.style, N_(\"use a diff3 based merge\"), XDL_MERGE_DIFF3),\n \t\tOPT_SET_INT(0, \"zdiff3\", &xmp.style, N_(\"use a zealous diff3 based merge\"),\n \t\t\t\tXDL_MERGE_ZEALOUS_DIFF3),\n@@ -71,8 +75,12 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\t\treturn error_errno(\"failed to redirect stderr to /dev/null\");\n \t}\n \n+\tif (object_id)\n+\t\tsetup_git_directory();\n+\n \tfor (i = 0; i < 3; i++) {\n \t\tchar *fname;\n+\t\tstruct object_id oid;\n \t\tmmfile_t *mmf = mmfs + i;\n \n \t\tif (!names[i])\n@@ -80,12 +88,22 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \n \t\tfname = prefix_filename(prefix, argv[i]);\n \n-\t\tif (read_mmfile(mmf, fname))\n+\t\tif (object_id) {\n+\t\t\tif (repo_get_oid(the_repository, argv[i], &oid))\n+\t\t\t\tret = error(_(\"object '%s' does not exist\"),\n+\t\t\t\t\t      argv[i]);\n+\t\t\telse if (!oideq(&oid, the_hash_algo->empty_blob))\n+\t\t\t\tread_mmblob(mmf, &oid);\n+\t\t\telse\n+\t\t\t\tread_mmfile(mmf, \"/dev/null\");\n+\t\t} else if (read_mmfile(mmf, fname)) {\n \t\t\tret = -1;\n-\t\telse if (mmf->size > MAX_XDIFF_SIZE ||\n-\t\t\t buffer_is_binary(mmf->ptr, mmf->size))\n+\t\t}\n+\t\tif (ret != -1 && (mmf->size > MAX_XDIFF_SIZE ||\n+\t\t    buffer_is_binary(mmf->ptr, mmf->size))) {\n \t\t\tret = error(\"Cannot merge binary files: %s\",\n \t\t\t\t    argv[i]);\n+\t\t}\n \n \t\tfree(fname);\n \t\tif (ret)\n@@ -99,20 +117,32 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);\n \n \tif (ret >= 0) {\n-\t\tconst char *filename = argv[0];\n-\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n-\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n+\t\tif (object_id && !to_stdout) {\n+\t\t\tstruct object_id oid;\n+\t\t\tif (result.size) {\n+\t\t\t\tif (write_object_file(result.ptr, result.size, OBJ_BLOB, &oid) < 0)\n+\t\t\t\t\tret = error(_(\"Could not write object file\"));\n+\t\t\t} else {\n+\t\t\t\toidcpy(&oid, the_hash_algo->empty_blob);\n+\t\t\t}\n+\t\t\tif (ret >= 0)\n+\t\t\t\tprintf(\"%s\\n\", oid_to_hex(&oid));\n+\t\t} else {\n+\t\t\tconst char *filename = argv[0];\n+\t\t\tchar *fpath = prefix_filename(prefix, argv[0]);\n+\t\t\tFILE *f = to_stdout ? stdout : fopen(fpath, \"wb\");\n \n-\t\tif (!f)\n-\t\t\tret = error_errno(\"Could not open %s for writing\",\n-\t\t\t\t\t  filename);\n-\t\telse if (result.size &&\n-\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n-\t\t\tret = error_errno(\"Could not write to %s\", filename);\n-\t\telse if (fclose(f))\n-\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tif (!f)\n+\t\t\t\tret = error_errno(\"Could not open %s for writing\",\n+\t\t\t\t\t\t  filename);\n+\t\t\telse if (result.size &&\n+\t\t\t\t fwrite(result.ptr, result.size, 1, f) != 1)\n+\t\t\t\tret = error_errno(\"Could not write to %s\", filename);\n+\t\t\telse if (fclose(f))\n+\t\t\t\tret = error_errno(\"Could not close %s\", filename);\n+\t\t\tfree(fpath);\n+\t\t}\n \t\tfree(result.ptr);\n-\t\tfree(fpath);\n \t}\n \n \tif (ret > 127)\ndiff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh\nindex 1a7082323d..2c92209eca 100755\n--- a/t/t6403-merge-file.sh\n+++ b/t/t6403-merge-file.sh\n@@ -65,11 +65,30 @@ test_expect_success 'merge with no changes' '\n \ttest_cmp test.txt orig.txt\n '\n \n+test_expect_success 'merge with no changes with --object-id' '\n+\tgit add orig.txt &&\n+\tgit merge-file -p --object-id :orig.txt :orig.txt :orig.txt >actual &&\n+\ttest_cmp actual orig.txt\n+'\n+\n test_expect_success \"merge without conflict\" '\n \tcp new1.txt test.txt &&\n \tgit merge-file test.txt orig.txt new2.txt\n '\n \n+test_expect_success 'merge without conflict with --object-id' '\n+\tgit add orig.txt new2.txt &&\n+\tgit merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n+test_expect_success 'can accept object ID with --object-id' '\n+\tgit merge-file --object-id $(test_oid empty_blob) $(test_oid empty_blob) :new2.txt >actual &&\n+\tgit rev-parse :new2.txt >expected &&\n+\ttest_cmp actual expected\n+'\n+\n test_expect_success 'works in subdirectory' '\n \tmkdir dir &&\n \tcp new1.txt dir/a.txt &&\n@@ -138,6 +157,31 @@ test_expect_success \"expected conflict markers\" '\n \ttest_cmp expect.txt test.txt\n '\n \n+test_expect_success \"merge with conflicts with --object-id\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id :backup.txt :orig.txt :new3.txt >actual &&\n+\tsed -e \"s/<< test.txt/<< :backup.txt/\" \\\n+\t    -e \"s/>> new3.txt/>> :new3.txt/\" \\\n+\t    expect.txt >expect &&\n+\ttest_cmp expect actual &&\n+\ttest_must_fail git merge-file --object-id :backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success \"merge with conflicts with --object-id with labels\" '\n+\tgit add backup.txt orig.txt new3.txt &&\n+\ttest_must_fail git merge-file -p --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >actual &&\n+\ttest_cmp expect.txt actual &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t-L test.txt -L orig.txt -L new3.txt \\\n+\t\t:backup.txt :orig.txt :new3.txt >oid &&\n+\tgit cat-file blob \"$(cat oid)\" >actual &&\n+\ttest_cmp expect.txt actual\n+'\n+\n test_expect_success \"merge conflicting with --ours\" '\n \tcp backup.txt test.txt &&\n \n@@ -256,6 +300,14 @@ test_expect_success 'binary files cannot be merged' '\n \tgrep \"Cannot merge binary files\" merge.err\n '\n \n+test_expect_success 'binary files cannot be merged with --object-id' '\n+\tcp \"$TEST_DIRECTORY\"/test-binary-1.png . &&\n+\tgit add orig.txt new1.txt test-binary-1.png &&\n+\ttest_must_fail git merge-file --object-id \\\n+\t\t:orig.txt :test-binary-1.png :new1.txt 2> merge.err &&\n+\tgrep \"Cannot merge binary files\" merge.err\n+'\n+\n test_expect_success 'MERGE_ZEALOUS simplifies non-conflicts' '\n \tsed -e \"s/deerit.\\$/deerit;/\" -e \"s/me;\\$/me./\" <new5.txt >new6.txt &&\n \tsed -e \"s/deerit.\\$/deerit,/\" -e \"s/me;\\$/me,/\" <new5.txt >new7.txt &&\n@@ -389,4 +441,10 @@ test_expect_success 'conflict sections match existing line endings' '\n \ttest $(tr \"\\015\" Q <nolf.txt | grep \"^[<=>].*Q$\" | wc -l) = 0\n '\n \n+test_expect_success '--object-id fails without repository' '\n+\tempty=\"$(test_oid empty_blob)\" &&\n+\tnongit test_must_fail git merge-file --object-id $empty $empty $empty 2>err &&\n+\tgrep \"not a git repository\" err\n+'\n+\n test_done\n"},{"id":"484313","messageId":"20231101192419.794162-1-sandals@crustytoothpaste.net","threadId":"60423","inReplyTo":"20231024195655.2413191-1-sandals@crustytoothpaste.net","subject":"[PATCH v3 0/2] Object ID support for git merge-file","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-11-01T19:24:17Z","receivedAt":"2023-11-01T19:24:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces an --object-id option to git merge-file such\nthat, instead of reading and writing from files on the system, it reads\nfrom and writes to the object store using blobs.\n\nChanges from v2:\n* Include a patch from Martin to pre-improve the documentation\n* Remove incorrect portion of commit message\n\nChanges from v1:\n* Improve error handling\n* Re-add `-p` argument for documentation\n\nMartin Ågren (1):\n  git-merge-file doc: drop \"-file\" from argument placeholders\n\nbrian m. carlson (1):\n  merge-file: add an option to process object IDs\n\n Documentation/git-merge-file.txt | 38 ++++++++++++++------\n builtin/merge-file.c             | 62 +++++++++++++++++++++++---------\n t/t6403-merge-file.sh            | 58 ++++++++++++++++++++++++++++++\n 3 files changed, 132 insertions(+), 26 deletions(-)\n\nRange-diff against v2:\n-:  ---------- > 1:  21a96acf42 git-merge-file doc: drop \"-file\" from argument placeholders\n1:  9cd4220a3b ! 2:  b1978a7b5c merge-file: add an option to process object IDs\n    @@ Commit message\n         Teach it an `--object-id` option which means that its arguments are\n         object IDs and not files to allow it to do so.\n     \n    -    Since we obviously won't be writing the data to the first argument,\n    -    imply the -p option so we write to standard output.\n    -\n         We handle the empty blob specially since read_mmblob doesn't read it\n         directly and otherwise users cannot specify an empty ancestor.\n     \n    @@ Commit message\n     \n      ## Documentation/git-merge-file.txt ##\n     @@ Documentation/git-merge-file.txt: SYNOPSIS\n    + [verse]\n      'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n      \t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n    - \t[--[no-]diff3] <current-file> <base-file> <other-file>\n    -+'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]\n    -+\t[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n    -+\t[--[no-]diff3] <current-oid> <base-oid> <other-oid>\n    +-\t[--[no-]diff3] <current> <base> <other>\n    ++\t[--[no-]diff3] [--object-id] <current> <base> <other>\n      \n      \n      DESCRIPTION\n    -@@ Documentation/git-merge-file.txt: however, these conflicts are resolved favouring lines from `<current-file>`,\n    - lines from `<other-file>`, or lines from both respectively.  The length of the\n    +@@ Documentation/git-merge-file.txt: however, these conflicts are resolved favouring lines from `<current>`,\n    + lines from `<other>`, or lines from both respectively.  The length of the\n      conflict markers can be given with the `--marker-size` option.\n      \n     +If `--object-id` is specified, exactly the same behavior occurs, except that\n"},{"id":"484327","messageId":"xmqq8r7howsq.fsf@gitster.g","threadId":"60423","inReplyTo":"20231101192419.794162-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-01T23:53:25Z","receivedAt":"2023-11-01T23:53:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> From: Martin Ågren <martin.agren@gmail.com>\n>\n> `git merge-file` takes three positional arguments. Each of them is\n> documented as `<foo-file>`. In preparation for teaching this command to\n> alternatively take three object IDs, make these placeholders a bit more\n\nMinor nit.  Don't we want to say \"three blob object names\"?  Unless\nwe plan to grow this feature into accepting three tree object names,\nthat is.\n\n"},{"id":"484328","messageId":"xmqq4ji5owpx.fsf@gitster.g","threadId":"60423","inReplyTo":"20231101192419.794162-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v3 0/2] Object ID support for git merge-file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-01T23:55:06Z","receivedAt":"2023-11-01T23:55:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> This series introduces an --object-id option to git merge-file such\n> that, instead of reading and writing from files on the system, it reads\n> from and writes to the object store using blobs.\n>\n> Changes from v2:\n> * Include a patch from Martin to pre-improve the documentation\n> * Remove incorrect portion of commit message\n\nLooking good.  Will replace.  Shall we mark the topic ready for\n'next'?\n\nThanks.\n\n"},{"id":"484344","messageId":"CAN0heSovsnNpmVWMQJeNP2rO9zB8gFCeWuDp-f2FP0dGBCw_Qw@mail.gmail.com","threadId":"60423","inReplyTo":"20231101192419.794162-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v3 2/2] merge-file: add an option to process object IDs","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-11-02T08:51:05Z","receivedAt":"2023-11-02T08:51:18Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 1 Nov 2023 at 20:24, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> From: \"brian m. carlson\" <bk2204@github.com>\n>\n> git merge-file knows how to merge files on the file system already.  It\n> would be helpful, however, to allow it to also merge single blobs.\n> Teach it an `--object-id` option which means that its arguments are\n> object IDs and not files to allow it to do so.\n>\n> We handle the empty blob specially since read_mmblob doesn't read it\n> directly and otherwise users cannot specify an empty ancestor.\n>\n> Signed-off-by: brian m. carlson <bk2204@github.com>\n\nOk, good solution to the misleading paragraph I noticed: dropping it.\n\n>  'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n>         [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]\n> -       [--[no-]diff3] <current> <base> <other>\n> +       [--[no-]diff3] [--object-id] <current> <base> <other>\n\nOk. This is where rebasing onto the new, initial patch could possibly\nhave tripped up. Of course you didn't. Looks good.\n\n\nMartin\n"},{"id":"484346","messageId":"CAN0heSrv7MPcEwkq4uEtv9uBbqm4FLKQLE3gdsEbqKkxPXOj5A@mail.gmail.com","threadId":"60423","inReplyTo":"xmqq8r7howsq.fsf@gitster.g","subject":"Re: [PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-11-02T08:53:36Z","receivedAt":"2023-11-02T08:53:53Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Thu, 2 Nov 2023 at 00:53, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>\n> > From: Martin Ågren <martin.agren@gmail.com>\n> >\n> > `git merge-file` takes three positional arguments. Each of them is\n> > documented as `<foo-file>`. In preparation for teaching this command to\n> > alternatively take three object IDs, make these placeholders a bit more\n>\n> Minor nit.  Don't we want to say \"three blob object names\"?  Unless\n> we plan to grow this feature into accepting three tree object names,\n> that is.\n\nHmm, yeah. Or just \"three non-filename arguments\". I do wonder: doesn't\nthis mean that the second patch could/should possibly move away from the\nnotion of \"object ID\"/`--object-id`? (That's not trying to shift any\nblame from one patch to the other, that's my honest reaction.)\n\nAh, yes, I thought I recognized this. Quoting your response [1] to v2:\n\n> I briefly thought about suggesting --blob-id instead of --object-id\n> simply because you'd never want to feed it trees and commits, but\n> the error message from read_mmblob() the users would get mentions\n> 'blob' to signal that non-blob objects are unwelcome, so the name of\n> the optionwould be OK as-is.\n\nMaybe you having a similar reaction a second time makes this smell a bit\nmore?\n\n[1] https://lore.kernel.org/git/xmqqo7ge2xzl.fsf@gitster.g/\n\n\nMartin\n"},{"id":"484347","messageId":"ZUNpVyQYUT8TA6An@tapette.crustytoothpaste.net","threadId":"60423","inReplyTo":"CAN0heSrv7MPcEwkq4uEtv9uBbqm4FLKQLE3gdsEbqKkxPXOj5A@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2023-11-02T09:18:15Z","receivedAt":"2023-11-02T09:18:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2023-11-02 at 08:53:36, Martin Ågren wrote:\n> On Thu, 2 Nov 2023 at 00:53, Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n> >\n> > > From: Martin Ågren <martin.agren@gmail.com>\n> > >\n> > > `git merge-file` takes three positional arguments. Each of them is\n> > > documented as `<foo-file>`. In preparation for teaching this command to\n> > > alternatively take three object IDs, make these placeholders a bit more\n> >\n> > Minor nit.  Don't we want to say \"three blob object names\"?  Unless\n> > we plan to grow this feature into accepting three tree object names,\n> > that is.\n> \n> Hmm, yeah. Or just \"three non-filename arguments\". I do wonder: doesn't\n> this mean that the second patch could/should possibly move away from the\n> notion of \"object ID\"/`--object-id`? (That's not trying to shift any\n> blame from one patch to the other, that's my honest reaction.)\n\nNot specifying an option would make this ambiguous.  What if I have a\nfile named \"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\"?  Is that the\nempty blob, or is it that file?  Normally we have ways to disambiguate\nthis, but those don't work here because of the positional arguments.\n\n> Ah, yes, I thought I recognized this. Quoting your response [1] to v2:\n> \n> > I briefly thought about suggesting --blob-id instead of --object-id\n> > simply because you'd never want to feed it trees and commits, but\n> > the error message from read_mmblob() the users would get mentions\n> > 'blob' to signal that non-blob objects are unwelcome, so the name of\n> > the optionwould be OK as-is.\n> \n> Maybe you having a similar reaction a second time makes this smell a bit\n> more?\n\nI think the name is fine.  We don't typically use the phrase \"blob ID\"\nanywhere, but we do say \"object ID\".  We'd need to say \"--blob\", but\nI'm not sure that's an improvement, and I fear it may be less\nunderstandable.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"484348","messageId":"CAN0heSot6a5fabjh7NPdWhCBoUcDDpNmV0ovJ0bby1mpw8M7YA@mail.gmail.com","threadId":"60423","inReplyTo":"ZUNpVyQYUT8TA6An@tapette.crustytoothpaste.net","subject":"Re: [PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2023-11-02T09:29:22Z","receivedAt":"2023-11-02T09:29:35Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Thu, 2 Nov 2023 at 10:18, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2023-11-02 at 08:53:36, Martin Ågren wrote:\n\n> > Hmm, yeah. Or just \"three non-filename arguments\". I do wonder: doesn't\n> > this mean that the second patch could/should possibly move away from the\n> > notion of \"object ID\"/`--object-id`? (That's not trying to shift any\n> > blame from one patch to the other, that's my honest reaction.)\n>\n> Not specifying an option would make this ambiguous.  What if I have a\n> file named \"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\"?  Is that the\n> empty blob, or is it that file?  Normally we have ways to disambiguate\n> this, but those don't work here because of the positional arguments.\n\nI didn't intend to suggest dropping the new option altogether, sorry for\nbeing unclear. I meant moving from `--object-id` to something else, such\nas `--blob` that you mention. Completely agreed that something explicit\nis needed here and that heuristics aren't possible.\n\n> I think the name is fine.  We don't typically use the phrase \"blob ID\"\n> anywhere, but we do say \"object ID\".  We'd need to say \"--blob\", but\n> I'm not sure that's an improvement, and I fear it may be less\n> understandable.\n\nAgreed. Thanks.\n\nMartin\n"},{"id":"484371","messageId":"xmqqpm0sm85o.fsf@gitster.g","threadId":"60423","inReplyTo":"CAN0heSrv7MPcEwkq4uEtv9uBbqm4FLKQLE3gdsEbqKkxPXOj5A@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-merge-file doc: drop \"-file\" from argument placeholders","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-02T16:28:35Z","receivedAt":"2023-11-02T16:28:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Ågren <martin.agren@gmail.com> writes:\n\n> Maybe you having a similar reaction a second time makes this smell a bit\n> more?\n\nNot at all.  I am perfectly OK with --object-*, not --blob-*, as the\nend-user facing option name.  I however strongly prefer to see our\nlog messages record the thought behind the design accurately in\norder to help future developers when they wonder what our intention\nwas back when the commit was created.\n\nIn this case, I want to see that we tell our future selves \"even\nthough we named the option 'object', we plan to support blobs and\nnothing else, at least for now\".\n\nThanks.\n"}]}