git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 1/9] show, log: provide a --remerge-diff capability

From
Elijah Newren <newren@gmail.com>
Date
Jan 20, 2022, 02:33 UTC
Message-ID
<CABPp-BH9+RWE1SQfQiAYFtGZze3eQpeNDYNr2v9dmT77Pns0nA@mail.gmail.com>
In-Reply-To
<220119.86lezbn21b.gmgdl@evledraar.gmail.com>

On Wed, Jan 19, 2022 at 8:06 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 101 quoted lines
>
> On Thu, Dec 30 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> > +     struct tmp_objdir *remerge_objdir = NULL;
> > +
> > +     if (rev->remerge_diff) {
> > +             remerge_objdir = tmp_objdir_create("remerge-diff");
> > +             if (!remerge_objdir)
> > +                     die_errno(_("unable to create temporary object directory"));
> > +             tmp_objdir_replace_primary_odb(remerge_objdir, 1);
> > +     }
>
> Re the errno feedback on v1
> https://lore.kernel.org/git/211221.864k71r6kz.gmgdl@evledraar.gmail.com/
> the API might lose the "errno" due to e.g. the remove_dir_recurse()
> codepath. This seems like it would take care of that:
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 944d9c0d9b5..d4b8b1aa4b6 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -424,9 +424,9 @@ static int cmd_log_walk(struct rev_info *rev)
>         int saved_dcctc = 0;
>
>         if (rev->remerge_diff) {
> -               rev->remerge_objdir = tmp_objdir_create("remerge-diff");
> +               rev->remerge_objdir = tmp_objdir_create_gently("remerge-diff", 0);
>                 if (!rev->remerge_objdir)
> -                       die_errno(_("unable to create temporary object directory"));
> +                       exit(128);
>                 tmp_objdir_replace_primary_odb(rev->remerge_objdir, 1);
>         }
>
> diff --git a/tmp-objdir.c b/tmp-objdir.c
> index adf6033549e..3c656120003 100644
> --- a/tmp-objdir.c
> +++ b/tmp-objdir.c
> @@ -121,19 +121,21 @@ static void env_replace(struct strvec *env, const char *key, const char *val)
>         strvec_pushf(env, "%s=%s", key, val);
>  }
>
> -static int setup_tmp_objdir(const char *root)
> +static int setup_tmp_objdir(const char *root, int quiet)
>  {
>         char *path;
>         int ret = 0;
>
>         path = xstrfmt("%s/pack", root);
>         ret = mkdir(path, 0777);
> +       if (!quiet && ret < 0)
> +               die_errno(_("unable to create temporary object directory '%s'"), path);
>         free(path);
>
>         return ret;
>  }
>
> -struct tmp_objdir *tmp_objdir_create(const char *prefix)
> +struct tmp_objdir *tmp_objdir_create_gently(const char *prefix, int quiet)
>  {
>         static int installed_handlers;
>         struct tmp_objdir *t;
> @@ -161,6 +163,8 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
>         strbuf_grow(&t->path, 1024);
>
>         if (!mkdtemp(t->path.buf)) {
> +               if (!quiet)
> +                       error_errno(_("unable to create temporary directory '%s'"), t->path.buf);
>                 /* free, not destroy, as we never touched the filesystem */
>                 tmp_objdir_free(t);
>                 return NULL;
> @@ -173,7 +177,7 @@ struct tmp_objdir *tmp_objdir_create(const char *prefix)
>                 installed_handlers++;
>         }
>
> -       if (setup_tmp_objdir(t->path.buf)) {
> +       if (setup_tmp_objdir(t->path.buf, quiet)) {
>                 tmp_objdir_destroy(t);
>                 return NULL;
>         }
> diff --git a/tmp-objdir.h b/tmp-objdir.h
> index 76efc7edee5..5072fb860d9 100644
> --- a/tmp-objdir.h
> +++ b/tmp-objdir.h
> @@ -24,8 +24,15 @@ struct tmp_objdir;
>  /*
>   * Create a new temporary object directory with the specified prefix;
>   * returns NULL on failure.
> + *
> + * The tmp_objdir_create() is an a wrapper for
> + * tmp_objdir_create_gently(..., 1).
>   */
> -struct tmp_objdir *tmp_objdir_create(const char *prefix);
> +struct tmp_objdir *tmp_objdir_create_gently(const char *prefix, int quiet);
> +static inline struct tmp_objdir *tmp_objdir_create(const char *prefix)
> +{
> +       return tmp_objdir_create_gently(prefix, 1);
> +}
>
>  /*
>   * Return a list of environment strings, suitable for use with

Yeah, I think this suggests that switching from die() to die_errno() was a mistake. Your patch looks right (though most of it belongs as part of ns/tmp-objdir rather than this series), but I think it makes the code uglier and I don't see why this theoretical error path is worth all this trouble. A die() is totally sufficient here.

Previous: Ævar Arnfjörð BjarmasonNext: Elijah Newren via GitGitGadget
Message 72 of 113 in “Add a new --remerge-diff capability to show & log”
  1. 0/9 Add a new --remerge-diff capability to show & logElijah Newren via GitGitGadget, Dec 21, 2021
  2. 1/9 tmp_objdir: add a helper function for discarding all contained objectsElijah Newren via GitGitGadget, Dec 21, 2021
  3. Junio C HamanoDec 21, 2021
  4. Elijah NewrenDec 21, 2021
  5. Junio C HamanoDec 22, 2021
  6. Elijah NewrenDec 25, 2021
  7. 2/9 ll-merge: make callers responsible for showing warningsElijah Newren via GitGitGadget, Dec 21, 2021
  8. Ævar Arnfjörð BjarmasonDec 21, 2021
  9. Elijah NewrenDec 21, 2021
  10. Ævar Arnfjörð BjarmasonDec 21, 2021
  11. Elijah NewrenDec 21, 2021
  12. Junio C HamanoDec 21, 2021
  13. Elijah NewrenDec 23, 2021
  14. 3/9 merge-ort: capture and print ll-merge warnings in our preferred fashionElijah Newren via GitGitGadget, Dec 21, 2021
  15. Junio C HamanoDec 22, 2021
  16. Elijah NewrenDec 23, 2021
  17. 4/9 merge-ort: mark a few more conflict messages as omittableElijah Newren via GitGitGadget, Dec 21, 2021
  18. Junio C HamanoDec 22, 2021
  19. Elijah NewrenDec 23, 2021
  20. 5/9 merge-ort: make path_messages available to external callersElijah Newren via GitGitGadget, Dec 21, 2021
  21. 6/9 diff: add ability to insert additional headers for pathsElijah Newren via GitGitGadget, Dec 21, 2021
  22. Junio C HamanoDec 22, 2021
  23. Elijah NewrenDec 25, 2021
  24. 7/9 merge-ort: format messages slightly different for use in headersElijah Newren via GitGitGadget, Dec 21, 2021
  25. 8/9 show, log: provide a --remerge-diff capabilityElijah Newren via GitGitGadget, Dec 21, 2021
  26. Ævar Arnfjörð BjarmasonDec 21, 2021
  27. Elijah NewrenDec 21, 2021
  28. 9/9 doc/diff-options: explain the new --remerge-diff optionElijah Newren via GitGitGadget, Dec 21, 2021
  29. Ævar Arnfjörð BjarmasonDec 21, 2021
  30. Elijah NewrenDec 21, 2021
  31. Ævar Arnfjörð BjarmasonDec 21, 2021
  32. Elijah NewrenDec 22, 2021
  33. Junio C HamanoDec 21, 2021
  34. Elijah NewrenDec 21, 2021
  35. Junio C HamanoDec 22, 2021
  36. 0/8 Add a new --remerge-diff capability to show & logElijah Newren via GitGitGadget, Dec 25, 2021
  37. 1/8 show, log: provide a --remerge-diff capabilityElijah Newren via GitGitGadget, Dec 25, 2021
  38. Johannes AltmanningerDec 28, 2021
  39. Elijah NewrenDec 28, 2021
  40. brian m. carlsonDec 28, 2021
  41. Elijah NewrenDec 28, 2021
  42. 2/8 log: clean unneeded objects during `log --remerge-diff`Elijah Newren via GitGitGadget, Dec 25, 2021
  43. 3/8 ll-merge: make callers responsible for showing warningsElijah Newren via GitGitGadget, Dec 25, 2021
  44. Johannes AltmanningerDec 28, 2021
  45. Elijah NewrenDec 28, 2021
  46. Johannes AltmanningerDec 28, 2021
  47. 4/8 merge-ort: capture and print ll-merge warnings in our preferred fashionElijah Newren via GitGitGadget, Dec 25, 2021
  48. 5/8 merge-ort: mark a few more conflict messages as omittableElijah Newren via GitGitGadget, Dec 25, 2021
  49. 6/8 merge-ort: format messages slightly different for use in headersElijah Newren via GitGitGadget, Dec 25, 2021
  50. In-tree strbuf "in-place" search/replace (was: [PATCH v2 6/8] merge-ort: format messages slightly different for use in headers)Ævar Arnfjörð Bjarmason, Dec 26, 2021
  51. Johannes AltmanningerDec 28, 2021
  52. Elijah NewrenDec 28, 2021
  53. 7/8 diff: add ability to insert additional headers for pathsElijah Newren via GitGitGadget, Dec 25, 2021
  54. Johannes AltmanningerDec 28, 2021
  55. Elijah NewrenDec 28, 2021
  56. Johannes AltmanningerDec 29, 2021
  57. Elijah NewrenDec 30, 2021
  58. Johannes AltmanningerDec 31, 2021
  59. 8/8 show, log: include conflict/warning messages in --remerge-diff headersElijah Newren via GitGitGadget, Dec 25, 2021
  60. Johannes AltmanningerDec 28, 2021
  61. Elijah NewrenDec 28, 2021
  62. Ævar Arnfjörð BjarmasonDec 26, 2021
  63. Elijah NewrenDec 27, 2021
  64. Ævar Arnfjörð BjarmasonJan 10, 2022
  65. Johannes AltmanningerDec 28, 2021
  66. 0/9 Add a new --remerge-diff capability to show & logElijah Newren via GitGitGadget, Dec 30, 2021
  67. 1/9 show, log: provide a --remerge-diff capabilityElijah Newren via GitGitGadget, Dec 30, 2021
  68. Ævar Arnfjörð BjarmasonJan 19, 2022
  69. Elijah NewrenJan 20, 2022
  70. Elijah NewrenJan 20, 2022
  71. Ævar Arnfjörð BjarmasonJan 19, 2022
  72. Elijah NewrenJan 20, 2022
  73. 2/9 log: clean unneeded objects during `log --remerge-diff`Elijah Newren via GitGitGadget, Dec 30, 2021
  74. 3/9 ll-merge: make callers responsible for showing warningsElijah Newren via GitGitGadget, Dec 30, 2021
  75. Ævar Arnfjörð BjarmasonJan 19, 2022
  76. Elijah NewrenJan 20, 2022
  77. 4/9 merge-ort: capture and print ll-merge warnings in our preferred fashionElijah Newren via GitGitGadget, Dec 30, 2021
  78. 5/9 merge-ort: mark a few more conflict messages as omittableElijah Newren via GitGitGadget, Dec 30, 2021
  79. 6/9 merge-ort: format messages slightly different for use in headersElijah Newren via GitGitGadget, Dec 30, 2021
  80. 7/9 diff: add ability to insert additional headers for pathsElijah Newren via GitGitGadget, Dec 30, 2021
  81. 8/9 show, log: include conflict/warning messages in --remerge-diff headersElijah Newren via GitGitGadget, Dec 30, 2021
  82. Ævar Arnfjörð BjarmasonJan 19, 2022
  83. Elijah NewrenJan 21, 2022
  84. Elijah NewrenJan 21, 2022
  85. 9/9 merge-ort: mark conflict/warning messages from inner merges as omittableElijah Newren via GitGitGadget, Dec 30, 2021
  86. Junio C HamanoDec 31, 2021
  87. 00/10 Add a new --remerge-diff capability to show & logElijah Newren via GitGitGadget, Jan 21, 2022
  88. 01/10 show, log: provide a --remerge-diff capabilityElijah Newren via GitGitGadget, Jan 21, 2022
  89. Ævar Arnfjörð BjarmasonFeb 1, 2022
  90. Elijah NewrenFeb 1, 2022
  91. 02/10 log: clean unneeded objects during `log --remerge-diff`Elijah Newren via GitGitGadget, Jan 21, 2022
  92. Ævar Arnfjörð BjarmasonFeb 1, 2022
  93. Elijah NewrenFeb 1, 2022
  94. Ævar Arnfjörð BjarmasonFeb 2, 2022
  95. 03/10 ll-merge: make callers responsible for showing warningsElijah Newren via GitGitGadget, Jan 21, 2022
  96. 04/10 merge-ort: capture and print ll-merge warnings in our preferred fashionElijah Newren via GitGitGadget, Jan 21, 2022
  97. 05/10 merge-ort: mark a few more conflict messages as omittableElijah Newren via GitGitGadget, Jan 21, 2022
  98. 06/10 merge-ort: format messages slightly different for use in headersElijah Newren via GitGitGadget, Jan 21, 2022
  99. 07/10 diff: add ability to insert additional headers for pathsElijah Newren via GitGitGadget, Jan 21, 2022
  100. 08/10 show, log: include conflict/warning messages in --remerge-diff headersElijah Newren via GitGitGadget, Jan 21, 2022
  101. 09/10 merge-ort: mark conflict/warning messages from inner merges as omittableElijah Newren via GitGitGadget, Jan 21, 2022
  102. 10/10 diff-merges: avoid history simplifications when diffing mergesElijah Newren via GitGitGadget, Jan 21, 2022
  103. 00/10 Add a new --remerge-diff capability to show & logElijah Newren via GitGitGadget, Feb 2, 2022
  104. 01/10 show, log: provide a --remerge-diff capabilityElijah Newren via GitGitGadget, Feb 2, 2022
  105. 02/10 log: clean unneeded objects during `log --remerge-diff`Elijah Newren via GitGitGadget, Feb 2, 2022
  106. 03/10 ll-merge: make callers responsible for showing warningsElijah Newren via GitGitGadget, Feb 2, 2022
  107. 04/10 merge-ort: capture and print ll-merge warnings in our preferred fashionElijah Newren via GitGitGadget, Feb 2, 2022
  108. 05/10 merge-ort: mark a few more conflict messages as omittableElijah Newren via GitGitGadget, Feb 2, 2022
  109. 07/10 diff: add ability to insert additional headers for pathsElijah Newren via GitGitGadget, Feb 2, 2022
  110. 10/10 diff-merges: avoid history simplifications when diffing mergesElijah Newren via GitGitGadget, Feb 2, 2022
  111. 08/10 show, log: include conflict/warning messages in --remerge-diff headersElijah Newren via GitGitGadget, Feb 2, 2022
  112. 06/10 merge-ort: format messages slightly different for use in headersElijah Newren via GitGitGadget, Feb 2, 2022
  113. 09/10 merge-ort: mark conflict/warning messages from inner merges as omittableElijah Newren via GitGitGadget, Feb 2, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.