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

Re: [PATCH v3 1/4] git-cherry-pick: add allow-empty option

From
Neil Horman <nhorman@tuxdriver.com>
Date
Apr 11, 2012, 00:44 UTC
Message-ID
<20120411004419.GA19616@neilslaptop.think-freely.org>
In-Reply-To
<7v4nsrpa4i.fsf@alter.siamese.dyndns.org>
On Tue, Apr 10, 2012 at 02:09:17PM -0700, Junio C Hamano wrote:
Show 35 quoted lines
> Neil Horman <nhorman@tuxdriver.com> writes:
> 
> > ...  What do you say to my proposal regarding the splitting of the device
> > dependent on how we were executed?  It seems we can't use a single advice string
> > in this case, as no matter which we choose there is a use case in which it fails
> > to make sense.
> 
> When your cherry-pick got --allow-empty, it should pass --allow-empty to
> its inner invocation of commit if it is picking an originally empty
> commit, and this advice will not trigger because commit will happily
> commit the no-change change.
> 
> When your cherry-pick got --allow-empty but not --keep-unnecessary-commit,
> its inner invocation of commit must not pass --allow-empty if it is _not_
> picking an originally empty commit.  Then the inner commit will fail if it
> auto resolves to no change, and the user sees the advice.  The current
> advice text is appropriate for this case.
> 
> When your cherry-pick did not get either of these flags, its inner
> invocation of commit must not pass --allow-empty.  The user sees the
> advice when the auto resolved result matches HEAD from the commit invoked
> by cherry-pick.  The current advice text is fine for this case, as we say
> "possibly", not "we definitely know it was due to conflict resolution".
> 
> Or your cherry-pick may have failed due to a conflict, regardless of the
> options like --allow-empty or --keep-unnecessary-commit given to it, and
> the user may have run commit after resolving the conflict.  The current
> advice text is fine for this case, too, as we say "possibly", and it
> indeed is what just happened.
> 
> So I do not think you need to change anything with respect to the advice
> message.
> 
> Am I missing some other cases?
>

No, you covered all the cases, but I disagree with your assertion that the advice is correct (or at least optimal) in any of these cases. If a cherry-pick without any options is preformed and the commit is empty (regardless of the reason), the advice given is that git commit --allow-empty should be used. With the addition of these new options, thats not true any longer. Instead of using git commit --allow-empty, you can use git cherry-pick --allow-empty.

Given your description above though, I'm ok rolling this back. Regardless of my disagreement, git commit --allow-empty still works just as well after the addition of these options, so while I still think its awkward to give git commit advice on the result of a git cherry-pick operation, its not any more problematic than the issues you've pointed out with my change. I'll roll this back in my next version and will look for a way to provide better advice in the future.

Regards Neil

