update-index --index-info producing spurious submodule commits

For reasons too complicated to go into, I have a repository B which has essentially been cloned from A, and there has been vast amounts of work on B (thousands of commits, many branches). These changes have not been merged back to A. I want to merge them back, but there's a directory foo that has changes in B that I can't release.

So, I ran filter-branch with an index filter

  found the merge base with A
  removed foo
  did ls-tree on foo from merge base
    and updated the index

The theory is to make each commit in B look like no changes to anything under foo, and otherwise the same.

After doing this, Richard noticed that the root tree of commits had a foo object, but that it was labeled a commit instead of a tree (but in fact it is a tree). He noticed because diffs looked like submodules.

I was able to produce a minimal test case, output below, script attached. The below output is with 1.7.5.4 on NetBSD/i386 (and /amd64). 1.7.6 (ubuntu/amd64) has the same problem.

So:

  Am I using "git update-index --index-info" wrong?

  Or is there a bug?

Thanks, Greg

Notice that "cat-file -p HEAD:" shows a tree before, and a commit afterwards:

+ git init
Initialized empty Git repository in /usr/home/gdt/GIT_TEST/.git/
+ mkdir foo
+ touch foo/bar
+ git add foo
+ git commit -minitial content
[master (root-commit) 6755919] initial content
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 foo/bar
+ git cat-file -p HEAD
tree 72d67e6de0599f72f1265c925316f91f78395787
author Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400
committer Greg Troxel <gdt@ir.bbn.com> 1313703545 -0400

initial content
+ git cat-file -p HEAD:
040000 tree d87cbcba0e2ede0752bdafc5938da35546803ba5	foo
+ git rm -r foo
rm 'foo/bar'
+ git ls-tree HEAD foo
040000 tree d87cbcba0e2ede0752bdafc5938da35546803ba5	foo
+ git ls-tree HEAD foo
+ git update-index --index-info
+ git diff --staged

Greg Troxel <gdt@ir.bbn.com> writes:

This --index-info definitely looks wrong, if "foo" is a directory, as the entries in the index are supposed to be either blobs or commits.

As "update-index --index-info" predates "submodule" by a few years or more, I wouldn't be surprised if the code didn't notice it was fed a wrong input and produced nonsensical result that happened to be a commit.

The command could just instead barf, saying the input is wrong, but the option was so low-level that it was deliberately written to accept and store anything you throw at it --- even when it is nonsensical for the version of plumbing, later updates to the data structure might have made it making sense, which was the way to ease development of the system.

By now, we should start enforcing more sanity on its input.

 builtin/update-index.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Junio C Hamano <gitster@pobox.com> writes:

In the man page for update-index in the --index-info section, what I'm doing seems to be covered by point 2, which specifically talks about output of ls-tree.

I realize the index is a data structure that has pairs of paths to blobs. But I also think of it as a representation of a tree object that would be referenced were one to commit (even if it isn't in that form yet). So I would argue that update-index with a tree should walk that tree and insert all the paths resulting from expansion into the index?

If what I'm doing is an abuse of update-index, do you or anyone else have a suggestion to make a directory in the index match a tree object? (I'm trying to use an index filter; it takes 11 hours to run filter-branch (5500 commits, 400K files in index, 800MB .git, ~2+GB working directory, tmpdir on NetBSD tmpfs (all in ram)).)

Thanks, Greg

Greg Troxel <gdt@ir.bbn.com> writes:

"ls-tree -r HEAD foo" is probably what you meant to say.

Junio C Hamano <gitster@pobox.com> writes:

Thanks very much for the clue - that works. The update-index documentation should probably say that only blobs (or perhaps commits intended to be submodules??) are acceptable, and perhaps say "ls-tree -r" instead of ls-tree. It could easily seem reasonable to someone to pass in a tree and expect that to result in that tree being logically in the index and to appear in the resulting commit.

For anyone else who is trying to do something like this, here's a revised script that (I think) correctly reverts a directory to another commit.

#!/bin/sh

if [ -d .git ]; then
   echo "existing .git"
   exit 1
fi

set -x

git init mkdir foo touch foo/bar git add foo git commit -m'initial content' git tag initial

touch foo/baz git add foo/baz git commit -m 'add baz'

git cat-file -p initial git cat-file -p initial: git rm --cached -r foo git ls-tree initial foo git ls-tree -r initial foo git ls-tree -r initial foo | git update-index --index-info

git diff git diff --staged git commit -m'munged foo'

git cat-file -p HEAD git cat-file -p HEAD:

Greg Troxel wrote:

Makes sense.  Please make it so.

By the way, for this particular application I wonder if something like

	git ls-files -z <dir> | git update-index -z --force-remove --stdin
	git read-tree --prefix=<dir>/ <tree>

would be easier.  Or a commit-filter. :)

	tree=$1
	shift
	tree=$(
		git ls-tree -z "$tree" |
		perl -0ne '
			chop;
			my ($info, $name) = split(/\t/, $_, 2);
			if ($name eq "<dir>") {
				printf("040000 tree <good tree>\t<dir>\0");
			} else {
				printf("%s\0", $_);
			}
		' |
		git mktree -z
	)
	git commit-tree "$tree" "$@"

Thanks, Jonathan

Junio C Hamano <gitster@pobox.com> writes:

The second paragraph needs a bit of clarification. What I meant to say was that the --index-info and its command line cousin --cacheinfo interfaces are designed to be used like using a hex editor on the disk block device to modify the file system in a random way, and just like a hex editor does not prevent you from writing a data to the disk that is not understood or misunderstood by the current filesystem implementations, ideally it should allow you to put data that is beyond the current design of the index, so that it can be used as a way to experiment while developing enhancements to the index further. That in fact was how I experimented with updates to the code to read from the index (in read-cache.c) in early days. Also they do not even look at the object name they are given, and that is very much deliberate---otherwise you cannot even stuff gitlinks in the index---and in general, the less sanity-checks we do in that interface, the better off we will be. After all we may someday start adding a tree entry in the index for a reason unknown to us today.

I am all for documenting that today's index holds only regular blobs (mode 100644), executable blobs (mode 100755), symlink blobs (mode 120000), and gitlinks (mode 160000), somewhere in the general part of the document not specific to these options, and also documenting that the result of the operation is undefined if anything outside the officially supported kinds of input is fed to --index-info/--cacheinfo.

Thanks.