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

Re: [PATCH v5 5/9] patch-id: document new behaviour

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 24, 2014, 22:12 UTC
Message-ID
<xmqqzjjatm0x.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140424212639.GA9129@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
Show 6 quoted lines
>> > +--unstable::
>> > +	Use a non-symmetrical sum of hashes, such that reordering
>> 
>> What is a non-symmetrical sum?
>
> Non-symmetrical combination function is better?
I do not think either is very good X-<.
The primary points to convey for "--stable" are:
 - Two patches produced by comparing the same two trees with two
   different settings for "-O<orderfile>" will result in the same
   patchc signature, thereby allowing the computed result to be used
   as a key to index some metainformation about the change between
   the two trees;
 - It will produce a result different from the plain vanilla
   patch-id has always produced even when used on a diff output
   taken without any use of "-O<orderfile>", thereby making existing
   databases keyed by patch-ids unusable.

The fact that we happened to use a patch-id that catches that somebody reordered the same patch into different file order and declares that they are two different changes is a more historical accident than a designed goal.

I would even say that we would have used the "stable" version from the beginning if we thought that "-O<orderfile>" would be widely used when these two features both appeared. Even though I was the guilty one who introduced it, I'd admit that "-O<orderfile>" has merely been a curiosity from its inception and has been a failed experiment, not in the sense that the feature does not work as adverertised (it does), but in the sense that it is not widely used (evidenced by the lack of complaints on missing diff.orderfile for a long time) at all. With "-O<orderfile>" being a failed experiment, the "unstability" did not matter, so it has stuck.

The only two things worth mentioning about "--unstable", if our future direction is to see diff.orderfile and "--stable" a lot more widely used, are:

 (1) it keeps producing the same patch-id as existing versions of
     Git, so users with existing databases (who do not deal with
     reordered patches) may want to use it; and perhaps
 (2) it will not consider a patch taken with "-O<orderfile>" and
     another without it from the same source the same patches.

Mathmatically speaking, mentioning "non-symmetrial" might be one way of expressing the latter point (2), but stressing on that alone without mentioning (1) misses the point. (2) is _not_ a designed feature, so it is not very interesting. Unless you have an existing database, there is no reason to use "--unstable".

On the other hand (1) is a very relevant thing to mention, as we are talking about a feature that, if unused, may break existing users' data.

Previous: Michael S. TsirkinNext: Michael S. Tsirkin
Message 16 of 21 in “diff: add a config option to control orderfile”
  1. 1/9 diff: add a config option to control orderfileMichael S. Tsirkin, Apr 24, 2014
  2. 2/9 test: add test_write_lines helperMichael S. Tsirkin, Apr 24, 2014
  3. Jonathan NiederApr 24, 2014
  4. Junio C HamanoApr 24, 2014
  5. 3/9 tests: new test for orderfile optionsMichael S. Tsirkin, Apr 24, 2014
  6. Jonathan NiederApr 24, 2014
  7. Junio C HamanoApr 24, 2014
  8. Michael S. TsirkinApr 24, 2014
  9. 4/9 patch-id: make it stable against hunk reorderingMichael S. Tsirkin, Apr 24, 2014
  10. Jonathan NiederApr 24, 2014
  11. Junio C HamanoApr 24, 2014
  12. Michael S. TsirkinApr 24, 2014
  13. 5/9 patch-id: document new behaviourMichael S. Tsirkin, Apr 24, 2014
  14. Jonathan NiederApr 24, 2014
  15. Michael S. TsirkinApr 24, 2014
  16. Junio C HamanoApr 24, 2014
  17. Michael S. TsirkinApr 27, 2014
  18. 6/9 patch-id-test: test stable and unstable behaviourMichael S. Tsirkin, Apr 24, 2014
  19. 7/9 patch-id: change default to stableMichael S. Tsirkin, Apr 24, 2014
  20. 8/9 t4204-patch-id.sh: default is now stableMichael S. Tsirkin, Apr 24, 2014
  21. 9/9 Documentation/git-patch-id.txt: default is stableMichael S. Tsirkin, Apr 24, 2014

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.