>
Previous: Junio C HamanoNext: Junio C Hamano
Message 50 of 121 in “Enhance git-rebases flexibiilty in handling empty commits”
  1. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Mar 30, 2012
  2. 1/4 git-cherry-pick: add keep-empty optionNeil Horman, Mar 30, 2012
  3. Junio C HamanoMar 30, 2012
  4. Jeff KingMar 30, 2012
  5. Neil HormanMar 31, 2012
  6. 2/4 git-rebase: add keep_empty flagNeil Horman, Mar 30, 2012
  7. Junio C HamanoMar 30, 2012
  8. Neil HormanMar 31, 2012
  9. 3/4 git-commit-am: Allow automatic rebasing to preserve empty commitsNeil Horman, Mar 30, 2012
  10. Junio C HamanoMar 30, 2012
  11. Neil HormanMar 31, 2012
  12. Junio C HamanoMar 30, 2012
  13. Neil HormanMar 31, 2012
  14. 4/4 git-commit-interactive: Allow rebasing to preserve empty commitsNeil Horman, Mar 30, 2012
  15. Junio C HamanoMar 30, 2012
  16. Neil HormanMar 31, 2012
  17. Junio C HamanoMar 30, 2012
  18. 0/5 Enhance git-rebases flexibiilty handling empty commits [v2]Neil Horman, Apr 5, 2012
  19. 1/5 argv-array: Add argv_array_pop function [v2]Neil Horman, Apr 5, 2012
  20. Junio C HamanoApr 5, 2012
  21. Neil HormanApr 5, 2012
  22. Neil HormanApr 6, 2012
  23. Jeff KingApr 6, 2012
  24. Neil HormanApr 6, 2012
  25. Junio C HamanoApr 6, 2012
  26. Jeff KingApr 6, 2012
  27. Cc tags in the commit message (Re: [PATCH 1/5] argv-array: Add argv_array_pop function [v2])Jonathan Nieder, Apr 6, 2012
  28. Junio C HamanoApr 6, 2012
  29. Jeff KingApr 6, 2012
  30. Junio C HamanoApr 6, 2012
  31. Neil HormanApr 6, 2012
  32. 2/5 git-cherry-pick: add allow-empty option [v2]Neil Horman, Apr 5, 2012
  33. 3/5 git-cherry-pick: Add ignore-if-made-empty option [v2]Neil Horman, Apr 5, 2012
  34. Junio C HamanoApr 5, 2012
  35. Neil HormanApr 5, 2012
  36. Junio C HamanoApr 6, 2012
  37. Neil HormanApr 6, 2012
  38. Johannes SixtApr 6, 2012
  39. 4/5 git-cherry-pick: Add test to validate new options [v2]Neil Horman, Apr 5, 2012
  40. 5/5 git-rebase: add keep_empty flag [v2]Neil Horman, Apr 5, 2012
  41. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Apr 10, 2012
  42. 1/4 git-cherry-pick: add allow-empty optionNeil Horman, Apr 10, 2012
  43. Junio C HamanoApr 10, 2012
  44. Neil HormanApr 10, 2012
  45. Junio C HamanoApr 10, 2012
  46. Neil HormanApr 10, 2012
  47. Junio C HamanoApr 10, 2012
  48. Neil HormanApr 10, 2012
  49. Junio C HamanoApr 10, 2012
  50. Neil HormanApr 11, 2012
  51. Junio C HamanoApr 11, 2012
  52. Neil HormanApr 11, 2012
  53. Junio C HamanoApr 11, 2012
  54. Neil HormanApr 11, 2012
  55. 2/4 git-cherry-pick: Add keep-redundant-commits optionNeil Horman, Apr 10, 2012
  56. Junio C HamanoApr 10, 2012
  57. Neil HormanApr 10, 2012
  58. 3/4 git-cherry-pick: Add test to validate new optionsNeil Horman, Apr 10, 2012
  59. 4/4 git-rebase: add keep_empty flagNeil Horman, Apr 10, 2012
  60. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Apr 13, 2012
  61. 1/4 git-cherry-pick: add allow-empty optionNeil Horman, Apr 13, 2012
  62. 2/4 git-cherry-pick: Add keep-redundant-commits optionNeil Horman, Apr 13, 2012
  63. Clemens BuchacherApr 15, 2012
  64. Neil HormanApr 16, 2012
  65. Clemens BuchacherApr 16, 2012
  66. Neil HormanApr 17, 2012
  67. Junio C HamanoApr 17, 2012
  68. Clemens BuchacherApr 17, 2012
  69. Neil HormanApr 18, 2012
  70. 3/4 git-cherry-pick: Add test to validate new optionsNeil Horman, Apr 13, 2012
  71. Clemens BuchacherApr 15, 2012
  72. Neil HormanApr 16, 2012
  73. Neil HormanApr 16, 2012
  74. Junio C HamanoApr 16, 2012
  75. Neil HormanApr 16, 2012
  76. Clemens BuchacherApr 16, 2012
  77. Neil HormanApr 17, 2012
  78. Clemens BuchacherApr 17, 2012
  79. Neil HormanApr 18, 2012
  80. Clemens BuchacherApr 18, 2012
  81. 4/4 git-rebase: add keep_empty flagNeil Horman, Apr 13, 2012
  82. Clemens BuchacherApr 15, 2012
  83. Neil HormanApr 16, 2012
  84. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Apr 17, 2012
  85. 1/4 git-cherry-pick: add allow-empty optionNeil Horman, Apr 17, 2012
  86. 2/4 git-cherry-pick: Add keep-redundant-commits optionNeil Horman, Apr 17, 2012
  87. Clemens BuchacherApr 17, 2012
  88. Neil HormanApr 18, 2012
  89. 3/4 git-cherry-pick: Add test to validate new optionsNeil Horman, Apr 17, 2012
  90. 4/4 git-rebase: add keep_empty flagNeil Horman, Apr 17, 2012
  91. Clemens BuchacherApr 17, 2012
  92. Neil HormanApr 18, 2012
  93. Junio C HamanoApr 18, 2012
  94. Neil HormanApr 19, 2012
  95. Junio C HamanoApr 19, 2012
  96. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Apr 18, 2012
  97. 1/4 git-cherry-pick: add allow-empty optionNeil Horman, Apr 18, 2012
  98. 2/4 git-cherry-pick: Add keep-redundant-commits optionNeil Horman, Apr 18, 2012
  99. Junio C HamanoApr 18, 2012
  100. 3/4 git-cherry-pick: Add test to validate new optionsNeil Horman, Apr 18, 2012
  101. 4/4 git-rebase: add keep_empty flagNeil Horman, Apr 18, 2012
  102. Zbigniew Jędrzejewski-SzmekApr 19, 2012
  103. Thomas RastApr 19, 2012
  104. Zbigniew Jędrzejewski-SzmekApr 19, 2012
  105. Neil HormanApr 19, 2012
  106. Junio C HamanoApr 19, 2012
  107. Junio C HamanoApr 19, 2012
  108. Neil HormanApr 20, 2012
  109. 0/4 Enhance git-rebases flexibiilty in handling empty commitsNeil Horman, Apr 20, 2012
  110. 1/4 git-cherry-pick: add allow-empty optionNeil Horman, Apr 20, 2012
  111. 2/4 git-cherry-pick: Add keep-redundant-commits optionNeil Horman, Apr 20, 2012
  112. Junio C HamanoApr 20, 2012
  113. Neil HormanApr 20, 2012
  114. 3/4 git-cherry-pick: Add test to validate new optionsNeil Horman, Apr 20, 2012
  115. 4/4 git-rebase: add keep_empty flagNeil Horman, Apr 20, 2012
  116. Junio C HamanoApr 25, 2012
  117. Neil HormanApr 25, 2012
  118. Martin von ZweigbergkJul 18, 2012
  119. Johannes SixtJul 18, 2012
  120. Martin von ZweigbergkJul 18, 2012
  121. Neil HormanJul 18, 2012

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.