* RE: git performance
From: Edward Ned Harvey @ 2008-10-22 21:55 UTC (permalink / raw)
To: git
In-Reply-To: <20081022203624.GA4585@coredump.intra.peff.net>
> Yes, it does stat all the files. How many files are you talking about,
> and what platform? From a warm cache on Linux, the 23,000 files kernel
> repo takes about a tenth of a second to stat all files for me (and this
> on a several year-old machine). And of course many operations don't
> require stat'ing at all (like looking at logs, or diffs that don't
> involve the working tree).
No worries. No solution can meet everyone's needs.
I'm talking about 40-50,000 files, on multi-user production linux, which means the cache is never warm, except when I'm benchmarking. Specifically RHEL 4 with the files on NFS mount. Cold cache "svn st" takes ~10 mins. Warm cache 20-30 sec. Surprisingly to me, performance was approx the same for files on local disk versus NFS. Probably the best solution for us is perforce, we just don't like the pricetag.
Out of curiosity, what are they talking about, when they say "git is fast?" Just the fact that it's all local disk, or is there more to it than that? I could see - git would probably outperform perforce for versioning of large files (let's say iso files) to benefit from sustained local disk IO, while perforce would probably outperform anything I can think of, operating on thousands of tiny files, because it will never walk the tree.
^ permalink raw reply
* [PATCH] Fixed git archive for bare repos
From: Charles Bailey @ 2008-10-22 21:47 UTC (permalink / raw)
To: git; +Cc: Deskin Miller, kenneth johansson, gitster
In-Reply-To: <20081022210913.GB22541@hashpling.org>
This moves the call to git config to a place where it doesn't break the
logic for using git archive in a bare repository but retains the fix to
make git archive respect core.autocrlf.
Signed-off-by: Charles Bailey <charles@hashpling.org>
---
OK, I've had a chance to have a quick look at this issue and I have an
altenative patch.
This is an alternative fix to the current git archive in a bare
repository issue.
It's been lightly tested and doesn't re-break the zip / autocrlf issue
that the previous introduction of git_config was designed to fix.
archive.c | 2 ++
builtin-archive.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/archive.c b/archive.c
index 849eed5..9ac455d 100644
--- a/archive.c
+++ b/archive.c
@@ -336,5 +336,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
parse_treeish_arg(argv, &args, prefix);
parse_pathspec_arg(argv + 1, &args);
+ git_config(git_default_config, NULL);
+
return ar->write_archive(&args);
}
diff --git a/builtin-archive.c b/builtin-archive.c
index 432ce2a..5ceec43 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -111,8 +111,6 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
{
const char *remote = NULL;
- git_config(git_default_config, NULL);
-
remote = extract_remote_arg(&argc, argv);
if (remote)
return run_remote_archiver(remote, argc, argv);
--
1.6.0.2.534.g5ab59
^ permalink raw reply related
* Re: [PATCH 2/3] Add -n/--no-prompt option to mergetool
From: Charles Bailey @ 2008-10-22 21:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, William Pursell, Andreas Ericsson,
Theodore Ts'o
In-Reply-To: <20081021122655.GA29294@hashpling.org>
On Tue, Oct 21, 2008 at 01:26:55PM +0100, Charles Bailey wrote:
> On Tue, Oct 21, 2008 at 01:49:30PM +0200, Andreas Ericsson wrote:
> >
> > There is discussion already about "-n should be for dry-run!" and git's
> > inconsistencies in such matters. Wouldn't -y ("assume yes on prompt")
> > be better?
> >
>
> I must have missed this discussion. I've just had a very quick look at
> a handful of basic modifying git commands (merge, pull, commit,
> checkout, reset, revert) and only found 'add' that used -n as
> --dry-run.
>
> That said, I've no real objections to -y if that makes for a better
> consensus.
>
I'm pretty keen on this patch, but have no strong opinions on which
short option is used, so are there any votes against -y?
If I re-roll with the favoured short option is there anything else
that would prevent its adoption into next or master?
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: [ANNOUCNE] repo - The Multiple Git Repository Tool
From: Shawn O. Pearce @ 2008-10-22 21:16 UTC (permalink / raw)
To: Leo Razoumov; +Cc: git
In-Reply-To: <ee2a733e0810221255u23aabef2i66d878bcbfa11816@mail.gmail.com>
Leo Razoumov <slonik.az@gmail.com> wrote:
> On 10/22/08, Shawn O. Pearce <spearce@spearce.org> wrote:
> >
> > Google developed two tools, repo and Gerrit, and open sourced them
> > under the Apache License:
> >
> > http://android.git.kernel.org/?p=tools/repo.git
> > http://android.git.kernel.org/?p=tools/gerrit.git
> >
> > git://android.git.kernel.org/tools/repo.git
> > git://android.git.kernel.org/tools/gerrit.git
> >
> > repo is a Python application to bind together Git repositories,
> > something like "git submodule",
>
> Are there any plans to make repo an official git command (e.g.
> git-repo) or merge its functionality with git-submodule?
I currently do not plan on asking Junio to consider making repo part
of git. However, I am also not adverse to it if repo development
slows down and there is sufficient community interest.
--
Shawn.
^ permalink raw reply
* Re: git performance
From: Peter Harris @ 2008-10-22 21:13 UTC (permalink / raw)
To: Jeff King; +Cc: Edward Ned Harvey, git
In-Reply-To: <20081022203624.GA4585@coredump.intra.peff.net>
On Wed, Oct 22, 2008 at 4:36 PM, Jeff King wrote:
> On Wed, Oct 22, 2008 at 04:17:16PM -0400, Edward Ned Harvey wrote:
>
>> So - I didn't see anything out there saying "git is fast because it
>> uses inotify" or anything like that. Perhaps git would not help me at
>> all? Because git still needs to stat all the files in the tree?
>
> Yes, it does stat all the files. How many files are you talking about,
> and what platform? From a warm cache on Linux, the 23,000 files kernel
> repo takes about a tenth of a second to stat all files for me (and this
> on a several year-old machine). And of course many operations don't
> require stat'ing at all (like looking at logs, or diffs that don't
> involve the working tree).
Windows is rather slower than Linux, so differences are more obvious.
I find git feels "only" about 2x as fast as svn at status. svn has to
stat all of its base files too, whereas git has the index. git pull
(vs svn update) feels better than 2x faster, since git doesn't need to
walk the tree and lock every sub-dir before it even connects to the
remote server.
So we're not talking 'inotify' fast, but maybe half a cup of coffee
instead of a full cup if you have that many files.
"git-svn" is really quite good. I recommend you try a quick (trunk and
maybe one branch only, last few revisions only) import of your svn
tree to test with.
Peter Harris
^ permalink raw reply
* Re: [ANNOUCNE] repo - The Multiple Git Repository Tool
From: Shawn O. Pearce @ 2008-10-22 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzlkwtpbq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Google developed two tools, repo and Gerrit, and open sourced them
> > under the Apache License:
> >
> > http://android.git.kernel.org/?p=tools/repo.git
> > http://android.git.kernel.org/?p=tools/gerrit.git
> >
> > git://android.git.kernel.org/tools/repo.git
> > git://android.git.kernel.org/tools/gerrit.git
>
> Heh, very nice, with a very shallow history ;-)
Yea, well, some of our history messages are downright messy and/or
contain information we don't want to disclose to the public. Its
apparently "normal" for Google to just drop history when it opens
a code base. :-\
The repo and Gerrit histories are short enough that I might
be able to go through them and clean them up some (remove any
comments revealing Google secrets) and publish the history under a
different repository. Folks can always graft it behind the (now)
real repository.
I can't promise anything, but I'll talk to my co-contributors and
consider doing this in the near future.
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH] archive: fix setup to work in bare repositories
From: Charles Bailey @ 2008-10-22 21:09 UTC (permalink / raw)
To: Deskin Miller; +Cc: kenneth johansson, git, gitster
In-Reply-To: <20081022203722.GD2015@riemann.deskinm.fdns.net>
On Wed, Oct 22, 2008 at 04:37:22PM -0400, Deskin Miller wrote:
> cmd_archive was calling git_config -> setup_git_env prior to
> write_archive calling setup_git_directory. In a bare repository, the
> former will set git_dir to be '.git' since the latter has not
> determined that it's operating in a bare repository yet.
>
> Things are complicated, however, by the fact that git archive --list
> should work from anywhere, not just in git repositories, so that
> argument needs to be checked for before setup_git_directory is called.
Just for some background, I think that this issue might be an
unintended consequence of this commit:
b99b5b40cffb5269e4aa38b6b60391b55039e27d
The only reason that the git_config call has been added is to ensure
the correct setting of core.autocrlf before convert_to_working_tree is
called on any blobs.
I haven't looked in detail, but would moving this call later make for
an cleaner change?
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: [RFC PATCH] archive: fix setup to work in bare repositories
From: Jeff King @ 2008-10-22 20:46 UTC (permalink / raw)
To: Deskin Miller; +Cc: kenneth johansson, git, gitster
In-Reply-To: <20081022203722.GD2015@riemann.deskinm.fdns.net>
On Wed, Oct 22, 2008 at 04:37:22PM -0400, Deskin Miller wrote:
> cmd_archive was calling git_config -> setup_git_env prior to
> write_archive calling setup_git_directory. In a bare repository, the
> former will set git_dir to be '.git' since the latter has not
> determined that it's operating in a bare repository yet.
>
> Things are complicated, however, by the fact that git archive --list
> should work from anywhere, not just in git repositories, so that
> argument needs to be checked for before setup_git_directory is called.
Should you perhaps be able to call setup_git_directory_gently(), and
then once you decide that you really do need the setup, call
setup_git_directory()?
You would have to add a "did we already do run" flag to
setup_git_directory_gently(), but I think it is already an error to call
it twice, so you wouldn't be hurting anything by that.
Note also that by moving the setup, you are moving the chdir() that
happens; you may need to prefix paths to any arguments to accomodate
this (I don't think it should matter, since git-archive shouldn't look
at any paths until after it would have done the setup_git_directory()
before, but I didn't check carefully).
-Peff
^ permalink raw reply
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Jeff King @ 2008-10-22 20:42 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <gdo1a3$akl$1@ger.gmane.org>
On Wed, Oct 22, 2008 at 10:12:54PM +0200, Jakub Narebski wrote:
> Currently you can use path_info URL for blob_plain
>
> http://repo.or.cz/w/git/gitbuild.git/platform:/jk/solaris/config.mak
Ah, I didn't know that. Thanks.
> Soon (thanks to Giuseppe patches) you would be able to use
>
> http://repo.or.cz/w/git/gitbuild.git/blob/platform:/jk/solaris/config.mak
Yes, those patches were actually what spurred my comment. :) I am
looking forward to playing with them once they are on repo.or.cz.
-Peff
^ permalink raw reply
* [RFC PATCH] archive: fix setup to work in bare repositories
From: Deskin Miller @ 2008-10-22 20:37 UTC (permalink / raw)
To: kenneth johansson; +Cc: git, gitster
In-Reply-To: <gdnsca$92r$2@ger.gmane.org>
cmd_archive was calling git_config -> setup_git_env prior to
write_archive calling setup_git_directory. In a bare repository, the
former will set git_dir to be '.git' since the latter has not
determined that it's operating in a bare repository yet.
Things are complicated, however, by the fact that git archive --list
should work from anywhere, not just in git repositories, so that
argument needs to be checked for before setup_git_directory is called.
---
On Wed, Oct 22, 2008 at 06:45:30PM +0000, kenneth johansson wrote:
> On Wed, 22 Oct 2008 09:08:29 -0400, Deskin Miller wrote:
>
> > On Wed, Oct 22, 2008 at 08:42:01AM +0000, kenneth johansson wrote:
> >> I was going to make a tar of the latest stable linux kernel. Done it
> >> before but now I got a strange problem.
> >>
> >> >git archive --format=tar v2.6.27.2
> >> fatal: Not a valid object name
> >
> > I had the same thing happen to me, while trying to make an archive of
> > Git. Were you perchance working in a bare repository, as I was? I spent
> > some time looking at it and I think git archive sets up the environment
> > in the wrong order, though of course I never finished a patch so I'm
> > going from memory:
>
> Yes it was a bare repository.
>
> >
> > After looking at the code again, I think the issue is that git_config is
> > called in builtin-archive.c:cmd_archive before setup_git_directory is
> > called in archive.c:write_archive. The former ends up setting GIT_DIR
> > to be '.git' even if you're in a bare repository. My coding skills
> > weren't up to fixing it easily; moving setup_git_directory before
> > git_config in builtin-archive caused last test of t5000 to fail:
> > GIT_DIR=some/nonexistent/path git archive --list should still display
> > the archive formats.
>
> if I do
> GIT_DIR=. git archive --format=tar v2.6.27.2
>
> it does work so it looks like you are on the right track.
Looks like this works, but I think it's really ugly; let me know if you have
any suggestions for improvement.
archive.c | 13 +++++++++++++
archive.h | 1 +
builtin-archive.c | 4 +++-
t/t5000-tar-tree.sh | 9 +++++++++
4 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/archive.c b/archive.c
index e2280df..d8e4373 100644
--- a/archive.c
+++ b/archive.c
@@ -325,6 +325,19 @@ static int parse_archive_args(int argc, const char **argv,
return argc;
}
+int archive_parse_options_early(int argc, const char **argv)
+{
+ int i;
+ for (i = 1; i < argc; ++i) {
+ if (!strcmp(argv[i], "--list")) {
+ for (i = 0; i < ARRAY_SIZE(archivers); i++)
+ printf("%s\n", archivers[i].name);
+ exit(0);
+ }
+ }
+ return 0;
+}
+
int write_archive(int argc, const char **argv, const char *prefix,
int setup_prefix)
{
diff --git a/archive.h b/archive.h
index 0b15b35..ff5b6cf 100644
--- a/archive.h
+++ b/archive.h
@@ -24,6 +24,7 @@ extern int write_tar_archive(struct archiver_args *);
extern int write_zip_archive(struct archiver_args *);
extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
+extern int archive_parse_options_early(int argc, const char **argv);
extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);
#endif /* ARCHIVE_H */
diff --git a/builtin-archive.c b/builtin-archive.c
index 432ce2a..e518113 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -111,6 +111,8 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
{
const char *remote = NULL;
+ archive_parse_options_early(argc, argv);
+ prefix = setup_git_directory();
git_config(git_default_config, NULL);
remote = extract_remote_arg(&argc, argv);
@@ -119,5 +121,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
- return write_archive(argc, argv, prefix, 1);
+ return write_archive(argc, argv, prefix, 0);
}
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e395ff4..53fe25c 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -192,4 +192,13 @@ test_expect_success \
'git archive --list outside of a git repo' \
'GIT_DIR=some/non-existing/directory git archive --list'
+test_expect_success \
+ 'git archive inside bare repository' \
+ 'git clone --bare "$(pwd)"/.git trash-bare &&
+ cd trash-bare &&
+ git archive --format=tar HEAD >/dev/null &&
+ cd .. &&
+ rm -rf trash-bare
+ '
+
test_done
--
1.6.0.2.554.g3041b
^ permalink raw reply related
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Jeff King @ 2008-10-22 20:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Alexander Gavrilov, git
In-Reply-To: <alpine.DEB.1.00.0810222220410.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Wed, Oct 22, 2008 at 10:29:39PM +0200, Johannes Schindelin wrote:
> > Not all greps support "-e", but in this case we can easily
> > convert it to a single extended regex.
>
> I really wonder if we cannot catch these things (unportable grep, sed, etc
> invokations) with a simple patch to the pre-commit hook.
We could probably write a hook for some of the simpler ones, but we
would have quite a few false negatives, I suspect.
FWIW, I am not finding these portability problems by hand. I am nightly
auto-building and testing Junio's maint, master, and next on Solaris 8
and FreeBSD, and Mike Ralphson is doing the same for AIX. So while we
could perhaps catch them sooner, I am very happy to have caught several
issues recently in next, _before_ they hit master.
-Peff
^ permalink raw reply
* Re: git performance
From: Jeff King @ 2008-10-22 20:36 UTC (permalink / raw)
To: Edward Ned Harvey; +Cc: git
In-Reply-To: <000801c93483$2fdad340$8f9079c0$@com>
On Wed, Oct 22, 2008 at 04:17:16PM -0400, Edward Ned Harvey wrote:
> So - I didn't see anything out there saying "git is fast because it
> uses inotify" or anything like that. Perhaps git would not help me at
> all? Because git still needs to stat all the files in the tree?
Yes, it does stat all the files. How many files are you talking about,
and what platform? From a warm cache on Linux, the 23,000 files kernel
repo takes about a tenth of a second to stat all files for me (and this
on a several year-old machine). And of course many operations don't
require stat'ing at all (like looking at logs, or diffs that don't
involve the working tree).
-Peff
^ permalink raw reply
* Older git version on the server
From: Diego Algorta @ 2008-10-22 20:33 UTC (permalink / raw)
To: git
Hi all,
= Background
We are a team of 3 developers. Each one has his own clone from a central
public repo in our server where we all push and pull from (you can tell
we come from svn). We do deployments on production from that repo's
master branch.
= Problem
We recently started using git submodules and now we're facing a problem
when switching between branches (git checkout) which have the same
submodule but linked from different sources. Some google research says
the new "git submodule sync" feature may be the solution, but we can't
use a version newer than 1.5.6.1 on the server.
= Question
Would it be a problem for us to use 1.6.0.3 on our development
environments and keep using 1.5.6.1 on the server to do the deployments?
Thank you!
--
Diego Algorta - Web Developer & SysAdmin -
http://www.oboxodo.com - http://diego.algorta.net
^ permalink raw reply
* [RFC PATCH 5/5] fix overlapping memcpy in normalize_absolute_path
From: Jeff King @ 2008-10-22 20:32 UTC (permalink / raw)
To: git
In-Reply-To: <20081022202810.GA4439@coredump.intra.peff.net>
The comments for normalize_absolute_path explicitly claim
that the source and destination buffers may be the same
(though they may not otherwise overlap). Thus the call to
memcpy may involve copying overlapping data, and memmove
should be used instead.
This fixes a valgrind error in t1504.
Signed-off-by: Jeff King <peff@peff.net>
---
An alternative fix, since we declare that only true equality is OK,
would be to keep the memcpy and explicitly check for "comp_start ==
dst". That might have a performance benefit if memcpy is faster than
memmove, but I don't know that normalize_absolute_path is in any
critical paths.
path.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/path.c b/path.c
index 76e8872..c1cb54b 100644
--- a/path.c
+++ b/path.c
@@ -348,7 +348,7 @@ int normalize_absolute_path(char *buf, const char *path)
goto next;
}
- memcpy(dst, comp_start, comp_len);
+ memmove(dst, comp_start, comp_len);
dst += comp_len;
next:
comp_start = comp_end;
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* [RFC PATCH 4/5] pack-objects: avoid reading uninitalized data
From: Jeff King @ 2008-10-22 20:31 UTC (permalink / raw)
To: git
In-Reply-To: <20081022202810.GA4439@coredump.intra.peff.net>
In the main loop of find_deltas, we do:
struct object_entry *entry = *list++;
...
if (!*list_size)
...
break
Because we look at and increment *list _before_ the check of
list_size, in the very last iteration of the loop we will
look at uninitialized data, and increment the pointer beyond
one past the end of the allocated space. Since we don't
actually do anything with the data until after the check,
this is not a problem in practice.
But since it technically violates the C standard, and
because it provokes a spurious valgrind warning, let's just
move the initialization of entry to a safe place.
This fixes valgrind errors in t5300, t5301, t5302, t303, and
t9400.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-pack-objects.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 59c30d1..15b80db 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1375,7 +1375,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
array = xcalloc(window, sizeof(struct unpacked));
for (;;) {
- struct object_entry *entry = *list++;
+ struct object_entry *entry;
struct unpacked *n = array + idx;
int j, max_depth, best_base = -1;
@@ -1384,6 +1384,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
progress_unlock();
break;
}
+ entry = *list++;
(*list_size)--;
if (!entry->preferred_base) {
(*processed)++;
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* git performance
From: Edward Ned Harvey @ 2008-10-22 20:17 UTC (permalink / raw)
To: git
I see things all over the Internet saying git is fast. I'm currently struggling with poor svn performance and poor attitude of svn developers, so I'd like to consider switching to git. A quick question first.
The core of the performance problem I'm facing is the need to "walk the tree" for many thousand files. Every time I do "svn update" or "svn status" the svn client must stat every file to check for local modifications (a coffee cup or a beer worth of stats). In essence, this is unavoidable if there is no mechanism to constantly monitor filesystem activity during normal operations. Analogous to filesystem journaling.
So - I didn't see anything out there saying "git is fast because it uses inotify" or anything like that. Perhaps git would not help me at all? Because git still needs to stat all the files in the tree?
^ permalink raw reply
* [RFC PATCH 3/5] correct cache_entry allocation
From: Jeff King @ 2008-10-22 20:30 UTC (permalink / raw)
To: git
In-Reply-To: <20081022202810.GA4439@coredump.intra.peff.net>
Most cache_entry structs are allocated by using the
cache_entry_size macro, which rounds the size of the struct
up to the nearest multiple of 8 bytes (presumably to avoid
memory fragmentation).
There is one exception: the special "conflict entry" is
allocated with an empty name, and so is explicitly given
just one extra byte to hold the NUL.
However, later code doesn't realize that this particular
struct has been allocated differently, and happily tries
reading and copying it based on the ce_size macro, which
assumes the 8-byte alignment.
This can lead to reading uninitalized data, though since
that data is simply padding, there shouldn't be any problem
as a result. Still, it makes sense to hold the padding
assumption so as not to surprise later maintainers.
This fixes valgrind errors in t1005, t3030, t4002, and
t4114.
Signed-off-by: Jeff King <peff@peff.net>
---
unpack-trees.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index e59d144..e5749ef 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -382,7 +382,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
o->merge_size = len;
if (!dfc)
- dfc = xcalloc(1, sizeof(struct cache_entry) + 1);
+ dfc = xcalloc(1, cache_entry_size(0));
o->df_conflict_entry = dfc;
if (len) {
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* [RFC PATCH 2/5] valgrind: ignore ldso errors
From: Jeff King @ 2008-10-22 20:30 UTC (permalink / raw)
To: git
In-Reply-To: <20081022202810.GA4439@coredump.intra.peff.net>
On some Linux systems, we get a host of Cond and Addr errors
from calls to dlopen that are caused by nss modules. We
should be able to safely ignore anything happening in
ld-*.so as "not our problem."
Signed-off-by: Jeff King <peff@peff.net>
---
This was from a Debian etch system, while my lenny systems seemed to
have no trouble. I'm sure we will pick up a few commits like these as
people run it on various platforms, but hopefully they should shake out
pretty quickly.
t/valgrind/default.supp | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 2482b3b..1013847 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -11,6 +11,18 @@
}
{
+ ignore-ldso-cond
+ Memcheck:Cond
+ obj:*ld-*.so
+}
+
+{
+ ignore-ldso-addr8
+ Memcheck:Addr8
+ obj:*ld-*.so
+}
+
+{
writing-data-from-zlib-triggers-errors
Memcheck:Param
write(buf)
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* [RFC PATCH 1/5] add valgrind support in test scripts
From: Jeff King @ 2008-10-22 20:29 UTC (permalink / raw)
To: git
In-Reply-To: <20081022202810.GA4439@coredump.intra.peff.net>
This patch adds the ability to use valgrind's memcheck tool
to diagnose memory problems in git while running the test
scripts. It works by placing a "fake" git in the front of
the test script's PATH; this fake git runs the real git
under valgrind. It also points the exec-path such that any
stand-alone dashed git programs are run using the same
script. In this way we avoid having to modify the actual git
code in any way.
The memcheck tool can be used by specifying
"GIT_TEST_OPTS=--memcheck" in the make invocation. Any
invocation of git that finds any errors under valgrind will
exit with failure code 126. Any valgrind output will go to
the usual stderr channel for tests (i.e., /dev/null, unless
-v has been specified).
A few default suppressions are included, since libz seems to
trigger quite a few false positives. We'll assume that libz
works and that we can ignore any errors which are reported
there.
Signed-off-by: Jeff King <peff@peff.net>
---
.gitignore | 1 +
Makefile | 12 ++++++++++--
t/test-lib.sh | 11 +++++++++++
t/valgrind/.gitignore | 1 +
t/valgrind/default.supp | 21 +++++++++++++++++++++
test-valgrind.sh | 24 ++++++++++++++++++++++++
6 files changed, 68 insertions(+), 2 deletions(-)
create mode 100644 t/valgrind/.gitignore
create mode 100644 t/valgrind/default.supp
create mode 100755 test-valgrind.sh
diff --git a/.gitignore b/.gitignore
index bbaf9de..45045cb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -151,6 +151,7 @@ test-match-trees
test-parse-options
test-path-utils
test-sha1
+test-valgrind
common-cmds.h
*.tar.gz
*.dsc
diff --git a/Makefile b/Makefile
index d6f3695..68f0172 100644
--- a/Makefile
+++ b/Makefile
@@ -279,6 +279,8 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
$(patsubst %.perl,%,$(SCRIPT_PERL)) \
git-instaweb
+VALGRIND_SH += test-valgrind.sh
+
# Empty...
EXTRA_PROGRAMS =
@@ -1139,7 +1141,7 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt
common-cmds.h: $(wildcard Documentation/git-*.txt)
$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@
-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
+$(patsubst %.sh,%,$(SCRIPT_SH) $(VALGRIND_SH)) : % : %.sh
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
@@ -1343,7 +1345,12 @@ all:: $(TEST_PROGRAMS)
export NO_SVN_TESTS
-test: all
+valgrind-setup: $(patsubst %.sh,%,$(VALGRIND_SH))
+ rm -rf t/valgrind/bin
+ mkdir t/valgrind/bin
+ for i in git $(PROGRAMS); do cp test-valgrind t/valgrind/bin/$$i; done
+
+test: all valgrind-setup
$(MAKE) -C t/ all
test-date$X: date.o ctype.o
@@ -1501,6 +1508,7 @@ endif
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: valgrind-setup
### Check documentation
#
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8936173..e753654 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -94,6 +94,8 @@ do
--no-python)
# noop now...
shift ;;
+ -m|--m|--me|--mem|--memc|--memch|--memche|--memchec|--memcheck)
+ memcheck=t shift ;;
*)
break ;;
esac
@@ -468,6 +470,15 @@ test_done () {
# t/ subdirectory and are run in 'trash directory' subdirectory.
TEST_DIRECTORY=$(pwd)
PATH=$TEST_DIRECTORY/..:$PATH
+if test -n "$memcheck"; then
+ PATH=$TEST_DIRECTORY/valgrind/bin:$PATH
+ GIT_VALGRIND=$TEST_DIRECTORY/valgrind; export GIT_VALGRIND
+ if ! test -f "$GIT_VALGRIND/bin/git"; then
+ echo >&2 'You need to setup the valgrind bin:'
+ echo >&2 'Run "make valgrind-setup" in the (toplevel) directory'
+ exit 1
+ fi
+fi
GIT_EXEC_PATH=$(pwd)/..
GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
unset GIT_CONFIG
diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore
new file mode 100644
index 0000000..ba077a4
--- /dev/null
+++ b/t/valgrind/.gitignore
@@ -0,0 +1 @@
+bin
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
new file mode 100644
index 0000000..2482b3b
--- /dev/null
+++ b/t/valgrind/default.supp
@@ -0,0 +1,21 @@
+{
+ ignore-zlib-errors-cond
+ Memcheck:Cond
+ obj:*libz.so*
+}
+
+{
+ ignore-zlib-errors-value4
+ Memcheck:Value4
+ obj:*libz.so*
+}
+
+{
+ writing-data-from-zlib-triggers-errors
+ Memcheck:Param
+ write(buf)
+ obj:/lib/ld-*.so
+ fun:write_in_full
+ fun:write_buffer
+ fun:write_loose_object
+}
diff --git a/test-valgrind.sh b/test-valgrind.sh
new file mode 100755
index 0000000..61c1428
--- /dev/null
+++ b/test-valgrind.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+base=`echo $0 | sed 's,.*/,,'`
+case "$base" in
+git-*)
+ set -- "$GIT_EXEC_PATH/$base" "$@"
+ ;;
+git)
+ set -- "$GIT_EXEC_PATH/git" --exec-path="$GIT_VALGRIND/bin" "$@"
+ ;;
+*)
+ echo >&2 "test-valgrind invoked on non-git command: $0"
+ exit 1
+esac
+
+exec valgrind -q --error-exitcode=126 \
+ --leak-check=no \
+ --suppressions="$GIT_VALGRIND/default.supp" \
+ --gen-suppressions=all \
+ --log-fd=4 \
+ --input-fd=4 \
+ ${GIT_VALGRIND_DEBUG:+--db-attach=yes} \
+ ${GIT_VALGRIND_DEBUGCMD:+--db-command="$GIT_VALGRIND_DEBUGCMD"} \
+ "$@"
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* [RFC PATCH 0/5] valgrind in test scripts
From: Jeff King @ 2008-10-22 20:28 UTC (permalink / raw)
To: git
I spent some time last week running git through the paces with
valgrind's memcheck tool. The good news is that I didn't find any
serious issues, so we are doing a pretty good job overall. The bad news
is that running the whole test suite with valgrind takes a few hours on
a quad-core (but thank goodness for "make -j4 test").
I did uncover a few potential problems, and patches are in the latter
part of the series. I suppose an argument could be made that these fixes
are code churn, since these are not problems in practice, but I think
they are worth fixing. The fixes are few in number and small, and we are
very close to a valgrind-error-free code-base, which would make it easy
to spot any new problems when they arise. We could always suppress these
errors, but there is the possibility of an overzealous suppression
masking a real problem (especially if one of these issues changes from
theoretical to practical).
There are a few things I don't like:
1. The "fake git PATH" is set up by the Makefile, since it needs to
know which dashed commands to override (based on $(PROGRAMS)).
I think it would be nicer if the test script itself set this up when
--memcheck was requested, so that we always know it is fresh. But:
- if the fake PATH isn't inside the trash directory, then we have
a problem with multiple tests trying to set it up at the same
time
- if the fake PATH is inside the trash directory, I'm not sure of
the best place to put it, as I don't want to influence the
outcome of the tests. It could go into .git/valgrind, without
hurting anything.
2. I wanted to have a completely clean valgrind run before posting.
With these patches, the only errors I get are for an uninitialized
"struct stat" in t4121 and t4127. After much looking (including
carefully reading the code, and using a a debugger, which shows the
data in question looking very much initialized) I can't figure out
what the problem might be. So it might simply be a false positive in
valgrind, but I would like another set of eyes.
-Peff
^ permalink raw reply
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Johannes Schindelin @ 2008-10-22 20:29 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Alexander Gavrilov, git
In-Reply-To: <20081022192253.GC31568@coredump.intra.peff.net>
Hi,
On Wed, 22 Oct 2008, Jeff King wrote:
> submodule: fix some non-portable grep invocations
>
> Not all greps support "-e", but in this case we can easily
> convert it to a single extended regex.
I really wonder if we cannot catch these things (unportable grep, sed, etc
invokations) with a simple patch to the pre-commit hook.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Jakub Narebski @ 2008-10-22 20:12 UTC (permalink / raw)
To: git
In-Reply-To: <20081022191415.GA31568@coredump.intra.peff.net>
Jeff King wrote:
>> Do people build with NO_EXTERNAL_GREP on older Solaris?
>
> Yep. See:
>
> http://repo.or.cz/w/git/gitbuild.git?a=blob;f=jk/solaris/config.mak;hb=platform
>
> for the gory details (boy, I wish we had nice PATH_INFO-based gitweb
> URLs...).
Currently you can use path_info URL for blob_plain
http://repo.or.cz/w/git/gitbuild.git/platform:/jk/solaris/config.mak
Soon (thanks to Giuseppe patches) you would be able to use
http://repo.or.cz/w/git/gitbuild.git/blob/platform:/jk/solaris/config.mak
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [ANNOUCNE] repo - The Multiple Git Repository Tool
From: Leo Razoumov @ 2008-10-22 19:55 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081022154245.GT14786@spearce.org>
On 10/22/08, Shawn O. Pearce <spearce@spearce.org> wrote:
> My "bundle related secret project" was released yesterday by Google
> as part of the Android open source release event. (I've mentioned
> it before on-list in the context of a modified "git status" output.)
>
> Google developed two tools, repo and Gerrit, and open sourced them
> under the Apache License:
>
> http://android.git.kernel.org/?p=tools/repo.git
> http://android.git.kernel.org/?p=tools/gerrit.git
>
> git://android.git.kernel.org/tools/repo.git
> git://android.git.kernel.org/tools/gerrit.git
>
> repo is a Python application to bind together Git repositories,
> something like "git submodule", except it can track a project's
> branch rather than a specific Git commit. repo is also able to
> natively import a tarball or zip file and use it to initialize a
> repository from an upstream source, then apply git based changes
> on top of that tarball. In other words, repo is (more or less)
> built to manage an OS distribution, in Git.
> [..snip..]
Are there any plans to make repo an official git command (e.g.
git-repo) or merge its functionality with git-submodule?
--Leo--
^ permalink raw reply
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Jeff King @ 2008-10-22 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alexander Gavrilov, git
In-Reply-To: <20081022191415.GA31568@coredump.intra.peff.net>
On Wed, Oct 22, 2008 at 03:14:16PM -0400, Jeff King wrote:
> > git-submodule.sh uses grep "-e" to look for two patterns and I suspect
> > older Solaris would have the same issue.
>
> Yes, that code will break on Solaris. Most of my portability fixes have
> been in direct response to tests, so I guess we are not testing
> git-submodule very well.
And here's a patch. Though I believe this is the last "grep -e", I
wonder if it wouldn't have been wiser to simply force people on such
platforms to use GNU grep (I already have to use GNU tools to build, and
bash to run the scripts).
-- >8 --
submodule: fix some non-portable grep invocations
Not all greps support "-e", but in this case we can easily
convert it to a single extended regex.
Signed-off-by: Jeff King <peff@peff.net>
---
Passes the test scripts, but I'm not sure they are exercising this code,
anyway, since it passed on Solaris. Please double-check my conversion.
git-submodule.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 65178ae..b63e5c3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -424,7 +424,7 @@ cmd_summary() {
cd_to_toplevel
# Get modified modules cared by user
modules=$(git diff-index $cached --raw $head -- "$@" |
- grep -e '^:160000' -e '^:[0-7]* 160000' |
+ egrep '^:([0-7]* )?160000' |
while read mod_src mod_dst sha1_src sha1_dst status name
do
# Always show modules deleted or type-changed (blob<->module)
@@ -438,7 +438,7 @@ cmd_summary() {
test -z "$modules" && return
git diff-index $cached --raw $head -- $modules |
- grep -e '^:160000' -e '^:[0-7]* 160000' |
+ egrep '^:([0-7]* )?160000' |
cut -c2- |
while read mod_src mod_dst sha1_src sha1_dst status name
do
--
1.6.0.2.825.g6d19d
^ permalink raw reply related
* Re: [PATCH] rebase-i-p: delay saving current-commit to REWRITTEN if squashing
From: Jeff King @ 2008-10-22 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Haberman, git
In-Reply-To: <7v4p34v42e.fsf@gitster.siamese.dyndns.org>
On Wed, Oct 22, 2008 at 12:10:33PM -0700, Junio C Hamano wrote:
> >> + if [ "$fast_forward" == "t" ]
> > This one even fails on my Linux box. :) "==" is a bash-ism.
>
> Thanks.
You're very welcome, and sorry for not saving you a little time by
writing my complaint in patch form in the first place.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox