Merge branch 'jc/send-pack-pipeline' into next

* jc/send-pack-pipeline:
  Documentation: illustrate send-pack pipeline.
  send-pack: fix pipeline.
  Fix 'git add' with .gitignore
  Revert "read_directory: show_both option."
  Add info about new test families (8 and 9) to t/README
  t5400 send-pack test: try a bit more nontrivial transfer.

Documentation/technical/send-pack-pipeline.txt

@@ -0,0 +1,112 @@
+git-send-pack
+=============
+
+Overall operation
+-----------------
+
+. Connects to the remote side and invokes git-receive-pack.
+
+. Learns what refs the remote has and what commit they point at.
+  Matches them to the refspecs we are pushing.
+
+. Checks if there are non-fast-forwards.  Unlike fetch-pack,
+  the repository send-pack runs in is supposed to be a superset
+  of the recipient in fast-forward cases, so there is no need
+  for want/have exchanges, and fast-forward check can be done
+  locally.  Tell the result to the other end.
+
+. Calls pack_objects() which generates a packfile and sends it
+  over to the other end.
+
+. If the remote side is new enough (v1.1.0 or later), wait for
+  the unpack and hook status from the other end.
+
+. Exit with appropriate error codes.
+
+
+Pack_objects pipeline
+---------------------
+
+This function gets one file descriptor (`out`) which is either a
+socket (over the network) or a pipe (local).  What's written to
+this fd goes to git-receive-pack to be unpacked.
+
+    send-pack ---> fd ---> receive-pack
+
+It somehow forks once, but does not wait for it.  I am not sure
+why.
+
+The forked child calls rev_list_generate() with that file
+descriptor (while the parent closes `out` -- the child will be
+the one that writes the packfile to the other end).
+
+    send-pack
+       |
+       rev-list-generate ---> fd ---> receive-pack
+
+
+Then rev-list-generate forks after creates a pipe; the child
+will become a pipeline "rev-list --stdin | pack-objects", which
+is the rev_list() function, while the parent feeds that pipeline
+the list of refs.
+
+    send-pack
+       |
+       rev-list-generate ---> fd ---> receive-pack
+          | ^ (pipe)
+	  v |
+         rev-list
+
+The child process, before calling rev-list, rearranges the file
+descriptors:
+
+. what it reads from rev-list-generate via pipe becomes the
+  stdin; this is to feed the upstream of the pipeline which will
+  be git-rev-list process.
+
+. what it writes to its stdout goes to the fd connected to
+  receive-pack.
+
+On the other hand, the parent process, before starting to feed
+the child pipeline, closes the reading side of the pipe and fd
+to receive-pack.
+
+    send-pack
+       |
+       rev-list-generate
+          |
+	  v [0]
+         rev-list [1] ---> receive-pack
+
+The parent then writes to the pipe and later closes it.  There
+is a commented out waitpid to wait for the rev-list side before
+it exits, I again do not understand why.
+
+The rev-list function further sets up a pipe and forks to run
+git-rev-list piped to git-pack-objects.  The child side, before
+exec'ing git-pack-objects, rearranges the file descriptors:
+
+. what it reads from the pipe becomes the stdin; this gets the
+  list of objects from the git-rev-list process.
+
+. its stdout is already connected to receive-pack, so what it
+  generates goes there.
+
+The parent process arranges its file descriptors before exec'ing
+git-rev-list:
+
+. its stdout is sent to the pipe to feed git-pack-objects.
+
+. its stdin is already connected to rev-list-generate and will
+  read the set of refs from it.
+
+
+    send-pack
+       |
+       rev-list-generate
+          |
+	  v [0]
+	  git-rev-list [1] ---> [0] git-pack-objects [1] ---> receive-pack
+
+
+

builtin-add.c

