* 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
* 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
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