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

Re: Strange effect merging empty file

From
Jeff King <peff@peff.net>
Date
Mar 22, 2012, 19:03 UTC
Message-ID
<20120322190303.GA32756@sigill.intra.peff.net>
In-Reply-To
<7v62dwxybd.fsf@alter.siamese.dyndns.org>
On Thu, Mar 22, 2012 at 11:52:54AM -0700, Junio C Hamano wrote:
> I still wonder why checking only the preimage side is sufficient, though.
> Shouldn't we check both sides?
> [...]
> > +		if (pair->status != 'R' || is_empty_blob_sha1(pair->one->sha1)) {

If the source is an empty file, then the other side must be an empty file, too, no? Otherwise it would not be a rename. It could not be exact, for obvious reasons. And it could not be inexact, because there is no content to analyze on the source side.

Ditto for the destination side. How could something be renamed to an empty file, but not be empty on the source side?

That is a slight layering violation, in that we are making assumptions about how the diffcore-rename subsystem works. In theory diffcore-rename could learn to read rename annotations from the commit message or something (bleh!). But then, we'd probably want to update merge-recursive in that instance to accept those "yes, it's definitely a rename" markers, even for an empty file.

I think it would be slightly cleaner to tell diffcore-rename "be conservative, and don't use empty files as sources" via an option. It's not as trivial as this one-liner, but it shouldn't be much more than the small patch you posted in the earlier thread.

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 18 of 25 in “Strange effect merging empty file”
  1. Ralf NyrenMar 21, 2012
  2. Zbigniew Jędrzejewski-SzmekMar 21, 2012
  3. Junio C HamanoMar 21, 2012
  4. Randal L. SchwartzMar 22, 2012
  5. Ralf NyrenMar 22, 2012
  6. Zbigniew Jędrzejewski-SzmekMar 22, 2012
  7. Jeff KingMar 22, 2012
  8. Junio C HamanoMar 22, 2012
  9. Jeff KingMar 22, 2012
  10. Jeff KingMar 22, 2012
  11. Jeff KingMar 22, 2012
  12. 1/3 drop casts from users EMPTY_TREE_SHA1_BINJeff King, Mar 22, 2012
  13. 2/3 make is_empty_blob_sha1 available everywhereJeff King, Mar 22, 2012
  14. 3/3 merge-recursive: don't detect renames from empty filesJeff King, Mar 22, 2012
  15. Jonathan NiederMar 22, 2012
  16. Jeff KingMar 22, 2012
  17. Junio C HamanoMar 22, 2012
  18. Jeff KingMar 22, 2012
  19. Junio C HamanoMar 22, 2012
  20. 0/2 merging renames of empty filesJeff King, Mar 22, 2012
  21. 1/2 teach diffcore-rename to optionally ignore empty contentJeff King, Mar 22, 2012
  22. 2/2 merge-recursive: don't detect renames of empty filesJeff King, Mar 22, 2012
  23. Junio C HamanoMar 22, 2012
  24. Jeff KingMar 23, 2012
  25. Junio C HamanoMar 23, 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.