{"thread":{"id":"46068","subject":"FORMAT_PATCH_NAME_MAX increase","startedAt":"2017-05-29T08:50:06Z","lastAt":"2017-05-30T15:06:20Z","messageCount":11,"participants":["Laszlo Ersek","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"320936","messageId":"99ac8ee7-67b5-5ae7-51cf-6be38de6217f@redhat.com","threadId":"46068","inReplyTo":null,"subject":"FORMAT_PATCH_NAME_MAX increase","fromName":"Laszlo Ersek","fromEmail":"lersek@redhat.com","sentAt":"2017-05-29T08:49:59Z","receivedAt":"2017-05-29T08:50:06Z","isPatch":false,"sender":{"key":"lersek@redhat.com","avatar":null},"body":"Hi,\n\nwould it be possible to\n\n- increase the FORMAT_PATCH_NAME_MAX macro from 64 to, say, 128?\n\n- Or else to introduce a new git-config knob for it?\n\nI have a small review-helper / interdiff script that matches patches\nfrom adjacent versions of a series against each other, based on subject\nline. (Using the formatted file name with the patch number stripped.)\nThe project in question uses long common prefixes in subjects, and the\ncurrent limit of 64 does not always ensure unicity (again, with the\nnumber stripped).\n\nThank you,\nLaszlo\n"},{"id":"320981","messageId":"xmqqh903dt4w.fsf@gitster.mtv.corp.google.com","threadId":"46068","inReplyTo":"99ac8ee7-67b5-5ae7-51cf-6be38de6217f@redhat.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-30T01:34:55Z","receivedAt":"2017-05-30T01:35:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laszlo Ersek <lersek@redhat.com> writes:\n\n> would it be possible to\n>\n> - increase the FORMAT_PATCH_NAME_MAX macro from 64 to, say, 128?\n>\n> - Or else to introduce a new git-config knob for it?\n\nIt's open source, so both are \"possible\", but you are interested in\nlearning if these are acceptable project-wide.\n\n> I have a small review-helper / interdiff script that matches patches\n> from adjacent versions of a series against each other, based on subject\n> line. (Using the formatted file name with the patch number stripped.)\n> The project in question uses long common prefixes in subjects, and the\n> current limit of 64 does not always ensure unicity (again, with the\n> number stripped).\n\nThe original motivation of the design is that leading numbers are to\nensure they are unique, and munging and truncating of title are to\nensure that they are safe on the filesystem while being identifiable\nfor eyeballing.  And the current truncation limit may not work well\nfor certain projects, failing the \"identifiabl\" goal.\n\nBut I do not think that is true only for your custom script.  For\nexample, with such a project, output from \"git shortlog\" or the\nlabels \"gitk\" gives to commits would not be very useful, because 2/3\nof your display are filled with and wasted by the same long prefix\nto the left.  The real issue is not that one particular \"length\nlimit\" used by format-patch is too short; it is that the project\nconvention wastes too many bytes for common things that can be left\nunsaid, and if we come up with a general way to address that, we'd\nmake all of Git that summarises commits more useful, not just\nformat-patch.\n\nSo I think lengthening FORMAT_PATCH_NAME_MAX, whether it is done\nunconditionally or conditionally, is rather an ad-hoc hack than\na real improvement.\n\nI cannot offhand guess what other places would suffer from such a\nproject convention, because I do not work with such a project, but\nyou may be able to come up with a list of various places in Git\nwhere the commit titles are used, and that if there were a mechanism\nto take these commit titles, pass them to your cutomization script,\nwhich abbreviates these \"long common prefixes\" to a shorter string,\nand to use the output from that script instead of the actual commit\ntitle, it would help all of these places.  The list of commits \"git\nmerge\" records when merging a side branch may benefit from the same\nmechanism.\n\nFor example, if the titles of your commit title look like this:\n\n    Subject: RedHat Kernel Maintenance: ipv4: add reference count\n    Subject: RedHat Kernel Maintenance: net: llc: add lock_sock\n    ...\n\nyour custom script may be\n\n    #!/bin/sh\n    exec sed -e \"s/^RedHat Kernel Maintenance: /RHKM-/\"\n\t... there may be more common patterns to shorten ...\n\nand the resulting output from format-patch might become\n\n\t0001-RHKM-ipv4-add-reference-count.patch\n\t0002-RHKM-net-llc-add-lock_sock.patch\n\nwhile \"git shortlog\" output and \"gitk\" display may also be helped\nby the same mechanism.\n\nIt's OK for the _initial_ application of such a mechanism were to\naffect the names of files format-patch creates and nothing else.\nInterested parties can then use the same mechanism in other programs\n(like \"gitk\" and \"git shortlog\"), as long as that mechansim is\ndesigned well.  And then the end users need to learn that mechanism\nonly once.\n"},{"id":"321012","messageId":"70d34f37-7209-e392-c269-21a61228a041@redhat.com","threadId":"46068","inReplyTo":"xmqqh903dt4w.fsf@gitster.mtv.corp.google.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Laszlo Ersek","fromEmail":"lersek@redhat.com","sentAt":"2017-05-30T11:03:38Z","receivedAt":"2017-05-30T11:03:45Z","isPatch":false,"sender":{"key":"lersek@redhat.com","avatar":null},"body":"On 05/30/17 03:34, Junio C Hamano wrote:\n\n> I cannot offhand guess what other places would suffer from such a\n> project convention, because I do not work with such a project, but\n> you may be able to come up with a list of various places in Git\n> where the commit titles are used, and that if there were a mechanism\n> to take these commit titles, pass them to your cutomization script,\n> which abbreviates these \"long common prefixes\" to a shorter string,\n> and to use the output from that script instead of the actual commit\n> title, it would help all of these places.\n\nThe problem is that I can't really automate the subject munging. The\nconcrete subjects in this case were:\n\n> OvmfPkg/QemuFwCfgLib: Implement SEV internal function for SEC phase\n> OvmfPkg/QemuFwCfgLib: Implement SEV internal functions for PEI phase\n> OvmfPkg/QemuFwCfgLib: Implement SEV internal function for Dxe phase\n\nwhich got formatted as\n\n> 0010-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch\n> 0011-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch\n> 0012-OvmfPkg-QemuFwCfgLib-Implement-SEV-internal-function.patch\n\nand these filenames differ only in the running counter on the left.\n\nThe patch subjects themselves aren't overlong (the longest is 68\ncharacters).\n\nAt best I could \"normalize away\" the \"OvmfPkg/QemuFwCfgLib\" prefix, but\nthat exact prefix is pretty accidental. Any standalone module (driver or\nlibrary instance) in the edk2 project is supposed to be named like this\nin patch subjects, so all those prefixes would have to be normalized\nsomehow.\n\nWe generally try to limit subjects (and commit messages in general) to\n74 columns. I think for one source of inspiration, we used the kernel\ndocumentation, when setting that limit.\n<https://www.kernel.org/doc/Documentation/process/submitting-patches.rst>\nsays, under section 14, \"The canonical patch format\",\n\n> The canonical patch subject line is::\n>\n>     Subject: [PATCH 001/123] subsystem: summary phrase\n>\n> The canonical patch message body contains the following:\n>\n>   [...]\n>\n>   - The body of the explanation, line wrapped at 75 columns, which will\n>     be copied to the permanent changelog to describe this patch.\n\nIt does not specify the subject length, but perhaps we can apply the\nbody line length to the subject as well.\n\nSo, even in kernel land, if subjects up to 75 columns are permitted, but\nFORMAT_PATCH_NAME_MAX is 64, conflicts are possible, at least in theory,\naren't they? With the numbers stripped, of course.\n\nThanks,\nLaszlo\n"},{"id":"321013","messageId":"CACBZZX5U=U1bpiFuuxH2t8ZWnmQQAjWm1ji8XYq_6-SJPGL1sw@mail.gmail.com","threadId":"46068","inReplyTo":"99ac8ee7-67b5-5ae7-51cf-6be38de6217f@redhat.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-30T11:36:41Z","receivedAt":"2017-05-30T11:37:08Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, May 29, 2017 at 10:49 AM, Laszlo Ersek <lersek@redhat.com> wrote:\n> Hi,\n>\n> would it be possible to\n>\n> - increase the FORMAT_PATCH_NAME_MAX macro from 64 to, say, 128?\n>\n> - Or else to introduce a new git-config knob for it?\n>\n> I have a small review-helper / interdiff script that matches patches\n> from adjacent versions of a series against each other, based on subject\n> line. (Using the formatted file name with the patch number stripped.)\n> The project in question uses long common prefixes in subjects, and the\n> current limit of 64 does not always ensure unicity (again, with the\n> number stripped).\n\nI don't see why this shouldn't be made configurable, but more\ngenerally if you have a script like this it seems like a futile effort\nin general to just make the subject longer to solve the general case,\nconsider:\n\n    (cd /tmp/; rm -rf test; git init test; cd test && for i in {1..3};\ndo touch $i && git add $i && git commit -m\"test\"; done && git\nformat-patch -2 && git reset --hard HEAD~2 && git am *patch)\n\nWhich now yields:\n\n    0001-test.patch\n    0002-test.patch\n\nGit projects in general will have plenty of patches like this, e.g.\n\"fix build\" or \"update tests\" or whatever. Isn't a better solution for\nyour script to e.g. key on git-patch-id?\n\n    $ grep \"^From \" *patch\n    0001-test.patch:From 870e37afa1a5aeb7eef76e607345adcfd4a9022d Mon\nSep 17 00:00:00 2001\n    0002-test.patch:From de8c37a1532a4f6ae71ffa65400479ba77438f3b Mon\nSep 17 00:00:00 2001\n    $ cat *patch | git patch-id\n    c71eb8f2c8c461ba6040668e9d79f996f5a04a61\n870e37afa1a5aeb7eef76e607345adcfd4a9022d\n    735aff6fb601d7ce99506dc7701be3e8a9b5d38c\nde8c37a1532a4f6ae71ffa65400479ba77438f3b\n\nOther potential heuristics could be keying not just on the subject but\non the subject + subject of the last N commits for each commit, which\nshould give more of a unique key, or key on the whole commit message\netc.\n"},{"id":"321015","messageId":"c2453701-979b-ebc7-dcc3-483a27d4c157@redhat.com","threadId":"46068","inReplyTo":"CACBZZX5U=U1bpiFuuxH2t8ZWnmQQAjWm1ji8XYq_6-SJPGL1sw@mail.gmail.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Laszlo Ersek","fromEmail":"lersek@redhat.com","sentAt":"2017-05-30T12:28:46Z","receivedAt":"2017-05-30T12:29:03Z","isPatch":false,"sender":{"key":"lersek@redhat.com","avatar":null},"body":"On 05/30/17 13:36, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, May 29, 2017 at 10:49 AM, Laszlo Ersek <lersek@redhat.com> wrote:\n>> Hi,\n>>\n>> would it be possible to\n>>\n>> - increase the FORMAT_PATCH_NAME_MAX macro from 64 to, say, 128?\n>>\n>> - Or else to introduce a new git-config knob for it?\n>>\n>> I have a small review-helper / interdiff script that matches patches\n>> from adjacent versions of a series against each other, based on subject\n>> line. (Using the formatted file name with the patch number stripped.)\n>> The project in question uses long common prefixes in subjects, and the\n>> current limit of 64 does not always ensure unicity (again, with the\n>> number stripped).\n> \n> I don't see why this shouldn't be made configurable, but more\n> generally if you have a script like this it seems like a futile effort\n> in general to just make the subject longer to solve the general case,\n> consider:\n> \n>     (cd /tmp/; rm -rf test; git init test; cd test && for i in {1..3};\n> do touch $i && git add $i && git commit -m\"test\"; done && git\n> format-patch -2 && git reset --hard HEAD~2 && git am *patch)\n> \n> Which now yields:\n> \n>     0001-test.patch\n>     0002-test.patch\n> \n> Git projects in general will have plenty of patches like this, e.g.\n> \"fix build\" or \"update tests\" or whatever. Isn't a better solution for\n> your script to e.g. key on git-patch-id?\n> \n>     $ grep \"^From \" *patch\n>     0001-test.patch:From 870e37afa1a5aeb7eef76e607345adcfd4a9022d Mon\n> Sep 17 00:00:00 2001\n>     0002-test.patch:From de8c37a1532a4f6ae71ffa65400479ba77438f3b Mon\n> Sep 17 00:00:00 2001\n>     $ cat *patch | git patch-id\n>     c71eb8f2c8c461ba6040668e9d79f996f5a04a61\n> 870e37afa1a5aeb7eef76e607345adcfd4a9022d\n>     735aff6fb601d7ce99506dc7701be3e8a9b5d38c\n> de8c37a1532a4f6ae71ffa65400479ba77438f3b\n> \n> Other potential heuristics could be keying not just on the subject but\n> on the subject + subject of the last N commits for each commit, which\n> should give more of a unique key, or key on the whole commit message\n> etc.\n> \n\nMy use case is the following:\n\n- Contributor posts version 1 of his/her patch series to the mailing\nlist, and also pushes the series to his/her publicly accessible git repo\non some branch. I review it and suggest a number of improvements.\n\n- Contributor posts version 2 of his/her patch series to the mailing\nlist, and pushes the series to another branch in his/her repo too. I\nwant to review the v2 series incrementally; that is, I'd like to diff\neach individual v2 patch against its v1 counterpart.\n\nWhile patches may be dropped, added, replaced between patch set\nversions, the general case is that most patches remain in the series,\nand they preserve their original subjects. Thus, finding the v1\ncounterpart for a v2 patch, based on the subject, as formatted by\ngit-format-patch into filenames, works reliably most of the time.\n\nNote that in such an incremental review, I specifically wish to compare\npatches against each other (i.e., I'd like to see diffs of diffs, AKA\ninterdiffs), and not the source tree at two, v1<->v2, commits into the\ntwo series. The latter would only give me the difference between the v1\nand v2 patch application results, and while that is valuable as well,\nI'd really like to diff the actual patches.\n\nOf course I could technically parse the Subject: header of the formatted\npatch files, and rename the files for interdiffing using full-length\nfilenames. But then I'd have to care about filesystem safety and such,\nand that's already handled perfectly by git-format-patch itself. The\nonly bit I'm missing is the arbitrary length in the formatted file\nnames. (I had patched git earlier to use 128 for FORMAT_PATCH_NAME_MAX,\nbut I'd like to stop carrying that change.)\n\n\"git-patch-id\" doesn't look applicable here, as I'd like to assign\npatches (across v1 and v2) to each other based on \"purpose\". I.e., the\nassignment should occur regardless of even functional changes between\ncorresponding v1 and v2 patches. I need the coupling exactly so I can\nreview those differences.\n\nI use the \"join\" utility on the number-stripped patch filenames to find\nthe pairs of patches between v1 and v2 that should be compared.\n\nThanks!\nLaszlo\n"},{"id":"321016","messageId":"85970ab4-e9c8-c6b6-11d3-faafd3ed0708@redhat.com","threadId":"46068","inReplyTo":"c2453701-979b-ebc7-dcc3-483a27d4c157@redhat.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Laszlo Ersek","fromEmail":"lersek@redhat.com","sentAt":"2017-05-30T12:33:08Z","receivedAt":"2017-05-30T12:33:14Z","isPatch":false,"sender":{"key":"lersek@redhat.com","avatar":null},"body":"(apologies for the self-followup:)\n\nOn 05/30/17 14:28, Laszlo Ersek wrote:\n\n> Note that in such an incremental review, I specifically wish to compare\n> patches against each other (i.e., I'd like to see diffs of diffs, AKA\n> interdiffs), and not the source tree at two, v1<->v2, commits into the\n> two series. The latter would only give me the difference between the v1\n> and v2 patch application results, and while that is valuable as well,\n> I'd really like to diff the actual patches.\n\n... One (but not the only) reason for that is that I'd like to see the\nv1->v2 commit message improvements as well. I quite frequently suggest\ncommit message improvements in review.\n\nThanks\nLaszlo\n"},{"id":"321017","messageId":"CACBZZX4-T50q_eVWwJuBzoC6ocuG+v14Tm8wfK8h7FB_cMQzdQ@mail.gmail.com","threadId":"46068","inReplyTo":"85970ab4-e9c8-c6b6-11d3-faafd3ed0708@redhat.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-30T12:41:03Z","receivedAt":"2017-05-30T12:41:50Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, May 30, 2017 at 2:33 PM, Laszlo Ersek <lersek@redhat.com> wrote:\n> (apologies for the self-followup:)\n>\n> On 05/30/17 14:28, Laszlo Ersek wrote:\n>\n>> Note that in such an incremental review, I specifically wish to compare\n>> patches against each other (i.e., I'd like to see diffs of diffs, AKA\n>> interdiffs), and not the source tree at two, v1<->v2, commits into the\n>> two series. The latter would only give me the difference between the v1\n>> and v2 patch application results, and while that is valuable as well,\n>> I'd really like to diff the actual patches.\n>\n> ... One (but not the only) reason for that is that I'd like to see the\n> v1->v2 commit message improvements as well. I quite frequently suggest\n> commit message improvements in review.\n\nThis all makes sense, I think a way to make this length configurable\nis the easiest/best solution to this.\n\nJust curious do you know about https://github.com/trast/tbdiff ? If\nnot it might have a high overlap with what you're doing.\n"},{"id":"321020","messageId":"xmqqk24ybhjj.fsf@gitster.mtv.corp.google.com","threadId":"46068","inReplyTo":"70d34f37-7209-e392-c269-21a61228a041@redhat.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-30T13:28:16Z","receivedAt":"2017-05-30T13:28:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laszlo Ersek <lersek@redhat.com> writes:\n\n> The problem is that I can't really automate the subject munging. The\n> concrete subjects in this case were:\n>\n>> OvmfPkg/QemuFwCfgLib: Implement SEV internal function for SEC phase\n>> OvmfPkg/QemuFwCfgLib: Implement SEV internal functions for PEI phase\n>> OvmfPkg/QemuFwCfgLib: Implement SEV internal function for Dxe phase\n> ...\n> So, even in kernel land, if subjects up to 75 columns are permitted, but\n> FORMAT_PATCH_NAME_MAX is 64, conflicts are possible, at least in theory,\n> aren't they? With the numbers stripped, of course.\n\nYup, configurable lengthening or unconditional lengthening to 75 or\nso do not sound _too_ bad.\n\nIf I sounded like I was opposed to lengthening, that wasn't what I\nmeant.  It was more like \"if you can meaningfully abbreviate, you\nmay help not just format-patch filenames but other use cases, and\nyou might even be able to get away without lengthening\"; if there\nis no meaningful way to abbreviate, raising the max length may be\nthe only workable solution.\n\n"},{"id":"321021","messageId":"xmqqfufmbh4f.fsf@gitster.mtv.corp.google.com","threadId":"46068","inReplyTo":"CACBZZX4-T50q_eVWwJuBzoC6ocuG+v14Tm8wfK8h7FB_cMQzdQ@mail.gmail.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-30T13:37:20Z","receivedAt":"2017-05-30T13:37:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Just curious do you know about https://github.com/trast/tbdiff ? If\n> not it might have a high overlap with what you're doing.\n\nYes, that is a very good suggestion.  You'd need to be able to\nactually apply the patches but the way I often do a review is very\nsimilar to (actually, I'd say it is identical workflow) Laszlo's,\nand it goes like this:\n\n    $ git checkout topic ;# previous round\n    $ git checkout master... ;# check out the fork point of previous one\n    $ git am mbox ;# apply the updated one\n    $ git tbdiff ..@{-1} @{-1}..\n\nWith the second step, the commit immediately before the previous\nround of patches were applied to is checked out as a detached HEAD,\nand then with the third step, the updated patches are applied.\nAfter these two steps, the history leading to HEAD is the latest\npatches, and the history leading to topic (which can be referred to\nas @{-1}, i.e. the branch we were previously on) is the previous\nround.\n\n\"git tbdiff\" takes two ranges and compares these two series of\ncommits.  The first one says \"commits included in the branch we are\npreviously on (i.e. topic), excluding the ones on the current HEAD\",\nwhich means \"the patches from the previous round\", and the second\none says \"commits included in the current HEAD, excluding the ones\non the previous branch (i.e. topic)\", which means \"the patches from\nthis latest round\".\n\nAfter that, I may conclude\n\n    $ git checkout -B @{-1}\n\nto update the tip of 'topic' with the latest set of patches (the\nreason why I type @{-1} is because that can stay constant in the\nworkflow, no matter what the actual topic branch is called).\n"},{"id":"321023","messageId":"CACBZZX7x7s-RfQMZjHuq=fbGtOeQzYin_Owqx7dK3v7dudh8zA@mail.gmail.com","threadId":"46068","inReplyTo":"xmqqfufmbh4f.fsf@gitster.mtv.corp.google.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-05-30T14:35:42Z","receivedAt":"2017-05-30T14:36:08Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, May 30, 2017 at 3:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> Just curious do you know about https://github.com/trast/tbdiff ? If\n>> not it might have a high overlap with what you're doing.\n>\n> Yes, that is a very good suggestion.  You'd need to be able to\n> actually apply the patches but the way I often do a review is very\n> similar to (actually, I'd say it is identical workflow) Laszlo's,\n> and it goes like this:\n>\n>     $ git checkout topic ;# previous round\n>     $ git checkout master... ;# check out the fork point of previous one\n>     $ git am mbox ;# apply the updated one\n>     $ git tbdiff ..@{-1} @{-1}..\n\nI wonder if this tool seemingly everyone on-list uses should just be\nintegrated into git.git.\n\nI only learned about it <2 weeks ago and it's been great. The diff\noutput was a bit nonsensical in some cases because it uses python's\ndiff engine instead of git's.\n\nWould you mind patches to just integrate it to git in python as-is,\nthen we could slowly convert it to C as we're doing with everything\nelse.\n"},{"id":"321025","messageId":"36557854-907f-9bb0-2059-c97328e1ca41@redhat.com","threadId":"46068","inReplyTo":"CACBZZX7x7s-RfQMZjHuq=fbGtOeQzYin_Owqx7dK3v7dudh8zA@mail.gmail.com","subject":"Re: FORMAT_PATCH_NAME_MAX increase","fromName":"Laszlo Ersek","fromEmail":"lersek@redhat.com","sentAt":"2017-05-30T15:06:11Z","receivedAt":"2017-05-30T15:06:20Z","isPatch":false,"sender":{"key":"lersek@redhat.com","avatar":null},"body":"On 05/30/17 16:35, Ævar Arnfjörð Bjarmason wrote:\n> On Tue, May 30, 2017 at 3:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>>\n>>> Just curious do you know about https://github.com/trast/tbdiff ? If\n>>> not it might have a high overlap with what you're doing.\n\nThank you for the suggestion, it does exactly what I need. It solves the\nAssignment Problem a whole lot better than my current, crude script.\n\nhttps://en.wikipedia.org/wiki/Assignment_problem\nhttps://en.wikipedia.org/wiki/Hungarian_algorithm\n\n(Mildly amusing, to me anyway: I happen to be Hungarian.)\n\nThe output of git-tbdiff is not exactly the way I like it (I like to\nreview interdiffs colorized, and fed to $PAGER individually), but I can\neasily do that on top of the \"--no-patches\" output.\n\n>> Yes, that is a very good suggestion.  You'd need to be able to\n>> actually apply the patches but the way I often do a review is very\n>> similar to (actually, I'd say it is identical workflow) Laszlo's,\n>> and it goes like this:\n>>\n>>     $ git checkout topic ;# previous round\n>>     $ git checkout master... ;# check out the fork point of previous one\n>>     $ git am mbox ;# apply the updated one\n>>     $ git tbdiff ..@{-1} @{-1}..\n> \n> I wonder if this tool seemingly everyone on-list uses should just be\n> integrated into git.git.\n> \n> I only learned about it <2 weeks ago and it's been great. The diff\n> output was a bit nonsensical in some cases because it uses python's\n> diff engine instead of git's.\n\nYes, I'd probably plug in git's diff engine (the patience algorithm at\nthat).\n\n> \n> Would you mind patches to just integrate it to git in python as-is,\n> then we could slowly convert it to C as we're doing with everything\n> else.\n\nThat would be wonderful. My preference would be to use core git features\nfor solving the problem; as a second preference, it would be great if\ngit-tbdiff were packaged & shipped by GNU/Linux distros.\n\nThomas (I see you are on To:/Cc: now -- I meant to CC you, snarfing your\nemail from the git.git history :) ), are you aware of any distros\nalready packaging git-tbdiff?\n\nStill the best would be if git provided this feature out of the box;\nincremental review of rerolled series appears a common activity for any\nserious reviewer / subsystem co-maintainer.\n\nThanks!\nLaszlo\n"}]}