* [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
* Re: [ANNOUCNE] repo - The Multiple Git Repository Tool
From: Junio C Hamano @ 2008-10-22 19:14 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081022154245.GT14786@spearce.org>
"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 ;-)
^ permalink raw reply
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Jeff King @ 2008-10-22 19:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alexander Gavrilov, git
In-Reply-To: <7vd4hsv46z.fsf@gitster.siamese.dyndns.org>
On Wed, Oct 22, 2008 at 12:07:48PM -0700, Junio C Hamano wrote:
> Yuck. Solaris 8 /usr/bin/grep does not even grok "-e", so we cannot do a
> more obvious:
>
> grep -e "^author " -e "^summary "
Yep. I already introduced one use of egrep for a similar case in
8753941 (tests: grep portability fixes).
> 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...).
> 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.
-Peff
^ permalink raw reply
* Re: [PATCH] rebase-i-p: delay saving current-commit to REWRITTEN if squashing
From: Junio C Hamano @ 2008-10-22 19:10 UTC (permalink / raw)
To: Jeff King; +Cc: Stephen Haberman, gitster, git
In-Reply-To: <20081022125149.GA17092@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Oct 15, 2008 at 02:44:36AM -0500, Stephen Haberman wrote:
>
>> + if [ "$fast_forward" == "t" ]
>
> This one even fails on my Linux box. :) "==" is a bash-ism.
Thanks.
-- >8 --
Subject: [PATCH] git-rebase--interactive.sh: comparision with == is bashism
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-rebase--interactive.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a563dea..0cae3be 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -170,7 +170,7 @@ pick_one_preserving_merges () {
if test -f "$DOTEST"/current-commit
then
- if [ "$fast_forward" == "t" ]
+ if test "$fast_forward" = t
then
cat "$DOTEST"/current-commit | while read current_commit
do
--
1.6.0.3.723.g757e
^ permalink raw reply related
* Re: [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Junio C Hamano @ 2008-10-22 19:07 UTC (permalink / raw)
To: Jeff King; +Cc: Alexander Gavrilov, git
In-Reply-To: <20081022082016.GA18473@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Oct 22, 2008 at 12:55:57AM +0400, Alexander Gavrilov wrote:
>
>> +test_expect_success \
>> + 'blame respects i18n.commitencoding' '
>> + git blame --incremental file | \
>> + grep "^\(author\|summary\) " > actual &&
>> + test_cmp actual expected
>
> Even though it is POSIX, using backslashed grouping in 'grep' isn't
> portable. It fails at least on Solaris 8, and you have to do:
>
> egrep "^(author|summary) "
>
> instead. Of course, I can't get your test to pass even with that change,
> but I think that is just a broken iconv on Solaris.
Yuck. Solaris 8 /usr/bin/grep does not even grok "-e", so we cannot do a
more obvious:
grep -e "^author " -e "^summary "
Do people build with NO_EXTERNAL_GREP on older Solaris?
git-submodule.sh uses grep "-e" to look for two patterns and I suspect
older Solaris would have the same issue.
^ permalink raw reply
* Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs
From: Junio C Hamano @ 2008-10-22 18:53 UTC (permalink / raw)
To: Eric Wong; +Cc: Björn Steinbrink, git, jsogo
In-Reply-To: <20081022081653.GC14966@untitled>
Eric Wong <normalperson@yhbt.net> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>
> Help with looking at what SVN does and writing testcases would
> definitely be appreciated on this matter. Or perhaps this can be done
> at GitTogether :)
I'm not sure it would be a good use of time at GitTogether to do something
whose spec is pretty much self-evident (essentially for this one it boils
down to "define what are the 'funny' bytes, and list the protocols
supported by svn, and come up with paths with funny bytes in it and
see what libsvn-perl gives to the underlying svn library, and what the svn
library does over the wire"). Ongoiong "fix start-up sequence around
worktree area" might be a better fit; I dunno.
>> The patch may make a path with '~' work, but it (neither in the patch text
>> nor in the commit log message) does not have much to give readers enough
>> confidence that the code after the patch is the _final_ one, as opposed to
>> being just a band-aid for a single symptom that happened to have been
>> discovered this time.
>
> This is definitely a band-aid fix until I or somebody else takes the
> time to figure out:
>
> 1. exactly which characters need to be escaped
> 2. for which protocols those characters need to be escaped
> 3. which part(s) of the URI they need to be escaped for
> (repository root vs SVN path)
> 4. which versions of SVN needs more (or less) escaping rules
>
> (I vote for somebody else, especially for #4 :)
Item 3. above disturbs me. Do you mean that in:
https://sucs.org/~welshbyte/svn/backuptool/trunk/foo~bar.txt
the two tildes might have to be sent to libsvn-perl differently?
Even if that is the case, I am inclined suggest taking the patch in the
meantime as an interim workaround, with the understanding that we know the
patch improves the situation for the tilde before welshbyte and even
though we do not know if the patch regresses for the latter one between
foo and bar, it would be much rarer to have tilde in such places.
Care to come up with an updated log message?
^ permalink raw reply
* Re: git archive
From: kenneth johansson @ 2008-10-22 18:45 UTC (permalink / raw)
To: git
In-Reply-To: <20081022130829.GC2015@riemann.deskinm.fdns.net>
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.
^ permalink raw reply
* Re: error: packfile while git fsck
From: Nicolas Pitre @ 2008-10-22 17:35 UTC (permalink / raw)
To: Nicolas Ferre; +Cc: git
In-Reply-To: <48FF4521.5070303@atmel.com>
On Wed, 22 Oct 2008, Nicolas Ferre wrote:
> Nicolas Pitre :
> > On Wed, 22 Oct 2008, Nicolas Ferre wrote:
> >
> > > Hi all,
> > > (please cc me on response)
> > >
> > > I am facing error during git status & git fsck on my tree.
> > > This tree is cloned from various linux kernel trees.
> > >
> > > Here are a sample of the error I see :
> > >
> > > $ git fsck
> > > error: packfile
> > > .git/objects/pack/pack-2ab31ad1f8cb69d091a56fe936634e4796606d49.pack does
> > > not
> > > match index
> > > error: packfile
> > > .git/objects/pack/pack-2ab31ad1f8cb69d091a56fe936634e4796606d49.pack
> > > cannot be
> > > accessed
> > [...]
> >
> > What git version?
>
> $ git --version
> git version 1.5.3.7
OK. Since this is not bleeding edge, it is pretty unlikely that the
corruption is due to git itself. Furthermore, the git packs are always
read only once they've been created, meaning that if they weren't
corrupted at some point then something outside of git caused the
corruption. You really should consider the possible causes for that
(dying disk, pilot error, etc).
As to recovery... That really depends if you have personal work
committed to your repository. If not then the easiest solution is
simply to recreate it by refetching from upstream. If you have
personal work in there then you could try to fetch your work branch into
the newly created repository. The latest git version could help with
the extraction of non-corrupted objects out of a bad pack, but if the
objects you are interested in are themselves corrupted then your only
hope is to have a copy of those objects somewhere else.
Nicolas
^ permalink raw reply
* Re: linux-next: stackprotector tree build failure
From: Johannes Schindelin @ 2008-10-22 17:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: Stephen Rothwell, Thomas Gleixner, H. Peter Anvin, linux-next,
Junio C Hamano, git
In-Reply-To: <20081022083139.GA4369@elte.hu>
Hi,
On Wed, 22 Oct 2008, Ingo Molnar wrote:
>
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> > On Wed, 22 Oct 2008 09:29:23 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> > >
> > > I've Cc:-ed Junio and the Git list as a general FYI - but it must be
> > > frustrating to get such a bugreport, because i have no reproducer.
> > >
> > > git-rerere sometimes seems to be picking up the wrong resolution. VERY
> > > rarely.
> > >
> > > It seems random and content dependent. Once it happened to
> > > arch/x86/kernel/traps_32.c and now to kernel/fork.c. Along the ~170
> > > successful resolutions i have in my tree right now. And i do many
> > > conflict resolutions every day - and it happened only once every 6
> > > months or so.
> > >
> > > (the arch/x86/kernel/traps_32.c one happened regularly, that's why i
> > > thought it's content sha1 dependent, and not some corruption.)
> > >
> > > Next time it happens i'll be on the watchout and will save the complete
> > > tree.
> >
> > I think rerere matches preimages on the SHA1 of the conflict (or its
> > reverse), so sufficiently similar pieces of code will match. I would
> > expect things like ext2/3/4 to be candidates. Did the traps_32.c one
> > match one for traps_64.c?
> >
> > I may be mistaken, but I once followed the code in rerere to try to
> > figure out how to fix a resolution.
>
> the traps_32.c one was that git-rerere put in a traps_64.c end result.
> So i ended up with a 32-bit kernel that tried to build a 64-bit piece of
> code - fireworks. That condition persisted - i had to fix it up manually
> all the time i integrated that portion of the tree. That too was i think
> centered around a header file chunk - perhaps the #include section of
> traps_32.c and traps_64.c was similar enough in that section?
I think it might be in order to explain how git-rerere works internally:
- in case of a conflicts, the files' conflicts are written into the
rr-cache _per file_.
First, the human readable stuff after the "<<<", "|||" and ">>>" lines
is removed.
Then, the "||| ... ===" part is removed, too (in effect turning the
diff3-style conflicts into an RCS merge-style conflicts).
The result is recorded as "preimage". The conflict is identified by the
SHA-1 of _just_ the conflicts (including the "<<<", "===" and ">>>"
lines).
After all the conflicted files have been handled, a list is written to
the file "MERGE_RR" containing the SHA-1s of the conflicts together with
the file names.
- when committing, the files whose conflicts were resolved are recorded
verbatim in the file "postimage".
Now, when rerere is called again and there are conflicted files, again the
files' conflicts are identified by their SHA-1. If a resolution exists, a
3-way merge is performed with the recorded preimage (the
original file with conflict lines) as base, the postimage (the originally
resolved file) and the current file with conflict lines.
The idea being: the diff between the recorded preimage and postimage gives
the resolution that you want, and the diff between the recorded preimage
and the current file with conflict lines gives you the changes that
happened to the file in-between.
So I think that you might hit the unfortunate case where two files
happened to have the same conflicts, but you needed to resolve them one
way for one file, and another way for the other file.
Ciao,
Dscho
^ permalink raw reply
* Re: [irq/urgent]: created 3786fc7: "irq: make variable static"
From: Johannes Schindelin @ 2008-10-22 17:04 UTC (permalink / raw)
To: Jeff King; +Cc: Jakub Narebski, Ingo Molnar, git
In-Reply-To: <20081022132148.GA17393@coredump.intra.peff.net>
Hi,
On Wed, 22 Oct 2008, Jeff King wrote:
> On Wed, Oct 22, 2008 at 03:50:52AM -0700, Jakub Narebski wrote:
>
> > About printing either forward (git-describe, e.g.
> > v1.6.0.2-590-g67f6062) or backward (git-name-rev, e.g.
> > tags/v1.6.0-rc2~8): you can use git-name-rev in filter mode (git log
> > ... | git name-rev --stdin), or "git log --decorate", or '%d' in
> > --pretty format specifier (this is very new thing).
>
> The "--decorate" and "%d" code just decorates branch _tips_. My
> impression is that he wanted to see the branch mentioned even if the
> commit was not at the tip. It would be possible to extend this to print
> name-rev output, but it would be computationally and memory-intensive, I
> suspect.
FWIW I tried to do this "on-the-fly", but that did not bode well with
things like
git log --decorate=any --no-walk master $(git rev-parse master~10)
If you're interested in code:
http://thread.gmane.org/gmane.comp.version-control.git/52123/focus=52126
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Implement git remote mv
From: Brandon Casey @ 2008-10-22 16:52 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
In-Reply-To: <1224634994-1664-1-git-send-email-vmiklos@frugalware.org>
Miklos Vajna wrote:
> +static int mv(int argc, const char **argv)
> +{
> + struct option options[] = {
> + OPT_END()
> + };
> + struct remote *oldremote, *newremote;
> + struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
> + struct string_list remote_branches = { NULL, 0, 0, 0 };
> + struct rename_info rename = { argv[1], argv[2], &remote_branches };
I think some non-c99 compilers would have issues with this run-time
initialization from function arguments. Plus, what if argv doesn't have
3 elements? I see you have a check for that below...
> + int i;
> +
> + if (argc != 3)
> + usage_with_options(builtin_remote_usage, options);
-brandon
^ permalink raw reply
* Terminology question: "tracking" branches
From: Björn Steinbrink @ 2008-10-22 16:13 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Marc Branchaud, Peter Harris, git
In-Reply-To: <48FF3FEE.8020209@drmicha.warpmail.net>
On 2008.10.22 16:59:58 +0200, Michael J Gruber wrote:
> - a remote branch (a branch in your local repo which is a copy of a
> branch in a remote repo; stored under refs/remotes, never to be modified
> locally)
> - a (remote) tracking branch (a local branch which is set up to pull
> from a remote branch by default)
(Remote) tracking branches are actually what you called remote branches,
at least according to the git glossary. But I wonder, what is the right
term for a branch that has the --track setup for pull?
On #git I usually fall back to some variation of "a branch that is
configured for 'git pull'" or something similarly verbose (and maybe
that's even partially wrong/inaccurate/incomplete?). And I always try to
stick to saying "remote tracking branch" and not just "tracking branch"
(as the glossary does) to avoid confusion as best as I can. But that
feels quite suboptimal.
So, is there some term that describes a local branch that has been
configured for "git pull"?
Thanks,
Björn
^ permalink raw reply
* Re: [PATCH] rebase-i-p: delay saving current-commit to REWRITTEN if squashing
From: Fredrik Skolmli @ 2008-10-22 15:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Stephen Haberman, gitster, git
In-Reply-To: <alpine.DEB.1.00.0810221721370.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Wed, Oct 22, 2008 at 05:21:53PM +0200, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 22 Oct 2008, Jeff King wrote:
>
> > On Wed, Oct 15, 2008 at 02:44:36AM -0500, Stephen Haberman wrote:
> >
> > > + if [ "$fast_forward" == "t" ]
> >
> > This one even fails on my Linux box. :) "==" is a bash-ism.
>
> Did we not also prefer "test" to "["?
We did.
Documentation/CodingGuidelines, line 51:
- We prefer "test" over "[ ... ]".
--
Kind regards,
Fredrik Skolmli
^ permalink raw reply
* [ANNOUCNE] repo - The Multiple Git Repository Tool
From: Shawn O. Pearce @ 2008-10-22 15:42 UTC (permalink / raw)
To: git
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.
Gerrit is a web based code review system, forked off the open
sourced Rietveld code review system. Gerrit runs on the highly
scaleable Google App Engine platform, but probably could be ported
to an open-source MySQL or PostgreSQL backend if people really
wanted to do that.
You can read some more of how Android has applied these tools to
its development process here:
http://source.android.com/download
http://source.android.com/submit-patches/workflow
http://source.android.com/download/using-repo
Although the current tool documentation is only on the Android
site, both the repo and Gerrit tools are not specific to Android
and are designed to be applied to any project that wants to use a
similar process.
repo and Gerrit are actually developed with themselves. You can
use repo to fetch repo:
curl http://android.git.kernel.org/repo >~/bin/repo
chomd a+x ~/bin/repo
mkdir myrepo
cd myrepo
repo init -u git://android.git.kernel.org/tools/manifest.git
repo sync
The "git status" output I was talking about before is the "repo
status" subcommand, e.g.:
$ echo '# test' >>repo/repo ; # modify a tracked file
$ repo status
project repo/ (*** NO BRANCH ***)
-m repo
--
Shawn.
^ 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