From 04509738b5779c33e59b93e4959b0ac19acab838 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 29 Dec 2006 02:25:04 -0800 Subject: [PATCH 1/6] t5400 send-pack test: try a bit more nontrivial transfer. Not that this reveals anything new, but I did test_tick shell function in test-lib and found it rather cute and nice. Signed-off-by: Junio C Hamano --- t/t5400-send-pack.sh | 43 ++++++++++++++++++++++++++++++++++--------- t/test-lib.sh | 11 +++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh index 28744b35e1..2c151912a3 100755 --- a/t/t5400-send-pack.sh +++ b/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. diff --git a/t/test-lib.sh b/t/test-lib.sh index 98f69d89f3..bf108d4226 100755 --- a/t/test-lib.sh +++ b/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. From 8757749ecbabbcf7f1a68c17529f02bf7702b424 Mon Sep 17 00:00:00 2001 From: Jakub Narebski Date: Fri, 29 Dec 2006 14:39:09 +0100 Subject: [PATCH 2/6] Add info about new test families (8 and 9) to t/README Signed-off-by: Jakub Narebski Signed-off-by: Junio C Hamano --- t/README | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/README b/t/README index c5db5804df..7abab1dafe 100644 --- a/t/README +++ b/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. From c889763bf33e9b54843dc93ba3bb38803f86a6cf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 29 Dec 2006 10:08:19 -0800 Subject: [PATCH 3/6] Revert "read_directory: show_both option." This reverts commit 4888c534099012d71d24051deb5b14319747bd1a. --- dir.c | 19 +++++++------------ dir.h | 6 ++---- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/dir.c b/dir.c index dd188a8c56..8477472c03 100644 --- a/dir.c +++ b/dir.c @@ -260,8 +260,7 @@ 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) +static void add_name(struct dir_struct *dir, const char *pathname, int len) { struct dir_entry *ent; @@ -274,7 +273,6 @@ 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->len = len; memcpy(ent->name, pathname, len); ent->name[len] = 0; @@ -316,7 +314,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 +322,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 +364,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); + add_name(dir, fullname, baselen + len); } exit_early: closedir(fdir); diff --git a/dir.h b/dir.h index 08c6345472..c919727949 100644 --- a/dir.h +++ b/dir.h @@ -13,8 +13,7 @@ struct dir_entry { - unsigned ignored_entry : 1; - unsigned int len : 15; + int len; char name[FLEX_ARRAY]; /* more */ }; @@ -30,8 +29,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; From 4d06f8ac4389ed8bcd2d03b2dcab4b7e2a858402 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 29 Dec 2006 11:01:31 -0800 Subject: [PATCH 4/6] Fix 'git add' with .gitignore When '*.ig' is ignored, and you have two files f.ig and d.ig/foo in the working tree, $ git add . correctly ignored f.ig but failed to ignore d.ig/foo. This was caused by a thinko in an earlier commit 4888c534, when we tried to allow adding otherwise ignored files. After reverting that commit, this takes a much simpler approach. When we have an unmatched pathspec that talks about an existing pathname, we know it is an ignored path the user tried to add, so we include it in the set of paths directory walker returned. This does not let you say "git add -f D" on an ignored directory D and add everything under D. People can submit a patch to further allow it if they want to, but I think it is a saner behaviour to require explicit paths to be spelled out in such a case. Signed-off-by: Junio C Hamano --- builtin-add.c | 52 ++++++++++++++++++++++++++------------------------ dir.c | 8 +++++--- dir.h | 5 ++++- t/t3700-add.sh | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 29 deletions(-) diff --git a/builtin-add.c b/builtin-add.c index 8ed4a6a9f3..e7a1b4d9ab 100644 --- a/builtin-add.c +++ b/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"); diff --git a/dir.c b/dir.c index 8477472c03..0338d6c4e0 100644 --- a/dir.c +++ b/dir.c @@ -260,12 +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) +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); @@ -273,10 +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 = 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) @@ -364,7 +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); + dir_add_name(dir, fullname, baselen + len); } exit_early: closedir(fdir); diff --git a/dir.h b/dir.h index c919727949..7233d65bbd 100644 --- a/dir.h +++ b/dir.h @@ -13,7 +13,9 @@ struct dir_entry { - int len; + unsigned int ignored : 1; + unsigned int ignored_dir : 1; + unsigned int len : 30; char name[FLEX_ARRAY]; /* more */ }; @@ -55,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 diff --git a/t/t3700-add.sh b/t/t3700-add.sh index c09c53f20b..e98786de32 100755 --- a/t/t3700-add.sh +++ b/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 From e40a9e2c9ed0723088e0fb65deb9b430fc91c367 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 29 Dec 2006 02:35:40 -0800 Subject: [PATCH 5/6] send-pack: fix pipeline. send-pack builds a pipeline that runs "rev-list | pack-objects" and sends the output from pack-objects to the other side, while feeding the input side of that pipe from itself. However, the file descriptor that is given to this pipeline (so that it can be dup2(2)'ed into file descriptor 1 of pack-objects) is closed by the caller before the complex fork+exec dance! Worse yet, the caller already dup2's it to 1, so the child process did not even have to. I do not understand how this code could possibly have been working, but it somehow was working by accident. Merging the sliding mmap() code reveals this problem, presumably because it keeps one extra file descriptor open for a packfile and changes the way file descriptors are allocated. I am too tired to diagnose the problem now, but this seems to be a sensible fix. Signed-off-by: Junio C Hamano --- send-pack.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/send-pack.c b/send-pack.c index cc884f3b2d..54de96e40c 100644 --- a/send-pack.c +++ b/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) From b5ffa5ceef250ae57b9088ac1de22e783faf7ff8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 29 Dec 2006 12:14:30 -0800 Subject: [PATCH 6/6] Documentation: illustrate send-pack pipeline. Signed-off-by: Junio C Hamano --- .../technical/send-pack-pipeline.txt | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 Documentation/technical/send-pack-pipeline.txt diff --git a/Documentation/technical/send-pack-pipeline.txt b/Documentation/technical/send-pack-pipeline.txt new file mode 100644 index 0000000000..bd32aff00b --- /dev/null +++ b/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 + + +