@@ -26,18 +26,9 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 	i = dir->nr;
 	while (--i >= 0) {
 		struct dir_entry *entry = *src++;
-		int how = match_pathspec(pathspec, entry->name, entry->len,
-					 prefix, seen);
-		/*
-		 * ignored entries can be added with exact match,
-		 * but not with glob nor recursive.
-		 */
-		if (!how ||
-		    (entry->ignored_entry && how != MATCHED_EXACTLY)) {
-			free(entry);
-			continue;
-		}
-		*dst++ = entry;
+		if (match_pathspec(pathspec, entry->name, entry->len,
+				   prefix, seen))
+			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
 
@@ -47,10 +38,20 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 		if (seen[i])
 			continue;
 
-		/* Existing file? We must have ignored it */
 		match = pathspec[i];
-		if (!match[0] || !lstat(match, &st))
+		if (!match[0])
 			continue;
+
+		/* Existing file? We must have ignored it */
+		if (!lstat(match, &st)) {
+			struct dir_entry *ent;
+
+			ent = dir_add_name(dir, match, strlen(match));
+			ent->ignored = 1;
+			if (S_ISDIR(st.st_mode))
+				ent->ignored_dir = 1;
+			continue;
+		}
 		die("pathspec '%s' did not match any files", match);
 	}
 }
@@ -62,8 +63,6 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec)
 
 	/* Set up the default git porcelain excludes */
 	memset(dir, 0, sizeof(*dir));
-	if (pathspec)
-		dir->show_both = 1;
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
 	if (!access(path, R_OK))
