{"thread":{"id":"3704","subject":"[RFC] Silent File Mods Being Committed","startedAt":"2006-03-23T04:04:32Z","lastAt":"2006-03-24T01:12:17Z","messageCount":6,"participants":["Jon Loeliger","Junio C Hamano","Petr Baudis"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"17805","messageId":"E1FMH3o-0001B5-Dw@jdl.com","threadId":"3704","inReplyTo":null,"subject":"[RFC] Silent File Mods Being Committed","fromName":"Jon Loeliger","fromEmail":"jdl@jdl.com","sentAt":"2006-03-23T04:04:32Z","receivedAt":"2006-03-23T04:04:32Z","isPatch":false,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"Folks,\n\nI sort of got blindsided by committing (accidental) mode\nchanges on a file that was also textually changed. Because\nit was textually changed, I happily expected 'git status'\nto show it as modified and subsequently 'git commit'-ed it.\n\nOnly after I 'git diff'-ed it later did I see that there\nwas also a mode change on the file! Argh!\n\nSecondarily, I think that the invisible file changes like\nthis caused an eariler (yesterday-ish) confusion where an\nindex looked clean, but was dirty due to stat issues and\npoor users (me) couldn't quite see why.\n\nOriginally, I was going to propose that git-ls-files be modified\nsuch that the mode and stat changes that ce_modified() correctly\nidentifies as changing are somehow translated into output that\n\"git status\" shows the user per-file _in_addition_ to the normal\n\"is modified\" header. So I did this mod:\n\n\ndiff --git a/ls-files.c b/ls-files.c\nindex df25c8c..3e7e55d 100644\n--- a/ls-files.c\n+++ b/ls-files.c\n@@ -450,7 +450,7 @@ static void show_killed_files(void)\n \t}\n }\n \n-static void show_ce_entry(const char *tag, struct cache_entry *ce)\n+static void show_ce_entry(const char *tag, struct cache_entry *ce, int changed)\n {\n \tint len = prefix_len;\n \tint offset = prefix_offset;\n@@ -482,6 +482,14 @@ static void show_ce_entry(const char *ta\n \t\tfputs(tag, stdout);\n \t\twrite_name_quoted(\"\", 0, ce->name + offset,\n \t\t\t\t line_terminator, stdout);\n+\t\tif (changed & (MTIME_CHANGED | CTIME_CHANGED)) {\n+\t\t putchar(' ');\n+\t\t putchar('T');\n+\t\t}\n+\t\tif (changed & MODE_CHANGED) {\n+\t\t putchar(' ');\n+\t\t putchar('M');\n+\t\t}\n \t\tputchar(line_terminator);\n \t}\n \telse {\n@@ -541,7 +549,7 @@ static void show_files(void)\n \t\t\t\tcontinue;\n \t\t\tif (show_unmerged && !ce_stage(ce))\n \t\t\t\tcontinue;\n-\t\t\tshow_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce);\n+\t\t\tshow_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce, 0);\n \t\t}\n \t}\n \tif (show_deleted | show_modified) {\n@@ -553,9 +561,13 @@ static void show_files(void)\n \t\t\t\tcontinue;\n \t\t\terr = lstat(ce->name, &st);\n \t\t\tif (show_deleted && err)\n-\t\t\t\tshow_ce_entry(tag_removed, ce);\n-\t\t\tif (show_modified && ce_modified(ce, &st, 0))\n-\t\t\t\tshow_ce_entry(tag_modified, ce);\n+\t\t\t\tshow_ce_entry(tag_removed, ce, 0);\n+\t\t\tif (show_modified) {\n+\t\t\t\tint changed = ce_modified(ce, &st, 0);\n+\t\t\t\tif (changed)\n+\t\t\t\t\tshow_ce_entry(tag_modified,\n+\t\t\t\t\t\t ce, changed);\n+\t\t\t}\n \t\t}\n \t}\n }\n\n\nAnd with that, \"git ls-file -m\" showed a trailing T or M to\nindicate a \"time\" or a \"mode\" change happened on each file.\n\nHowever, 'git status' didn't show that output....\n\nAnd that is because it is driven by the diffcore instead!\nSo I _think_ diff_resolve_rename_copy() has to be consulted\nto get that DIFF_STATUS_MODIFIED indicator. Except that it is\nshared with the SHA1 compare too:\n\n else if (memcmp(p->one->sha1, p->two->sha1, 20) ||\n p->one->mode != p->two->mode)\n p->status = DIFF_STATUS_MODIFIED;\n\nBut I haven't tracked it back to see how to propagate that\nstatus back up to show_modified() in diff-files.c yet...\n\nMaybe there is an easier way...?\n\njdl\n"},{"id":"17806","messageId":"7vek0t68we.fsf@assigned-by-dhcp.cox.net","threadId":"3704","inReplyTo":"E1FMH3o-0001B5-Dw@jdl.com","subject":"Re: [RFC] Silent File Mods Being Committed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-23T05:13:21Z","receivedAt":"2006-03-23T05:13:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Loeliger writes:\n\n> However, 'git status' didn't show that output....\n>\n> And that is because it is driven by the diffcore instead!\n> So I _think_ diff_resolve_rename_copy() has to be consulted\n> to get that DIFF_STATUS_MODIFIED indicator. Except that it is\n> shared with the SHA1 compare too:\n>\n> else if (memcmp(p->one->sha1, p->two->sha1, 20) ||\n> p->one->mode != p->two->mode)\n> p->status = DIFF_STATUS_MODIFIED;\n>\n> But I haven't tracked it back to see how to propagate that\n> status back up to show_modified() in diff-files.c yet...\n\nI think the cleanest way would be either\n\n (1) define another output format to diffcore similar to\n --name-status, let that format perform most of what\n DIFF_FORMAT_NAME_STATUS does, and update\n diff.c:diff_flush_raw() to show the mode change as M+ (old\n was not executable but new is) or M- (the other way); or\n\n (2) audit all the scripts to make sure they do not get upset if\n we add trailing +/- to the status letter, and do that\n unconditionally, like the attached patch does.\n\nIf you go the latter route, you would get something like this:\n\n $ ./git-diff-files --abbrev\n :100644 100755 c0548ee... 0000000... M+\tdiff.c\n\t$ ./git-diff-files --name-status\n M+\tdiff.c\n\n---\ndiff --git a/diff.c b/diff.c\nindex c0548ee..09b8f7e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1034,6 +1034,10 @@ static void diff_flush_raw(struct diff_f\n \t\tstatus[0] = p->status;\n \t\tstatus[1] = 0;\n \t}\n+\n+\tif (p->one->mode && p->two->mode && p->one->mode != p->two->mode)\n+\t\tstrcat(status, (p->two->mode & 01) ? \"+\" : \"-\");\n+\n \tswitch (p->status) {\n \tcase DIFF_STATUS_COPIED:\n \tcase DIFF_STATUS_RENAMED:\n"},{"id":"17828","messageId":"20060323214710.GV18185@pasky.or.cz","threadId":"3704","inReplyTo":"7vek0t68we.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] Silent File Mods Being Committed","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-03-23T21:47:10Z","receivedAt":"2006-03-23T21:47:10Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter\nwhere Junio C Hamano said that...\n> (2) audit all the scripts to make sure they do not get upset if\n> we add trailing +/- to the status letter, and do that\n> unconditionally, like the attached patch does.\n\nCogito will get upset since we assume the mode field is one-char in our\nregexps, and when we don't, we compare the mode field with strings and\nthat would obviously fail if you add random stuff to it.\n\nOtherwise, I like this idea, though.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nRight now I am having amnesia and deja-vu at the same time. I think\nI have forgotten this before.\n"},{"id":"17837","messageId":"7vodzw4u13.fsf@assigned-by-dhcp.cox.net","threadId":"3704","inReplyTo":"20060323214710.GV18185@pasky.or.cz","subject":"Re: [RFC] Silent File Mods Being Committed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-23T23:32:08Z","receivedAt":"2006-03-23T23:32:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter\n> where Junio C Hamano said that...\n>> (2) audit all the scripts to make sure they do not get upset if\n>> we add trailing +/- to the status letter, and do that\n>> unconditionally, like the attached patch does.\n>\n> Cogito will get upset since we assume the mode field is one-char in our\n> regexps, and when we don't, we compare the mode field with strings and\n> that would obviously fail if you add random stuff to it.\n>\n> Otherwise, I like this idea, though.\n\nLikewise. If it was not obvious, I am not going to commit that\nmyself. If jdl or somebody cares enough, he or she can prepare\na patch to accomodate set of patches to git-core, Cogito and\nStGIT (at least these three should be covered) _and_ parrot my\npatch back at me. Hint, hint...\n"},{"id":"17838","messageId":"7vk6ak4tzp.fsf@assigned-by-dhcp.cox.net","threadId":"3704","inReplyTo":"20060323214710.GV18185@pasky.or.cz","subject":"Re: [RFC] Silent File Mods Being Committed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-23T23:32:58Z","receivedAt":"2006-03-23T23:32:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter\n> where Junio C Hamano said that...\n>> (2) audit all the scripts to make sure they do not get upset if\n>> we add trailing +/- to the status letter, and do that\n>> unconditionally, like the attached patch does.\n>\n> Cogito will get upset since we assume the mode field is one-char in our\n> regexps, and when we don't, we compare the mode field with strings and\n> that would obviously fail if you add random stuff to it.\n>\n> Otherwise, I like this idea, though.\n\nLikewise. If it was not obvious, I am not going to commit that\nmyself. If jdl or somebody cares enough, he or she can prepare\na a set of patches to git-core, Cogito and StGIT (at least these\nthree should be covered) to teach them the trailing +/- letter,\n_and_ parrot my patch back at me. Hint, hint...\n"},{"id":"17846","messageId":"20060324011217.GP19263@pasky.or.cz","threadId":"3704","inReplyTo":"7vk6ak4tzp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] Silent File Mods Being Committed","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-03-24T01:12:17Z","receivedAt":"2006-03-24T01:12:17Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Fri, Mar 24, 2006 at 12:32:58AM CET, I got a letter\nwhere Junio C Hamano said that...\n> Petr Baudis writes:\n> \n> > Dear diary, on Thu, Mar 23, 2006 at 06:13:21AM CET, I got a letter\n> > where Junio C Hamano said that...\n> >> (2) audit all the scripts to make sure they do not get upset if\n> >> we add trailing +/- to the status letter, and do that\n> >> unconditionally, like the attached patch does.\n> >\n> > Cogito will get upset since we assume the mode field is one-char in our\n> > regexps, and when we don't, we compare the mode field with strings and\n> > that would obviously fail if you add random stuff to it.\n> >\n> > Otherwise, I like this idea, though.\n> \n> Likewise. If it was not obvious, I am not going to commit that\n> myself. If jdl or somebody cares enough, he or she can prepare\n> a a set of patches to git-core, Cogito and StGIT (at least these\n> three should be covered) to teach them the trailing +/- letter,\n> _and_ parrot my patch back at me. Hint, hint...\n\nWell, or it can just add the option to Core Git. Just unconditional\nimplementation of (2) is no-go because it would break backwards\ncompatibility, which is baaad.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nRight now I am having amnesia and deja-vu at the same time. I think\nI have forgotten this before.\n"}]}