{"thread":{"id":"24274","subject":"'git commit --short' without touching index?","startedAt":"2010-07-03T08:30:31Z","lastAt":"2010-07-06T12:10:22Z","messageCount":7,"participants":["Daniel","Jeff King","friedan@muon.rutgers.edu","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"144735","messageId":"loom.20100703T102242-536@post.gmane.org","threadId":"24274","inReplyTo":null,"subject":"'git commit --short' without touching index?","fromName":"Daniel","fromEmail":"friedan@muon.rutgers.edu","sentAt":"2010-07-03T08:30:31Z","receivedAt":"2010-07-03T08:30:31Z","isPatch":false,"sender":{"key":"friedan@muon.rutgers.edu","avatar":null},"body":"'git commit --short' touches .git/index ('--short' implies '--dry-run').\n\nIs there any equivalent command that does not touch .git/index?\n\nI'm using 'git commit --short' in a script to check if the repository needs\nattention.  At present, it touches .git/index, which causes unnecessary activity\nin another script running rsync.\n\nthanks,\nDaniel\n"},{"id":"144738","messageId":"20100703091748.GA11714@sigill.intra.peff.net","threadId":"24274","inReplyTo":"loom.20100703T102242-536@post.gmane.org","subject":"Re: 'git commit --short' without touching index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-03T09:17:48Z","receivedAt":"2010-07-03T09:17:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 03, 2010 at 08:30:31AM +0000, Daniel wrote:\n\n> 'git commit --short' touches .git/index ('--short' implies '--dry-run').\n> \n> Is there any equivalent command that does not touch .git/index?\n> \n> I'm using 'git commit --short' in a script to check if the repository needs\n> attention.  At present, it touches .git/index, which causes\n> unnecessary activity in another script running rsync.\n\nIf you are just calling \"git commit --short\" without arguments (i.e.,\nyou care about the current state, not about \"what would happen if I\ncommit with these arguments\"), then you probably want \"git status\".\nAlso, you probably should use \"git status --porcelain\", which is the\nsame as --short but is guaranteed not to change in the future.\n\nHowever, both \"status\" and \"commit --dry-run\" will opportunistically\nrefresh the index. If you don't want to touch the index at all, you can\nuse a combination of:\n\n  # show changes between HEAD and index\n  git diff-index HEAD\n\n  # show changes between index and working tree\n  git diff-files\n\n  # show untracked files\n  git ls-files --exclude-standard -o\n\nOne thing you will find, though, is that not refreshing the index means\nthat stat-dirty files (e.g., ones which have no content changed but\nwhich have had their timestamps changed) will appear the diff-files\nabove.\n\nI don't think there is a simple way to tell \"git status\" to refresh the\nindex internally, but not to write it out. It will do this if it doesn't\nhave write permissions, but chmod'ing your index before running the\nscript seems like an awful hack. You could refresh to a temporary index,\nbut that also seems like a hack.\n\nIt would be nice if the index-refreshing code only wrote to the index if\nthere was something worth writing. I'm not sure how hard that would be\nto implement, though.\n\n-Peff\n"},{"id":"144740","messageId":"4C2F0C20.1020004@muon.rutgers.edu","threadId":"24274","inReplyTo":"20100703091748.GA11714@sigill.intra.peff.net","subject":"Re: 'git commit --short' without touching index?","fromName":"","fromEmail":"friedan@muon.rutgers.edu","sentAt":"2010-07-03T10:08:32Z","receivedAt":"2010-07-03T10:08:32Z","isPatch":false,"sender":{"key":"friedan@muon.rutgers.edu","avatar":null},"body":"Thanks.  Your suggestion seems to work.  Much appreciated.\n\nDaniel\n\nOn 2010.07.03 9:17 AM, Jeff King wrote:\n> However, both \"status\" and \"commit --dry-run\" will opportunistically\n> refresh the index. If you don't want to touch the index at all, you can\n> use a combination of:\n> \n>   # show changes between HEAD and index\n>   git diff-index HEAD\n> \n>   # show changes between index and working tree\n>   git diff-files\n> \n>   # show untracked files\n>   git ls-files --exclude-standard -o\n> \n"},{"id":"144845","messageId":"7veifhy9j0.fsf@alter.siamese.dyndns.org","threadId":"24274","inReplyTo":"20100703091748.GA11714@sigill.intra.peff.net","subject":"Re: 'git commit --short' without touching index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-05T20:10:43Z","receivedAt":"2010-07-05T20:10:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It would be nice if the index-refreshing code only wrote to the index if\n> there was something worth writing. I'm not sure how hard that would be\n> to implement, though.\n\nHmm, don't we already do that with \"istate->cache_changed\"?\n\nIn any case, we should diagnose \"commit --short\" (and \"--procelain\") as an\nerror, perhaps by splitting option parsers for commit and status further.\n"},{"id":"144847","messageId":"20100705205651.GA32728@sigill.intra.peff.net","threadId":"24274","inReplyTo":"7veifhy9j0.fsf@alter.siamese.dyndns.org","subject":"Re: 'git commit --short' without touching index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-05T20:56:52Z","receivedAt":"2010-07-05T20:56:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 05, 2010 at 01:10:43PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > It would be nice if the index-refreshing code only wrote to the index if\n> > there was something worth writing. I'm not sure how hard that would be\n> > to implement, though.\n> \n> Hmm, don't we already do that with \"istate->cache_changed\"?\n\nApparently not:\n\n  $ stat .git/index | grep -i modify\n  Modify: 2010-07-05 16:52:11.000000000 -0400\n  $ git status\n  # On branch master\n  nothing to commit\n  $ stat .git/index | grep -i modify\n  Modify: 2010-07-05 16:53:09.000000000 -0400\n\nand it is not just updating some stat-dirtiness. Doing it over and over\nwill keep updating the index. It looks like we unconditionally do the\nlock and write in cmd_status, but I haven't looked further.\n\n> In any case, we should diagnose \"commit --short\" (and \"--procelain\") as an\n> error, perhaps by splitting option parsers for commit and status further.\n\nI don't think it's an error. \"commit --short\" implies \"commit --dry-run\n--short\", which is actually a useful thing (well, _I_ don't find it\nuseful, but I believe it was kept intentionally during the \"status is no\nlonger commit --dry-run\" conversion).\n\n-Peff\n"},{"id":"144848","messageId":"7vk4p9wrdb.fsf@alter.siamese.dyndns.org","threadId":"24274","inReplyTo":"20100705205651.GA32728@sigill.intra.peff.net","subject":"Re: 'git commit --short' without touching index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-05T21:28:16Z","receivedAt":"2010-07-05T21:28:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Jul 05, 2010 at 01:10:43PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > It would be nice if the index-refreshing code only wrote to the index if\n>> > there was something worth writing. I'm not sure how hard that would be\n>> > to implement, though.\n>> \n>> Hmm, don't we already do that with \"istate->cache_changed\"?\n>\n> Apparently not:\n>\n>   $ stat .git/index | grep -i modify\n>   Modify: 2010-07-05 16:52:11.000000000 -0400\n>   $ git status\n>   # On branch master\n>   nothing to commit\n>   $ stat .git/index | grep -i modify\n>   Modify: 2010-07-05 16:53:09.000000000 -0400\n>\n> and it is not just updating some stat-dirtiness. Doing it over and over\n> will keep updating the index. It looks like we unconditionally do the\n> lock and write in cmd_status, but I haven't looked further.\n\nSomething like this, plus possibly a similar fix to \"git commit $path\"\ncodepath, perhaps?\n\nWe may want to audit all uses of write_cache() and write_index() that are\nnot protected with active_cache_changed (or istate->cache_changed); I am\nreluctant to suggest placing that logic into write_index() at this point,\nthough, as we may be updating the index in bulk, without marking\nactive_cache_changed bit, exactly because we know we will unconditionally\nwrite the result out.\n\n builtin/commit.c |   10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c101f00..86d3926 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -343,9 +343,13 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n \t\trefresh_cache_or_die(refresh_flags);\n-\t\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t    commit_locked_index(&index_lock))\n-\t\t\tdie(\"unable to write new_index file\");\n+\t\tif (active_cache_changed) {\n+\t\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t\t    commit_locked_index(&index_lock))\n+\t\t\t\tdie(\"unable to write new_index file\");\n+\t\t} else {\n+\t\t\trollback_lock_file(&index_lock);\n+\t\t}\n \t\tcommit_style = COMMIT_AS_IS;\n \t\treturn get_index_file();\n \t}\n"},{"id":"144897","messageId":"20100706121022.GB15413@sigill.intra.peff.net","threadId":"24274","inReplyTo":"7vk4p9wrdb.fsf@alter.siamese.dyndns.org","subject":"Re: 'git commit --short' without touching index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-06T12:10:22Z","receivedAt":"2010-07-06T12:10:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 05, 2010 at 02:28:16PM -0700, Junio C Hamano wrote:\n\n> > Apparently not:\n> >\n> >   $ stat .git/index | grep -i modify\n> >   Modify: 2010-07-05 16:52:11.000000000 -0400\n> >   $ git status\n> >   # On branch master\n> >   nothing to commit\n> >   $ stat .git/index | grep -i modify\n> >   Modify: 2010-07-05 16:53:09.000000000 -0400\n> >\n> > and it is not just updating some stat-dirtiness. Doing it over and over\n> > will keep updating the index. It looks like we unconditionally do the\n> > lock and write in cmd_status, but I haven't looked further.\n> \n> Something like this, plus possibly a similar fix to \"git commit $path\"\n> codepath, perhaps?\n\nYeah, that looks sane, though the one for \"git status\" is actually:\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8258a16..5e578af 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1090,9 +1090,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \tfd = hold_locked_index(&index_lock, 0);\n \tif (0 <= fd) {\n-\t\tif (!write_cache(fd, active_cache, active_nr))\n+\t\tif (active_cache_changed &&\n+\t\t    !write_cache(fd, active_cache, active_nr))\n \t\t\tcommit_locked_index(&index_lock);\n-\t\trollback_lock_file(&index_lock);\n+\t\telse\n+\t\t\trollback_lock_file(&index_lock);\n \t}\n \n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n\nand I did confirm that it works (does not touch the index when nothing\nchanges, and does write out the index even for a stat-dirty entry).\n\n> We may want to audit all uses of write_cache() and write_index() that are\n> not protected with active_cache_changed (or istate->cache_changed); I am\n> reluctant to suggest placing that logic into write_index() at this point,\n> though, as we may be updating the index in bulk, without marking\n> active_cache_changed bit, exactly because we know we will unconditionally\n> write the result out.\n\nI don't think changing write_index() is a good idea. There are a lot of\ncalls, and I would rather have each one opt into this optimization than\nbreak them in a mass change.\n\nI was thinking that we might be able to simplify all of these lines into\na \"lock, write, and commit\" function that would either rollback or die\non failure, depending on a parameter. But there's often more complex\nlogic going on between those functions, so I don't think it would really\nsave us much.\n\n-Peff\n"}]}