@@ -154,7 +153,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	if (show_only) {
 		const char *sep = "", *eof = "";
 		for (i = 0; i < dir.nr; i++) {
-			if (!ignored_too && dir.entries[i]->ignored_entry)
+			if (!ignored_too && dir.entries[i]->ignored)
 				continue;
 			printf("%s%s", sep, dir.entries[i]->name);
 			sep = " ";
@@ -168,16 +167,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		die("index file corrupt");
 
 	if (!ignored_too) {
-		int has_ignored = -1;
-		for (i = 0; has_ignored < 0 && i < dir.nr; i++)
-			if (dir.entries[i]->ignored_entry)
-				has_ignored = i;
-		if (0 <= has_ignored) {
+		int has_ignored = 0;
+		for (i = 0; i < dir.nr; i++)
+			if (dir.entries[i]->ignored)
+				has_ignored = 1;
+		if (has_ignored) {
 			fprintf(stderr, ignore_warning);
-			for (i = has_ignored; i < dir.nr; i++) {
-				if (!dir.entries[i]->ignored_entry)
+			for (i = 0; i < dir.nr; i++) {
+				if (!dir.entries[i]->ignored)
 					continue;
-				fprintf(stderr, "%s\n", dir.entries[i]->name);
+				fprintf(stderr, "%s", dir.entries[i]->name);
+				if (dir.entries[i]->ignored_dir)
+					fprintf(stderr, " (directory)");
+				fputc('\n', stderr);
 			}
 			fprintf(stderr,
 				"Use -f if you really want to add them.\n");

dir.c

@@ -260,13 +260,12 @@ int excluded(struct dir_struct *dir, const char *pathname)
 	return 0;
 }
 
-static void add_name(struct dir_struct *dir, const char *pathname, int len,
-		     int ignored_entry)
+struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
 {
 	struct dir_entry *ent;
 
 	if (cache_name_pos(pathname, len) >= 0)
-		return;
+		return NULL;
 
 	if (dir->nr == dir->alloc) {
 		int alloc = alloc_nr(dir->alloc);
@@ -274,11 +273,12 @@ static void add_name(struct dir_struct *dir, const char *pathname, int len,
 		dir->entries = xrealloc(dir->entries, alloc*sizeof(ent));
 	}
 	ent = xmalloc(sizeof(*ent) + len + 1);
-	ent->ignored_entry = ignored_entry;
+	ent->ignored = ent->ignored_dir = 0;
 	ent->len = len;
 	memcpy(ent->name, pathname, len);
 	ent->name[len] = 0;
 	dir->entries[dir->nr++] = ent;
+	return ent;
 }
 
 static int dir_exists(const char *dirname, int len)
@@ -316,7 +316,6 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
-			int ignored_entry;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||
@@ -325,12 +324,11 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 				continue;
 			len = strlen(de->d_name);
 			memcpy(fullname + baselen, de->d_name, len+1);
-			ignored_entry = excluded(dir, fullname);
-
-			if (!dir->show_both &&
-			    (ignored_entry != dir->show_ignored) &&
-			    (!dir->show_ignored || DTYPE(de) != DT_DIR))
-				continue;
+			if (excluded(dir, fullname) != dir->show_ignored) {
+				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+					continue;
+				}
+			}
 
 			switch (DTYPE(de)) {
 			struct stat st;
@@ -368,8 +366,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (check_only)
 				goto exit_early;
 			else
-				add_name(dir, fullname, baselen + len,
-					 ignored_entry);
+				dir_add_name(dir, fullname, baselen + len);
 		}
 exit_early:
 		closedir(fdir);

dir.h

@@ -13,8 +13,9 @@
 
 
 struct dir_entry {
-	unsigned ignored_entry : 1;
-	unsigned int len : 15;
+	unsigned int ignored : 1;
+	unsigned int ignored_dir : 1;
+	unsigned int len : 30;
 	char name[FLEX_ARRAY]; /* more */
 };
 
@@ -30,8 +31,7 @@ struct exclude_list {
 
 struct dir_struct {
 	int nr, alloc;
-	unsigned int show_both: 1,
-		     show_ignored:1,
+	unsigned int show_ignored:1,
 		     show_other_directories:1,
 		     hide_empty_directories:1;
 	struct dir_entry **entries;
@@ -57,5 +57,6 @@ extern void add_excludes_from_file(struct dir_struct *, const char *fname);
 extern void add_exclude(const char *string, const char *base,
 			int baselen, struct exclude_list *which);
 extern int file_exists(const char *);
+extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len);
 
 #endif

send-pack.c

@@ -58,7 +58,7 @@ static void exec_rev_list(struct ref *refs)
 /*
  * Run "rev-list --stdin | pack-objects" pipe.
  */
-static void rev_list(int fd, struct ref *refs)
+static void rev_list(struct ref *refs)
 {
 	int pipe_fd[2];
 	pid_t pack_objects_pid;
@@ -71,10 +71,8 @@ static void rev_list(int fd, struct ref *refs)
 		 * and writes to the original fd
 		 */
 		dup2(pipe_fd[0], 0);
-		dup2(fd, 1);
 		close(pipe_fd[0]);
 		close(pipe_fd[1]);
-		close(fd);
 		exec_pack_objects();
 		die("pack-objects setup failed");
 	}
@@ -85,7 +83,6 @@ static void rev_list(int fd, struct ref *refs)
 	dup2(pipe_fd[1], 1);
 	close(pipe_fd[0]);
 	close(pipe_fd[1]);
-	close(fd);
 	exec_rev_list(refs);
 }
 
@@ -111,7 +108,7 @@ static void rev_list_generate(int fd, struct ref *refs)
 		close(pipe_fd[0]);
 		close(pipe_fd[1]);
 		close(fd);
-		rev_list(fd, refs);
+		rev_list(refs);
 		die("rev-list setup failed");
 	}
 	if (rev_list_generate_pid < 0)

t/README

@@ -74,6 +74,8 @@ First digit tells the family:
 	5 - the pull and exporting commands
 	6 - the revision tree commands (even e.g. merge-base)
 	7 - the porcelainish commands concerning the working tree
+	8 - the porcelainish commands concerning forensics
+	9 - the git tools
 
 Second digit tells the particular command we are testing.
 

t/t3700-add.sh

@@ -51,4 +51,37 @@ test_expect_success \
 	 *) echo fail; git-ls-files --stage xfoo3; (exit 1);;
 	 esac'
 
+test_expect_success '.gitignore test setup' '
+	echo "*.ig" >.gitignore &&
+	mkdir c.if d.ig &&
+	>a.ig && >b.if &&
+	>c.if/c.if && >c.if/c.ig &&
+	>d.ig/d.if && >d.ig/d.ig
+'
+
+test_expect_success '.gitignore is honored' '
+	git-add . &&
+	! git-ls-files | grep "\\.ig"
+'
+
+test_expect_success 'error out when attempting to add ignored ones without -f' '
+	! git-add a.?? &&
+	! git-ls-files | grep "\\.ig"
+'
+
+test_expect_success 'error out when attempting to add ignored ones without -f' '
+	! git-add d.?? &&
+	! git-ls-files | grep "\\.ig"
+'
+
+test_expect_success 'add ignored ones with -f' '
+	git-add -f a.?? &&
+	git-ls-files --error-unmatch a.ig
+'
+
+test_expect_success 'add ignored ones with -f' '
+	git-add -f d.??/* &&
+	git-ls-files --error-unmatch d.ig/d.if d.ig/d.ig
+'
+
 test_done

t/t5400-send-pack.sh

@@ -8,38 +8,63 @@ test_description='See why rewinding head breaks send-pack
 '
 . ./test-lib.sh
 
-touch cpio-test
-test_expect_success 'working cpio' 'echo cpio-test | cpio -o > /dev/null'
-
-cnt='1'
+cnt=64
 test_expect_success setup '
+	test_tick &&
+	mkdir mozart mozart/is &&
+	echo "Commit #0" >mozart/is/pink &&
+	git-update-index --add mozart/is/pink &&
 	tree=$(git-write-tree) &&
 	commit=$(echo "Commit #0" | git-commit-tree $tree) &&
 	zero=$commit &&
 	parent=$zero &&
-	for i in $cnt
+	i=0 &&
+	while test $i -le $cnt
 	do
-	    sleep 1 &&
+	    i=$(($i+1)) &&
+	    test_tick &&
+	    echo "Commit #$i" >mozart/is/pink &&
+	    git-update-index --add mozart/is/pink &&
+	    tree=$(git-write-tree) &&
 	    commit=$(echo "Commit #$i" | git-commit-tree $tree -p $parent) &&
+	    git-update-ref refs/tags/commit$i $commit &&
 	    parent=$commit || return 1
 	done &&
 	git-update-ref HEAD "$commit" &&
-	git-clone -l ./. victim &&
+	git-clone ./. victim &&
 	cd victim &&
 	git-log &&
 	cd .. &&
 	git-update-ref HEAD "$zero" &&
 	parent=$zero &&
-	for i in $cnt
+	i=0 &&
+	while test $i -le $cnt
 	do
-	    sleep 1 &&
+	    i=$(($i+1)) &&
+	    test_tick &&
+	    echo "Rebase #$i" >mozart/is/pink &&
+	    git-update-index --add mozart/is/pink &&
+	    tree=$(git-write-tree) &&
 	    commit=$(echo "Rebase #$i" | git-commit-tree $tree -p $parent) &&
+	    git-update-ref refs/tags/rebase$i $commit &&
 	    parent=$commit || return 1
 	done &&
 	git-update-ref HEAD "$commit" &&
 	echo Rebase &&
 	git-log'
 
+test_expect_success 'pack the source repository' '
+	git repack -a -d &&
+	git prune
+'
+
+test_expect_success 'pack the destination repository' '
+	cd victim &&
+	git repack -a -d &&
+	git prune &&
+	cd ..
+'
+
 test_expect_success \
         'pushing rewound head should not barf but require --force' ' 
 	# should not fail but refuse to update.

t/test-lib.sh

@@ -96,6 +96,17 @@ test_count=0
 
 trap 'echo >&5 "FATAL: Unexpected exit with code $?"; exit 1' exit
 
+test_tick () {
+	if test -z "${test_tick+set}"
+	then
+		test_tick=432630000
+	else
+		test_tick=$(($test_tick + 60))
+	fi
+	GIT_COMMITTER_DATE=$test_tick
+	GIT_AUTHOR_DATE=$test_tick
+	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+}
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.