Re: git-p4 out of memory for very large repository
On 23/08/13 02:12, Corey Thompson wrote: Hello, Has anyone actually gotten git-p4 to clone a large Perforce repository? Yes. I've cloned repos with a couple of Gig of files. I have one codebase in particular that gets to about 67%, then consistently gets get-fast-import (and often times a few other processes) killed by the OOM killer. What size is this codebase? Which version and platform of git are you using? Maybe it's a regression, or perhaps you've hit some new, previously unknown size limit? Thanks Luke I've found some patches out there that claim to resolve this, but they're all for versions of git-p4.py from several years ago. Not only will they not apply cleanly, but as far as I can tell the issues that these patches are meant to address aren't in the current version, anyway. Any suggestions would be greatly appreciated. Thanks, Corey -- 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 -- 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: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push
Felipe Contreras felipe.contre...@gmail.com writes: On Wed, Aug 21, 2013 at 4:36 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Aug 13, 2013 at 8:31 AM, Matthieu Moy matthieu@imag.fr wrote: Felipe: Is this the right fix for git-remote-mediawiki? Any better idea? Why not keep track of the revisions yourself? You can have file where you store which was the last revision that was fetched. I don't really understand the point of the private namespace anymore I guess. Why do we have both refs/remotes/$remote and refs/$foreign_vcs/$remote, if they are always kept in sync? They are not always in sync; if a push fails, the private namespace is not updated. [...] As I said, they are not exactly the same. It is possible refs/remotes point to a mercurial revision on the remote server, and refs/hg points to a mercurial revision on the local internal repository, and they are not the same. This is assuming you follow the scheme git - local repo for other vcs - remote repo for other vcs which itself more or less assumes that the other VCS is a decentralized VCS. I understand this is a good idea for hg or bzr remote helpers, but not applicable for git-remote-mediawiki. I find this very unclear in the doc. How about adding something like this in the 'refspec' refspec:: part? --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -176,6 +176,12 @@ applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by the list command. If no 'refspec' capability is advertised, there is an implied `refspec *:*`. ++ +When writing remote-helpers for decentralized version control +systems, it is advised to keep a local copy of the repository to +interact with, and to let the private namespace refs point to this +local repository, while the refs/remotes namespace is used to track +the remote repository. 'bidi-import':: This modifies the 'import' capability. So, now, I understand the point of the private namespace in the case of DVCS, but still note really for interactions with centralized VCS. Back to my original problem: the point of dumb push is to make sure the next import will re-import the pushed revisions. if I use something other than the private namespace to keep track of the last imported, then I'll need to do a non-fast forward update to the private ref. I could do that by sending feature force\n it the beginning of the fast-import stream when importing, but that loses one safety feature (e.g. I detected the breakage thanks to the non-fast forward error message, while the tests incorrectly pass if I enable force). -- 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
[PATCH] Documentation/fast-import: clarify summary for `feature` command
In most cases, feature foo does not just require that the feature exists, but also changes the behavior to enable it. Cases where the feature is only requested like cat-blob, notes or ls are clearly documented below. Signed-off-by: Matthieu Moy matthieu@imag.fr --- Documentation/git-fast-import.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index bf1a02a..4a9cc74 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -361,8 +361,8 @@ and control the current import process. More detailed discussion `--cat-blob-fd` or `stdout` if unspecified. `feature`:: - Require that fast-import supports the specified feature, or - abort if it does not. + Enable the specified feature. This requires that fast-import + supports the specified feature, and aborts if it does not. `option`:: Specify any of the options listed under OPTIONS that do not -- 1.8.4.rc4.4.g57d5d3d -- 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: t3010 broken by 2eac2a4
On Fri, Aug 23, 2013 at 1:36 AM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: I sent a patch [1] which resolves the problem, although the solution is not especially pretty (due to some ugliness in the existing implementation). Yeah, thanks. I tend to agree with you that fixing the icase callee not to rely on having the trailing slash (which is looking past the end of the given string), instead of working that breakage around on the caller's side like your patch did, would be a better alternative, though. My concern with fixing directory_exists_in_index_icase() to add the '/' itself was that it would have to copy the string to make space for the '/', which could be expensive. However, I reworked the code so that the existing strbufs now get passed to directory_exists_in_index_icase(), which allows it to add its needed '/' without duplicating the string. So, the trailing '/' requirement of directory_exists_in_index_icase() is now a private implementation detail, placing no burden on the caller. I'll send the revised patch series later today since the commit message needs a rewrite. Also, I'd like to try to split the change into a couple patches -- one to pass around strbufs, which has a noisy diff, and one to fix the actual bug -- though I fear that splitting may not be possible. -- 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-p4 out of memory for very large repository
On Fri, Aug 23, 2013 at 08:16:58AM +0100, Luke Diamand wrote: On 23/08/13 02:12, Corey Thompson wrote: Hello, Has anyone actually gotten git-p4 to clone a large Perforce repository? Yes. I've cloned repos with a couple of Gig of files. I have one codebase in particular that gets to about 67%, then consistently gets get-fast-import (and often times a few other processes) killed by the OOM killer. What size is this codebase? Which version and platform of git are you using? Maybe it's a regression, or perhaps you've hit some new, previously unknown size limit? Thanks Luke I've found some patches out there that claim to resolve this, but they're all for versions of git-p4.py from several years ago. Not only will they not apply cleanly, but as far as I can tell the issues that these patches are meant to address aren't in the current version, anyway. Any suggestions would be greatly appreciated. Thanks, Corey -- 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 Sorry, I guess I could have included more details in my original post. Since then, I have also made an attempt to clone another (slightly more recent) branch, and at last had success. So I see this does indeed work, it just seems to be very unhappy with one particular branch. So, here are a few statistics I collected on the two branches. branch-that-fails: total workspace disk usage (current head): 12GB 68 files over 20MB largest three being about 118MB branch-that-clones: total workspace disk usage (current head): 11GB 22 files over 20MB largest three being about 80MB I suspect that part of the problem here might be that my company likes to submit very large binaries into our repo (.tar.gzs, pre-compiled third party binaries, etc.). Is there any way I can clone this in pieces? The best I've come up with is to clone only up to a change number just before it tends to fail, and then rebase to the latest. My clone succeeded, but the rebase still runs out of memory. It would be great if I could specify a change number to rebase up to, so that I can just take this thing a few hundred changes at a time. Thanks, Corey -- 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-p4 out of memory for very large repository
On Fri, Aug 23, 2013 at 07:48:56AM -0400, Corey Thompson wrote: Sorry, I guess I could have included more details in my original post. Since then, I have also made an attempt to clone another (slightly more recent) branch, and at last had success. So I see this does indeed work, it just seems to be very unhappy with one particular branch. So, here are a few statistics I collected on the two branches. branch-that-fails: total workspace disk usage (current head): 12GB 68 files over 20MB largest three being about 118MB branch-that-clones: total workspace disk usage (current head): 11GB 22 files over 20MB largest three being about 80MB I suspect that part of the problem here might be that my company likes to submit very large binaries into our repo (.tar.gzs, pre-compiled third party binaries, etc.). Is there any way I can clone this in pieces? The best I've come up with is to clone only up to a change number just before it tends to fail, and then rebase to the latest. My clone succeeded, but the rebase still runs out of memory. It would be great if I could specify a change number to rebase up to, so that I can just take this thing a few hundred changes at a time. Thanks, Corey And I still haven't told you anything about my platform or git version... This is on Fedora Core 11, with git 1.8.3.4 built from the github repo (117eea7e). -- 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
[BUGLET] git bisect visualize does not show BISECT_HEAD
[v1.8.3.4]
git bisect visualize when run without --no-checkout has the standard gitk
handling of
HEAD showing what the current revision being tested is (as a yellow node).
Now when using --no-checkout, the information current revision we may be
looking for
has nothing to do with HEAD any longer: we need BISECT_HEAD instead - and if by
any chance
HEAD would happen to be in the displayed scope, the user may do wrong
assumptions about
it (maybe).
Wondering whether there would be any flags that would pass to gitk through
bisect visualize,
I naively tried:
$ git bisect visualize -h
usage: git log [options] [revision range] [[--] path...]
or: git show [options] object...
--quiet suppress diff output
--source show source
--use-mailmap Use mail map file
--decorate[=...] decorate options
-L n,m:file Process line range n,m in file, counting from 1
Wandering away from what I was look for:
$ git bisect visualize --decorate
... some git log output
That seems unfortunate in its own right as well...
Back to the problem of visualizing the info, it looks like gitk would need a
way to display
refs that are not displayed by default, when we need them. Something like:
gitk --explicit-refs=BISECT_HEAD,refs/whatever
That would also be helpful when one tries to look at the reflog graphically:
whereas
gitk accepts to show whatever@{1} and friends, it never tells us which revision
corresponds
to which reflog entry, and --explicit-refs=whatever@{1},whatever@{2} would help
here, as would
something like --explicit-refs=whatever@{*} or --explicit-refs=whatever@{1..5},
but that starts
to be more tricky to formalize.
Thoughts, anyone ?
--
Yann Dirson - Bertin Technologies
--
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] git-commit: search author pattern against mailmap
When committing for someone else, using the --author option, it can be
nice to use the mailmap file to find the correct name spelling and email
address.
Currently, you would have to find the correct mapping in mailmap file
first, and then use the full ident form when committing.
Let's allow git-commit to find if an entry exists in mailmap file for
that pattern, and use that instead.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Hi,
I would use that feature at work where I happen to commit some work for
other colleagues, while we heavily rely on mailmap file to have decent indents.
On the other hand, I'm kind of embarrassed to add this new option to
git-commit.
Documentation/git-commit.txt | 6 +-
builtin/commit.c | 16 +++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..9e3fe04 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--dry-run] [(-c | -C | --fixup | --squash) commit]
[-F file | -m msg] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=author]
- [--date=date] [--cleanup=mode] [--[no-]status]
+ [--use-mailmap] [--date=date] [--cleanup=mode] [--[no-]status]
[-i | -o] [-S[keyid]] [--] [file...]
DESCRIPTION
@@ -131,6 +131,10 @@ OPTIONS
commit by that author (i.e. rev-list --all -i --author=author);
the commit author is then copied from the first such commit found.
+--use-mailmap::
+ When used with `--author=author`, match the author pattern
+ against mapped name and email. See linkgit:git-shortlog[1].
+
--date=date::
Override the author date used in the commit.
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..fbd0664 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -30,6 +30,7 @@
#include column.h
#include sequencer.h
#include notes-utils.h
+#include mailmap.h
static const char * const builtin_commit_usage[] = {
N_(git commit [options] [--] pathspec...),
@@ -87,6 +88,7 @@ static enum {
} commit_style;
static const char *logfile, *force_author;
+static int mailmap;
static const char *template_file;
/*
* The _message variables are commit names from which to take
@@ -945,13 +947,24 @@ static const char *find_author_by_nickname(const char
*name)
av[++ac] = buf.buf;
av[++ac] = NULL;
setup_revisions(ac, av, revs, NULL);
+ if (mailmap) {
+ revs.mailmap = xcalloc(1, sizeof(struct string_list));
+ read_mailmap(revs.mailmap, NULL);
+ }
prepare_revision_walk(revs);
commit = get_revision(revs);
if (commit) {
struct pretty_print_context ctx = {0};
+ const char *format;
+
+ if (mailmap)
+ format = %aN %aE;
+ else
+ format = %an %ae;
+
ctx.date_mode = DATE_NORMAL;
strbuf_release(buf);
- format_commit_message(commit, %an %ae, buf, ctx);
+ format_commit_message(commit, format, buf, ctx);
return strbuf_detach(buf, NULL);
}
die(_(No existing author found with '%s'), name);
@@ -1428,6 +1441,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_GROUP(N_(Commit message options)),
OPT_FILENAME('F', file, logfile, N_(read message from
file)),
OPT_STRING(0, author, force_author, N_(author),
N_(override author for commit)),
+ OPT_BOOLEAN(0, use-mailmap, mailmap, N_(Use mailmap file
when searching for author)),
OPT_STRING(0, date, force_date, N_(date), N_(override
date for commit)),
OPT_CALLBACK('m', message, message, N_(message),
N_(commit message), opt_parse_m),
OPT_STRING('c', reedit-message, edit_message, N_(commit),
N_(reuse and edit message from specified commit)),
--
1.8.4.rc4.1.g0d8beaa.dirty
--
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] Remove dead link
From: Michal Nazarewicz min...@mina86.com Signed-off-by: Michal Nazarewicz min...@mina86.com --- Documentation/technical/pack-heuristics.txt | 6 -- 1 file changed, 6 deletions(-) diff --git a/Documentation/technical/pack-heuristics.txt b/Documentation/technical/pack-heuristics.txt index 8b7ae1c..b7bd951 100644 --- a/Documentation/technical/pack-heuristics.txt +++ b/Documentation/technical/pack-heuristics.txt @@ -366,12 +366,6 @@ been detailed! linus Yes, we always write out most recent first -For the other record: - -pasky njs`: http://pastebin.com/547965 - -The 'net never forgets, so that should be good until the end of time. - njs` And, yeah, I got the part about deeper-in-history stuff having worse IO characteristics, one sort of doesn't care. -- 1.8.4.rc1 -- 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] Remove dead link
From: Michal Nazarewicz min...@mina86.com Signed-off-by: Michal Nazarewicz min...@mina86.com --- Documentation/technical/pack-heuristics.txt | 6 -- 1 file changed, 6 deletions(-) diff --git a/Documentation/technical/pack-heuristics.txt b/Documentation/tech nical/pack-heuristics.txt index 8b7ae1c..b7bd951 100644 --- a/Documentation/technical/pack-heuristics.txt +++ b/Documentation/technical/pack-heuristics.txt @@ -366,12 +366,6 @@ been detailed! linus Yes, we always write out most recent first -For the other record: - -pasky njs`: http://pastebin.com/547965 - -The 'net never forgets, so that should be good until the end of time. - njs` And, yeah, I got the part about deeper-in-history stuff having worse IO characteristics, one sort of doesn't care. Hmm. Something oddly self-referential about this quip being removed from the document. Perhaps we should notify the Boy from Paradise By The Dashboard Light that he can rest easily now without the Girl as it seems the end of time has come at last. Acked-by: Jon Loeliger j...@jdl.com jdl -- 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 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
Jeff King p...@peff.net writes:
Furthermore, we know that one of our endpoints must be
the edge of the run of duplicates. For example, given this
sequence:
idx 0 1 2 3 4 5
key A C C C C D
If we are searching for B, we might hit the duplicate run
at lo=1, hi=3 (e.g., by first mi=3, then mi=0). But we can
never have lo 1, because B C. That is, if our key is
less than the run, we know that lo is the edge, but we can
say nothing of hi. Similarly, if our key is greater than
the run, we know that hi is the edge, but we can say
nothing of lo. But that is enough for us to return not
only not found, but show the position at which we would
insert the new item.
This is somewhat tricky and may deserve an in-code comment.
diff --git a/sha1-lookup.c b/sha1-lookup.c
index c4dc55d..614cbb6 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -204,7 +204,30 @@ int sha1_entry_pos(const void *table,
* byte 0 thru (ofs-1) are the same between
* lo and hi; ofs is the first byte that is
* different.
+ *
+ * If ofs==20, then no bytes are different,
+ * meaning we have entries with duplicate
+ * keys. We know that we are in a solid run
+ * of this entry (because the entries are
+ * sorted, and our lo and hi are the same,
+ * there can be nothing but this single key
+ * in between). So we can stop the search.
+ * Either one of these entries is it (and
+ * we do not care which), or we do not have
+ * it.
*/
+ if (ofs == 20) {
+ mi = lo;
+ mi_key = base + elem_size * mi + key_offset;
+ cmp = memcmp(mi_key, key, 20);
It think we already know that mi_key[0:ofs_0] and key[0:ofs_0] are
the same at this point and we do not have to compare full 20 bytes
again, but this is done only once and a better readablity of the
above trumps micro-optimization possibility, I think.
+ if (!cmp)
+ return mi;
+ if (cmp 0)
+ return -1 - hi;
+ else
+ return -1 - lo;
+ }
+
hiv = hi_key[ofs_0];
if (ofs_0 19)
hiv = (hiv 8) | hi_key[ofs_0+1];
diff --git a/t/lib-pack.sh b/t/lib-pack.sh
new file mode 100644
index 000..61c5d19
--- /dev/null
+++ b/t/lib-pack.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Support routines for hand-crafting weird or malicious packs.
+#
+# You can make a complete pack like:
+#
+# pack_header 2 foo.pack
+# pack_obj e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 foo.pack
+# pack_obj e68fe8129b546b101aee9510c5328e7f21ca1d18 foo.pack
+# pack_trailer foo.pack
+
+# Print the big-endian 4-byte octal representation of $1
+uint32_octal() {
micronit (style):
uint32_octal () {
+ n=$1
+ printf '\%o' $(($n / 16777216)); n=$((n % 16777216))
+ printf '\%o' $(($n /65536)); n=$((n %65536))
+ printf '\%o' $(($n / 256)); n=$((n % 256))
+ printf '\%o' $(($n ));
+}
+
+# Print the big-endian 4-byte binary representation of $1
+uint32_binary() {
+ printf $(uint32_octal $1)
+}
+
+# Print a pack header, version 2, for a pack with $1 objects
+pack_header() {
+ printf 'PACK'
+ printf '\0\0\0\2'
+ uint32_binary $1
+}
+
+# Print the pack data for object $1, as a delta against object $2 (or as a
full
+# object if $2 is missing or empty). The output is suitable for including
+# directly in the packfile, and represents the entirety of the object entry.
+# Doing this on the fly (especially picking your deltas) is quite tricky, so
we
+# have hardcoded some well-known objects. See the case statements below for
the
+# complete list.
Cute ;-) I like the idea of having this function with a right API in
place, and cheating by limiting its implementation to what is
necessary.
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: t3010 broken by 2eac2a4
Eric Sunshine sunsh...@sunshineco.com writes: On Fri, Aug 23, 2013 at 1:36 AM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: I sent a patch [1] which resolves the problem, although the solution is not especially pretty (due to some ugliness in the existing implementation). Yeah, thanks. I tend to agree with you that fixing the icase callee not to rely on having the trailing slash (which is looking past the end of the given string), instead of working that breakage around on the caller's side like your patch did, would be a better alternative, though. My concern with fixing directory_exists_in_index_icase() to add the '/' itself was that it would have to copy the string to make space for the '/', which could be expensive. However, I reworked the code so that the existing strbufs now get passed to directory_exists_in_index_icase(), which allows it to add its needed '/' without duplicating the string. So, the trailing '/' requirement of directory_exists_in_index_icase() is now a private implementation detail, placing no burden on the caller. When 5102c617 (Add case insensitivity support for directories when using git status, 2010-10-03) added the directories to the name-hash with trailing slash, there was only a single name hash table to which both real cache entries and leading directory prefixes are registered, so it made some sense to register them with trailing slashes so that we can tell what kind of entry is being returned. But since 2092678c (name-hash.c: fix endless loop with core.ignorecase=true, 2013-02-28), these directory entries that are not the cache entries are kept track of in a separate hashtable, which makes me wonder if it still makes sense to register directories with trailing slashes. And if we stop doing that (and instead if we shrunk the namelen when an unconverted caller asks for a name with a trailing slash to see if a directory exists in the index), wouldn't it automatically fix the directory_exists_in_index_icase()? It does not need to assume that dirname[len] has '/'; after all, it may not even be a valid memory location in the first place. -- 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
How to set tab size for hunks in “git add -p”?
Hi! I used the solution from here http://stackoverflow.com/questions/15132325/how-to-set-tab-size-for-pager-used-in-git-diff to change tab size in |git diff||| output. That works fine with git diff. I have these settings in my *.gitconfig*: |[core] whitespace = tabsize=4,indent-with-non-tab pager = less -FSRX -x4 | But those settings seems does not affect on |git add -p|. How to set tab size for hunks in *git add -p* command? Thanks in advance! -- Best regards, Alexander Yancharuk a...@itvault.info mailto:a...@itvault.info -- 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] git-commit: search author pattern against mailmap
Antoine Pelisse apeli...@gmail.com writes:
When committing for someone else, using the --author option, it can be
nice to use the mailmap file to find the correct name spelling and email
address.
Currently, you would have to find the correct mapping in mailmap file
first, and then use the full ident form when committing.
Let's allow git-commit to find if an entry exists in mailmap file for
that pattern, and use that instead.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
---
Hi,
I would use that feature at work where I happen to commit some work for
other colleagues, while we heavily rely on mailmap file to have decent
indents.
On the other hand, I'm kind of embarrassed to add this new option to
git-commit.
My initial reaction was Why should something as important as 'git
commit' should be playing a guessing-game? ;-) and I am kind of
ashamed to have added 146ea068 (git commit --author=$name: look
$name up in existing commits, 2008-08-26) and then am embarrased to
have completely forgotten about it. I never use the feature myself.
But for that old and established --author parameter that does not
use the standard format guesses feature to be useful, I agree that
it should honor the mailmap.
I wonder if it would hurt anybody if we made this unconditional, not
even with --no-mailmap override? Opinions?
Documentation/git-commit.txt | 6 +-
builtin/commit.c | 16 +++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..9e3fe04 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--dry-run] [(-c | -C | --fixup | --squash) commit]
[-F file | -m msg] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=author]
-[--date=date] [--cleanup=mode] [--[no-]status]
+[--use-mailmap] [--date=date] [--cleanup=mode] [--[no-]status]
[-i | -o] [-S[keyid]] [--] [file...]
DESCRIPTION
@@ -131,6 +131,10 @@ OPTIONS
commit by that author (i.e. rev-list --all -i --author=author);
the commit author is then copied from the first such commit found.
+--use-mailmap::
+ When used with `--author=author`, match the author pattern
+ against mapped name and email. See linkgit:git-shortlog[1].
+
--date=date::
Override the author date used in the commit.
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..fbd0664 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -30,6 +30,7 @@
#include column.h
#include sequencer.h
#include notes-utils.h
+#include mailmap.h
static const char * const builtin_commit_usage[] = {
N_(git commit [options] [--] pathspec...),
@@ -87,6 +88,7 @@ static enum {
} commit_style;
static const char *logfile, *force_author;
+static int mailmap;
static const char *template_file;
/*
* The _message variables are commit names from which to take
@@ -945,13 +947,24 @@ static const char *find_author_by_nickname(const char
*name)
av[++ac] = buf.buf;
av[++ac] = NULL;
setup_revisions(ac, av, revs, NULL);
+ if (mailmap) {
+ revs.mailmap = xcalloc(1, sizeof(struct string_list));
+ read_mailmap(revs.mailmap, NULL);
+ }
prepare_revision_walk(revs);
commit = get_revision(revs);
if (commit) {
struct pretty_print_context ctx = {0};
+ const char *format;
+
+ if (mailmap)
+ format = %aN %aE;
+ else
+ format = %an %ae;
+
ctx.date_mode = DATE_NORMAL;
strbuf_release(buf);
- format_commit_message(commit, %an %ae, buf, ctx);
+ format_commit_message(commit, format, buf, ctx);
return strbuf_detach(buf, NULL);
}
die(_(No existing author found with '%s'), name);
@@ -1428,6 +1441,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_GROUP(N_(Commit message options)),
OPT_FILENAME('F', file, logfile, N_(read message from
file)),
OPT_STRING(0, author, force_author, N_(author),
N_(override author for commit)),
+ OPT_BOOLEAN(0, use-mailmap, mailmap, N_(Use mailmap file
when searching for author)),
OPT_STRING(0, date, force_date, N_(date), N_(override
date for commit)),
OPT_CALLBACK('m', message, message, N_(message),
N_(commit message), opt_parse_m),
OPT_STRING('c', reedit-message, edit_message, N_(commit),
N_(reuse and edit message from specified commit)),
--
1.8.4.rc4.1.g0d8beaa.dirty
--
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] Remove dead link
Michal Nazarewicz m...@google.com writes: From: Michal Nazarewicz min...@mina86.com Signed-off-by: Michal Nazarewicz min...@mina86.com --- Documentation/technical/pack-heuristics.txt | 6 -- 1 file changed, 6 deletions(-) diff --git a/Documentation/technical/pack-heuristics.txt b/Documentation/technical/pack-heuristics.txt index 8b7ae1c..b7bd951 100644 --- a/Documentation/technical/pack-heuristics.txt +++ b/Documentation/technical/pack-heuristics.txt @@ -366,12 +366,6 @@ been detailed! linus Yes, we always write out most recent first -For the other record: - -pasky njs`: http://pastebin.com/547965 - -The 'net never forgets, so that should be good until the end of time. - njs` And, yeah, I got the part about deeper-in-history stuff having worse IO characteristics, one sort of doesn't care. That is unfortunate, especially given the last line that the patch removes. Has anybody asked pastebin folks why it is gone and if it can be resurrected? -- 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: [PATCHv2] git-diff: Clarify operation when not inside a repository.
From: Junio C Hamano gits...@pobox.com I suspect that it may be a good idea to split the section altogether to reduce confusion like what triggered this thread, e.g. 'git diff' [--options] [--] [path...]:: This form is to view the changes you made relative to the index (staging area for the next commit). In other words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. 'git diff' --no-index [--options] [--] path path:: This form is to compare the given two paths on the filesystem. When run in a working tree controlled by Git, if at least one of the paths points outside the working tree, or when run outside a working tree controlled by Git, you can omit the `--no-index` option. For now, I'll queue your version as-is modulo style fixes, while waiting for others to help polishing the documentation better. It'd difficult to figure out how to describe it well. In my opinion, the problem here is the DWIM nature of the command, which means that there is a lot of interaction between the options that are specified, the number of path arguments, and the circumstances. My preference is for do what I say, that the options restrict the command to operate in exactly one way, which determines the way the paths are used (and thus their number) and the context in which it can be used. But that's not how git-diff works. Dale -- 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: How to set tab size for hunks in “git add -p”?
On Fri, Aug 23, 2013 at 7:28 PM, Янчарук Александр a...@itvault.info wrote: But those settings seems does not affect on |git add -p|. How to set tab size for hunks in *git add -p* command? That's because git add -p doesn't go through less/pager. You can certainly change the tabs size for your terminal with tabs -4 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: [PATCH 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
On Fri, Aug 23, 2013 at 09:41:57AM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Furthermore, we know that one of our endpoints must be
the edge of the run of duplicates. For example, given this
sequence:
idx 0 1 2 3 4 5
key A C C C C D
If we are searching for B, we might hit the duplicate run
at lo=1, hi=3 (e.g., by first mi=3, then mi=0). But we can
never have lo 1, because B C. That is, if our key is
less than the run, we know that lo is the edge, but we can
say nothing of hi. Similarly, if our key is greater than
the run, we know that hi is the edge, but we can say
nothing of lo. But that is enough for us to return not
only not found, but show the position at which we would
insert the new item.
This is somewhat tricky and may deserve an in-code comment.
Do you want me to re-roll, pushing it down into the comment, or do you
want to mark it up yourself? I think there might be some value in the
latter as your re-writing of it as a comment may cross-check that my
logic is sound.
+ if (ofs == 20) {
+ mi = lo;
+ mi_key = base + elem_size * mi + key_offset;
+ cmp = memcmp(mi_key, key, 20);
It think we already know that mi_key[0:ofs_0] and key[0:ofs_0] are
the same at this point and we do not have to compare full 20 bytes
again, but this is done only once and a better readablity of the
above trumps micro-optimization possibility, I think.
Yes, I had the same idea, and came to the same conclusion. Though if
anybody did want to try it, note that we have just overwritten the old
ofs_0, so you would want to bump the new code up above that line).
+uint32_octal() {
micronit (style):
uint32_octal () {
Hmph. I always forget which one we prefer, and we seem to have equal
numbers of both already. Again, want a re-roll or to mark it up
yourself?
+# Print the pack data for object $1, as a delta against object $2 (or as a
full
+# object if $2 is missing or empty). The output is suitable for including
+# directly in the packfile, and represents the entirety of the object
entry.
+# Doing this on the fly (especially picking your deltas) is quite tricky,
so we
+# have hardcoded some well-known objects. See the case statements below
for the
+# complete list.
Cute ;-) I like the idea of having this function with a right API in
place, and cheating by limiting its implementation to what is
necessary.
Just for reference, the procedure I used to generate the base data is
reasonably straight forward:
sha1=$(printf %s $content | git hash-object -w --stdin)
echo $sha1 | git pack-objects --stdout tmp.pack
tail -c +13 tmp.pack no-header.pack
head -c -20 no-header.pack no-trailer.pack
od -b no-trailer.pack | grep ' ' | cut -d' ' -f2- | tr ' ' '\\'
Since we want binary, we can skip the od call at the end (I needed it
to convert to something readable to hand printf). But head -c is not
portable, nor is head with a negative count.
To find items in the same fanout, I just used for-loops to calculate the
sha1s of all 2-byte blobs. And that is why we have the odd magic \7\76
blob.
Making the deltas was considerably less elegant, since we cannot provoke
pack-objects to pick arbitrary deltas (and it will not even try to delta
tiny objects, anyway, which would bloat our samples). I ended up with
the horrible patch below. We _could_ clean it up (error-checking? Who
needs it?) and make it a debug-and-testing-only option for pack-objects,
but I just didn't think the grossness was worth it. Still, it's probably
worth documenting here on the list in case somebody else ever needs to
add new samples to lib-pack.sh.
---
builtin/pack-objects.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8da2a66..e8937f5 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2442,6 +2442,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
const char *rp_av[6];
int rp_ac = 0;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
+ int magic = 0;
struct option pack_objects_options[] = {
OPT_SET_INT('q', quiet, progress,
N_(do not show progress meter), 0),
@@ -2505,6 +2506,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
N_(pack compression level)),
OPT_SET_INT(0, keep-true-parents, grafts_replace_parents,
N_(do not hide commits by grafts), 0),
+ OPT_BOOL(0, magic, magic, make deltas),
OPT_END(),
};
@@ -2520,6 +2522,34 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
argc = parse_options(argc, argv, prefix, pack_objects_options,
Re: t3010 broken by 2eac2a4
On Fri, Aug 23, 2013 at 10:15:55AM -0700, Junio C Hamano wrote: When 5102c617 (Add case insensitivity support for directories when using git status, 2010-10-03) added the directories to the name-hash with trailing slash, there was only a single name hash table to which both real cache entries and leading directory prefixes are registered, so it made some sense to register them with trailing slashes so that we can tell what kind of entry is being returned. But since 2092678c (name-hash.c: fix endless loop with core.ignorecase=true, 2013-02-28), these directory entries that are not the cache entries are kept track of in a separate hashtable, which makes me wonder if it still makes sense to register directories with trailing slashes. And if we stop doing that (and instead if we shrunk the namelen when an unconverted caller asks for a name with a trailing slash to see if a directory exists in the index), wouldn't it automatically fix the directory_exists_in_index_icase()? It does not need to assume that dirname[len] has '/'; after all, it may not even be a valid memory location in the first place. Yeah, I think that is sane overall direction. When I did the sketch that eventually turned into Karsten's 2092678c, that was one of the goals. But I did not keep up with his response and the final patch, and I'm not sure if the slashes still serve some function. So it would definitely need somebody looking carefully at the current logic. More details are in this sub-thread: http://thread.gmane.org/gmane.comp.version-control.git/215820/focus=216284 -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 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
On Fri, 23 Aug 2013, Jeff King wrote: Making the deltas was considerably less elegant, since we cannot provoke pack-objects to pick arbitrary deltas (and it will not even try to delta tiny objects, anyway, which would bloat our samples). I ended up with the horrible patch below. We _could_ clean it up (error-checking? Who needs it?) and make it a debug-and-testing-only option for pack-objects, but I just didn't think the grossness was worth it. Still, it's probably worth documenting here on the list in case somebody else ever needs to add new samples to lib-pack.sh. Maybe using test-delta (from test-delta.c) would have helped here? In any case, if something needs to be permanently added into the code to help in the creation of test objects, I think test-delta.c is a far better place than pack-objects.c. Nicolas -- 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 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
On Fri, Aug 23, 2013 at 02:54:19PM -0400, Nicolas Pitre wrote: On Fri, 23 Aug 2013, Jeff King wrote: Making the deltas was considerably less elegant, since we cannot provoke pack-objects to pick arbitrary deltas (and it will not even try to delta tiny objects, anyway, which would bloat our samples). I ended up with the horrible patch below. We _could_ clean it up (error-checking? Who needs it?) and make it a debug-and-testing-only option for pack-objects, but I just didn't think the grossness was worth it. Still, it's probably worth documenting here on the list in case somebody else ever needs to add new samples to lib-pack.sh. Maybe using test-delta (from test-delta.c) would have helped here? In any case, if something needs to be permanently added into the code to help in the creation of test objects, I think test-delta.c is a far better place than pack-objects.c. *forehead palm* I didn't even know we had test-delta. Yes, that is obviously a way better place (I initially looked at pack-objects because it has the helpers to do compression and the type/size header properly). -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] git-commit: search author pattern against mailmap
Jeff King p...@peff.net writes:
But for that old and established --author parameter that does not
use the standard format guesses feature to be useful, I agree that
it should honor the mailmap.
I wonder if it would hurt anybody if we made this unconditional, not
even with --no-mailmap override? Opinions?
I think it would be OK. You can always override by giving the actual
full address you want instead of a partial one.
OK, so how about labelling it as a bugfix, like this perhaps? We
obviously need a test or two, though.
-- 8 --
From: Antoine Pelisse apeli...@gmail.com
Date: Fri, 23 Aug 2013 15:48:31 +0200
Subject: [PATCH] commit: search author pattern against mailmap
git commit --author=$name sets the author to one whose name
matches the given string from existing commits, when $name is not in
the Name e-mail format. However, it does not honor the mailmap
to use the canonical name for the author found this way.
Fix it by telling the logic to find a matching existing author to
honor the mailmap, and use the name and email after applying the
mailmap.
Signed-off-by: Antoine Pelisse apeli...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/commit.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..5b7d969 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -30,6 +30,7 @@
#include column.h
#include sequencer.h
#include notes-utils.h
+#include mailmap.h
static const char * const builtin_commit_usage[] = {
N_(git commit [options] [--] pathspec...),
@@ -945,13 +946,16 @@ static const char *find_author_by_nickname(const char
*name)
av[++ac] = buf.buf;
av[++ac] = NULL;
setup_revisions(ac, av, revs, NULL);
+ revs.mailmap = xcalloc(1, sizeof(struct string_list));
+ read_mailmap(revs.mailmap, NULL);
+
prepare_revision_walk(revs);
commit = get_revision(revs);
if (commit) {
struct pretty_print_context ctx = {0};
ctx.date_mode = DATE_NORMAL;
strbuf_release(buf);
- format_commit_message(commit, %an %ae, buf, ctx);
+ format_commit_message(commit, %aN %aE, buf, ctx);
return strbuf_detach(buf, NULL);
}
die(_(No existing author found with '%s'), name);
--
1.8.4-rc4-299-g7e07a8d
--
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] Remove dead link
On Fri, Aug 23 2013, Junio C Hamano wrote: That is unfortunate, especially given the last line that the patch removes. Has anybody asked pastebin folks why it is gone and if it can be resurrected? Way Back Machine has nothing. What was under that link? If IRC logs, then those appear to be available: http://pasky.or.cz/cp/%23git/2006-02-10.log -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
Am 23.08.2013 01:14, schrieb Jeff King: +++ b/t/t5308-pack-detect-duplicates.sh @@ -0,0 +1,73 @@ +#!/bin/sh + +test_description='handling of duplicate objects in incoming packfiles' +. ./test-lib.sh +. ../lib-pack.sh This should be . $TEST_DIRECTORY/lib-pack.sh to support running tests with --root (also in patch 3/6). -- Hannes -- 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-p4 out of memory for very large repository
I think I've cloned files as large as that or larger. If you just want to clone this and move on, perhaps you just need a bit more memory? What's the size of your physical memory and swap partition? Per process memory limit? On 23 Aug 2013 12:59, Corey Thompson cmt...@gmail.com wrote: On 23/08/13 12:59, Corey Thompson wrote: On Fri, Aug 23, 2013 at 07:48:56AM -0400, Corey Thompson wrote: Sorry, I guess I could have included more details in my original post. Since then, I have also made an attempt to clone another (slightly more recent) branch, and at last had success. So I see this does indeed work, it just seems to be very unhappy with one particular branch. So, here are a few statistics I collected on the two branches. branch-that-fails: total workspace disk usage (current head): 12GB 68 files over 20MB largest three being about 118MB branch-that-clones: total workspace disk usage (current head): 11GB 22 files over 20MB largest three being about 80MB I suspect that part of the problem here might be that my company likes to submit very large binaries into our repo (.tar.gzs, pre-compiled third party binaries, etc.). Is there any way I can clone this in pieces? The best I've come up with is to clone only up to a change number just before it tends to fail, and then rebase to the latest. My clone succeeded, but the rebase still runs out of memory. It would be great if I could specify a change number to rebase up to, so that I can just take this thing a few hundred changes at a time. Thanks, Corey And I still haven't told you anything about my platform or git version... This is on Fedora Core 11, with git 1.8.3.4 built from the github repo (117eea7e). -- 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] git-commit: search author pattern against mailmap
On Fri, Aug 23, 2013 at 9:03 PM, Junio C Hamano gits...@pobox.com wrote: OK, so how about labelling it as a bugfix, like this perhaps? We obviously need a test or two, though. OK, I will resubmit tomorrow with some tests. -- 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: CPPCheck found 24 high risk bugs in Git v.1.8.3.4 (fetch.c L588)
From: Philip Oakley philipoak...@iee.org Sent: Monday, August 19, 2013 10:46 PM From: Koch, Rick (Subcontractor) rick.k...@tbe.com Ran CPPCheck 1.5.6 on Windows-XP. Hi Rick, Thank you for the clarification. Normal practice on the list is to use Reply All, so everyone can participate in the discussion. It looks like most of the reports are false positives. My bikeshedding thought would be that it is common in Git to inspect all the call sites such that they don't create the various problems, rather than protect against the problems within the various functions, which may be a cause of the reports (i.e. different philosophical approach to checking). I have double checked the reported: wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588. fprintf(stderr, x %-*s %-*s - %s\n, TRANSPORT_SUMMARY(_([deleted])), REFCOL_WIDTH, _((none)), prettify_refname(ref-name)); At first it did look like there were not enough parameters to satisfy the %-*s format strings, given that the second invocation has an obvious width. This is the only usage within the prune_refs function. A little further looking shows that the %-*s format is used extensively in the wider fetch.c and that the TRANSPORT_SUMMARY(), macro returns two values as required by the fprintf. Inaddition those other invocations aren't flagged showing that this is a false positive, and is a good example for feeding back to CPPCheck (If you wish Rick) as an example so they can see what went wrong. Does CPPCheck give more details of 'why' it thinks the other faults are present? (e.g. the double pointer checks which can be tricky) regards Philip --- v/r Roderick (Rick) Koch OSF - Information Assurance Team Teledyne / Sentar Inc. Work: 256-726-1253 rick.k...@tbe.com -Original Message- From: Philip Oakley [mailto:philipoak...@iee.org] From: Koch, Rick (Subcontractor) rick.k...@tbe.com Sent: Monday, August 19, 2013 6:09 PM I'm directing to this e-mail, as it seems to be the approved forum for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs. Please see the attachment xlsx. Is there a method to post to the Git community to allow the community to review and debunk as faults positive or develop patches to fix lists code files? v/r Roderick (Rick) Koch Information Assurance rick.k...@tbe.com What OS version / CPPCheck version was this checked on? In case other readers don't have a .xlsx reader here is Rick's list in plain text (may be white space damaged). I expect some will be false positives, and some will just be being too cautious. Philip description resourceFilePath fileName lineNumber nullPointer(CppCheck) \git-master\builtin\add.c add.c 286 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c fetch.c 588 False positive. nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c 144 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2803 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2802 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c 2805 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c syslog.c 46 True report. uninitvar(CppCheck) \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c 419 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156 uninitvar(CppCheck) \git-master\merge-recursive.c merge-recursive.c 1887 uninitvar(CppCheck) \git-master\notes.c notes.c 805 uninitvar(CppCheck) \git-master\notes.c notes.c 805 deallocret(CppCheck) \git-master\pretty.c pretty.c 677 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125 doubleFree(CppCheck) \git-master\shell.c shell.c 130 -- 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: [RFC/PATCH] git-remote-mediawiki: reset private ref after non-dumb push
On Fri, Aug 23, 2013 at 3:25 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: This is assuming you follow the scheme git - local repo for other vcs - remote repo for other vcs which itself more or less assumes that the other VCS is a decentralized VCS. I understand this is a good idea for hg or bzr remote helpers, but not applicable for git-remote-mediawiki. A centralized repository is a subset of decentralized workflows. Back to my original problem: the point of dumb push is to make sure the next import will re-import the pushed revisions. if I use something other than the private namespace to keep track of the last imported, then I'll need to do a non-fast forward update to the private ref. I could do that by sending feature force\n it the beginning of the fast-import stream when importing, but that loses one safety feature (e.g. I detected the breakage thanks to the non-fast forward error message, while the tests incorrectly pass if I enable force). I don't know if a dumb push is the right thing to do here, but supposing it is, you can still report non-fast-forward errors: https://github.com/felipec/git/commit/0f7f0a223d710d29a4f1a03fc27348a8690d8a19 https://github.com/felipec/git/commit/b67a8914bc1bb3ad23591875611165b84135aaf9 If it's too much hassle to find non-fast-forward situations by yourself, then perhaps it would make sense to update the remote namespace only when a certain feature has been flagged. -- Felipe Contreras -- 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 v2 1/2] t3103: demonstrate dir.c:treat_one_path() core.ignorecase failure
2eac2a4cc4bdc8d7 (ls-files -k: a directory only can be killed if the index has a non-directory; 2013-08-15) adds a caller of directory_exists_in_index(dirname,len) which forgets to satisfy the undocumented requirement that a '/' must be present at dirname[len] (despite being past the end-of-string). This oversight leads to incorrect behavior when core.ignorecase is true. Demonstrate this. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- t/t3010-ls-files-killed-modified.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 3120efd..198d308 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -103,6 +103,14 @@ test_expect_success 'validate git ls-files -k output.' ' test_cmp .expected .output ' +test_expect_success 'git ls-files -k to show killed files (icase).' ' + git -c core.ignorecase=true ls-files -k .output +' + +test_expect_failure 'validate git ls-files -k output (icase).' ' + test_cmp .expected .output +' + test_expect_success 'git ls-files -m to show modified files.' ' git ls-files -m .output ' -- 1.8.4.rc4.557.g46c5bb2 -- 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 v2 2/2] dir: fix core.ignorecase inconsistency with missing '/'
Although undocumented, directory_exists_in_index_icase(dirname,len)
unconditionally assumes the presence of a '/' at dirname[len], despite
being past the end-of-string. Callers are expected to respect this
assumption by ensuring that a '/' is present beyond the last character
of the passed path.
directory_exists_in_index(), on the other hand, expects no trailing '/'
(and never looks beyond end-of-string). Callers are expected to respect
this by ensuring the absence of '/'.
2eac2a4cc4bdc8d7 (ls-files -k: a directory only can be killed if the
index has a non-directory; 2013-08-15) adds a caller which forgets to
ensure the trailing '/' beyond end-of-string, which leads to
inconsistent behavior between directory_exists_in_index() and
directory_exists_in_index_icase(), depending upon the setting of
core.ignorecase.
One approach to fix this would be to augment the new caller (added by
2eac2a4cc4bdc8d7) to add a '/' beyond end-of-string, but this places
extra burden on the caller to account for an implementation detail of
directory_exists_in_index_icase().
Another approach would be for directory_exists_in_index_icase() to take
responsibility for its own requirements by copying the incoming path and
adding a trailing '/', but such copying can become expensive.
The approach taken by this patch is to pass the strbufs already used by
their callers into directory_exists_in_index() and
directory_exists_in_index_icase() rather than 'const char *' + 'size_t
len'. This allows both functions to satisfy their own internal
requirements -- by manipulating the strbuf to add or remove the trailing
'/' -- without placing burden upon callers and without having to make
expensive copies of each incoming string.
This also fixes an initially-unnoticed failure, when core.ignorecase is
true, in a t3010 test added by 3c56875176390eee (t3010: update to
demonstrate ls-files -k optimization pitfalls; 2013-08-15).
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
dir.c | 42 +++--
t/t3010-ls-files-killed-modified.sh | 2 +-
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/dir.c b/dir.c
index edd666a..6f761a1 100644
--- a/dir.c
+++ b/dir.c
@@ -887,14 +887,21 @@ enum exist_status {
* the directory name; instead, use the case insensitive
* name hash.
*/
-static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
+static enum exist_status directory_exists_in_index_icase(struct strbuf
*dirname)
{
- const struct cache_entry *ce = cache_name_exists(dirname, len + 1,
ignore_case);
+ const struct cache_entry *ce;
unsigned char endchar;
+ size_t len = dirname-len;
+ int need_slash = len dirname-buf[len - 1] != '/';
+
+ if (need_slash)
+ strbuf_addch(dirname, '/');
+ ce = cache_name_exists(dirname-buf, dirname-len, ignore_case);
+ strbuf_setlen(dirname, len);
if (!ce)
return index_nonexistent;
- endchar = ce-name[len];
+ endchar = ce-name[need_slash ? len : len - 1];
/*
* The cache_entry structure returned will contain this dirname
@@ -922,21 +929,25 @@ static enum exist_status
directory_exists_in_index_icase(const char *dirname, in
* the files it contains) will sort with the '/' at the
* end.
*/
-static enum exist_status directory_exists_in_index(const char *dirname, int
len)
+static enum exist_status directory_exists_in_index(struct strbuf *dirname)
{
- int pos;
+ int pos, len;
if (ignore_case)
- return directory_exists_in_index_icase(dirname, len);
+ return directory_exists_in_index_icase(dirname);
+
+ len = dirname-len;
+ if (len dirname-buf[len - 1] == '/')
+ len--;
- pos = cache_name_pos(dirname, len);
+ pos = cache_name_pos(dirname-buf, len);
if (pos 0)
pos = -pos-1;
while (pos active_nr) {
const struct cache_entry *ce = active_cache[pos++];
unsigned char endchar;
- if (strncmp(ce-name, dirname, len))
+ if (strncmp(ce-name, dirname-buf, len))
break;
endchar = ce-name[len];
if (endchar '/')
@@ -983,11 +994,10 @@ static enum exist_status directory_exists_in_index(const
char *dirname, int len)
* (c) otherwise, we recurse into it.
*/
static enum path_treatment treat_directory(struct dir_struct *dir,
- const char *dirname, int len, int exclude,
+ struct strbuf *dirname, int exclude,
const struct path_simplify *simplify)
{
- /* The len-1 is to strip the final '/' */
- switch (directory_exists_in_index(dirname, len-1)) {
+ switch (directory_exists_in_index(dirname)) {
case index_directory:
return path_recurse;
@@ -999,7 +1009,7 @@ static enum path_treatment treat_directory(struct
[PATCH v2 0/2] fix t3010 failure when core.ignorecase=true
This is a re-roll of [1] which fixes a bug in dir.c resulting in failure of a newly added test in t3010 when core.ignorecase=true. Recent mailing list discussion [2][3] suggests that changes at a more fundamental level (if they pan out) would be a more appropriate approach to resolving this bug. Such an approach would (happily) make patch 2/2 unnecessary, however, I'm posting this series for posterity since it was already written before [2][3]. Also, since I'm learning the relevant git internals as I work on these issues, any fix at the more fundamental level (if it comes from me) is going to be correspondingly delayed since there is so much more to digest. Changes since v1: patch 1: place the new test in t3010 instead of t3103 [4] patch 2: make directory_exists_in_index() and directory_exists_in_index_icase() responsible for their own internal requirements regarding presence or lack of trailing '/' [5]; also, pass in already-existing strbufs so that directory_exists_in_index_icase() doesn't have to make its own copy of the incoming string in order to append '/' [1]: http://thread.gmane.org/gmane.comp.version-control.git/232796 [2]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232813 [3]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232822 [4]: http://thread.gmane.org/gmane.comp.version-control.git/232796/focus=232814 [5]: http://thread.gmane.org/gmane.comp.version-control.git/232727/focus=232800 Eric Sunshine (2): t3103: demonstrate dir.c:treat_one_path() core.ignorecase failure dir: test_one_path: fix inconsistent behavior due to missing '/' dir.c | 42 +++-- t/t3010-ls-files-killed-modified.sh | 8 +++ 2 files changed, 34 insertions(+), 16 deletions(-) -- 1.8.4.rc4.557.g46c5bb2 -- 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] git-commit: search author pattern against mailmap
Antoine Pelisse apeli...@gmail.com writes: On Fri, Aug 23, 2013 at 9:03 PM, Junio C Hamano gits...@pobox.com wrote: OK, so how about labelling it as a bugfix, like this perhaps? We obviously need a test or two, though. OK, I will resubmit tomorrow with some tests. Thanks. Also, after I sent out that like this patch, I realized that the mailmap string-list no longer has to be on the heap if we are doing it unconditionally (it can be an auto variable in the function, sitting next to struct strbuf buf). -- 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/2] fix t3010 failure when core.ignorecase=true
On Fri, Aug 23, 2013 at 5:00 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: Recent mailing list discussion [2][3] suggests that changes at a more fundamental level (if they pan out) would be a more appropriate approach to resolving this bug. Such an approach would (happily) make patch 2/2 unnecessary, however, I'm posting this series for posterity since it was already written before [2][3]. Fair enough. In the meantime, given the reaction from Peff, I am tempted to squash your work around in the caller side as a fix to 2eac2a4c (ls-files -k: a directory only can be killed if the index has a non-directory, 2013-08-15) and squash in the patch to run ls-files -k with -c core.ignorecase=true to 3c568751 (t3010: update to demonstrate ls-files -k optimization pitfalls, 2013-08-15). That way, the eventual fix of not adding '/' at the end do not have to revert the changes to the caller, and the tests added to t3010 by the latter will be optimization pitfalls as before. That sounds like a reasonable interim solution. -- 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/2] fix t3010 failure when core.ignorecase=true
Eric Sunshine sunsh...@sunshineco.com writes: Recent mailing list discussion [2][3] suggests that changes at a more fundamental level (if they pan out) would be a more appropriate approach to resolving this bug. Such an approach would (happily) make patch 2/2 unnecessary, however, I'm posting this series for posterity since it was already written before [2][3]. Fair enough. In the meantime, given the reaction from Peff, I am tempted to squash your work around in the caller side as a fix to 2eac2a4c (ls-files -k: a directory only can be killed if the index has a non-directory, 2013-08-15) and squash in the patch to run ls-files -k with -c core.ignorecase=true to 3c568751 (t3010: update to demonstrate ls-files -k optimization pitfalls, 2013-08-15). That way, the eventual fix of not adding '/' at the end do not have to revert the changes to the caller, and the tests added to t3010 by the latter will be optimization pitfalls as before. 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
[ANNOUNCE] Git v1.8.4
The latest feature release Git v1.8.4 is now available at the usual places. It contains 870+ changes from ~100 contributors (among which 33 people are new) since v1.8.3. We will have two more releases til the end of this year; the release after that could be Git 2.0. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 2a361a2d185b8bc604f7f2ce2f502d0dea9d3279 git-1.8.4.tar.gz f130398eb623c913497ef51a6e61d916fe7e31c8 git-htmldocs-1.8.4.tar.gz 8c67a7bc442d6191bc17633c7f2846c71bda71cf git-manpages-1.8.4.tar.gz The following public repositories all have a copy of the v1.8.4 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Also, http://www.kernel.org/pub/software/scm/git/ has copies of the release tarballs. Git v1.8.4 Release Notes Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics that pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. Updates since v1.8.3 Foreign interfaces, subsystems and ports. * Cygwin port has been updated for more recent Cygwin 1.7. * git rebase -i now honors --strategy and -X options. * Git-gui has been updated to its 0.18.0 version. * MediaWiki remote helper (in contrib/) has been updated to use the credential helper interface from Git.pm. * Update build for Cygwin 1.[57]. Torsten Bögershausen reports that this is fine with Cygwin 1.7 ($gmane/225824) so let's try moving it ahead. * The credential helper to talk to keychain on OS X (in contrib/) has been updated to kick in not just when talking http/https but also imap(s) and smtp. * Remote transport helper has been updated to report errors and maintain ref hierarchy used to keep track of its own state better. * With export remote-helper protocol, (1) a push that tries to update a remote ref whose name is different from the pushing side does not work yet, and (2) the helper may not know how to do --dry-run; these problematic cases are disabled for now. * git-remote-hg/bzr (in contrib/) updates. * git-remote-mw (in contrib/) hints users to check the certificate, when https:// connection failed. * git-remote-mw (in contrib/) adds a command to allow previewing the contents locally before pushing it out, when working with a MediaWiki remote. UI, Workflows Features * Sample post-receive-email hook script got an enhanced replacement multimail (in contrib/). * Also in contrib/ is a new contacts script that runs git blame to find out the people who may be interested in a set of changes. * git clean command learned an interactive mode. * The --head option to git show-ref was only to add HEAD to the list of candidate refs to
Re: [PATCH 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
On Fri, Aug 23, 2013 at 09:41:39PM +0200, Johannes Sixt wrote: Am 23.08.2013 01:14, schrieb Jeff King: +++ b/t/t5308-pack-detect-duplicates.sh @@ -0,0 +1,73 @@ +#!/bin/sh + +test_description='handling of duplicate objects in incoming packfiles' +. ./test-lib.sh +. ../lib-pack.sh This should be . $TEST_DIRECTORY/lib-pack.sh to support running tests with --root (also in patch 3/6). Doh, you would think that I would remember that, as the one who introduced --root in the first place. Will fix. Thanks for noticing. -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
[PATCH 1/6] test-sha1: add a binary output mode
The test-sha1 helper program will run our internal sha1
routines over its input and output the 40-byte hex sha1.
Sometimes, however, it is useful to have the binary 20-byte
sha1 (and it's a pain to convert back in the shell). Let's
add a -b option to output the binary version.
Signed-off-by: Jeff King p...@peff.net
Acked-by: Nicolas Pitre n...@fluxnic.net
---
test-sha1.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/test-sha1.c b/test-sha1.c
index 80daba9..e57eae1 100644
--- a/test-sha1.c
+++ b/test-sha1.c
@@ -5,10 +5,15 @@ int main(int ac, char **av)
git_SHA_CTX ctx;
unsigned char sha1[20];
unsigned bufsz = 8192;
+ int binary = 0;
char *buffer;
- if (ac == 2)
- bufsz = strtoul(av[1], NULL, 10) * 1024 * 1024;
+ if (ac == 2) {
+ if (!strcmp(av[1], -b))
+ binary = 1;
+ else
+ bufsz = strtoul(av[1], NULL, 10) * 1024 * 1024;
+ }
if (!bufsz)
bufsz = 8192;
@@ -42,6 +47,10 @@ int main(int ac, char **av)
git_SHA1_Update(ctx, buffer, this_sz);
}
git_SHA1_Final(sha1, ctx);
- puts(sha1_to_hex(sha1));
+
+ if (binary)
+ fwrite(sha1, 1, 20, stdout);
+ else
+ puts(sha1_to_hex(sha1));
exit(0);
}
--
1.8.4.rc2.28.g6bb5f3f
--
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 2/6] sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
The sha1_entry_pos function tries to be smart about
selecting the middle of a range for its binary search by
looking at the value differences between the lo and hi
constraints. However, it is unable to cope with entries with
duplicate keys in the sorted list.
We may hit a point in the search where both our lo and
hi point to the same key. In this case, the range of
values between our endpoints is 0, and trying to scale the
difference between our key and the endpoints over that range
is undefined (i.e., divide by zero). The current code
catches this with an assert(lov hiv).
Moreover, after seeing that the first 20 byte of the key are
the same, we will try to establish a value from the 21st
byte. Which is nonsensical.
Instead, we can detect the case that we are in a run of
duplicates, and simply do a final comparison against any one
of them (since they are all the same, it does not matter
which). If the keys match, we have found our entry (or one
of them, anyway). If not, then we know that we do not need
to look further, as we must be in a run of the duplicate
key.
Signed-off-by: Jeff King p...@peff.net
Acked-by: Nicolas Pitre n...@fluxnic.net
---
sha1-lookup.c | 47 +++
t/lib-pack.sh | 78 +++
t/t5308-pack-detect-duplicates.sh | 73
3 files changed, 198 insertions(+)
create mode 100644 t/lib-pack.sh
create mode 100755 t/t5308-pack-detect-duplicates.sh
diff --git a/sha1-lookup.c b/sha1-lookup.c
index c4dc55d..2dd8515 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -204,7 +204,54 @@ int sha1_entry_pos(const void *table,
* byte 0 thru (ofs-1) are the same between
* lo and hi; ofs is the first byte that is
* different.
+*
+* If ofs==20, then no bytes are different,
+* meaning we have entries with duplicate
+* keys. We know that we are in a solid run
+* of this entry (because the entries are
+* sorted, and our lo and hi are the same,
+* there can be nothing but this single key
+* in between). So we can stop the search.
+* Either one of these entries is it (and
+* we do not care which), or we do not have
+* it.
+*
+* Furthermore, we know that one of our
+* endpoints must be the edge of the run of
+* duplicates. For example, given this
+* sequence:
+*
+* idx 0 1 2 3 4 5
+* key A C C C C D
+*
+* If we are searching for B, we might
+* hit the duplicate run at lo=1, hi=3
+* (e.g., by first mi=3, then mi=0). But we
+* can never have lo 1, because B C.
+* That is, if our key is less than the
+* run, we know that lo is the edge, but
+* we can say nothing of hi. Similarly,
+* if our key is greater than the run, we
+* know that hi is the edge, but we can
+* say nothing of lo.
+*
+* Therefore if we do not find it, we also
+* know where it would go if it did exist:
+* just on the far side of the edge that we
+* know about.
*/
+ if (ofs == 20) {
+ mi = lo;
+ mi_key = base + elem_size * mi + key_offset;
+ cmp = memcmp(mi_key, key, 20);
+ if (!cmp)
+ return mi;
+ if (cmp 0)
+ return -1 - hi;
+ else
+ return -1 - lo;
+ }
+
hiv = hi_key[ofs_0];
if (ofs_0 19)
hiv = (hiv 8) | hi_key[ofs_0+1];
diff --git a/t/lib-pack.sh b/t/lib-pack.sh
new file mode 100644
index 000..fecd5a0
--- /dev/null
+++ b/t/lib-pack.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Support routines for hand-crafting weird or malicious packs.
+#
+# You can make a complete pack like:
+#
+# pack_header 2 foo.pack
+# pack_obj e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 foo.pack
+# pack_obj e68fe8129b546b101aee9510c5328e7f21ca1d18 foo.pack
+# pack_trailer foo.pack
+
+# Print
[PATCH 3/6] add tests for indexing packs with delta cycles
If we receive a broken or malicious pack from a remote, we
will feed it to index-pack. As index-pack processes the
objects as a stream, reconstructing and hashing each object
to get its name, it is not very susceptible to doing the
wrong with bad data (it simply notices that the data is
bogus and aborts).
However, one question raised on the list is whether it could
be susceptible to problems during the delta-resolution
phase. In particular, can a cycle in the packfile deltas
cause us to go into an infinite loop or cause any other
problem?
The answer is no.
We cannot have a cycle of delta-base offsets, because they
go only in one direction (the OFS_DELTA object mentions its
base by an offset towards the beginning of the file, and we
explicitly reject negative offsets).
We can have a cycle of REF_DELTA objects, which refer to
base objects by sha1 name. However, index-pack does not know
these sha1 names ahead of time; it has to reconstruct the
objects to get their names, and it cannot do so if there is
a delta cycle (in other words, it does not even realize
there is a cycle, but only that there are items that cannot
be resolved).
Even though we can reason out that index-pack should handle
this fine, let's add a few tests to make sure it behaves
correctly.
Signed-off-by: Jeff King p...@peff.net
Acked-by: Nicolas Pitre n...@fluxnic.net
---
t/lib-pack.sh| 22 +
t/t5309-pack-delta-cycles.sh | 59
2 files changed, 81 insertions(+)
create mode 100755 t/t5309-pack-delta-cycles.sh
diff --git a/t/lib-pack.sh b/t/lib-pack.sh
index fecd5a0..7e8685b 100644
--- a/t/lib-pack.sh
+++ b/t/lib-pack.sh
@@ -55,6 +55,28 @@ pack_obj () {
printf '\062\170\234\143\267\3\0\0\116\0\106'
return
;;
+ 01d7713666f4de822776c7622c10f1b07de280dc)
+ printf '\165\1\327\161\66\146\364\336\202\47\166'
+ printf '\307\142\54\20\361\260\175\342\200\334\170'
+ printf '\234\143\142\142\142\267\003\0\0\151\0\114'
+ return
+ ;;
+ esac
+ ;;
+
+ # blob containing \7\0
+ 01d7713666f4de822776c7622c10f1b07de280dc)
+ case $2 in
+ '')
+ printf '\062\170\234\143\147\0\0\0\20\0\10'
+ return
+ ;;
+ e68fe8129b546b101aee9510c5328e7f21ca1d18)
+ printf '\165\346\217\350\22\233\124\153\20\32\356'
+ printf '\225\20\305\62\216\177\41\312\35\30\170\234'
+ printf '\143\142\142\142\147\0\0\0\53\0\16'
+ return
+ ;;
esac
;;
esac
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
new file mode 100755
index 000..1640bf7
--- /dev/null
+++ b/t/t5309-pack-delta-cycles.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='test index-pack handling of delta cycles in packfiles'
+. ./test-lib.sh
+. $TEST_DIRECTORY/lib-pack.sh
+
+# Two similar-ish objects that we have computed deltas between.
+A=01d7713666f4de822776c7622c10f1b07de280dc
+B=e68fe8129b546b101aee9510c5328e7f21ca1d18
+
+# double-check our hand-constucted packs
+test_expect_success 'index-pack works with a single delta (A-B)' '
+ clear_packs
+ {
+ pack_header 2
+ pack_obj $A $B
+ pack_obj $B
+ } ab.pack
+ pack_trailer ab.pack
+ git index-pack --stdin ab.pack
+ git cat-file -t $A
+ git cat-file -t $B
+'
+
+test_expect_success 'index-pack works with a single delta (B-A)' '
+ clear_packs
+ {
+ pack_header 2
+ pack_obj $A
+ pack_obj $B $A
+ } ba.pack
+ pack_trailer ba.pack
+ git index-pack --stdin ba.pack
+ git cat-file -t $A
+ git cat-file -t $B
+'
+
+test_expect_success 'index-pack detects missing base objects' '
+ clear_packs
+ {
+ pack_header 1
+ pack_obj $A $B
+ } missing.pack
+ pack_trailer missing.pack
+ test_must_fail git index-pack --fix-thin --stdin missing.pack
+'
+
+test_expect_success 'index-pack detects REF_DELTA cycles' '
+ clear_packs
+ {
+ pack_header 2
+ pack_obj $A $B
+ pack_obj $B $A
+ } cycle.pack
+ pack_trailer cycle.pack
+ test_must_fail git index-pack --fix-thin --stdin cycle.pack
+'
+
+test_done
--
1.8.4.rc2.28.g6bb5f3f
--
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 5/6] index-pack: optionally reject packs with duplicate objects
Git should never generate packs with duplicate objects.
However, we may see such packs due to bugs in Git or other
implementations (e.g., JGit had such a bug a few years ago).
In theory, such packs should not be a problem for us (we
will simply find one of the instances of the object when
looking in the pack). However, the JGit bug report mentioned
possible infinite loops during repacking due to cycles in
the delta chain. Though this problem hasn't specifically
been reproduced on modern git, there is no reason not to be
careful with incoming packs, given that only buggy
implementations should be producing such packs, anyway.
This patch introduces the pack.indexDuplicates option to
allow or reject such packs from index-pack. The default
remains to allow it.
Signed-off-by: Jeff King p...@peff.net
Acked-by: Nicolas Pitre n...@fluxnic.net
---
builtin/index-pack.c | 4
pack-write.c | 24
pack.h| 2 ++
t/t5308-pack-detect-duplicates.sh | 8
4 files changed, 38 insertions(+)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 79dfe47..72e19a0 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1364,6 +1364,10 @@ static int git_index_pack_config(const char *k, const
char *v, void *cb)
#endif
return 0;
}
+ if (!strcmp(k, pack.indexduplicates)) {
+ opts-allow_duplicates = git_config_bool(k, v);
+ return 0;
+ }
return git_default_config(k, v, cb);
}
diff --git a/pack-write.c b/pack-write.c
index ca9e63b..da946a7 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -7,6 +7,7 @@ void reset_pack_idx_option(struct pack_idx_option *opts)
memset(opts, 0, sizeof(*opts));
opts-version = 2;
opts-off32_limit = 0x7fff;
+ opts-allow_duplicates = 1;
}
static int sha1_compare(const void *_a, const void *_b)
@@ -37,6 +38,19 @@ static int need_large_offset(off_t offset, const struct
pack_idx_option *opts)
sizeof(ofsval), cmp_uint32);
}
+static void *find_duplicate(void *vbase, size_t n, size_t size,
+ int (*cmp)(const void *, const void *))
+{
+ unsigned char *base = vbase;
+ while (n 1) {
+ if (!cmp(base, base + size))
+ return base;
+ base += size;
+ n--;
+ }
+ return NULL;
+}
+
/*
* On entry *sha1 contains the pack content SHA1 hash, on exit it is
* the SHA1 hash of sorted object names. The objects array passed in
@@ -68,6 +82,16 @@ const char *write_idx_file(const char *index_name, struct
pack_idx_entry **objec
else
sorted_by_sha = list = last = NULL;
+ if (!opts-allow_duplicates) {
+ struct pack_idx_entry **dup;
+
+ dup = find_duplicate(sorted_by_sha, nr_objects,
+sizeof(*sorted_by_sha), sha1_compare);
+ if (dup)
+ die(pack has duplicate entries for %s,
+ sha1_to_hex((*dup)-sha1));
+ }
+
if (opts-flags WRITE_IDX_VERIFY) {
assert(index_name);
f = sha1fd_check(index_name);
diff --git a/pack.h b/pack.h
index aa6ee7d..45217b6 100644
--- a/pack.h
+++ b/pack.h
@@ -44,6 +44,8 @@ struct pack_idx_option {
uint32_t version;
uint32_t off32_limit;
+ int allow_duplicates;
+
/*
* List of offsets that would fit within off32_limit but
* need to be written out as 64-bit entity for byte-for-byte
diff --git a/t/t5308-pack-detect-duplicates.sh
b/t/t5308-pack-detect-duplicates.sh
index 04fe242..97ce2e0 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -70,4 +70,12 @@ test_expect_success 'lookup in duplicated pack
(GIT_USE_LOOKUP)' '
test_cmp expect actual
'
+test_expect_success 'index-pack can reject packs with duplicates' '
+ clear_packs
+ create_pack dups.pack 2
+ test_must_fail \
+ git -c pack.indexDuplicates=0 index-pack --stdin dups.pack
+ test_expect_code 1 git cat-file -e $LO_SHA1
+'
+
test_done
--
1.8.4.rc2.28.g6bb5f3f
--
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 6/6] default pack.indexDuplicates to false
We should never see duplicate objects in packs, and it is unknown what kind of complications such packs could create during the repacking process. The previous commit introduced a safety valve for checking packs coming into the repository and being indexed by index-pack. Let's turn the safety valve on by default. This helps protect sites receiving packfiles from potentially malicious strangers, and shouldn't affect normal use (and if it does, it will have helped us identify a buggy packfile writer). In a pinch, users can recover by turning off the option, or by resorting to unpack-objects to explode the pack. Note that this also turns the option on for packs we write ourselves (e.g., via pack-objects, fast-import, or streaming large blobs). We do not expect maliciously constructed packfiles in these code paths, of course, but it can serve as an extra check that we have no accidentally created a buggy pack (and the check itself is cheap to perform). Signed-off-by: Jeff King p...@peff.net Acked-by: Nicolas Pitre n...@fluxnic.net --- pack-write.c | 1 - t/t5308-pack-detect-duplicates.sh | 9 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pack-write.c b/pack-write.c index da946a7..1e3c459 100644 --- a/pack-write.c +++ b/pack-write.c @@ -7,7 +7,6 @@ void reset_pack_idx_option(struct pack_idx_option *opts) memset(opts, 0, sizeof(*opts)); opts-version = 2; opts-off32_limit = 0x7fff; - opts-allow_duplicates = 1; } static int sha1_compare(const void *_a, const void *_b) diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh index 97ce2e0..e0ba5e1 100755 --- a/t/t5308-pack-detect-duplicates.sh +++ b/t/t5308-pack-detect-duplicates.sh @@ -37,10 +37,10 @@ test_expect_success 'index-pack will allow duplicate objects by default' ' git index-pack --stdin no-dups.pack ' -test_expect_success 'index-pack will allow duplicate objects by default' ' +test_expect_success 'index-pack will allow duplicate objects if asked' ' clear_packs create_pack dups.pack 100 - git index-pack --stdin dups.pack + git -c pack.indexDuplicates=true index-pack --stdin dups.pack ' test_expect_success 'create batch-check test vectors' ' @@ -70,11 +70,10 @@ test_expect_success 'index-pack can reject packs with duplicates' ' test_cmp expect actual ' -test_expect_success 'index-pack can reject packs with duplicates' ' +test_expect_success 'index-pack rejects packs with duplicates by default' ' clear_packs create_pack dups.pack 2 - test_must_fail \ - git -c pack.indexDuplicates=0 index-pack --stdin dups.pack + test_must_fail git index-pack --stdin dups.pack test_expect_code 1 git cat-file -e $LO_SHA1 ' -- 1.8.4.rc2.28.g6bb5f3f -- 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 4/6] test index-pack on packs with recoverable delta cycles
The previous commit added tests to show that index-pack
correctly bails in unrecoverable situations. There are some
situations where the data could be recovered, but it is not
currently:
1. If we can break the cycle using an object from another
pack via --fix-thin.
2. If we can break the cycle using a duplicate of one of
the objects found in the same pack.
Note that neither of these is particularly high priority; a
delta cycle within a pack should never occur, and we have no
record of even a buggy git implementation creating such a
pack.
However, it's worth adding these tests for two reasons. One,
to document that we do not currently handle the situation,
even though it is possible. And two, to exercise the code
that runs in this situation; even though it fails, by
running it we can confirm that index-pack detects the
situation and aborts, and does not misbehave (e.g., by
following the cycle in an infinite loop).
In both cases, we hit an assert that aborts index-pack.
Signed-off-by: Jeff King p...@peff.net
Acked-by: Nicolas Pitre n...@fluxnic.net
---
t/t5309-pack-delta-cycles.sh | 18 ++
1 file changed, 18 insertions(+)
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
index 1640bf7..3e7861b 100755
--- a/t/t5309-pack-delta-cycles.sh
+++ b/t/t5309-pack-delta-cycles.sh
@@ -56,4 +56,22 @@ test_expect_success 'index-pack detects REF_DELTA cycles' '
test_must_fail git index-pack --fix-thin --stdin cycle.pack
'
+test_expect_failure 'failover to an object in another pack' '
+ clear_packs
+ git index-pack --stdin ab.pack
+ git index-pack --stdin --fix-thin cycle.pack
+'
+
+test_expect_failure 'failover to a duplicate object in the same pack' '
+ clear_packs
+ {
+ pack_header 3
+ pack_obj $A $B
+ pack_obj $B $A
+ pack_obj $A
+ } recoverable.pack
+ pack_trailer recoverable.pack
+ git index-pack --fix-thin --stdin recoverable.pack
+'
+
test_done
--
1.8.4.rc2.28.g6bb5f3f
--
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 v3 17/24] read-cache: read cache-tree in index-v5
On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer t.gumme...@gmail.com wrote:
+/*
+ * This function modifies the directory argument that is given to it.
+ * Don't use it if the directory entries are still needed after.
+ */
There goes my hope of keeping directory_entry* in core so that at
write-time, after validation, we only need to recreate some trees
instead of all of them..
Or we could make cache-tree keep references to directory_entry. If a
cache-tree is not invalidated, then the attached directory_tree should
be reused..
+static struct cache_tree *cache_tree_convert_v5(struct directory_entry *de)
+{
+ if (!de-de_nentries)
+ return NULL;
+ sort_directories(de);
+ return convert_one(de);
+}
+
static int read_entries(struct index_state *istate, struct directory_entry
*de,
unsigned int first_entry_offset, void *mmap,
unsigned long mmap_size, unsigned int *nr,
@@ -591,6 +668,7 @@ static int read_index_v5(struct index_state *istate, void
*mmap,
}
de = de-next;
}
+ istate-cache_tree = cache_tree_convert_v5(root_directory);
istate-cache_nr = nr;
return 0;
}
Otherwise we do need to free root_directory down to the deepest
subtrees, I think. People have been complaining about read-cache
leaking memory like mad, so this is a real issue. Even if you keep
references in cache-tree, you still need to free it
cache_tree_invalidate_path() to avoid leaking
--
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
[PATCH v2 1/3] t3404: make tests more self-contained
As its very first action, t3404 installs (via set_fake_editor) a specialized $EDITOR which simplifies automated 'rebase -i' testing. Many tests rely upon this setting, thus tests which need a different editor must take extra care upon completion to restore $EDITOR in order to avoid breaking following tests. This places extra burden upon such tests and requires that they undesirably have extra knowledge about surrounding tests. Ease this burden by having each test install the $EDITOR it requires, rather than relying upon a global setting. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- t/t3404-rebase-interactive.sh | 65 +-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 49ccb38..f857161 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -29,8 +29,6 @@ Initial setup: . $TEST_DIRECTORY/lib-rebase.sh -set_fake_editor - # WARNING: Modifications to the initial repository can change the SHA ID used # in the expect2 file for the 'stop on conflicting pick' test. @@ -72,6 +70,7 @@ export SHELL test_expect_success 'rebase -i with the exec command' ' git checkout master ( + set_fake_editor FAKE_LINES=1 exec_touch-one 2 exec_touch-two exec_false exec_touch-three 3 4 exec_\touch-file__name_with_spaces\;_touch-after-semicolon 5 @@ -93,6 +92,7 @@ test_expect_success 'rebase -i with the exec command' ' test_expect_success 'rebase -i with the exec command runs from tree root' ' git checkout master mkdir subdir (cd subdir + set_fake_editor FAKE_LINES=1 exec_touch-subdir \ git rebase -i HEAD^ ) @@ -103,6 +103,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' ' test_expect_success 'rebase -i with the exec command checks tree cleanness' ' git checkout master ( + set_fake_editor FAKE_LINES=exec_echo_foo_file1 1 export FAKE_LINES test_must_fail git rebase -i HEAD^ @@ -116,6 +117,7 @@ test_expect_success 'rebase -i with exec of inexistent command' ' git checkout master test_when_finished git rebase --abort ( + set_fake_editor FAKE_LINES=exec_this-command-does-not-exist 1 export FAKE_LINES test_must_fail git rebase -i HEAD^ actual 21 @@ -125,6 +127,7 @@ test_expect_success 'rebase -i with exec of inexistent command' ' test_expect_success 'no changes are a nop' ' git checkout branch2 + set_fake_editor git rebase -i F test $(git symbolic-ref -q HEAD) = refs/heads/branch2 test $(git rev-parse I) = $(git rev-parse HEAD) @@ -134,6 +137,7 @@ test_expect_success 'test the [branch] option' ' git checkout -b dead-end git rm file6 git commit -m stop here + set_fake_editor git rebase -i F branch2 test $(git symbolic-ref -q HEAD) = refs/heads/branch2 test $(git rev-parse I) = $(git rev-parse branch2) @@ -142,6 +146,7 @@ test_expect_success 'test the [branch] option' ' test_expect_success 'test --onto branch' ' git checkout -b test-onto branch2 + set_fake_editor git rebase -i --onto branch1 F test $(git symbolic-ref -q HEAD) = refs/heads/test-onto test $(git rev-parse HEAD^) = $(git rev-parse branch1) @@ -151,6 +156,7 @@ test_expect_success 'test --onto branch' ' test_expect_success 'rebase on top of a non-conflicting commit' ' git checkout branch1 git tag original-branch1 + set_fake_editor git rebase -i branch2 test file6 = $(git diff --name-only original-branch1) test $(git symbolic-ref -q HEAD) = refs/heads/branch1 @@ -163,6 +169,7 @@ test_expect_success 'reflog for the branch shows state before rebase' ' ' test_expect_success 'exchange two commits' ' + set_fake_editor FAKE_LINES=2 1 git rebase -i HEAD~2 test H = $(git cat-file commit HEAD^ | sed -ne \$p) test G = $(git cat-file commit HEAD | sed -ne \$p) @@ -188,6 +195,7 @@ EOF test_expect_success 'stop on conflicting pick' ' git tag new-branch1 + set_fake_editor test_must_fail git rebase -i master test $(git rev-parse HEAD~3) = $(git rev-parse master) test_cmp expect .git/rebase-merge/patch @@ -208,6 +216,7 @@ test_expect_success 'abort' ' test_expect_success 'abort with error when new base cannot be checked out' ' git rm --cached file1 git commit -m remove file in base + set_fake_editor test_must_fail git rebase -i master output 21 grep The following untracked working tree files would be overwritten by checkout: \ output @@ -222,6 +231,7 @@ test_expect_success 'retain authorship' ' test_tick
[PATCH v2 0/3] fix interactive rebase short SHA-1 collision bug
This is a re-roll of [1] which fixes a short SHA-1 collision bug during rebase -i. v1 graduated to 'next' quickly, before I was able to post a re-roll addressing Junio's review comments, so I instead posted fixes as incremental patches atop 'next' [2]. Also, after already in 'next', I discovered an issue in patch 3/3 where it ignored core.commentchar. This was fixed by another incremental patch atop 'next' [3]. Junio suggested [4] that, when 'next' is ready to be rewound after 1.8.4 is released, a replacement to the original series should be sent with all the incremental patches squashed in. This re-roll does just that. It incorporates all the incremental patches and presents the series afresh. This series is meant to supersede all of the following commits in 'next': * v1 series 4ababc19e3b6 (t3404: restore specialized rebase-editor following commentchar test; 2013-08-12) cda44c3b5098 (t3404: rebase: interactive: demonstrate short SHA-1 collision; 2013-08-12) 9a46c25bdbf7 (rebase: interactive: fix short SHA-1 collision; 2013-08-12) * patches addressing review comments general cleanup 4bd24da943d9 (t3404: preserve test_tick state across short SHA-1 collision test; 2013-08-21) d63f172cfd90 (t3404: make tests more self-contained; 2013-08-21) 7883ad2419a3 (t3404: simplify short SHA-1 collision test; 2013-08-21) * core.commentchar bug fix cd5ce1625074 (rebase -i: fix core.commentchar regression; 2013-08-18) [This series does not include the similar core.commentchar bug fix 7bca7afeff6f (rebase -i: fix cases ignoring core.commentchar; 2013-08-16) currently in 'next' which is intended for 'maint' and 'master'.] [1]: http://thread.gmane.org/gmane.comp.version-control.git/232146 [2]: http://thread.gmane.org/gmane.comp.version-control.git/232718 [3]: http://thread.gmane.org/gmane.comp.version-control.git/232482/focus=232483 [4]: http://thread.gmane.org/gmane.comp.version-control.git/232718/focus=232771 Eric Sunshine (2): t3404: make tests more self-contained t3404: rebase -i: demonstrate short SHA-1 collision Junio C Hamano (1): rebase -i: fix short SHA-1 collision git-rebase--interactive.sh| 30 +++ t/t3404-rebase-interactive.sh | 89 ++- 2 files changed, 117 insertions(+), 2 deletions(-) -- 1.8.4.557.g34b3a2e -- 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 v2 3/3] rebase -i: fix short SHA-1 collision
From: Junio C Hamano gits...@pobox.com
The 'todo' sheet for interactive rebase shows abbreviated SHA-1's and
then performs its operations upon those shortened values. This can lead
to an abort if the SHA-1 of a reworded or edited commit is no longer
unique within the abbreviated SHA-1 space and a subsequent SHA-1 in the
todo list has the same abbreviated value.
For example:
edit f00dfad first
pick badbeef second
If, after editing, the new SHA-1 of first also has prefix badbeef,
then the subsequent 'pick badbeef second' will fail since badbeef is no
longer a unique SHA-1 abbreviation:
error: short SHA1 badbeef is ambiguous.
fatal: Needed a single revision
Invalid commit name: badbeef
Fix this problem by expanding the SHA-1's in the todo list before
performing the operations.
[es: also collapse expand SHA-1's for --edit-todo; respect
core.commentchar in transform_todo_ids(); compose commit message]
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
git-rebase--interactive.sh| 30 ++
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 83d6d46..b97a1d0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -689,6 +689,32 @@ skip_unnecessary_picks () {
die Could not skip unnecessary pick commands
}
+transform_todo_ids () {
+ while read -r command rest
+ do
+ case $command in
+ $comment_char* | exec)
+ # Be careful for oddball commands like 'exec'
+ # that do not have a SHA-1 at the beginning of $rest.
+ ;;
+ *)
+ sha1=$(git rev-parse --verify --quiet $@ ${rest%% *})
+ rest=$sha1 ${rest#* }
+ ;;
+ esac
+ printf '%s\n' $command${rest:+ }$rest
+ done $todo $todo.new
+ mv -f $todo.new $todo
+}
+
+expand_todo_ids() {
+ transform_todo_ids
+}
+
+collapse_todo_ids() {
+ transform_todo_ids --short=7
+}
+
# Rearrange the todo list that has both pick sha1 msg and
# pick sha1 fixup!/squash! msg appears in it so that the latter
# comes immediately after the former, and change pick to
@@ -841,6 +867,7 @@ skip)
edit-todo)
git stripspace --strip-comments $todo $todo.new
mv -f $todo.new $todo
+ collapse_todo_ids
append_todo_help
git stripspace --comment-lines $todo \EOF
@@ -852,6 +879,7 @@ EOF
git_sequence_editor $todo ||
die Could not execute editor
+ expand_todo_ids
exit
;;
@@ -1008,6 +1036,8 @@ git_sequence_editor $todo ||
has_action $todo ||
die_abort Nothing to do
+expand_todo_ids
+
test -d $rewritten || test -n $force_rebase || skip_unnecessary_picks
GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 64a02a0..6cdc2ea 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1049,7 +1049,7 @@ test_expect_success 'short SHA-1 setup' '
)
'
-test_expect_failure 'short SHA-1 collide' '
+test_expect_success 'short SHA-1 collide' '
test_when_finished reset_rebase git checkout master
git checkout collide
(
--
1.8.4.557.g34b3a2e
--
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 v2 2/3] t3404: rebase -i: demonstrate short SHA-1 collision
The 'todo' sheet for interactive rebase shows abbreviated SHA-1's and then performs its operations upon those shortened values. This can lead to an abort if the SHA-1 of a reworded or edited commit is no longer unique within the abbreviated SHA-1 space and a subsequent SHA-1 in the todo list has the same abbreviated value. For example: edit f00dfad first pick badbeef second If, after editing, the new SHA-1 of first also has prefix badbeef, then the subsequent 'pick badbeef second' will fail since badbeef is no longer a unique SHA-1 abbreviation: error: short SHA1 badbeef is ambiguous. fatal: Needed a single revision Invalid commit name: badbeef Demonstrate this problem with a couple of specially crafted commits which initially have distinct abbreviated SHA-1's, but for which the abbreviated SHA-1's collide after a simple rewording of the first commit's message. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- t/t3404-rebase-interactive.sh | 24 1 file changed, 24 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index f857161..64a02a0 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1037,4 +1037,28 @@ test_expect_success 'rebase -i with --strategy and -X' ' test $(cat file1) = Z ' +test_expect_success 'short SHA-1 setup' ' + test_when_finished git checkout master + git checkout --orphan collide + git rm -rf . + ( + unset test_tick + test_commit collide1 collide + test_commit --notick collide2 collide + test_commit --notick collide3 collide + ) +' + +test_expect_failure 'short SHA-1 collide' ' + test_when_finished reset_rebase git checkout master + git checkout collide + ( + unset test_tick + test_tick + set_fake_editor + FAKE_COMMIT_MESSAGE=collide2 ac4f2ee \ + FAKE_LINES=reword 1 2 git rebase -i HEAD~2 + ) +' + test_done -- 1.8.4.557.g34b3a2e -- 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
It is Utterly Private
It is Private I am George Daniels, a Banker and credit system programmer (HSBC bank). I saw your email address while browsing through the bank D.T.C Screen in my office yesterday so I decided to use this very chance to know you. I believe we should use every opportunity to know each other better. However, I am contacting you for obvious reason which you will understand. I am sending this mail just to know if this email address is OK, reply me back so that I will send more details to you. I have a very important thing to discuss with you, I look forward to receiving your response at georgedani...@postino.net. Have a pleasant day. George Daniels -- 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-p4 out of memory for very large repository
On Fri, Aug 23, 2013 at 08:42:44PM +0100, Luke Diamand wrote: I think I've cloned files as large as that or larger. If you just want to clone this and move on, perhaps you just need a bit more memory? What's the size of your physical memory and swap partition? Per process memory limit? The machine has 32GB of memory, so I hope that should be more than sufficient! $ ulimit -a core file size (blocks, -c) 0 data seg size (kbytes, -d) unlimited scheduling priority (-e) 0 file size (blocks, -f) unlimited pending signals (-i) 268288 max locked memory (kbytes, -l) 64 max memory size (kbytes, -m) unlimited open files (-n) 1024 pipe size(512 bytes, -p) 8 POSIX message queues (bytes, -q) 819200 real-time priority (-r) 0 stack size (kbytes, -s) 10240 cpu time (seconds, -t) unlimited max user processes (-u) 1024 virtual memory (kbytes, -v) unlimited file locks (-x) unlimited Admittedly I don't typically look at ulimit, so please excuse me if I interpret this wrong, but I feel like this is indicating that the only artificial limit in place is a maximum of 64kB mlock()'d memory. Thanks, Corey -- 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] write_index: optionally allow broken null sha1s
Commit 4337b58 (do not write null sha1s to on-disk index,
2012-07-28) unconditionally prevents git from writing null
sha1s into the index. The intent was to catch errors in
other parts of the code that might let such an entry slip
into the index (or worse, a tree).
However, some repositories have history in which the trees
contain entries with null sha1s. Because the restriction is
unconditional, it can be difficult to work with these older
histories (for example, you cannot even check out the
history, because you are forbidden to write the broken
index). Worst of all, you cannot use git-filter-branch's
index-filter to repair such a broken history, because
filter-branch will try to write an index for each tree.
We could potentially work around this by using a
commit-filter, and munging the tree manually. However,
filter-branch unconditionally writes the index, even if we
only asked for a commit-filter, so this doesn't work. That
would be possible to fix, but figuring out that a
commit-filter is needed (and what it should contain) is
somewhat non-intuitive.
Instead, let's introduce an environment variable for
relaxing this check, and use it when checking out the index
in filter-branch. We turn it on by default, so users do not
have to learn any special tricks to perform such a
filter-branch. But we only do so for the index check-out, so
any further manipulations the user does will fail unless
they remove the broken entry.
We cannot catch every case, though, since some filter-branch
invocations might not rewrite the index at all. But we still
print a loud warning, so it is unlikely to go unnoticed by
the user.
Signed-off-by: Jeff King p...@peff.net
---
I was tempted to not involve filter-branch in this commit at all, and
instead require the user to manually invoke
GIT_ALLOW_NULL_SHA1=1 git filter-branch ...
to perform such a filter. That would be slightly safer, but requires
some specialized knowledge from the user (and advice on using
filter-branch to remove such entries already exists on places like
stackoverflow, and this patch makes it Just Work on recent versions of
git).
git-filter-branch.sh | 5 ++--
read-cache.c | 13 +++--
t/t7009-filter-branch-null-sha1.sh | 54 ++
3 files changed, 68 insertions(+), 4 deletions(-)
create mode 100755 t/t7009-filter-branch-null-sha1.sh
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ac2a005..98e8fe4 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -283,11 +283,12 @@ while read commit parents; do
case $filter_subdir in
)
- git read-tree -i -m $commit
+ GIT_ALLOW_NULL_SHA1=1 git read-tree -i -m $commit
;;
*)
# The commit may not have the subdirectory at all
- err=$(git read-tree -i -m $commit:$filter_subdir 21) || {
+ err=$(GIT_ALLOW_NULL_SHA1=1 \
+ git read-tree -i -m $commit:$filter_subdir 21) || {
if ! git rev-parse -q --verify $commit:$filter_subdir
then
rm -f $GIT_INDEX_FILE
diff --git a/read-cache.c b/read-cache.c
index c3d5e35..83a7414 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1817,8 +1817,17 @@ int write_index(struct index_state *istate, int newfd)
continue;
if (!ce_uptodate(ce) is_racy_timestamp(istate, ce))
ce_smudge_racily_clean_entry(ce);
- if (is_null_sha1(ce-sha1))
- return error(cache entry has null sha1: %s, ce-name);
+ if (is_null_sha1(ce-sha1)) {
+ static const char msg[] = cache entry has null sha1:
%s;
+ static int allow = -1;
+
+ if (allow 0)
+ allow = git_env_bool(GIT_ALLOW_NULL_SHA1, 0);
+ if (allow)
+ warning(msg, ce-name);
+ else
+ return error(msg, ce-name);
+ }
if (ce_write_entry(c, newfd, ce, previous_name) 0)
return -1;
}
diff --git a/t/t7009-filter-branch-null-sha1.sh
b/t/t7009-filter-branch-null-sha1.sh
new file mode 100755
index 000..2944ac9
--- /dev/null
+++ b/t/t7009-filter-branch-null-sha1.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='filter-branch removal of trees with null sha1'
+. ./test-lib.sh
+
+test_expect_success 'create base commits' '
+ test_commit one
+ test_commit two
+ test_commit three
+'
+
+test_expect_success 'create a commit with a bogus null sha1 in the tree' '
+ test_tick
+ tree=$(
+ {
+ git ls-tree HEAD
+ printf 16 commit $_z40\\tbroken
+ } | git mktree
+ )
+ commit=$(
+
[PATCH 1/3] Make test using invalid commit with -C more strict
In the test 'using invalid commit with -C' git-commit would have failed even if the -C option had been given the correct commit, as there was nothing to commit. Fix it by making sure there is always something to commit and git-commit fails because of the invalid commit provided to it. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t7501-commit.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 99ce36f..699a603 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' ' ' test_expect_success 'using invalid commit with -C' ' - test_must_fail git commit -C bogus + echo bong file + git add file + test_must_fail git commit -C bogus + git reset ' test_expect_success 'nothing to commit' ' -- 1.8.3.4 From 8ad7ef374e1b05cb73d6cf445c44f10e5ec36be3 Mon Sep 17 00:00:00 2001 From: Kacper Kornet drae...@pld-linux.org Date: Sat, 24 Aug 2013 03:58:05 +0100 Subject: [PATCH 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite The test 'patch mode ignores unmerged entries' uses git-add -p, so it depends on the perl code. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t3701-add-interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 9fab25c..8514220 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk add -p (edit)' ' ! grep ^+15 actual ' -test_expect_success 'patch mode ignores unmerged entries' ' +test_expect_success PERL 'patch mode ignores unmerged entries' ' git reset --hard test_commit conflict test_commit non-conflict -- 1.8.3.4 From 14f135b178bfaba67653bc3bedf65ec45e38bc76 Mon Sep 17 00:00:00 2001 From: Kacper Kornet drae...@pld-linux.org Date: Sat, 24 Aug 2013 04:01:02 +0100 Subject: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite The test 'reset -p' uses git-reset -p, so it depends on the perl code. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t7106-reset-unborn-branch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh index 8062cf5..499cd88 100755 --- a/t/t7106-reset-unborn-branch.sh +++ b/t/t7106-reset-unborn-branch.sh @@ -27,7 +27,7 @@ test_expect_success 'reset $file' ' test $(git ls-files) = b ' -test_expect_success 'reset -p' ' +test_expect_success PERL 'reset -p' ' rm .git/index git add a echo y | git reset -p -- 1.8.3.4 -- 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 0/3] Fixes for tests run without perl
This is a set of fixes for problems found while running test suite without perl installed. Kacper -- 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 2/3] t/t3701-add-interactive.sh: Add PERL prerequisite
The test 'patch mode ignores unmerged entries' uses git-add -p, so it depends on the perl code. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t3701-add-interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 9fab25c..8514220 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -330,7 +330,7 @@ test_expect_success PERL 'split hunk add -p (edit)' ' ! grep ^+15 actual ' -test_expect_success 'patch mode ignores unmerged entries' ' +test_expect_success PERL 'patch mode ignores unmerged entries' ' git reset --hard test_commit conflict test_commit non-conflict -- 1.8.3.4 -- 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 1/3] Make test using invalid commit with -C more strict
In the test 'using invalid commit with -C' git-commit would have failed even if the -C option had been given the correct commit, as there was nothing to commit. Fix it by making sure there is always something to commit and git-commit fails because of the invalid commit provided to it. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- t/t7501-commit.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 99ce36f..699a603 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -53,7 +53,10 @@ test_expect_success PERL 'can use paths with --interactive' ' ' test_expect_success 'using invalid commit with -C' ' - test_must_fail git commit -C bogus + echo bong file + git add file + test_must_fail git commit -C bogus + git reset ' test_expect_success 'nothing to commit' ' -- 1.8.3.4 -- 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 v3 18/24] read-cache: write index-v5
On Mon, Aug 19, 2013 at 2:42 AM, Thomas Gummerer t.gumme...@gmail.com wrote: Write the index version 5 file format to disk. This version doesn't write the cache-tree data and resolve-undo data to the file. I keep having things to add after sending my emails. Now that we have all conflicted entries in file block, the conflict data block becomes optional, it functions exactly (I think) like resolve-undo extension, which makes me think it might make sense to make conflict data block an extension. If we make it so, we might want to move cr and ncr fields out of direntries. I don't see a solution yet, but I think it's interesting because future extensions might want to attach stuff to direntries, just like cr/ncr from conflict extension. We may want to think now how that might be done (and conflict extension is a good exercise to see how it works out) -- Duy I -- 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 v3 00/24] Index-v5
On Mon, Aug 19, 2013 at 2:41 AM, Thomas Gummerer t.gumme...@gmail.com wrote: Hi, previous rounds (without api) are at $gmane/202752, $gmane/202923, $gmane/203088 and $gmane/203517, the previous rounds with api were at $gmane/229732 and $gmane/230210. Thanks to Duy for reviewing the the last round and Junio and Ramsay for additional comments. I'm done reviewing this version (I neglected the extension writing patches because after spending hours on the main write patch I don't want to look at them anymore :p). Now that rc period is over, with a partial write proof-of-concept, I think it's enough to call Junio's attention on the series, see if we have any chance of merging it. The partial write POC is needed to make sure we don't overlook anything, just support update-index is enough. -- 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
