[PATCH] Update gitweb.perl to current CGI.pm API
Hi all, This simple two-line patch fixes a bug that makes gitweb unusable on systems where the installed CGI.pm is version 4.04 or later (such as on Debian unstable). That version removed the startform method, which had previously been deprecated in favour of start_form since 2009. I don't have any specific tests for that change, but it does help fixing the testsuite of FusionForge (which includes a test of its Git plugin, involving gitweb). For reference, this is Debian bug #765525 (http://bugs.debian.org/765525). (I'm not subscribed to the git@vger mailing-list; please Cc me on replies.) Thanks, Roland. From 1b74cfb8568927a307f165e428455789398f6d61 Mon Sep 17 00:00:00 2001 From: Roland Mas lola...@debian.org Date: Thu, 16 Oct 2014 00:05:25 +0200 Subject: [PATCH] Update gitweb.perl to current CGI.pm API CGI.pm 4.04 removed the startform method, which had previously been deprecated in favour of start_form. Updated gitweb.perl accordingly. Signed-off-by: Roland Mas lola...@debian.org --- gitweb/gitweb.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a9f57d6..ccf7516 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4100,7 +4100,7 @@ sub print_search_form { if ($use_pathinfo) { $action .= /.esc_url($project); } - print $cgi-startform(-method = get, -action = $action) . + print $cgi-start_form(-method = get, -action = $action) . div class=\search\\n . (!$use_pathinfo $cgi-input({-name=p, -value=$project, -type=hidden}) . \n) . @@ -5510,7 +5510,7 @@ sub git_project_search_form { } print div class=\projsearch\\n; - print $cgi-startform(-method = 'get', -action = $my_uri) . + print $cgi-start_form(-method = 'get', -action = $my_uri) . $cgi-hidden(-name = 'a', -value = 'project_list') . \n; print $cgi-hidden(-name = 'pf', -value = $project_filter). \n if (defined $project_filter); -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fsck option to remove corrupt objects - why/why not?
On Thu, Oct 16, 2014 at 2:13 AM, Ben Aveling bena@optusnet.com.au wrote: On 14/10/2014 19:21, Jeff King wrote: On Mon, Oct 13, 2014 at 09:37:27AM +1100, Ben Aveling wrote: A question about fsck - is there a reason it doesn't have an option to delete bad objects? If the objects are reachable, then deleting them would create other big problems (i.e., we would be breaking the object graph!). The man page for fsck advises: Any corrupt objects you will have to find in backups or other archives (i.e., you can just remove them and do an /rsync/ with some other site in the hopes that somebody else has the object you have corrupted). And that seems sensible to me - the object is corrupt, it is unusable, the object graph is already broken, we already have big problems, removing the corrupt object(s) doesn't create any new problems, and it allows the possibility that the damaged objects can be restored. I ask because I have a corrupt repository, and every time I run fsck, it reports one corrupt object, then stops. I could write a script to repeatedly call fsck and then remove the next corrupt object, but it raises the question for me; could it make sense to extend fsck with the option to do to the removes? I am positive to this idea. Yesterday a colleague of mine came to me with a repo containing a single corrupt object (in a 1.2GB packfile). We were lucky, since we had a copy of the repo with a good copy of the same object. However, we were lucky in a couple of other respects, as well: I simply copied the packfile containing the good copy into the corrupted repo, and then ran a git gc, which happened to use the good copy of the corrupted object and complete successfully (instead of barfing on the bad copy). The GC then removed the old (now-obsolete) packfiles, and thus the corruption was gone. However, exactly _why_ git happened to prefer the good copy in my copied packfile instead of the bad copy in the existing packfile, I do not know. I suspect some amount of pure luck was involved. Indeed, I feared I would have to explode the corrupt pack, then manually replace the )(now-loose) bad copy with a good copy (from a similarly exploded pristine pack), and then finally repack everything again. That said, I'm not at all sure that Git would be able to successfully explode a pack containing corrupt objects... I think a better solution would be to tell fsck to remove the corrupt object(s), as you suggest above, and then copy in the good pack. In that case, there would be no question that the good copy would be used in the subsequent GC. Or even better, do the removes and then do the necessary [r]sync, assuming the user has another repository that has a good copy of the bad objects, which in this case I do. Hmm. I am not sure we want to automate the syncing step. First, git cannot know _which_ remote is likely to have a good copy of the bad object. Second, we do not necessarily know what caused the corruption in the first place, and whether syncing with a remote (which will create certain amount of write activity on a possibly dying disk drive) is a good idea at all. Finally, this syncing step will have to bypass Git's usual reachability analysis (which easily skips fetching a corrupt blob from otherwise-reachable history), is more involved than simply calling out to git fetch... ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git download --- Virus
Hi I downloaded and started to Install Git. There is a Virus on you setup. Program that appears to have trojan-like features or behavior. Git/bin/pdfinfo.exe trojan.generic.[variant], gen:trojan.[variant] Why??? Br. Risto -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git download --- Virus
On Thu, 16 Oct 2014 12:35:33 +0300 (EEST) risto.makini...@pp.inet.fi risto.makini...@pp.inet.fi wrote: I downloaded and started to Install Git. There is a Virus on you setup. Program that appears to have trojan-like features or behavior. Git/bin/pdfinfo.exe trojan.generic.[variant], gen:trojan.[variant] Why??? Because your antivirus software applies its (seemingly imperfect) heuristics and thinks there's a virus while there's none. To state this in a more blunt way: no there's no any virus in the Git for Windows installation package. The other possibility is you obtaining the installation package from a place other than http://git-scm.com or some malware active on your computer is changing the packages you're downloading on the fly. The latter is highly unlikely though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fsck option to remove corrupt objects - why/why not?
Ben Aveling bena@optusnet.com.au writes: And that seems sensible to me - the object is corrupt, it is unusable, the object graph is already broken, we already have big problems, removing the corrupt object(s) doesn't create any new problems, and it allows the possibility that the damaged objects can be restored. Removing completely may remove a chance to restore the corrupt object (rather unlikely, but I can imagine fine binary file surgery to un-break a broken object file). But we could move them out of Git's object directory (a bit like .git/lost-found, we could have .git/corrupt). For unpacked objects, it's trivial (just mv them in the directory). For packed objects, I don't know what happens in case they are corrupt. That would solve essentially any problem that you can solve by removing the file, but it makes the operation reversible. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fsck option to remove corrupt objects - why/why not?
On Thu, Oct 16, 2014 at 11:04:04AM +0200, Johan Herland wrote: I simply copied the packfile containing the good copy into the corrupted repo, and then ran a git gc, which happened to use the good copy of the corrupted object and complete successfully (instead of barfing on the bad copy). The GC then removed the old (now-obsolete) packfiles, and thus the corruption was gone. However, exactly _why_ git happened to prefer the good copy in my copied packfile instead of the bad copy in the existing packfile, I do not know. I suspect some amount of pure luck was involved. I'm not sure that it is luck, but more like 8eca0b4 (implement some resilience against pack corruptions, 2008-06-23) working as intended[1]. Generally, git should be able to warn about corrupted objects and look in other packs for them (both for regular operations, and for repacking). -Peff [1] That's just one of the many commits dealing with this. Try running git log --author=Nicolas.Pitre --grep=corrupt for more. :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fsck option to remove corrupt objects - why/why not?
On Thu, Oct 16, 2014 at 2:25 PM, Jeff King p...@peff.net wrote: On Thu, Oct 16, 2014 at 11:04:04AM +0200, Johan Herland wrote: I simply copied the packfile containing the good copy into the corrupted repo, and then ran a git gc, which happened to use the good copy of the corrupted object and complete successfully (instead of barfing on the bad copy). The GC then removed the old (now-obsolete) packfiles, and thus the corruption was gone. However, exactly _why_ git happened to prefer the good copy in my copied packfile instead of the bad copy in the existing packfile, I do not know. I suspect some amount of pure luck was involved. I'm not sure that it is luck, but more like 8eca0b4 (implement some resilience against pack corruptions, 2008-06-23) working as intended[1]. Generally, git should be able to warn about corrupted objects and look in other packs for them (both for regular operations, and for repacking). -Peff [1] That's just one of the many commits dealing with this. Try running git log --author=Nicolas.Pitre --grep=corrupt for more. :) Indeed, from reading the logs, it seems what I assumed was a lucky strike, was actually carefully designed behavior. With that in mind, I'm no longer so sure that fsck actually needs an option to remove corrupt objects. Instead, it's probably better to leave the corrupt object in place until a good copy can be located and copied into the repo, at which point Nicolas' brilliant work will make sure a simple repack takes care of fixing the corruption. That said, we should consider documenting this strategy for fixing corruptions: - Locate the a good copy of the affected objects in another repo - Copy relevant pack file or loose object into this repo - Run git gc - Profit! ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git describe oddity with GIT_DIR
Hi, I've encountered an oddity with git describe. Consider the following snippet: - mkdir test cd test git init echo 1 file git add file git commit -m changes $ git describe --always --dirty 8ad486e $ cd .. $ git --git-dir=test/.git describe --always --dirty 8ad486e-dirty $ GIT_DIR=test/.git git describe --always --dirty 8ad486e-dirty - The -dirty suffix appears if invoking git not from the worktree itself, but should actually never appear. According to my research this behaviour is there since 9f67d2e8 (Teach git describe --dirty option, 2009-10-27). Is that to expected? Thanks Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diff: Handle process substitution
For example: git diff --color-words (echo a b c) (echo a d c) Changes to struct diff_filespec: - is_stdin renamed to skip_hashing, which is what it did - follow_symlinks added, causing diff_populate_filespec to look at file contents instead of the readlink value Paths that are handled specially (using skip_hashing and follow_symlinks): /dev/stdin added as an alternative to - /dev/fd/* /proc/self/fd/* The first two are standard ways to refer to file descriptors, and there is precedence for handling them specially (bash redirections for example). The last one is there to support zsh process substitution, which on Linux uses an OS-specific path. --- Documentation/git-diff.txt | 7 +++ diff-no-index.c| 8 +++- diff.c | 26 +- diffcore.h | 8 +--- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index bbab35f..f4ca476 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -99,10 +99,17 @@ include::diff-options.txt[] path...:: The paths parameters, when given, are used to limit the diff to the named paths (you can give directory names and get diff for all files under them). + + + With --no-index, or with paths outside the worktree, + some paths are handled specially: - and /dev/stdin + refer to standard input, and /dev/fd/* can be used + to refer to existing file descriptors, as in: + + $ git diff --color-words (echo a b c) (echo a d c) include::diff-format.txt[] EXAMPLES diff --git a/diff-no-index.c b/diff-no-index.c index 265709b..586036b 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -70,11 +70,11 @@ static int populate_from_stdin(struct diff_filespec *s) s-should_munmap = 0; s-data = strbuf_detach(buf, size); s-size = size; s-should_free = 1; - s-is_stdin = 1; + s-skip_hashing = 1; return 0; } static struct diff_filespec *noindex_filespec(const char *name, int mode) { @@ -84,10 +84,16 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode) name = /dev/null; s = alloc_filespec(name); fill_filespec(s, null_sha1, 0, mode); if (name == file_from_standard_input) populate_from_stdin(s); + else if (!strcmp(name, /dev/stdin) || +!strncmp(name, /dev/fd/, 8) || +!strncmp(name, /proc/self/fd/, 14)) { + s-skip_hashing = 1; + s-follow_symlinks = 1; + } return s; } static int queue_diff(struct diff_options *o, const char *name1, const char *name2) diff --git a/diff.c b/diff.c index d7a5c81..c1150d7 100644 --- a/diff.c +++ b/diff.c @@ -2711,22 +2711,26 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags) reuse_worktree_file(s-path, s-sha1, 0)) { struct strbuf buf = STRBUF_INIT; struct stat st; int fd; - if (lstat(s-path, st) 0) { + if (s-follow_symlinks) + err = stat(s-path, st); + else + err = lstat(s-path, st); + if (err 0) { if (errno == ENOENT) { err_empty: err = -1; empty: s-data = (char *); s-size = 0; return err; } } s-size = xsize_t(st.st_size); - if (!s-size) + if (S_ISREG(st.st_mode) !s-size) goto empty; if (S_ISLNK(st.st_mode)) { struct strbuf sb = STRBUF_INIT; if (strbuf_readlink(sb, s-path, s-size)) @@ -2744,13 +2748,25 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags) return 0; } fd = open(s-path, O_RDONLY); if (fd 0) goto err_empty; - s-data = xmmap(NULL, s-size, PROT_READ, MAP_PRIVATE, fd, 0); + if (S_ISREG(st.st_mode)) { + s-data = xmmap(NULL, s-size, PROT_READ, MAP_PRIVATE, fd, 0); + s-should_munmap = 1; + } else { + struct strbuf sb = STRBUF_INIT; + if (strbuf_read(sb, fd, s-size) 0) { + err = error(error while reading from %s: %s, +s-path, strerror(errno)); + goto err_empty; + } + s-data = strbuf_detach(sb, s-size); + s-should_munmap = 0;
Re: [PATCH] clone: --dissociate option to mark that reference is only temporary
On 14-10-15 05:50 PM, Junio C Hamano wrote: Marc Branchaud marcn...@xiplink.com writes: Yes, but we're cloning gko, not the neighbour. Doesn't that mean that the clone operation won't know about any of the neighbour's refs? No. --reference (and a natural implementation of --borrow, I would imagine) peeks the refs of the repository we borrow from and that is how clone can say I already have objects reachable from these refs, so please send me the remainder to the repository it is cloning from. By know about I meant want to use. Sorry for being a bit dense about this; let me try again. (BTW, it occurs to me that your patch -- if I read it right -- doesn't fulfill your scenario since it disassociates the clone from all repos, regardless of whether they are specified with --reference or --borrow. In the following I assume a --borrow that only disassociates from the specified repo and leaves the --reference repo(s) alone.) Since we're cloning gko's refs, all of the neighbour's unique refs and objects can be ignored. Even though paths to the neighbour and the local pool will be in the clone's alternates file, any refs the clone operation creates won't need to use any objects from the neighbour. The clone operation gives us no way to refer to the neighbour's unique objects. I just don't see what difference the --borrow option makes. Consider the two cases: With just --reference=/local/pool/linux.git: 1. Set up the alternates file with that path. 2. Copy gko's refs into refs/remotes/origin/. 3. Set up refs/heads/master to refer to gko's HEAD. 4. Checkout refs/heads/master (uses objects from local pool). With both that --reference and --borrow=../my/neighbour/linux-hack.git: 1. Set up the alternates file with both paths. 2. Copy gko's refs into refs/remotes/origin/. 3. Set up refs/heads/master to refer to gko's HEAD. 4. Checkout refs/heads/master (uses objects from local pool). 5. Disassociate ourselves from the neighbour repo. In both cases the first four actions have no need of the neighbour repo. The second case's fifth action surgically removes the neighbour as an alternate object store, and we're left with the same clone we got in the first case. What was the point? It seems that in order to make something like --borrow useful, git clone would somehow need to know which of the neighbour's refs you want to *also* clone, then copy any unique objects from the neighbour before disassociating from it. M. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git log with follow author not working
I have relocated a file into another directory and committed that. Using the --follow command on the NEW path of the file, I want to find all commits to that file by a specific author: $ git log --follow --author david -- new/path/to/file.cpp When I do this, I get NO results. When I use the OLD path to the file, it works: $ git log --follow --author david -- OLD/path/to/file.cpp Also --follow seems to work fine on the NEW path if I do not specify --author. Is this a bug or am I using this command incorrectly? Follow up assistance is very much appreciated. Thanks guys. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fsck option to remove corrupt objects - why/why not?
Johan Herland jo...@herland.net writes: I simply copied the packfile containing the good copy into the corrupted repo, and then ran a git gc, which happened to use the good copy of the corrupted object and complete successfully (instead of barfing on the bad copy). The GC then removed the old (now-obsolete) packfiles, and thus the corruption was gone. However, exactly _why_ git happened to prefer the good copy in my copied packfile instead of the bad copy in the existing packfile, I do not know. By design ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git describe oddity with GIT_DIR
Thomas Braun thomas.br...@virtuell-zuhause.de writes: I've encountered an oddity with git describe. Consider the following snippet: - mkdir test cd test git init echo 1 file git add file git commit -m changes $ git describe --always --dirty 8ad486e $ cd .. $ git --git-dir=test/.git describe --always --dirty 8ad486e-dirty $ GIT_DIR=test/.git git describe --always --dirty 8ad486e-dirty - The -dirty suffix appears if invoking git not from the worktree itself, but should actually never appear. This is not oddity with describe. You are using --git-dir incorrectly. When you tell Git where its repository resides with the $GIT_DIR environment variable or the --git-dir command-line option, unless you tell it where the top-level of your working tree is, you are telling that your current working directory is the top-level of your working tree. You are asking git describe to describe the state of the HEAD including the dirtyness of the working tree in various ways. With the first invocation, you do not tell Git where things are and let it correctly figure it out, i.e. you are in 'test' directory and relative to where you are, .git is the repository and . is the top of the working tree. The commit recorded in the .git/HEAD, i.e. 8ad486e, is used, and its compared with the working tree to determine dirtiness. Specifically, the blob object 8ad486e:file is the same as ./file (that is test/file relative to where you started with mkdir test above). With the latter two, you are asking the same question but you go one level up and then tell Git that the repository is test/.git (correct) and the top of the working tree is . (a lie). Again, test/.git/HEAD records the same commit, but when trying to compare the contents of its tree, e.g. file at the top-level in the commit, you do not have file in the working tree. Git is led to believe that you removed file, hence your working tree state is dirty. Make it a habit to always specify GIT_WORK_TREE when you use GIT_DIR, unless you know you will always start from the top of the working tree. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/25] isxdigit: cast input to unsigned char
Jeff King p...@peff.net writes: Otherwise, callers must do so or risk triggering warnings -Wchar-subscript (and rightfully so; a signed char might cause us to use a bogus negative index into the hexval_table). While we are dropping the now-unnecessary casts from the caller in urlmatch.c, we can get rid of similar casts in actually parsing the hex by using the hexval() helper, which implicitly casts to unsigned (but note that we cannot implement isxdigit in terms of hexval(), as it also casts its return value to unsigned). Signed-off-by: Jeff King p...@peff.net --- The patch that added more calls to isxdigit later in the series actually got reworked. So this is purely a cleanup and can be dropped if need be, though I still think it is an improvement. Yes, thanks. git-compat-util.h | 2 +- urlmatch.c| 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index fb41118..44890d5 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -677,7 +677,7 @@ extern const unsigned char sane_ctype[256]; #define iscntrl(x) (sane_istest(x,GIT_CNTRL)) #define ispunct(x) sane_istest(x, GIT_PUNCT | GIT_REGEX_SPECIAL | \ GIT_GLOB_SPECIAL | GIT_PATHSPEC_MAGIC) -#define isxdigit(x) (hexval_table[x] != -1) +#define isxdigit(x) (hexval_table[(unsigned char)(x)] != -1) #define tolower(x) sane_case((unsigned char)(x), 0x20) #define toupper(x) sane_case((unsigned char)(x), 0) #define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC) diff --git a/urlmatch.c b/urlmatch.c index 3d4c54b..618d216 100644 --- a/urlmatch.c +++ b/urlmatch.c @@ -43,11 +43,11 @@ static int append_normalized_escapes(struct strbuf *buf, from_len--; if (ch == '%') { if (from_len 2 || - !isxdigit((unsigned char)from[0]) || - !isxdigit((unsigned char)from[1])) + !isxdigit(from[0]) || + !isxdigit(from[1])) return 0; - ch = hexval_table[(unsigned char)*from++] 4; - ch |= hexval_table[(unsigned char)*from++]; + ch = hexval(*from++) 4; + ch |= hexval(*from++); from_len -= 2; was_esc = 1; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/25] object_array: factor out slopbuf-freeing logic
Jeff King p...@peff.net writes: This is not a lot of code, but it's a logical construct that should not need to be repeated (and we are about to add a third repetition). Good, but I have two and a half tangential comments about the context that appears in this patch ;-) void object_array_filter(struct object_array *array, object_array_each_func_t want, void *cb_data) { @@ -367,8 +377,7 @@ void object_array_filter(struct object_array *array, objects[dst] = objects[src]; dst++; } else { - if (objects[src].name != object_array_slopbuf) - free(objects[src].name); + object_array_release_entry(objects[src]); } } array-nr = dst; @@ -400,8 +409,7 @@ void object_array_remove_duplicates(struct object_array *array) objects[array-nr] = objects[src]; array-nr++; } else { - if (objects[src].name != object_array_slopbuf) - free(objects[src].name); + object_array_release_entry(objects[src]); } } } 1. These two functions both remove elements from a given array in-place, the former being in a more generic form that takes a caller-specified criterion while the latter uses a hardcoded condition to decide what to filter. aeb4a51e (object_array: add function object_array_filter(), 2013-05-25) and later 1506510c (object_array_remove_duplicates(): rewrite to reduce copying, 2013-05-25) should have refactored the latter further to implement it in terms of the former, perhaps? 1.5 I would have expected a function to remove duplicates from an array to remove duplicates from the array by comparing the objects contained in the array, not entries that may (or may not) point at different objects but happens to share the same name; I think this function is misnamed. 2. We use object_array_remove_duplicates() to de-dup git bundle create x master master, which came from b2a6d1c6 (bundle: allow the same ref to be given more than once, 2009-01-17), which is still the sole caller of the function, and I think this is bogus. Comparing .name would not de-dup git bundle create x master refs/heads/master. I think the right way to fix these two and a half problems is to do the following: - object_array_remove_duplicates() (and contains_name() helper it uses) should be removed from object.c; - create_bundle() in bundle.c should implement a helper that is similar to contains_name() but knows about ref dwimming and use it to call object_array_filter() to replace its call to object_array_remove_duplicates(). I am not doing this myself, and I do not expect either you or Michael to do so, either. I am just writing this down to point out a low hanging fruit to aspiring new contributors (hint, hint). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/25] reachable: use traverse_commit_list instead of custom walk
Jeff King p...@peff.net writes: To find the set of reachable objects, we add a bunch of possible sources to our rev_info, call prepare_revision_walk, and then launch into a custom walker that handles each object top. This is a subset of what traverse_commit_list does, so we can just reuse that code (it can also handle more complex cases like UNINTERESTING commits and pathspecs, but we don't use those features). Signed-off-by: Jeff King p...@peff.net --- I was concerned this would be slower because traverse_commit_list is more featureful. To my surprise, it was consistently about 3-4% faster! The major difference is that traverse_commit_list will hit all of the commits first, and then the trees. For reachability that doesn't matter either way, but I suspect the new way has slightly better cache locality, leading to the minor speedup. I am not very surprised, as custom walk hasn't changed much ever since it was done in ba84a797 (builtin git prune, 2006-07-06), while the generic traversal code has been worked heavily while it was still in builtin-rev-list.c and then later moved to list-objects.c. reachable.c | 130 1 file changed, 17 insertions(+), 113 deletions(-) ;-) ;-) ;-) ;-) ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh
Brandon Turner bt at brandonturner.net writes: On Thu, Oct 9, 2014 at 5:11 PM, Junio C Hamano gitster at pobox.com wrote: Actually the patch was slightly wrong. It did not quite matter as cd '' is a no-op, but git -C '' cmd is not that lenient (which may be something we may want to fix) and breaks t9902 by exposing an existing breakage in the callchain. Here is a replacement. I did some more testing on this iteration as well - looks good. Sorry, for the late reply, guys... I've tested it too and it seems to work just fine. As for my comments about using $=2 instead of $2: When zsh runs through this code it does so while emulating ksh. Thus the reliance on splitting unquoted expansions is not a problem. I was unaware of this until ten minutes ago. Øsse -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 21/25] rev-list: add --index-objects option
Jeff King p...@peff.net writes: There is currently no easy way to ask the revision traversal machinery to include objects reachable from the index (e.g., blobs and trees that have not yet been committed). This patch adds an option to do so. Signed-off-by: Jeff King p...@peff.net --- I was tempted to call this just --index, because I could not think of what else --index would mean in the context of rev-list. But I also worried about weird interactions with other commands that take revision arguments. And since this is mostly for internal use anyway, I figured the more verbose name is not too bad. I could be convinced otherwise, though. I agree that --index is a bad name as it usually is used in a particular context: the command can work on various combination of working tree and the index, and I am asking it to work on both (e.g. apply --index as opposed to apply --cached). diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 4cf94c6..03ab343 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -172,6 +172,13 @@ explicitly. Pretend as if all objects mentioned by reflogs are listed on the command line as `commit`. +--index-objects:: This risks index getting misunderstood as a verb, e.g. please enumerate the objects and assign labels to later refer to them, doesn't it? --indexed-objects (short for --show-objects-in-the-index) or something? + Pretend as if all objects used by the index (any blobs, and any + trees which are mentioned by the index's cache-tree extension) + ad listed on the command line. Note that you probably want to s/ad/are/, probably? + use `--objects`, too, as there are by definition no commits in + the index. For gitlinks/submodules, the index records names of the commit objects, they are not listed, and that is the right behaviour, but this description invites some confusion. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git describe oddity with GIT_DIR
Am 16.10.2014 um 18:57 schrieb Junio C Hamano: Thomas Braun thomas.br...@virtuell-zuhause.de writes: I've encountered an oddity with git describe. Consider the following snippet: - mkdir test cd test git init echo 1 file git add file git commit -m changes $ git describe --always --dirty 8ad486e $ cd .. $ git --git-dir=test/.git describe --always --dirty 8ad486e-dirty $ GIT_DIR=test/.git git describe --always --dirty 8ad486e-dirty - The -dirty suffix appears if invoking git not from the worktree itself, but should actually never appear. This is not oddity with describe. You are using --git-dir incorrectly. When you tell Git where its repository resides with the $GIT_DIR environment variable or the --git-dir command-line option, unless you tell it where the top-level of your working tree is, you are telling that your current working directory is the top-level of your working tree. You are asking git describe to describe the state of the HEAD including the dirtyness of the working tree in various ways. With the first invocation, you do not tell Git where things are and let it correctly figure it out, i.e. you are in 'test' directory and relative to where you are, .git is the repository and . is the top of the working tree. The commit recorded in the .git/HEAD, i.e. 8ad486e, is used, and its compared with the working tree to determine dirtiness. Specifically, the blob object 8ad486e:file is the same as ./file (that is test/file relative to where you started with mkdir test above). With the latter two, you are asking the same question but you go one level up and then tell Git that the repository is test/.git (correct) and the top of the working tree is . (a lie). Again, test/.git/HEAD records the same commit, but when trying to compare the contents of its tree, e.g. file at the top-level in the commit, you do not have file in the working tree. Git is led to believe that you removed file, hence your working tree state is dirty. Make it a habit to always specify GIT_WORK_TREE when you use GIT_DIR, unless you know you will always start from the top of the working tree. Thanks a lot Junio for the in-depth explanation. I'll promise to do more research next time :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clone: --dissociate option to mark that reference is only temporary
Junio C Hamano gits...@pobox.com writes: Marc Branchaud marcn...@xiplink.com writes: I think things would be more understandable if the option was --dissociate repository and was an explicit alternative to --reference: [[--reference | --dissociate] repository] I'm still not liking the name --dissociate though. The original suggestion of --borrow is better. Perhaps --library or --local-cache? I dunno... I was not thinking when I originally started the topic with --borrow, until I realized that it would not make much sense, primarily because we allow multiple references. What should this command line do, and how would you implement such a behaviour? $ git clone \ --reference=/local/pool/linux.git \ --borrow=../my/neighbour/linux-hack.git \ git://git.kernel.org/./linux.git With do the usual --reference thing, but then dissociate the result from referents option, there is no ambiguity and that is why I did not go with the --borrow option suggested in the original thread. Another approach we could take is to add --borrow and then forbid mixing --reference and --borrow on the same command line, until somebody comes up with an implementation to allow us dissociate from borrowed repositories while still depending on referenced ones, at which time we can lift that limitation. But if that future extension is not going to happen, there is not much difference. The end result will be either - the one that is very clear to the users that they cannot selectively dissociate because there is no such option documented (i.e. --reference, --dissociate and no --borrow); and - the other one that gives a slight hope to the users that the combination might work (i.e. with --reference, --borrow and no --dissociate) but refuses to do so when it actually is run. Between the two, the former might actually be easier to the users to understand, as it keeps their expectation in line with the reality. So I dunno. I certainly am *not* going to do the selective dissociation change myself. Do we have a volunteer (hint: this probably does not fall into low-hanging fruit category)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] core.filemode may need manual action
core.filemode is set automatically when a repo is created. But when a repo is exported via CIFS or cygwin is mixed with Git for Windows core.filemode may better be set manually to false. Update and improve the documentation. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Torsten Bögershausen tbo...@web.de --- Does this reflect the discussion via email ? Or is more tweaking needed ? Documentation/config.txt | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 4333636..b4fea43 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -204,8 +204,23 @@ advice.*:: -- core.fileMode:: - If false, the executable bit differences between the index and - the working tree are ignored; useful on broken filesystems like FAT. + Tells Git if the executable bit of files in the working tree + is to be honored. + + Some filesystems lose the executable bit when a file that is + marked as executable is checked out, or checks out an + non-executable file with executable bit on. git init and + git clone probe the filesystem to see if it records + executable bit correctly when they create a new repository + and this variable is automatically set as necessary. + + A repository, however, may be on a filesystem that records + the filemode correctly, and this variable is set to 'true' + when created, but later may be made accessible from another + environment that loses the filemode (e.g. exporting ext4 via + CIFS mount, visiting a Cygwin managed repository with + MsysGit). In such a case, it may be necessary to set this + variable to 'false'. See linkgit:git-update-index[1]. + The default is true, except linkgit:git-clone[1] or linkgit:git-init[1] -- 2.0.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Update gitweb.perl to current CGI.pm API
Looks sensible to me; Jakub, ack? Roland Mas lola...@debian.org writes: Hi all, This simple two-line patch fixes a bug that makes gitweb unusable on systems where the installed CGI.pm is version 4.04 or later (such as on Debian unstable). That version removed the startform method, which had previously been deprecated in favour of start_form since 2009. I don't have any specific tests for that change, but it does help fixing the testsuite of FusionForge (which includes a test of its Git plugin, involving gitweb). For reference, this is Debian bug #765525 (http://bugs.debian.org/765525). (I'm not subscribed to the git@vger mailing-list; please Cc me on replies.) Thanks, Roland. From 1b74cfb8568927a307f165e428455789398f6d61 Mon Sep 17 00:00:00 2001 From: Roland Mas lola...@debian.org Date: Thu, 16 Oct 2014 00:05:25 +0200 Subject: [PATCH] Update gitweb.perl to current CGI.pm API CGI.pm 4.04 removed the startform method, which had previously been deprecated in favour of start_form. Updated gitweb.perl accordingly. Signed-off-by: Roland Mas lola...@debian.org --- gitweb/gitweb.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a9f57d6..ccf7516 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4100,7 +4100,7 @@ sub print_search_form { if ($use_pathinfo) { $action .= /.esc_url($project); } - print $cgi-startform(-method = get, -action = $action) . + print $cgi-start_form(-method = get, -action = $action) . div class=\search\\n . (!$use_pathinfo $cgi-input({-name=p, -value=$project, -type=hidden}) . \n) . @@ -5510,7 +5510,7 @@ sub git_project_search_form { } print div class=\projsearch\\n; - print $cgi-startform(-method = 'get', -action = $my_uri) . + print $cgi-start_form(-method = 'get', -action = $my_uri) . $cgi-hidden(-name = 'a', -value = 'project_list') . \n; print $cgi-hidden(-name = 'pf', -value = $project_filter). \n if (defined $project_filter); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] core.filemode may need manual action
Am 16.10.2014 um 21:29 schrieb Torsten Bögershausen: core.filemode is set automatically when a repo is created. But when a repo is exported via CIFS or cygwin is mixed with Git for Windows core.filemode may better be set manually to false. Update and improve the documentation. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Torsten Bögershausen tbo...@web.de --- Does this reflect the discussion via email ? Or is more tweaking needed ? Documentation/config.txt | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 4333636..b4fea43 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -204,8 +204,23 @@ advice.*:: -- core.fileMode:: - If false, the executable bit differences between the index and - the working tree are ignored; useful on broken filesystems like FAT. + Tells Git if the executable bit of files in the working tree + is to be honored. + + Some filesystems lose the executable bit when a file that is + marked as executable is checked out, or checks out an + non-executable file with executable bit on. git init and + git clone probe the filesystem to see if it records + executable bit correctly when they create a new repository + and this variable is automatically set as necessary. + + A repository, however, may be on a filesystem that records + the filemode correctly, and this variable is set to 'true' + when created, but later may be made accessible from another + environment that loses the filemode (e.g. exporting ext4 via + CIFS mount, visiting a Cygwin managed repository with + MsysGit). In such a case, it may be necessary to set this + variable to 'false'. See linkgit:git-update-index[1]. + The default is true, except linkgit:git-clone[1] or linkgit:git-init[1] [CC'ing msysgit aka git-for-windows/sdk for input] I'm not really happy with the term MsysGit here. Would it be too bold to say [... ] visiting a Cygwin managed repository with Git for Windows. ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] core.filemode may need manual action
Hi, On Thu, 16 Oct 2014, Thomas Braun wrote: Am 16.10.2014 um 21:29 schrieb Torsten Bögershausen: core.filemode is set automatically when a repo is created. But when a repo is exported via CIFS or cygwin is mixed with Git for Windows core.filemode may better be set manually to false. Update and improve the documentation. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Torsten Bögershausen tbo...@web.de --- Does this reflect the discussion via email ? Or is more tweaking needed ? Documentation/config.txt | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 4333636..b4fea43 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -204,8 +204,23 @@ advice.*:: -- core.fileMode:: - If false, the executable bit differences between the index and - the working tree are ignored; useful on broken filesystems like FAT. + Tells Git if the executable bit of files in the working tree + is to be honored. + + Some filesystems lose the executable bit when a file that is + marked as executable is checked out, or checks out an + non-executable file with executable bit on. git init and + git clone probe the filesystem to see if it records + executable bit correctly when they create a new repository + and this variable is automatically set as necessary. + + A repository, however, may be on a filesystem that records + the filemode correctly, and this variable is set to 'true' + when created, but later may be made accessible from another + environment that loses the filemode (e.g. exporting ext4 via + CIFS mount, visiting a Cygwin managed repository with + MsysGit). In such a case, it may be necessary to set this + variable to 'false'. See linkgit:git-update-index[1]. + The default is true, except linkgit:git-clone[1] or linkgit:git-init[1] [CC'ing msysgit aka git-for-windows/sdk for input] I'm not really happy with the term MsysGit here. Would it be too bold to say [... ] visiting a Cygwin managed repository with Git for Windows. ? I agree that msysGit is the wrong term. Not only is it about to be replaced by the Git for Windows SDK, it is *actively* wrong because msysGit is just the *development environment* to build Git for Windows. Most users do *not* need msysGit at all. Ciao, Dscho
Re: [PATCH] core.filemode may need manual action
Torsten Bögershausen tbo...@web.de writes: core.filemode is set automatically when a repo is created. But when a repo is exported via CIFS or cygwin is mixed with Git for Windows core.filemode may better be set manually to false. Update and improve the documentation. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Torsten Bögershausen tbo...@web.de --- Does this reflect the discussion via email ? Or is more tweaking needed ? Documentation/config.txt | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 4333636..b4fea43 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -204,8 +204,23 @@ advice.*:: -- core.fileMode:: - If false, the executable bit differences between the index and - the working tree are ignored; useful on broken filesystems like FAT. + Tells Git if the executable bit of files in the working tree + is to be honored. + + Some filesystems lose the executable bit when a file that is + marked as executable is checked out, or checks out an + non-executable file with executable bit on. git init and + git clone probe the filesystem to see if it records + executable bit correctly when they create a new repository + and this variable is automatically set as necessary. + + A repository, however, may be on a filesystem that records + the filemode correctly, and this variable is set to 'true' + when created, but later may be made accessible from another + environment that loses the filemode (e.g. exporting ext4 via + CIFS mount, visiting a Cygwin managed repository with + MsysGit). In such a case, it may be necessary to set this + variable to 'false'. See linkgit:git-update-index[1]. + The default is true, except linkgit:git-clone[1] or linkgit:git-init[1] I suspect that the above will not format very well. Hint: what is the lone + line before The default is true... doing there? Aside from Is MsysGit the old name for Git for Windows? raised by others, I think it may be worthwhile to mention Eclipse, as that is where the original (by the way, it would have been nice if you left some pointer to the original discussion when saying the discussion via email---it took me a while to recall what you are talking about) from Hilco Wijbenga was about ([$gmane/257689]). So, perhaps s/with MsysGit/with Eclipse or Git for Windows/; or something. Other than that, I do not see anything wrong in there. Thanks. As a separate topic, however, we may want to start thinking about adding a cheat-sheet on platform-specific bits to our documentation. The alphabetical listing of configuration variables we see here is a very good way for people to go from variable names to what they do (e.g. find a variable defined in configuration file of a neighbour or be instructed to set a variable by project lead and want to learn why setting the variable to the value is a good idea), but not a good way to go in the other direction (e.g. have trouble running Git on a filesystem that mangles filenames and want to find out if there already is a way to work it around). The cheat-sheet could be just a list of configuration and environment variables e.g. Those on Windows may want to check into these settings. [Reference] $gmane/257689: http://thread.gmane.org/gmane.comp.version-control.git/257558/focus=257689 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Update gitweb.perl to current CGI.pm API
Jakub Narębski jna...@gmail.com writes: On Thu, Oct 16, 2014 at 9:36 PM, Junio C Hamano gits...@pobox.com wrote: Looks sensible to me; Jakub, ack? Ack. Nb. this code follows back to original gitweb.cgi by Kay Sievers, very early in the development (2005) Thanks. I realize that Ack sounds as if Yeah, I acknowledge and admit I was in the wrong earlier, but I didn't mean I think this is your bug---do you think it is a good fix? I just meant the latter half of that sentence. Here is what I'll queue (note: I've retitled so that an entry in git shortlog would mean something). Thanks, both. -- 8 -- From: Roland Mas lola...@debian.org Subject: [PATCH] gitweb: use start_form, not startform that was removed in CGI.pm 4.04 CGI.pm 4.04 removed the startform method, which had previously been deprecated in favour of start_form. Changes file for CGI.pm says: 4.04 2014-09-04 [ REMOVED / DEPRECATIONS ] - startform and endform methods removed (previously deprecated, you should be using the start_form and end_form methods) Signed-off-by: Roland Mas lola...@debian.org Reviewed-by: Jakub Narębski jna...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- gitweb/gitweb.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a9f57d6..ccf7516 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4100,7 +4100,7 @@ sub print_search_form { if ($use_pathinfo) { $action .= /.esc_url($project); } - print $cgi-startform(-method = get, -action = $action) . + print $cgi-start_form(-method = get, -action = $action) . div class=\search\\n . (!$use_pathinfo $cgi-input({-name=p, -value=$project, -type=hidden}) . \n) . @@ -5510,7 +5510,7 @@ sub git_project_search_form { } print div class=\projsearch\\n; - print $cgi-startform(-method = 'get', -action = $my_uri) . + print $cgi-start_form(-method = 'get', -action = $my_uri) . $cgi-hidden(-name = 'a', -value = 'project_list') . \n; print $cgi-hidden(-name = 'pf', -value = $project_filter). \n if (defined $project_filter); -- 2.1.2-561-gc401a55 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] Consolidate ref parsing code
Michael Haggerty mhag...@alum.mit.edu writes: This is a rif on Duy's oldish patch series [1]. I started reviewing his patch series, but found that some of his patches did multiple things, making it harder to review. I started pulling it apart into smaller changes to aid my review, and I guess I got carried away :-/ Hmmm... You are aware of another large change in flight in the same area, and after having reviewed that series a few times I was hoping that you would have a better understanding of how ready the other topic is. And then I see this series that conflicts with that other topic. Is this your way to say that the other topic is not quite ready? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
On Wed, Oct 15, 2014 at 08:57:20PM +0200, Jens Lehmann wrote: Am 15.10.2014 um 00:15 schrieb Max Kirillov: I think the logic can be simple: it a submodule is not checked-out in the repository checkout --to is called from, then it is not checked-out to the new one also. If it is, then checkout calls itself recursively in the submodule and works like being run in standalone repository. But when I later decide to populate the submodule in a checkout --to work tree, should it automagically also use the central storage, creating the modules/name directory there if it doesn't exist yet? I think that'd make sense to avoid having the work tree layout depend on the order commands were ran in. And imagine new submodules, they should not be handled differently from those already present. Like place the common directory to $MAIN_REPO/.git/modules/$SUB/ and worktree-specific part to $MAIN_REPO/.git/worktrees/$WORKTREE/modules/$SUB, rather than placing all into the socond one? It would make sense to make, but then it would be imposible to checkout a diferent repository into the same submodule in different superproject checkouts. However stupid is sounds, there could be cases if, for example, at some moment submodule is being replaced by another one, and older worktrees should work with older submodule, while newer uses the newer submodule. Maybe, there could be some options to tell the command which populates submodules (which commands that are? submodule update and other submodule subcommands? or there is something else?) to use the curent checkout space or the main one. But I would still leave it depend on what user explicitly calls and where the initial submodule update is executed. Also, could you clarify the usage of the /modules/ directory. I did not notice it to affect anything after the submofule is placed there. Submodule operations use the submodule repositories directly (through the git link, which can point anywhere), or in .gitmodules file, or maybe in .git/config. So there is actually no need to have that gitdir there. Is it correct? -- Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
Somewhere in this series the object enumeration seems to get broken. git clone --no-local of git.git repository died with Cloning into 'victim-7'... remote: Counting objects: 173727, done. remote: Compressing objects: 100% (43697/43697), done. remote: Total 173727 (delta 130908), reused 170009 (delta 128151) Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s, done. Resolving deltas: 100% (130908/130908), done. fatal: did not receive expected object a74380c3117994efba501df1707418eb6feb9284 fatal: index-pack failed where a74380c... is such an object. If you have a clone of https://github.com/git/git.git $ git rev-list --parents --objects --all | grep a74380c3117994 would be an easy way to reproduce. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
Junio C Hamano gits...@pobox.com writes: Somewhere in this series the object enumeration seems to get broken. git clone --no-local of git.git repository died with Cloning into 'victim-7'... remote: Counting objects: 173727, done. remote: Compressing objects: 100% (43697/43697), done. remote: Total 173727 (delta 130908), reused 170009 (delta 128151) Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s, done. Resolving deltas: 100% (130908/130908), done. fatal: did not receive expected object a74380c3117994efba501df1707418eb6feb9284 fatal: index-pack failed where a74380c... is such an object. Ehh, such is a nonsense. It is a blob that directly is pointed by a tag that is in refs/tags/*. If you have a clone of https://github.com/git/git.git $ git rev-list --parents --objects --all | grep a74380c3117994 would be an easy way to reproduce. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
On Thu, Oct 16, 2014 at 02:07:47PM -0700, Junio C Hamano wrote: Somewhere in this series the object enumeration seems to get broken. git clone --no-local of git.git repository died with Cloning into 'victim-7'... remote: Counting objects: 173727, done. remote: Compressing objects: 100% (43697/43697), done. remote: Total 173727 (delta 130908), reused 170009 (delta 128151) Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s, done. Resolving deltas: 100% (130908/130908), done. fatal: did not receive expected object a74380c3117994efba501df1707418eb6feb9284 fatal: index-pack failed where a74380c... is such an object. If you have a clone of https://github.com/git/git.git Hrm. I cannot reproduce the clone failure... $ git rev-list --parents --objects --all | grep a74380c3117994 would be an easy way to reproduce. But this I can. Digging into it... -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
On Thu, Oct 16, 2014 at 05:21:12PM -0400, Jeff King wrote: Hrm. I cannot reproduce the clone failure... Oh, because I have bitmaps turned on, and we do not run the list-objects code at all in that case. The bug unsurprisingly bisects to traverse_commit_list: support pending blobs/trees with paths. The problem is that I didn't notice that handle_commit actually rewrites the object pointer when unwrapping the tags, and that commit reuses the original pointer from the entry. So we end up putting two copies of the tag object into the pending list, rather than the tag and the blob. The fix is: diff --git a/revision.c b/revision.c index 9a0f99a..8030fc8 100644 --- a/revision.c +++ b/revision.c @@ -231,12 +231,6 @@ static void add_pending_object_with_mode(struct rev_info *revs, add_pending_object_with_path(revs, obj, name, mode, NULL); } -static void add_pending_object_from_entry(struct rev_info *revs, - struct object_array_entry *e) -{ - add_pending_object_with_path(revs, e-item, e-name, e-mode, e-path); -} - void add_pending_object(struct rev_info *revs, struct object *obj, const char *name) { @@ -283,6 +277,8 @@ static struct commit *handle_commit(struct rev_info *revs, { struct object *object = entry-item; const char *name = entry-name; + const char *path = entry-path; + unsigned int mode = entry-mode; unsigned long flags = object-flags; /* @@ -301,6 +297,14 @@ static struct commit *handle_commit(struct rev_info *revs, die(bad object %s, sha1_to_hex(tag-tagged-sha1)); } object-flags |= flags; + /* +* We'll handle the tagged object by looping or dropping +* through to the non-tag handlers below. Do not +* propagate data from the tag's pending entry. +*/ + name = NULL; + path = NULL; + mode = 0; } /* @@ -332,7 +336,7 @@ static struct commit *handle_commit(struct rev_info *revs, mark_tree_contents_uninteresting(tree); return NULL; } - add_pending_object_from_entry(revs, entry); + add_pending_object_with_path(revs, object, name, mode, path); return NULL; } @@ -344,7 +348,7 @@ static struct commit *handle_commit(struct rev_info *revs, return NULL; if (flags UNINTERESTING) return NULL; - add_pending_object_from_entry(revs, entry); + add_pending_object_with_path(revs, object, name, mode, path); return NULL; } die(%s is unknown object, name); We should probably add a test for cloning a tag of an otherwise unreferenced object, too. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Oct 2014, #04; Thu, 16)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * bw/trace-no-inline-getnanotime (2014-09-29) 1 commit (merged to 'next' on 2014-10-14 at 19facbb) + trace.c: do not mark getnanotime() as inline No file-scope static variables in an inlined function, please. * jc/completion-no-chdir (2014-10-09) 1 commit (merged to 'next' on 2014-10-14 at 1cf12e1) + completion: use git -C $there instead of (cd $there git ...) * po/everyday-doc (2014-10-10) 3 commits (merged to 'next' on 2014-10-13 at daf1d03) + doc: add 'everyday' to 'git help' + doc: Makefile regularise OBSOLETE_HTML list building + doc: modernise everyday.txt wording and format in man page style git help everyday to show the Everyday Git document. -- [New Topics] * da/mergetool-temporary-directory (2014-10-16) 2 commits - t7610-mergetool: add test cases for mergetool.writeToTemp - mergetool: add an option for writing to a temporary directory (this branch uses da/mergetool-temporary-filename and da/mergetool-tests; is tangled with da/mergetool-tool-help.) Allow a temporary directory specified to be used while running git mergetool backend. Will merge to 'next'. * da/mergetool-tests (2014-10-16) 4 commits - test-lib-functions: adjust style to match CodingGuidelines - t7610-mergetool: use test_config to isolate tests - t7610-mergetool: add missing and remove commented-out code - t7610-mergetool: use tabs instead of a mix of tabs and spaces (this branch is used by da/mergetool-temporary-directory and da/mergetool-temporary-filename; is tangled with da/mergetool-tool-help.) The clean-up of this test script was long overdue and is a very welcome change. Will merge to 'next'. * bc/asciidoctor (2014-10-15) 2 commits - Documentation: implement linkgit macro for Asciidoctor - Documentation: move some AsciiDoc parameters into variables (this branch uses bc/asciidoc.) Add machinery to alternatively use AsciiDoctor to format our documentation. Will merge to 'next'. * da/mergetool-meld (2014-10-16) 1 commit - mergetools/meld: make usage of `--output` configurable and more robust Newer versions of 'meld' breaks the auto-detection we use to see if they are new enough to support the `--output` option. Will merge to 'next'. * rm/gitweb-start-form (2014-10-16) 1 commit - gitweb: use start_form, not startform that was removed in CGI.pm 4.04 Will merge to 'next'. * ss/contrib-subtree-contacts (2014-10-15) 2 commits - contacts: add a Makefile to generate docs and install - subtree: add an install-html target Will merge to 'next'. -- [Stalled] * je/quiltimport-no-fuzz (2014-09-26) 2 commits - git-quiltimport: flip the default not to allow fuzz - git-quiltimport.sh: allow declining fuzz with --exact option quiltimport drove git apply always with -C1 option to reduce context of the patch in order to give more chance to somewhat stale patches to apply. Add an --exact option to disable, and also -C$n option to customize this behaviour. The top patch optionally flips the default to --exact. Waiting for an Ack. * eb/no-pthreads (2014-10-13) 2 commits - pack-objects: set number of threads before checking and warning - index-pack: fix compilation with NO_PTHREADS Allow us build with NO_PTHREADS=NoThanks compilation option. The last change (not queued) needs a bit more explanation in its log message. * tr/remerge-diff (2014-09-08) 8 commits - log --remerge-diff: show what the conflict resolution changed - name-hash: allow dir hashing even when !ignore_case - merge-recursive: allow storing conflict hunks in index - merge_diff_mode: fold all merge diff variants into an enum - combine-diff: do not pass revs-dense_combined_merges redundantly - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: internal flag to avoid touching the worktree - merge-recursive: remove dead conditional in update_stages() log -p output learns a new way to let users inspect a merge commit by showing the differences between the automerged result with conflicts the person who recorded the merge would have seen and the final conflict resolution that was recorded in the merge. Waiting for a reroll ($gmane/256591). * hv/submodule-config (2014-06-30) 4 commits - do not die on error of parsing fetchrecursesubmodules option - use new config API for worktree configurations of submodules - extract functions for submodule config set and lookup - implement submodule config cache for lookup of submodule names Kicked back to 'pu' per request
Re: [PATCH v2 00/11] Consolidate ref parsing code
Junio C Hamano gits...@pobox.com writes: Michael Haggerty mhag...@alum.mit.edu writes: This is a rif on Duy's oldish patch series [1]. I started reviewing his patch series, but found that some of his patches did multiple things, making it harder to review. I started pulling it apart into smaller changes to aid my review, and I guess I got carried away :-/ Hmmm... You are aware of another large change in flight in the same area, and after having reviewed that series a few times I was hoping that you would have a better understanding of how ready the other topic is. And then I see this series that conflicts with that other topic. Is this your way to say that the other topic is not quite ready? The last one was a rhetorical question and I regret that it came out a bit harsher than I intended to. I just wanted to see a bit better inter-developer coordination, that's all. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
Jeff King p...@peff.net writes: On Thu, Oct 16, 2014 at 05:21:12PM -0400, Jeff King wrote: Hrm. I cannot reproduce the clone failure... Oh, because I have bitmaps turned on, and we do not run the list-objects code at all in that case. ;-) We should probably add a test for cloning a tag of an otherwise unreferenced object, too. Yeah, that too ;-) Thanks for a quick diag. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
From: brian m. carlson sand...@crustytoothpaste.net This series is designed to implement the changes necessary to build Git using Asciidoctor instead of AsciiDoc. [..] Even with these patches, Asciidoctor warns about everyday.txt and user-manual.txt. I'm not sending patches for these right now because I've seen recent series including those and don't want to cause a merge conflict. Does the new version for giteveryday.txt and everyday.txt which graduated to master, 1cb3324 (Merge branch 'po/everyday-doc', 2014-10-16) format OK? i.e. does 'git help everyday' now correct the Asciidoctor warnings. I don't have access to Asciidoctor (on MsysGit), but did make sure the header underlines were updated. -- Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] Consolidate ref parsing code
On 10/16/2014 10:47 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: This is a rif on Duy's oldish patch series [1]. I started reviewing his patch series, but found that some of his patches did multiple things, making it harder to review. I started pulling it apart into smaller changes to aid my review, and I guess I got carried away :-/ Hmmm... You are aware of another large change in flight in the same area, and after having reviewed that series a few times I was hoping that you would have a better understanding of how ready the other topic is. And then I see this series that conflicts with that other topic. Is this your way to say that the other topic is not quite ready? No, not at all. To be honest, I thought that the changes in this patch series were localized in an area different than Ronnie and Jonathan's patches were touching, but stupidly I didn't check for conflicts. Sorry about that. There is nothing urgent in this patch series, so I suggest we just put it back on ice and I will reroll after the dust has settled on rs/ref-transactions. The conflicts shouldn't be super hard to resolve (the series don't overlap much *conceptually*), but I'd rather not have to do it multiple times. Regarding rs/ref-transaction, I will reply on that thread. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v23 0/25] rs/ref-transaction (Use ref transactions, part 3)
On 10/15/2014 02:45 AM, Jonathan Nieder wrote: This series by Ronnie was last seen on-list at [1]. It includes some improvements to the ref-transaction API, improves handling of poorly named refs (which should make it easier to tighten the refname format checks in the future), and is a stepping stone toward later series that use the ref-transaction API more and make it support alternate backends (yay!). The changes since (a merge of 'master' and) v22 are very minor and can be seen below[2]. The more important change is that it's rebased against current 'master'. Review comments from Michael and Junio were very helpful in getting this in shape. Thanks much to both. The series can also be found at git://repo.or.cz/git/jrn.git tags/rs/ref-transaction It is based against current 'master' (670a3c1d, 2014-10-14) and intended for 'next'. Thoughts welcome, as always. Improvements preferred in the form of patches on top of the series. Thanks for the new version. I reviewed the previous version pretty carefully in Gerrit and was very happy with the overall series, though there were still some rough spots. It seems to be much more polished now. Given that I will be traveling for the next two weeks I probably won't be able to do another thorough review in time. But I just scanned through patches 01 - 19 and didn't notice any problems. Unfortunately I have to stop there. Cheers, Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] Consolidate ref parsing code
On Wed, Oct 15, 2014 at 10:06 PM, Michael Haggerty mhag...@alum.mit.edu wrote: As far as I know, Duy isn't actively working on this, so I hope my reroll is not unwelcome. I couldn't be happier that someone does the work for me and I still benefit from it ;) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(no subject)
-- Are you in a financial distress? do you need financial assistant to start up a business or to pay bills? we offer loans to individual's and company investors at 3% interest rate.Email us at adeyemisu...@gmail.com Fill out the details below: Full name: Country: Address: State: Zip code: Amount needed: Duration: Tel: Reason: Contact us by reply and we give you more response. Contact Online Manager Adeyemi Sule adeyemisu...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
On Thu, Oct 16, 2014 at 03:18:34PM -0700, Junio C Hamano wrote: We should probably add a test for cloning a tag of an otherwise unreferenced object, too. Yeah, that too ;-) Here's that test (after the scissors below). It can be applied totally separately, though I stuck it in as patch 18.5/25 in the existing series to confirm that my original series does cause the test to fail, and that it passes with the patch I posted earlier. Do you want to just squash the fix I posted earlier (into patch 19, the traverse_commit_list: ... one)? I'm happy to repost the revised patch, but my impression of your workflow is that squashing is just as easy than replacing a patch (i.e., you're running rebase -i either way). Or I'm happy to re-post the whole series, but it's rather long. :) Thanks for a quick diag. I'm impressed that you found the bug so quickly. :) My biggest fear with the whole series is that it's disrupting and refactoring some fundamental parts of the code that might cause regressions. I put a lot of my faith in the test suite, but that did not work out here (I do still feel pretty good about the series overall, though I think I'd cook it longer than usual given the complexity and the areas it touches). -- 8 -- Subject: t5516: test pushing a tag of an otherwise unreferenced blob It's not unreasonable to have a tag that points to a blob that is not part of the normal history. We do this in git.git to distribute gpg keys. However, we never explicitly checked in our test suite that this actually works (i.e., that pack-objects actually sends the blob because of the tag mentioning it). It does in fact work fine, but a recent patch under discussion broke this, and the test suite didn't notice. Let's make the test suite more complete. Signed-off-by: Jeff King p...@peff.net --- The should have below is belt-and-suspenders. The test actually fails with my series without the cat-file check, but I'm concerned a bug that affects pack-objects could also affect the connectivity check on the receiving side. t/t5516-fetch-push.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 67e0ab3..7c8a769 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1277,4 +1277,17 @@ EOF git push --no-thin --receive-pack=$rcvpck no-thin/.git refs/heads/master:refs/heads/foo ' +test_expect_success 'pushing a tag pushes the tagged object' ' + rm -rf dst.git + blob=$(echo unreferenced | git hash-object -w --stdin) + git tag -m foo tag-of-blob $blob + git init --bare dst.git + git push dst.git tag-of-blob + # the receiving index-pack should have noticed + # any problems, but we double check + echo unreferenced expect + git --git-dir=dst.git cat-file blob tag-of-blob actual + test_cmp expect actual +' + test_done -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 21/25] rev-list: add --index-objects option
On Thu, Oct 16, 2014 at 11:41:35AM -0700, Junio C Hamano wrote: I agree that --index is a bad name as it usually is used in a particular context: the command can work on various combination of working tree and the index, and I am asking it to work on both (e.g. apply --index as opposed to apply --cached). Thanks. I wasn't sure if I was just being paranoid or not, but now there are at least two of us. +--index-objects:: This risks index getting misunderstood as a verb, e.g. please enumerate the objects and assign labels to later refer to them, doesn't it? --indexed-objects (short for --show-objects-in-the-index) or something? That sounds reasonable. We could technically do `--indexed` as that is different from `--index`, but maybe they are still confusingly close. + Pretend as if all objects used by the index (any blobs, and any + trees which are mentioned by the index's cache-tree extension) + ad listed on the command line. Note that you probably want to s/ad/are/, probably? Yeah, sorry, vi cruft I think (at least I didn't accidentally insert C-a C-k ;) ). + use `--objects`, too, as there are by definition no commits in + the index. For gitlinks/submodules, the index records names of the commit objects, they are not listed, and that is the right behaviour, but this description invites some confusion. Good point. How about this: diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 03ab343..3301fde 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -172,12 +172,10 @@ explicitly. Pretend as if all objects mentioned by reflogs are listed on the command line as `commit`. ---index-objects:: - Pretend as if all objects used by the index (any blobs, and any - trees which are mentioned by the index's cache-tree extension) - ad listed on the command line. Note that you probably want to - use `--objects`, too, as there are by definition no commits in - the index. +--indexed-objects:: + Pretend as if all trees and blobs used by the index are listed + on the command line. Note that you probably want to use + `--objects`, too. --ignore-missing:: Upon seeing an invalid object name in the input, pretend as if I was tempted to not document this at all (nor to add documentation for --reflog), as I think these are really only going to be used internally. But it's nice to have documentation even for this internal stuff (if anything, we should probably be making sure they are just limited to rev-list plumbing, and not included in the log manpage). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git-bundle rev handling and de-duping
[subject tweaked as we have veered quite far off the original, and this might get more attention from interested people] On Thu, Oct 16, 2014 at 10:39:54AM -0700, Junio C Hamano wrote: 2. We use object_array_remove_duplicates() to de-dup git bundle create x master master, which came from b2a6d1c6 (bundle: allow the same ref to be given more than once, 2009-01-17), which is still the sole caller of the function, and I think this is bogus. Comparing .name would not de-dup git bundle create x master refs/heads/master. Hmm. We also doesn't respect the UNINTERESTING flag, so it will dedup foo and ^foo into a single entry. I _think_ this doesn't have any bad side effects, because they are equivalent (i.e., the flag is carried on the object, not the pending entry). I would expect this: git bundle create ... foo ^foo to barf (and it does) because the bundle is empty. But with this: git bundle create ... another-ref foo ^foo I would expect the resulting bundle to contain the objects from another-ref, but still mention foo as an update (we didn't need to send any objects, since presumably the other side already had them). It doesn't, but that is not because of the dedup. It is because we generate the list of refs to send based on the pending list, and we skip all UNINTERESTING objects (we must, because otherwise ^foo would make an entry). I.e., it is the same the flag is on the object, not the pending entry that saved us above now causing its own bug. Obviously the example above is sort of silly, but if you have: # two branches point at the same object git branch foo master # the other side already has master. Let's send them foo. # this will fail because the bundle is empty. That's mildly # annoying because we really want to tell them hey, update # your foo branch. But at least we get an error. git bundle create tmp.bundle foo ^master # now the same thing, but we're sending them some other objects. # This one succeeds, but silently omits foo from the bundle! git bundle create tmp.bundle foo another-ref ^master I have a feeling that the list needs to be generated from revs.cmdline, not revs.pending, and the de-duplication needs to happen there (with the ref resolution that you mention). I also have the feeling that fast-export had to deal with this exact same issue. It looks like we use revs.cmdline there. I seem to recall there was some ongoing work in that area, but I stopped paying close attention due to some personality conflicts, and I don't know if all of the issues were ever resolved. I think the right way to fix these two and a half problems is to do the following: - object_array_remove_duplicates() (and contains_name() helper it uses) should be removed from object.c; - create_bundle() in bundle.c should implement a helper that is similar to contains_name() but knows about ref dwimming and use it to call object_array_filter() to replace its call to object_array_remove_duplicates(). Agreed. The loop in create_bundle right after the de-dup does the dwimming. Probably it would be simple to just skip duplicates there using a hashmap or sorted list. I am not doing this myself, and I do not expect either you or Michael to do so, either. I am just writing this down to point out a low hanging fruit to aspiring new contributors (hint, hint). I am also not planning on working on it soon, but now we have hopefully fed plenty of possibilities to anybody who wants to. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 21/25] rev-list: add --index-objects option
On Thu, Oct 16, 2014 at 08:12:31PM -0400, Jeff King wrote: --indexed-objects (short for --show-objects-in-the-index) or something? That sounds reasonable. We could technically do `--indexed` as that is different from `--index`, but maybe they are still confusingly close. Here's a re-roll of the final 5 patches of the series with the updated name (`--indexed-objects`). The name change cascades from patch 22 to patches 23 and 25 (and I renamed the matching pack-objects option as well). 24 and 26 are unchanged, but I figured it is easier on you to replace the whole block of patches at once. [22/26]: rev-list: add --indexed-objects option [23/26]: reachable: use revision machinery's --indexed-objects code [24/26]: pack-objects: use argv_array [25/26]: repack: pack objects mentioned by the index [26/26]: pack-objects: double-check options before discarding objects -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 23/26] reachable: use revision machinery's --indexed-objects code
This does the same thing as our custom code, so let's not repeat ourselves. Signed-off-by: Jeff King p...@peff.net --- reachable.c | 52 +--- 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/reachable.c b/reachable.c index 0176a88..a647267 100644 --- a/reachable.c +++ b/reachable.c @@ -32,56 +32,6 @@ static int add_one_ref(const char *path, const unsigned char *sha1, int flag, vo return 0; } -static void add_one_tree(const unsigned char *sha1, struct rev_info *revs) -{ - struct tree *tree = lookup_tree(sha1); - if (tree) - add_pending_object(revs, tree-object, ); -} - -static void add_cache_tree(struct cache_tree *it, struct rev_info *revs) -{ - int i; - - if (it-entry_count = 0) - add_one_tree(it-sha1, revs); - for (i = 0; i it-subtree_nr; i++) - add_cache_tree(it-down[i]-cache_tree, revs); -} - -static void add_cache_refs(struct rev_info *revs) -{ - int i; - - read_cache(); - for (i = 0; i active_nr; i++) { - struct blob *blob; - - /* -* The index can contain blobs and GITLINKs, GITLINKs are hashes -* that don't actually point to objects in the repository, it's -* almost guaranteed that they are NOT blobs, so we don't call -* lookup_blob() on them, to avoid populating the hash table -* with invalid information -*/ - if (S_ISGITLINK(active_cache[i]-ce_mode)) - continue; - - blob = lookup_blob(active_cache[i]-sha1); - if (blob) - blob-object.flags |= SEEN; - - /* -* We could add the blobs to the pending list, but quite -* frankly, we don't care. Once we've looked them up, and -* added them as objects, we've really done everything -* there is to do for a blob -*/ - } - if (active_cache_tree) - add_cache_tree(active_cache_tree, revs); -} - /* * The traversal will have already marked us as SEEN, so we * only need to handle any progress reporting here. @@ -213,7 +163,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog, revs-tree_objects = 1; /* Add all refs from the index file */ - add_cache_refs(revs); + add_index_objects_to_pending(revs, 0); /* Add all external refs */ for_each_ref(add_one_ref, revs); -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 22/26] rev-list: add --indexed-objects option
There is currently no easy way to ask the revision traversal machinery to include objects reachable from the index (e.g., blobs and trees that have not yet been committed). This patch adds an option to do so. Signed-off-by: Jeff King p...@peff.net --- Documentation/rev-list-options.txt | 5 revision.c | 51 ++ revision.h | 1 + t/t6000-rev-list-misc.sh | 23 + 4 files changed, 80 insertions(+) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 4cf94c6..3301fde 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -172,6 +172,11 @@ explicitly. Pretend as if all objects mentioned by reflogs are listed on the command line as `commit`. +--indexed-objects:: + Pretend as if all trees and blobs used by the index are listed + on the command line. Note that you probably want to use + `--objects`, too. + --ignore-missing:: Upon seeing an invalid object name in the input, pretend as if the bad input was not given. diff --git a/revision.c b/revision.c index 8030fc8..a6c5dc3 100644 --- a/revision.c +++ b/revision.c @@ -17,6 +17,7 @@ #include mailmap.h #include commit-slab.h #include dir.h +#include cache-tree.h volatile show_early_output_fn_t show_early_output; @@ -1303,6 +1304,53 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) for_each_reflog(handle_one_reflog, cb); } +static void add_cache_tree(struct cache_tree *it, struct rev_info *revs, + struct strbuf *path) +{ + size_t baselen = path-len; + int i; + + if (it-entry_count = 0) { + struct tree *tree = lookup_tree(it-sha1); + add_pending_object_with_path(revs, tree-object, , +04, path-buf); + } + + for (i = 0; i it-subtree_nr; i++) { + struct cache_tree_sub *sub = it-down[i]; + strbuf_addf(path, %s%s, baselen ? / : , sub-name); + add_cache_tree(sub-cache_tree, revs, path); + strbuf_setlen(path, baselen); + } + +} + +void add_index_objects_to_pending(struct rev_info *revs, unsigned flags) +{ + int i; + + read_cache(); + for (i = 0; i active_nr; i++) { + struct cache_entry *ce = active_cache[i]; + struct blob *blob; + + if (S_ISGITLINK(ce-ce_mode)) + continue; + + blob = lookup_blob(ce-sha1); + if (!blob) + die(unable to add index blob to traversal); + add_pending_object_with_path(revs, blob-object, , +ce-ce_mode, ce-name); + } + + if (active_cache_tree) { + struct strbuf path = STRBUF_INIT; + add_cache_tree(active_cache_tree, revs, path); + strbuf_release(path); + } +} + static int add_parents_only(struct rev_info *revs, const char *arg_, int flags) { unsigned char sha1[20]; @@ -1653,6 +1701,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg !strcmp(arg, --reflog) || !strcmp(arg, --not) || !strcmp(arg, --no-walk) || !strcmp(arg, --do-walk) || !strcmp(arg, --bisect) || starts_with(arg, --glob=) || + !strcmp(arg, --indexed-objects) || starts_with(arg, --exclude=) || starts_with(arg, --branches=) || starts_with(arg, --tags=) || starts_with(arg, --remotes=) || starts_with(arg, --no-walk=)) @@ -2082,6 +2131,8 @@ static int handle_revision_pseudo_opt(const char *submodule, clear_ref_exclusion(revs-ref_excludes); } else if (!strcmp(arg, --reflog)) { add_reflogs_to_pending(revs, *flags); + } else if (!strcmp(arg, --indexed-objects)) { + add_index_objects_to_pending(revs, *flags); } else if (!strcmp(arg, --not)) { *flags ^= UNINTERESTING | BOTTOM; } else if (!strcmp(arg, --no-walk)) { diff --git a/revision.h b/revision.h index e644044..e6dcd5d 100644 --- a/revision.h +++ b/revision.h @@ -277,6 +277,7 @@ extern void add_pending_sha1(struct rev_info *revs, extern void add_head_to_pending(struct rev_info *); extern void add_reflogs_to_pending(struct rev_info *, unsigned int flags); +extern void add_index_objects_to_pending(struct rev_info *, unsigned int flags); enum commit_action { commit_ignore, diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh index 3794e4c..2602086 100755 --- a/t/t6000-rev-list-misc.sh +++ b/t/t6000-rev-list-misc.sh @@ -73,4 +73,27 @@ test_expect_success 'symleft flag bit is propagated down from tag' ' test_cmp expect actual ' +test_expect_success 'rev-list can show index
[PATCH v3 24/26] pack-objects: use argv_array
This saves us from having to bump the rp_av count when we add new traversal options. Signed-off-by: Jeff King p...@peff.net --- builtin/pack-objects.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 4df9499..b26276b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -22,6 +22,7 @@ #include pack-bitmap.h #include reachable.h #include sha1-array.h +#include argv-array.h static const char *pack_usage[] = { N_(git pack-objects --stdout [options...] [ ref-list | object-list]), @@ -2614,8 +2615,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) int use_internal_rev_list = 0; int thin = 0; int all_progress_implied = 0; - const char *rp_av[6]; - int rp_ac = 0; + struct argv_array rp = ARGV_ARRAY_INIT; int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0; struct option pack_objects_options[] = { OPT_SET_INT('q', quiet, progress, @@ -2705,24 +2705,24 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (pack_to_stdout != !base_name || argc) usage_with_options(pack_usage, pack_objects_options); - rp_av[rp_ac++] = pack-objects; + argv_array_push(rp, pack-objects); if (thin) { use_internal_rev_list = 1; - rp_av[rp_ac++] = --objects-edge; + argv_array_push(rp, --objects-edge); } else - rp_av[rp_ac++] = --objects; + argv_array_push(rp, --objects); if (rev_list_all) { use_internal_rev_list = 1; - rp_av[rp_ac++] = --all; + argv_array_push(rp, --all); } if (rev_list_reflog) { use_internal_rev_list = 1; - rp_av[rp_ac++] = --reflog; + argv_array_push(rp, --reflog); } if (rev_list_unpacked) { use_internal_rev_list = 1; - rp_av[rp_ac++] = --unpacked; + argv_array_push(rp, --unpacked); } if (!reuse_object) @@ -2766,8 +2766,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (!use_internal_rev_list) read_object_list_from_stdin(); else { - rp_av[rp_ac] = NULL; - get_object_list(rp_ac, rp_av); + get_object_list(rp.argc, rp.argv); + argv_array_clear(rp); } cleanup_preferred_base(); if (include_tag nr_result) -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 25/26] repack: pack objects mentioned by the index
When we pack all objects, we use only the objects reachable from references and reflogs. This misses any objects which are reachable from the index, but not yet referenced. By itself this isn't a big deal; the objects can remain loose until they are actually used in a commit. However, it does create a problem when we drop packed but unreachable objects. We try to optimize out the writing of objects that we will immediately prune, which means we must follow the same rules as prune in determining what is reachable. And prune uses the index for this purpose. This is rather uncommon in practice, as objects in the index would not usually have been packed in the first place. But it could happen in a sequence like: 1. You make a commit on a branch that references blob X. 2. You repack, moving X into the pack. 3. You delete the branch (and its reflog), so that X is unreferenced. 4. You git add blob X so that it is now referenced only by the index. 5. You repack again with git-gc. The pack-objects we invoke will see that X is neither referenced nor recent and not bother loosening it. Signed-off-by: Jeff King p...@peff.net --- builtin/pack-objects.c | 8 builtin/repack.c | 1 + t/t7701-repack-unpack-unreachable.sh | 13 + 3 files changed, 22 insertions(+) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index b26276b..0cf95c9 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2617,6 +2617,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) int all_progress_implied = 0; struct argv_array rp = ARGV_ARRAY_INIT; int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0; + int rev_list_index = 0; struct option pack_objects_options[] = { OPT_SET_INT('q', quiet, progress, N_(do not show progress meter), 0), @@ -2663,6 +2664,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) { OPTION_SET_INT, 0, reflog, rev_list_reflog, NULL, N_(include objects referred by reflog entries), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 }, + { OPTION_SET_INT, 0, indexed-objects, rev_list_index, NULL, + N_(include objects referred to by the index), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 }, OPT_BOOL(0, stdout, pack_to_stdout, N_(output pack to stdout)), OPT_BOOL(0, include-tag, include_tag, @@ -2720,6 +2724,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) use_internal_rev_list = 1; argv_array_push(rp, --reflog); } + if (rev_list_index) { + use_internal_rev_list = 1; + argv_array_push(rp, --indexed-objects); + } if (rev_list_unpacked) { use_internal_rev_list = 1; argv_array_push(rp, --unpacked); diff --git a/builtin/repack.c b/builtin/repack.c index 2aae05d..2845620 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -209,6 +209,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) argv_array_push(cmd_args, --non-empty); argv_array_push(cmd_args, --all); argv_array_push(cmd_args, --reflog); + argv_array_push(cmd_args, --indexed-objects); if (window) argv_array_pushf(cmd_args, --window=%s, window); if (window_memory) diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh index b8d4cde..aad8a9c 100755 --- a/t/t7701-repack-unpack-unreachable.sh +++ b/t/t7701-repack-unpack-unreachable.sh @@ -109,4 +109,17 @@ test_expect_success 'do not bother loosening old objects' ' test_must_fail git cat-file -p $obj2 ' +test_expect_success 'keep packed objects found only in index' ' + echo my-unique-content file + git add file + git commit -m make it reachable + git gc + git reset HEAD^ + git reflog expire --expire=now --all + git add file + test-chmtime =-86400 .git/objects/pack/* + git gc --prune=1.hour.ago + git cat-file blob :file +' + test_done -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 26/26] pack-objects: double-check options before discarding objects
When we are given an expiration time like --unpack-unreachable=2.weeks.ago, we avoid writing out old, unreachable loose objects entirely, under the assumption that running prune would simply delete them immediately anyway. However, this is only valid if we computed the same set of reachable objects as prune would. In practice, this is the case, because only git-repack uses the --unpack-unreachable option with an expiration, and it always feeds as many objects into the pack as possible. But we can double-check at runtime just to be sure. Signed-off-by: Jeff King p...@peff.net --- builtin/pack-objects.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 0cf95c9..64123d4 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2757,6 +2757,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) if (keep_unreachable unpack_unreachable) die(--keep-unreachable and --unpack-unreachable are incompatible.); + if (!rev_list_all || !rev_list_reflog || !rev_list_index) + unpack_unreachable_expiration = 0; if (!use_internal_rev_list || !pack_to_stdout || is_repository_shallow()) use_bitmap_index = 0; -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pack-objects: turn off bitmaps when we split packs
If a pack.packSizeLimit is set, we may split the pack data across multiple packfiles. This means we cannot generate .bitmap files, as they require that all of the reachable objects are in the same pack. We check that condition when we are generating the list of objects to pack (and disable bitmaps if we are not packing everything), but we forgot to update it when we notice that we needed to split (which doesn't happen until the actual write phase). The resulting bitmaps are quite bogus (they mention entries that do not exist in the pack!) and can cause a fetch or push to send insufficient objects. Signed-off-by: Jeff King p...@peff.net --- You should generally avoid splitting packs anyway, as it introduces other inefficiencies, so I'm not really concerned about making bitmaps work here (besides which, it would require a big change to the on-disk format). The important thing here is avoiding creating the broken bitmaps. builtin/pack-objects.c | 1 + t/t5310-pack-bitmaps.sh | 9 + 2 files changed, 10 insertions(+) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index d391934..a4022a7 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -811,6 +811,7 @@ static void write_pack_file(void) fixup_pack_header_footer(fd, sha1, pack_tmp_name, nr_written, sha1, offset); close(fd); + write_bitmap_index = 0; } if (!pack_to_stdout) { diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 0580258..6003490 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -170,4 +170,13 @@ test_expect_success JGIT 'jgit can read our bitmaps' ' ) ' +test_expect_success 'splitting packs does not generate bogus bitmaps' ' + test-genrandom foo $((1024 * 1024)) rand + git add rand + git commit -m commit with big file + git -c pack.packSizeLimit=500k repack -adb + git init --bare no-bitmaps.git + git -C no-bitmaps.git fetch .. HEAD +' + test_done -- 2.1.2.596.g7379948 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/25] prune-safety
On Thu, Oct 16, 2014 at 09:13:39PM -0700, Junio C Hamano wrote: Just being curious, but would the same bug, if allowed to be triggered cutting repacking of your repository, have corrupted the resulting bitmap? I didn't test, but yes, almost certainly. The bug was in list-objects.c, which is used by pack-objects to generate the list of objects to pack, as well as to build the bitmaps. So not only would it have corrupted the bitmaps, a `git repack -ad`[1] would have dropped the object completely, corrupting the repository! -Peff [1] git-gc uses `repack -Ad`, of course. So assuming you had packed more recently than 2 weeks ago, it would have just been ejected to a loose object. Small comfort. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html