{"thread":{"id":"28156","subject":"update-index --index-info producing spurious submodule commits","startedAt":"2011-08-18T21:53:15Z","lastAt":"2011-08-20T02:15:15Z","messageCount":7,"participants":["Greg Troxel","Junio C Hamano","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"173791","messageId":"rmivctuv12s.fsf@fnord.ir.bbn.com","threadId":"28156","inReplyTo":null,"subject":"update-index --index-info producing spurious submodule commits","fromName":"Greg Troxel","fromEmail":"gdt@ir.bbn.com","sentAt":"2011-08-18T21:53:15Z","receivedAt":"2011-08-18T21:53:15Z","isPatch":false,"sender":{"key":"gdt@ir.bbn.com","avatar":null},"body":"\nFor reasons too complicated to go into, I have a repository B which has\nessentially been cloned from A, and there has been vast amounts of work\non B (thousands of commits, many branches).   These changes have not\nbeen merged back to A.  I want to merge them back, but there's a\ndirectory foo that has changes in B that I can't release.\n\nSo, I ran filter-branch with an index filter\n\n  found the merge base with A\n  removed foo\n  did ls-tree on foo from merge base\n    and updated the index\n\nThe theory is to make each commit in B look like no changes to anything\nunder foo, and otherwise the same.\n\nAfter doing this, Richard noticed that the root tree of commits had a\nfoo object, but that it was labeled a commit instead of a tree (but in\nfact it is a tree).  He noticed because diffs looked like submodules.\n\nI was able to produce a minimal test case, output below, script\nattached.  The below output is with 1.7.5.4 on NetBSD/i386 (and /amd64).\n1.7.6 (ubuntu/amd64) has the same problem.\n\nSo:\n\n  Am I using \"git update-index --index-info\" wrong?\n\n  Or is there a bug?\n\nThanks,\nGreg\n\nNotice that \"cat-file -p HEAD:\" shows a tree before, and a commit\nafterwards:\n\n\n+ git init\nInitialized empty Git repository in /usr/home/gdt/GIT_TEST/.git/\n+ mkdir foo\n+ touch foo/bar\n+ git add foo\n+ git commit -minitial content\n[master (root-commit) 6755919] initial content\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 foo/bar\n+ git cat-file -p HEAD\ntree 72d67e6de0599f72f1265c925316f91f78395787\nauthor Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400\ncommitter Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400\n\ninitial content\n+ git cat-file -p HEAD:\n040000 tree d87cbcba0e2ede0752bdafc5938da35546803ba5\tfoo\n+ git rm -r foo\nrm 'foo/bar'\n+ git ls-tree HEAD foo\n040000 tree d87cbcba0e2ede0752bdafc5938da35546803ba5\tfoo\n+ git ls-tree HEAD foo\n+ git update-index --index-info\n+ git diff --staged\ndiff --git a/foo b/foo\nnew file mode 160000\nindex 0000000..d87cbcb\n--- /dev/null\n+++ b/foo\n@@ -0,0 +1 @@\n+Subproject commit d87cbcba0e2ede0752bdafc5938da35546803ba5\ndiff --git a/foo/bar b/foo/bar\ndeleted file mode 100644\nindex e69de29..0000000\n+ git commit -mmunged foo\n[master 3348447] munged foo\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 160000 foo\n delete mode 100644 foo/bar\n+ git cat-file -p HEAD\ntree 04fbd499dbd01afb3241d7f0af8171fde008bfe3\nparent 6755919e289665ec46d270672d29b594f992fa03\nauthor Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400\ncommitter Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400\n\nmunged foo\n+ git cat-file -p HEAD:\n160000 commit d87cbcba0e2ede0752bdafc5938da35546803ba5\tfoo\n\n\n\n\n#!/bin/sh\n\nif [ -d .git ]; then\n   echo \"existing .git\"\n   exit 1\nfi\n\nset -x\n\ngit init\nmkdir foo\ntouch foo/bar\ngit add foo\ngit commit -m'initial content'\ngit cat-file -p HEAD\ngit cat-file -p HEAD:\ngit rm -r foo\ngit ls-tree HEAD foo\ngit ls-tree HEAD foo | git update-index --index-info\ngit diff --staged\ngit commit -m'munged foo'\ngit cat-file -p HEAD\ngit cat-file -p HEAD:\n"},{"id":"173809","messageId":"7vd3g272tk.fsf@alter.siamese.dyndns.org","threadId":"28156","inReplyTo":"rmivctuv12s.fsf@fnord.ir.bbn.com","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T22:49:27Z","receivedAt":"2011-08-18T22:49:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Troxel <gdt@ir.bbn.com> writes:\n\n> git ls-tree HEAD foo\n> git ls-tree HEAD foo | git update-index --index-info\n\nThis --index-info definitely looks wrong, if \"foo\" is a directory, as the\nentries in the index are supposed to be either blobs or commits.\n\nAs \"update-index --index-info\" predates \"submodule\" by a few years or\nmore, I wouldn't be surprised if the code didn't notice it was fed a wrong\ninput and produced nonsensical result that happened to be a commit.\n\nThe command could just instead barf, saying the input is wrong, but the\noption was so low-level that it was deliberately written to accept and\nstore anything you throw at it --- even when it is nonsensical for the\nversion of plumbing, later updates to the data structure might have made\nit making sense, which was the way to ease development of the system.\n\nBy now, we should start enforcing more sanity on its input.\n\n builtin/update-index.c |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex a6a23fa..4b32bfe 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -220,6 +220,12 @@ static int process_path(const char *path)\n \treturn add_one_path(ce, path, len, &st);\n }\n \n+static int verify_mode(unsigned int mode)\n+{\n+\treturn (mode == 0160000 || mode == 0120000 ||\n+\t\tmode == 0100644 || mode == 0100755);\n+}\n+\n static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \t\t\t const char *path, int stage)\n {\n@@ -229,6 +235,9 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \tif (!verify_path(path))\n \t\treturn error(\"Invalid path '%s'\", path);\n \n+\tif (!verify_mode(mode))\n+\t\treturn error(\"Invalid mode '%o'\", mode);\n+\n \tlen = strlen(path);\n \tsize = cache_entry_size(len);\n \tce = xcalloc(1, size);\n"},{"id":"173824","messageId":"rmiliuq2qlg.fsf@fnord.ir.bbn.com","threadId":"28156","inReplyTo":"7vd3g272tk.fsf@alter.siamese.dyndns.org","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Greg Troxel","fromEmail":"gdt@ir.bbn.com","sentAt":"2011-08-19T00:27:07Z","receivedAt":"2011-08-19T00:27:07Z","isPatch":false,"sender":{"key":"gdt@ir.bbn.com","avatar":null},"body":"\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Greg Troxel <gdt@ir.bbn.com> writes:\n>\n>> git ls-tree HEAD foo\n>> git ls-tree HEAD foo | git update-index --index-info\n>\n> This --index-info definitely looks wrong, if \"foo\" is a directory, as the\n> entries in the index are supposed to be either blobs or commits.\n\nIn the man page for update-index in the --index-info section, what I'm\ndoing seems to be covered by point 2, which specifically talks about\noutput of ls-tree.\n\nI realize the index is a data structure that has pairs of paths to\nblobs.  But I also think of it as a representation of a tree object that\nwould be referenced were one to commit (even if it isn't in that form\nyet).  So I would argue that update-index with a tree should walk that\ntree and insert all the paths resulting from expansion into the index?\n\n> The command could just instead barf, saying the input is wrong, but the\n> option was so low-level that it was deliberately written to accept and\n> store anything you throw at it --- even when it is nonsensical for the\n> version of plumbing, later updates to the data structure might have made\n> it making sense, which was the way to ease development of the system.\n\nIf what I'm doing is an abuse of update-index, do you or anyone else\nhave a suggestion to make a directory in the index match a tree object?\n(I'm trying to use an index filter; it takes 11 hours to run\nfilter-branch (5500 commits, 400K files in index, 800MB .git, ~2+GB\nworking directory, tmpdir on NetBSD tmpfs (all in ram)).)\n\nThanks,\nGreg\n"},{"id":"173827","messageId":"7vpqk2593g.fsf@alter.siamese.dyndns.org","threadId":"28156","inReplyTo":"rmiliuq2qlg.fsf@fnord.ir.bbn.com","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-19T04:16:51Z","receivedAt":"2011-08-19T04:16:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Troxel <gdt@ir.bbn.com> writes:\n\n> If what I'm doing is an abuse of update-index, do you or anyone else\n> have a suggestion to make a directory in the index match a tree object?\n\n\"ls-tree -r HEAD foo\" is probably what you meant to say.\n"},{"id":"173847","messageId":"rmi39gxacp1.fsf@fnord.ir.bbn.com","threadId":"28156","inReplyTo":"7vpqk2593g.fsf@alter.siamese.dyndns.org","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Greg Troxel","fromEmail":"gdt@ir.bbn.com","sentAt":"2011-08-19T11:00:10Z","receivedAt":"2011-08-19T11:00:10Z","isPatch":false,"sender":{"key":"gdt@ir.bbn.com","avatar":null},"body":"\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Greg Troxel <gdt@ir.bbn.com> writes:\n>\n>> If what I'm doing is an abuse of update-index, do you or anyone else\n>> have a suggestion to make a directory in the index match a tree object?\n>\n> \"ls-tree -r HEAD foo\" is probably what you meant to say.\n\nThanks very much for the clue - that works.  The update-index\ndocumentation should probably say that only blobs (or perhaps commits\nintended to be submodules??) are acceptable, and perhaps say \"ls-tree\n-r\" instead of ls-tree.  It could easily seem reasonable to someone to\npass in a tree and expect that to result in that tree being logically in\nthe index and to appear in the resulting commit.\n\nFor anyone else who is trying to do something like this, here's a\nrevised script that (I think) correctly reverts a directory to another commit.\n\n#!/bin/sh\n\nif [ -d .git ]; then\n   echo \"existing .git\"\n   exit 1\nfi\n\nset -x\n\ngit init\nmkdir foo\ntouch foo/bar\ngit add foo\ngit commit -m'initial content'\ngit tag initial\n\ntouch foo/baz\ngit add foo/baz\ngit commit -m 'add baz'\n\ngit cat-file -p initial\ngit cat-file -p initial:\ngit rm --cached -r foo\ngit ls-tree initial foo\ngit ls-tree -r initial foo\ngit ls-tree -r initial foo | git update-index --index-info\n\ngit diff\ngit diff --staged\ngit commit -m'munged foo'\n\ngit cat-file -p HEAD\ngit cat-file -p HEAD:\n"},{"id":"173868","messageId":"7vk4a948to.fsf@alter.siamese.dyndns.org","threadId":"28156","inReplyTo":"7vd3g272tk.fsf@alter.siamese.dyndns.org","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-19T17:20:19Z","receivedAt":"2011-08-19T17:20:19Z","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> As \"update-index --index-info\" predates \"submodule\" by a few years or\n> more, I wouldn't be surprised if the code didn't notice it was fed a wrong\n> input and produced nonsensical result that happened to be a commit.\n>\n> The command could just instead barf, saying the input is wrong, but the\n> option was so low-level that it was deliberately written to accept and\n> store anything you throw at it --- even when it is nonsensical for the\n> version of plumbing, later updates to the data structure might have made\n> it making sense, which was the way to ease development of the system.\n\nThe second paragraph needs a bit of clarification. What I meant to say was\nthat the --index-info and its command line cousin --cacheinfo interfaces\nare designed to be used like using a hex editor on the disk block device\nto modify the file system in a random way, and just like a hex editor does\nnot prevent you from writing a data to the disk that is not understood or\nmisunderstood by the current filesystem implementations, ideally it should\nallow you to put data that is beyond the current design of the index, so\nthat it can be used as a way to experiment while developing enhancements\nto the index further. That in fact was how I experimented with updates to\nthe code to read from the index (in read-cache.c) in early days. Also they\ndo not even look at the object name they are given, and that is very much\ndeliberate---otherwise you cannot even stuff gitlinks in the index---and\nin general, the less sanity-checks we do in that interface, the better off\nwe will be. After all we may someday start adding a tree entry in the\nindex for a reason unknown to us today.\n\nI am all for documenting that today's index holds only regular blobs (mode\n100644), executable blobs (mode 100755), symlink blobs (mode 120000), and\ngitlinks (mode 160000), somewhere in the general part of the document not\nspecific to these options, and also documenting that the result of the\noperation is undefined if anything outside the officially supported kinds\nof input is fed to --index-info/--cacheinfo.\n\nThanks.\n"},{"id":"173906","messageId":"20110820021430.GA14281@elie.sbx02827.chicail.wayport.net","threadId":"28156","inReplyTo":"rmi39gxacp1.fsf@fnord.ir.bbn.com","subject":"Re: update-index --index-info producing spurious submodule commits","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-20T02:15:15Z","receivedAt":"2011-08-20T02:15:15Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Greg Troxel wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n\n>> \"ls-tree -r HEAD foo\" is probably what you meant to say.\n>\n> Thanks very much for the clue - that works.  The update-index\n> documentation should probably say that only blobs (or perhaps commits\n> intended to be submodules??) are acceptable, and perhaps say \"ls-tree\n> -r\" instead of ls-tree.\n\nMakes sense.  Please make it so.\n\nBy the way, for this particular application I wonder if something like\n\n\tgit ls-files -z <dir> | git update-index -z --force-remove --stdin\n\tgit read-tree --prefix=<dir>/ <tree>\n\nwould be easier.  Or a commit-filter. :)\n\n\ttree=$1\n\tshift\n\ttree=$(\n\t\tgit ls-tree -z \"$tree\" |\n\t\tperl -0ne '\n\t\t\tchop;\n\t\t\tmy ($info, $name) = split(/\\t/, $_, 2);\n\t\t\tif ($name eq \"<dir>\") {\n\t\t\t\tprintf(\"040000 tree <good tree>\\t<dir>\\0\");\n\t\t\t} else {\n\t\t\t\tprintf(\"%s\\0\", $_);\n\t\t\t}\n\t\t' |\n\t\tgit mktree -z\n\t)\n\tgit commit-tree \"$tree\" \"$@\"\n\nThanks,\nJonathan\n"}]}