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

Re: [PATCH] Revert 'diff-merges: let "-m" imply "-p"'

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Aug 7, 2021, 13:51 UTC
Message-ID
<YQ6P0OWPPksWG5Hf@google.com>
In-Reply-To
<035b4e99-e708-f31b-2f13-e255d99dea33@kdbg.org>
Hi Hannes,
Johannes Sixt wrote:
> Am 07.08.21 um 03:55 schrieb Jonathan Nieder:
Show 15 quoted lines
>> The motivating example (Rust) shows that there is at least one script
>> that _did_ use "-m" in this way.  Rust has mitigation, but the above
>> logic leads me to believe that they are not the only project that will
>> be affected.  And more generally, when a script author has a
>> reasonable reason to believe something will work, they write scripts
>> where it _does_ work, and then an update breaks their script, I think
>> it's reasonable for them not to be happy.
>
> As you know, we have "plumbing" commands with a stable interface and
> "porcelain" commands for which we reserve to change the behavior without
> advance notice. By your reasoning we would not need to distinguish
> between the two categories and were forced to keep all behavior stable.
> This undoing of a behavior change in a "porcelain" command with the
> argument that one script depended on the old behavior and that others
> might do so as well would set an unwanted precedent.

Hm, this is worth elucidating a bit more, since I am definitely in favor of continuing to change porcelain commands for the better where we can. If we decide that "git log --format=<fmt>" is no longer part of the stable scripting interface we provide, then that would be a huge change for our callers (and it's probably too late), but I would certainly be in favor of us going back in time and doing that. :)

More generally, we've been able to make changes to porcelain commands that don't hurt our ability to act as a platform for scripts, and I want us to continue to be able to do that. "Do not break any script" is certainly not the standard I think we should apply, as illustrated by my thoughts upthread when I thought '-m' in this Rust example was a typo.

But by now it's very clear to me that it was not a typo.
In other words:
- this isn't only about one obscure script.  The point of the "this
  was not a typo" logic is to illustrate that in addition to the
  examples that we know about it is very likely that there are
  examples that we don't know about, in teams' script collections
  beyond the reach of search engines.
- In fact, in addition to the motivating example that makes it
  possible to build Rust, we had multiple in-tree scripts that would
  also have broken by this, if they had not been adapted to work
  around that in the same series!  I should have noticed that in
  review, and I'm sorry that I didn't.
> Perhaps we need to point script authors to "plumbing" commands more clearly?

I think the existence of "plumbing" is fairly well known, but users don't always have an easy time using it. The "porcelain" is what ends up getting the most attention in improvements, and so while I encourage script authors to use 'git rev-list <revs> | git diff-tree -s --stdin --format=<fmt>' in place of 'git log --format=<fmt> <revs>', most do not listen, and I can't really blame them given how much more convenient the latter is and how many more options it supports.

I don't think that situation will change unless we
 a. Maintain a second, parallel implementation of each porcelain
    command that only uses plumbing.  This would provide an example of
    how to use plumbing and would ensure that the plumbing grows in
    capability at the same time as the corresponding porcelain.  Or
 b. Expose a library interface, so that we can expose the actual
    helpers that support the standard implementation of porcelain
    commands.

I tried a little of (a) years ago by updating contrib/examples/ to pass tests: https://lore.kernel.org/git/20100817065147.GA18293@burratino/. It was fun but I don't think it's really sustainable.

In the long term, I think (b) is going to be an important thing to do, and I think it will be helpful. Some automated callers would appreciate the ability to pass structured input instead of having to pretend to be shell scripts. :) True shell scripts would also benefit because the plumbing commands can more directly map to such a library API.

> (BTW, I have no opinion on whether -m should or should not imply -p.)
Nevertheless, thanks for weighing in.
Jonathan
Previous: Johannes SixtNext: Junio C Hamano
Message 124 of 129 in “Why doesn't `git log -m` imply `-p`?”
  1. Alex HenrieApr 29, 2021
  2. Junio C HamanoApr 29, 2021
  3. Sergey OrganovApr 29, 2021
  4. Alex HenrieApr 29, 2021
  5. Sergey OrganovApr 29, 2021
  6. Alex HenrieApr 29, 2021
  7. Sergey OrganovApr 29, 2021
  8. Felipe ContrerasMay 4, 2021
  9. Sergey OrganovMay 4, 2021
  10. Junio C HamanoApr 29, 2021
  11. Junio C HamanoApr 30, 2021
  12. Sergey OrganovApr 30, 2021
  13. Junio C HamanoMay 1, 2021
  14. Sergey OrganovMay 3, 2021
  15. Junio C HamanoMay 4, 2021
  16. Sergey OrganovMay 4, 2021
  17. Junio C HamanoMay 4, 2021
  18. Sergey OrganovMay 4, 2021
  19. Junio C HamanoMay 5, 2021
  20. Sergey OrganovMay 5, 2021
  21. Junio C HamanoMay 6, 2021
  22. Sergey OrganovMay 6, 2021
  23. Junio C HamanoMay 6, 2021
  24. Sergey OrganovMay 6, 2021
  25. Alex HenrieMay 7, 2021
  26. Sergey OrganovMay 10, 2021
  27. Alex HenrieMay 10, 2021
  28. 0/6 diff-merges: let -m imply -pSergey Organov, May 10, 2021
  29. 1/6 t4013: add test for "git diff-index -m"Sergey Organov, May 10, 2021
  30. 2/6 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 10, 2021
  31. Junio C HamanoMay 11, 2021
  32. Junio C HamanoMay 11, 2021
  33. Junio C HamanoMay 11, 2021
  34. Sergey OrganovMay 11, 2021
  35. 3/6 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 10, 2021
  36. 4/6 stash list: stop passing "-m" to "git list"Sergey Organov, May 10, 2021
  37. 5/6 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 10, 2021
  38. 6/6 diff-merges: let -m imply -pSergey Organov, May 10, 2021
  39. Junio C HamanoMay 11, 2021
  40. Junio C HamanoMay 11, 2021
  41. Sergey OrganovMay 11, 2021
  42. Alex HenrieMay 11, 2021
  43. Sergey OrganovMay 11, 2021
  44. Alex HenrieMay 11, 2021
  45. Sergey OrganovMay 11, 2021
  46. Felipe ContrerasMay 12, 2021
  47. Elijah NewrenMay 11, 2021
  48. Sergey OrganovMay 11, 2021
  49. Elijah NewrenMay 11, 2021
  50. Sergey OrganovMay 11, 2021
  51. Junio C HamanoMay 11, 2021
  52. Sergey OrganovMay 11, 2021
  53. Junio C HamanoMay 11, 2021
  54. Jonathan NiederMay 19, 2021
  55. Sergey OrganovMay 20, 2021
  56. Felipe ContrerasMay 21, 2021
  57. Sergey OrganovMay 11, 2021
  58. Sergey OrganovMay 17, 2021
  59. Sergey OrganovMay 11, 2021
  60. Jonathan NiederMay 19, 2021
  61. Sergey OrganovMay 19, 2021
  62. Junio C HamanoMay 19, 2021
  63. Sergey OrganovMay 20, 2021
  64. Jonathan NiederMay 20, 2021
  65. Sergey OrganovMay 20, 2021
  66. 0/9 diff-merges: let -m imply -pSergey Organov, May 17, 2021
  67. 1/9 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 17, 2021
  68. 4/9 t4013: test "git diff-index -m"Sergey Organov, May 17, 2021
  69. 3/9 t4013: test "git -m --stat"Sergey Organov, May 17, 2021
  70. 2/9 t4013: test "git -m --raw"Sergey Organov, May 17, 2021
  71. Bagas SanjayaMay 18, 2021
  72. Sergey OrganovMay 18, 2021
  73. 6/9 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 17, 2021
  74. 8/9 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 17, 2021
  75. 5/9 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 17, 2021
  76. Junio C HamanoMay 17, 2021
  77. Sergey OrganovMay 17, 2021
  78. Junio C HamanoMay 17, 2021
  79. Sergey OrganovMay 17, 2021
  80. 9/9 diff-merges: let "-m" imply "-p"Sergey Organov, May 17, 2021
  81. 7/9 stash list: stop passing "-m" to "git list"Sergey Organov, May 17, 2021
  82. Junio C HamanoMay 17, 2021
  83. Sergey OrganovMay 17, 2021
  84. Bagas SanjayaMay 18, 2021
  85. Sergey OrganovMay 18, 2021
  86. Sergey OrganovMay 18, 2021
  87. Junio C HamanoMay 18, 2021
  88. Sergey OrganovMay 18, 2021
  89. 0/9 diff-merges: let -m imply -pSergey Organov, May 19, 2021
  90. 1/9 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 19, 2021
  91. 2/9 t4013: test "git log -m --raw"Sergey Organov, May 19, 2021
  92. 3/9 t4013: test "git log -m --stat"Sergey Organov, May 19, 2021
  93. 4/9 t4013: test "git diff-index -m"Sergey Organov, May 19, 2021
  94. 5/9 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 19, 2021
  95. 6/9 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 19, 2021
  96. 7/9 stash list: stop passing "-m" to "git log"Sergey Organov, May 19, 2021
  97. 8/9 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 19, 2021
  98. 9/9 diff-merges: let "-m" imply "-p"Sergey Organov, May 19, 2021
  99. 00/10 diff-merges: let -m imply -pSergey Organov, May 20, 2021
  100. 02/10 t4013: test "git log -m --raw"Sergey Organov, May 20, 2021
  101. 01/10 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 20, 2021
  102. 03/10 t4013: test "git log -m --stat"Sergey Organov, May 20, 2021
  103. 04/10 t4013: test "git diff-tree -m"Sergey Organov, May 20, 2021
  104. 05/10 t4013: test "git diff-index -m"Sergey Organov, May 20, 2021
  105. 06/10 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 20, 2021
  106. 07/10 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 20, 2021
  107. 08/10 stash list: stop passing "-m" to "git log"Sergey Organov, May 20, 2021
  108. 09/10 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 20, 2021
  109. 10/10 diff-merges: let "-m" imply "-p"Sergey Organov, May 20, 2021
  110. Jonathan NiederAug 5, 2021
  111. Revert 'diff-merges: let "-m" imply "-p"'Jonathan Nieder, Aug 6, 2021
  112. Junio C HamanoAug 6, 2021
  113. Junio C HamanoAug 6, 2021
  114. Jonathan NiederAug 6, 2021
  115. Junio C HamanoAug 8, 2021
  116. Sergey OrganovAug 17, 2021
  117. Junio C HamanoAug 17, 2021
  118. Sergey OrganovAug 18, 2021
  119. Junio C HamanoAug 19, 2021
  120. Junio C HamanoAug 19, 2021
  121. Sergey OrganovAug 20, 2021
  122. Jonathan NiederAug 7, 2021
  123. Johannes SixtAug 7, 2021
  124. Jonathan NiederAug 7, 2021
  125. Junio C HamanoAug 7, 2021
  126. Jonathan NiederAug 7, 2021
  127. Junio C HamanoAug 8, 2021
  128. Sergey OrganovAug 17, 2021
  129. Sergey OrganovAug 16, 2021

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.