{"thread":{"id":"29785","subject":"filtering out mode-change-only changes","startedAt":"2012-02-29T02:31:13Z","lastAt":"2012-03-03T22:16:43Z","messageCount":7,"participants":["Neal Kreitzinger","Junio C Hamano","Pete Harlan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"185691","messageId":"jik2le$2lb$1@dough.gmane.org","threadId":"29785","inReplyTo":null,"subject":"filtering out mode-change-only changes","fromName":"Neal Kreitzinger","fromEmail":"neal@rsss.com","sentAt":"2012-02-29T02:31:13Z","receivedAt":"2012-02-29T02:31:13Z","isPatch":false,"sender":{"key":"neal@rsss.com","avatar":null},"body":"What is the best way to filter out the \"mode change only\" entries from a \n\"name-status diff result\" listing of changed files?\n\nReason-for-this:\nget a list of files whose content changed and feed that list into a \ngui-diff-tool for visual review of \"merge\" (rebase) results.\n\nPartial-Solutions:\n--name-status does not have mode-change info.\n--raw has mode change info but I would have to parse it out and compare it \nmyself.\n--summary has that info, but not the content modification info.\n\nBefore I write a script to discern \"mode-change-only changes\" and remove \nthem from the list of changed files (thus leaving only content-changed files \nin the list), I'd like to see if there is some way that git \nalready-does-this-for-you, or if someone already has a script that does \nthis.\n\nThanks in advance for any tips.\n\nv/r,\nneal \n"},{"id":"185695","messageId":"7vmx82wbmr.fsf@alter.siamese.dyndns.org","threadId":"29785","inReplyTo":"jik2le$2lb$1@dough.gmane.org","subject":"Re: filtering out mode-change-only changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T03:40:12Z","receivedAt":"2012-02-29T03:40:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Neal Kreitzinger\" <neal@rsss.com> writes:\n\n> What is the best way to filter out the \"mode change only\" entries from a \n> \"name-status diff result\" listing of changed files?\n>\n> Reason-for-this:\n> get a list of files whose content changed and feed that list into a \n> gui-diff-tool for visual review of \"merge\" (rebase) results.\n\nLater I have some words on this.\n\n> ... I'd like to see if there is some way that git \n> already-does-this-for-you, or if someone already has a script that does \n> this.\n\nI do not know about random scripts people write, but there is nothing\nbuilt-in.\n\nBut *if* the _real_ reason you want to do this is because you do not want\nto see unnecessary mode changes caused by your filesystem that screws up\nfile modes for whatever breakage, perhaps setting core.filemode to false\nso that mode changes made to the working tree files by your mode breaking\nfilesystem might be the real solution.\n\nOf course I do not know if that is the real reason you are asking for that\nor not.\n"},{"id":"185697","messageId":"7vipiqwb2g.fsf@alter.siamese.dyndns.org","threadId":"29785","inReplyTo":"7vmx82wbmr.fsf@alter.siamese.dyndns.org","subject":"Re: filtering out mode-change-only changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T03:52:23Z","receivedAt":"2012-02-29T03:52:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Neal Kreitzinger\" <neal@rsss.com> writes:\n>\n>> What is the best way to filter out the \"mode change only\" entries from a \n>> \"name-status diff result\" listing of changed files?\n>> ...\n> I do not know about random scripts people write, but there is nothing\n> built-in.\n\nHaving said that, if we _were_ to do this built-in, an obvious logical\nplace to do so is to define a new DIFF_OPT_IGNORE_EXECUTABLE_BIT, teach\n\"--ignore-executable-bit\" command line option to diff_opt_parse(), and\nthen teach diff_resolve_rename_copy() to consider this bit when the code\noriginally set DIFF_STATUS_MODIFIED.  Instead, the updated code that is\nworking under --ignore-executable-bit option would drop such a filepair\nfrom diff_queued_diff.\n\nI do not know if such a change is worth doing, though.  It depends on the\nreal reason why do you have so many \"mode change only\" changes that would\nmake rebasing or cherry-picking too troublesome.\n"},{"id":"185733","messageId":"4F4E7847.9030402@gmail.com","threadId":"29785","inReplyTo":"7vipiqwb2g.fsf@alter.siamese.dyndns.org","subject":"Re: filtering out mode-change-only changes","fromName":"Neal Kreitzinger","fromEmail":"nkreitzinger@gmail.com","sentAt":"2012-02-29T19:11:03Z","receivedAt":"2012-02-29T19:11:03Z","isPatch":false,"sender":{"key":"nkreitzinger@gmail.com","avatar":null},"body":"On 2/28/2012 9:52 PM, Junio C Hamano wrote:\n> Junio C Hamano<gitster@pobox.com>  writes:\n>\n>> \"Neal Kreitzinger\"<neal@rsss.com>  writes:\n>>\n>>> What is the best way to filter out the \"mode change only\" entries from a\n>>> \"name-status diff result\" listing of changed files?\n>>> ...\n>> I do not know about random scripts people write, but there is nothing\n>> built-in.\n>\n> Having said that, if we _were_ to do this built-in, an obvious logical\n> place to do so is to define a new DIFF_OPT_IGNORE_EXECUTABLE_BIT, teach\n> \"--ignore-executable-bit\" command line option to diff_opt_parse(), and\n> then teach diff_resolve_rename_copy() to consider this bit when the code\n> originally set DIFF_STATUS_MODIFIED.  Instead, the updated code that is\n> working under --ignore-executable-bit option would drop such a filepair\n> from diff_queued_diff.\n>\n> I do not know if such a change is worth doing, though.  It depends on the\n> real reason why do you have so many \"mode change only\" changes that would\n> make rebasing or cherry-picking too troublesome.\n>\nI see three parts to this issue that are related but also independent:\nQuestions:\n(Q1) Is the user handling filemodes correctly in git?\n(Q2) Why does the user need to interrogate filemodes in git?\n(Q3) How are file modes interrogated by the user in git?\n\nSome Answers:\nQ1: Is the user handling filemodes correctly in git?\n\nA1-1: (My Context)\nPerhaps I'm not, but I'm not prepared to ignore filemodes.  I think I \nneed to be aware of what's changing.  Blasting everything with the linux \nchmod 777 shotgun, or the git core.filemode=false shotgun does not seem \nlike the right answer to me.  I need to do more homework on linux \npermissions and git executable bit tracking.\n\nA1-2: (General Context)\nSome users do legitimately choose to have core.filemode=true and are \ncorrect in doing so.\n\nQ2: Why does the user need to interrogate filemodes in git?\n\nA2-1: (My Context)\nAfter a rebase we need to review what changed with a 4-way diff to have \nthe full context of merge-base, topic, upstream, and merged. Because we \nare mere-mortals, we want to use gui side-by-side diff (ie, diffuse) \ninstead of 4-way combined diff. git-difftool only takes two file parms \nso I have to write my own script. git-mergetool's can display 4-way diff \nbut insist on mangling the $MERGED file with their own attempts at \nredoing the merge, ie. creating their own merge conflicts even though \n$MERGED has no conflict markers on input.\n\nA2-2: (General Context)\n\"Vendor code drops\" (see git-rm manpage) can have substantial \nfile-mode-only changes along with real content changes due to incorrect \ntar procedures and/or the vendor's filesystem being a \"mode breaking \nfilesystem\". Also, there are these human stdin's going about \ncapriciously with a free-will doing chmod's.\n\nQ3: How are file modes interrogated by the user in git?\n\nA3-1: (Some Current Options)\n--name-status lumps file-mode-only changes and content changes together \nunder status \"M\".\n--raw can be parsed to discern filemode changes concurrent with \nidentical content sha1's.\n--summary \"mode change\" entries might also be usable to apply a filter \nto --name-status results.\n\nA3-2: (Some Desired Options)\n--name-status learns a new status for file-mode-only changes (ie, \"P\" \nfor \"P\"ermissions).\n--raw learns \"P+x\" and \"P-x\" in the status column to tell you if the \nexecutable bit was added or removed.\n\nI wonder if filemode tracking was somewhat of an afterthought of the \ncontent-is-king design of git and that is why it is semi-opaque.\n\nv/r,\nneal\n"},{"id":"185738","messageId":"7vsjhts9hu.fsf@alter.siamese.dyndns.org","threadId":"29785","inReplyTo":"4F4E7847.9030402@gmail.com","subject":"Re: filtering out mode-change-only changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T19:52:13Z","receivedAt":"2012-02-29T19:52:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neal Kreitzinger <nkreitzinger@gmail.com> writes:\n\n> A3-2: (Some Desired Options)\n> --name-status learns a new status for file-mode-only changes (ie, \"P\"\n> for \"P\"ermissions).\n\nAfter reading everything above I omitted from your response in my quote, I\nstill do not get the feeling that these willy-nilly mode changes that you\nare suffering from is a problem that is general enough to warrant such a\nchange, even if such a change is done as an optional feature.\n\nCalling our executable bits \"Permission\" is a misnomer, by the way. It is\nmore about \"is this an executable file?\" attribute, and is not \"are you\nallowed to execute this?\" permission.\n\n> --raw learns \"P+x\" and \"P-x\" in the status column to tell you if the\n> executable bit was added or removed.\n\nAs --raw output by definition is designed to be read by scripts that can\nand do parse its output, I do not see how this can be a useful addition at\nall. The same information is available in the separate mode column already.\n\n> I wonder if filemode tracking was somewhat of an afterthought of the\n> content-is-king design of git and that is why it is semi-opaque.\n\nOur blobs represent contents.  Whether your shell script has or does not\nhave executable bits, the file has the same contents.\n\nThe executable-ness of a particular file starts to matter only when you\nextract it to your working tree.  The executable-ness matters equally as\nits contents, and as the path at which it is extracted.  All three are\nrecorded in an entry in a tree object and in an entry in the index, as a\n<mode, object name, path> tuple.\n\nAt the end-user level, you seldom use the blob contents (identified by the\nobject name in the three tuple explained above) alone without the\nsurrounding context (the other two members of the three tuple) the blob\nappears in.  Especially in \"diff\", you not only want to view the content\nchange, but also need to know the change of the context in which the blobs\nappear in.  And that is why we show \"This content, or a related old\nversion of it, appeared at this old path, but now it appears at this new\npath with the following change, and it lost the executable bit during the\nchange.\"\n\nI do see nothing afterthought about the design at the storage or at the\npresentation level.\n"},{"id":"185750","messageId":"7v7gz5s2rg.fsf@alter.siamese.dyndns.org","threadId":"29785","inReplyTo":"7vipiqwb2g.fsf@alter.siamese.dyndns.org","subject":"Re: filtering out mode-change-only changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-29T22:17:39Z","receivedAt":"2012-02-29T22:17:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Having said that, if we _were_ to do this built-in, an obvious logical\n> place to do so is to define a new DIFF_OPT_IGNORE_EXECUTABLE_BIT, teach\n> \"--ignore-executable-bit\" command line option to diff_opt_parse(), and\n> then teach diff_resolve_rename_copy() to consider this bit when the code\n> originally set DIFF_STATUS_MODIFIED.  Instead, the updated code that is\n> working under --ignore-executable-bit option would drop such a filepair\n> from diff_queued_diff.\n\nA patch to do so may look like this (untested, of course).  A few things\nto note on the changes:\n\n - The \"header\" strbuf holds the lines starting from \"diff --git\" and the\n   meta-information lines such as mode change, similarity, etc.  It is\n   passed to the the interface code fn_out_consume() via the xdiff\n   machinery when an actual diff is found and emitted before the first\n   hunk of the diff.\n\n - The must_show_header toggle is set at the strategic places when\n   information is added to the \"header\" strbuf that makes the output for\n   this filepair a must, even if it turns out that xdiff machinery does\n   not find any content changes.  Before this patch, we flipped this\n   toggle when we noticed a mode change, so that the header is shown even\n   if there is no content change. The patch has to make make it\n   conditional, which is what the first hunk is about.\n\n - The second hunk is not related but I think it is a worthy bit-rot fix.\n   The original code before \"must_show_header\" was introduced showed the\n   header upfront unless we are ignoring any whitespace changes, the\n   reasoning behind it being that two different blobs may produce no patch\n   under --ignore-space-change and in such a case we do not want to show\n   anything for the filepair.  When 296c6bb (diff: fix \"git show -C -C\"\n   output when renaming a binary file, 2010-05-26) introduced\n   \"must_show_header\", it retained that logic.\n\n   But I think it was a mistake.  If xdiff machinery decides there is no\n   patch to show, taking the --ignore-space-change into account, our\n   fn_out_consume() won't be called so we won't show the header\n   unnecessarily.  And when we do see a line of patch, fn_out_consume()\n   will show the header.\n\n\n\n diff.c |   31 +++++++++++++++++++++++++++++--\n diff.h |    1 +\n 2 files changed, 30 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nold mode 100644\nnew mode 100755\ndiff --git a/diff.c b/diff.c\nindex a1c06b5..acf7232 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2152,7 +2152,8 @@ static void builtin_diff(const char *name_a,\n \t\tif (one->mode != two->mode) {\n \t\t\tstrbuf_addf(&header, \"%s%sold mode %06o%s\\n\", line_prefix, set, one->mode, reset);\n \t\t\tstrbuf_addf(&header, \"%s%snew mode %06o%s\\n\", line_prefix, set, two->mode, reset);\n-\t\t\tmust_show_header = 1;\n+\t\t\tif (!DIFF_OPT_TST(o, IGNORE_MODE_CHANGE))\n+\t\t\t\tmust_show_header = 1;\n \t\t}\n \t\tif (xfrm_msg)\n \t\t\tstrbuf_addstr(&header, xfrm_msg);\n@@ -2207,7 +2208,7 @@ static void builtin_diff(const char *name_a,\n \t\tstruct emit_callback ecbdata;\n \t\tconst struct userdiff_funcname *pe;\n \n-\t\tif (!DIFF_XDL_TST(o, WHITESPACE_FLAGS) || must_show_header) {\n+\t\tif (must_show_header) {\n \t\t\tfprintf(o->file, \"%s\", header.buf);\n \t\t\tstrbuf_reset(&header);\n \t\t}\n@@ -3446,6 +3447,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t}\n \telse if (!strcmp(arg, \"--no-renames\"))\n \t\toptions->detect_rename = 0;\n+\telse if (!strcmp(arg, \"--ignore-mode-change\"))\n+\t\tDIFF_OPT_SET(options, IGNORE_MODE_CHANGE);\n \telse if (!strcmp(arg, \"--relative\"))\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n \telse if (!prefixcmp(arg, \"--relative=\")) {\n@@ -4509,10 +4512,34 @@ void diffcore_fix_diff_index(struct diff_options *options)\n \tqsort(q->queue, q->nr, sizeof(q->queue[0]), diffnamecmp);\n }\n \n+static void diffcore_ignore_mode_change(struct diff_options *diffopt)\n+{\n+\tint i;\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct diff_queue_struct outq;\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\n+\t\tif (DIFF_FILE_VALID(p->one) &&\n+\t\t    DIFF_FILE_VALID(p->two) &&\n+\t\t    (p->one->sha1_valid && p->two->sha1_valid) &&\n+\t\t    !hashcmp(p->one->sha1, p->two->sha1))\n+\t\t\tdiff_free_filepair(p); /* skip this */\n+\t\telse\n+\t\t\tdiff_q(&outq, p);\n+\t}\n+\tfree(q->queue);\n+\t*q = outq;\n+}\n+\n void diffcore_std(struct diff_options *options)\n {\n \tif (options->skip_stat_unmatch)\n \t\tdiffcore_skip_stat_unmatch(options);\n+\tif (DIFF_OPT_TST(options, IGNORE_MODE_CHANGE))\n+\t\tdiffcore_ignore_mode_change(options);\n \tif (!options->found_follow) {\n \t\t/* See try_to_follow_renames() in tree-diff.c */\n \t\tif (options->break_opt != -1)\ndiff --git a/diff.h b/diff.h\nindex 7af5f1e..fabcc96 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -82,6 +82,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n+#define DIFF_OPT_IGNORE_MODE_CHANGE  (1 << 30)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n"},{"id":"186017","messageId":"4F52984B.5050805@pcharlan.com","threadId":"29785","inReplyTo":"7vsjhts9hu.fsf@alter.siamese.dyndns.org","subject":"Re: filtering out mode-change-only changes","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2012-03-03T22:16:43Z","receivedAt":"2012-03-03T22:16:43Z","isPatch":false,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 2/29/2012 11:52 AM, Junio C Hamano wrote:\n> Neal Kreitzinger<nkreitzinger@gmail.com>  writes:\n>\n>> A3-2: (Some Desired Options)\n>> --name-status learns a new status for file-mode-only changes (ie, \"P\"\n>> for \"P\"ermissions).\n>\n> After reading everything above I omitted from your response in my quote, I\n> still do not get the feeling that these willy-nilly mode changes that you\n> are suffering from is a problem that is general enough to warrant such a\n> change, even if such a change is done as an optional feature.\n\nI'll add a vote for the usefulness of the option.\n\nI'm dealing with a repo whose history occasionally has commits that do a \nhandful of real changes mixed in with tens of thousands of mistaken mode \nchanges.  (New workflow is fixed but it is infeasible to fix history at \nthis point.)  It's cumbersome filtering away the noise.\n\nIf Git's diff engine could ignore mode changes that would be a big help.\n\nRegards,\n\n--\nPete Harlan\npgit@pcharlan.com\n"}]}