* Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git
From: Thomas Rast @ 2009-01-07 12:24 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: martin f krafft, git, pasky
In-Reply-To: <20090107112754.GA15158@roro3>
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
Kirill Smelkov wrote:
> On Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:
> > I find this very confusing. Why not simply
> >
> > TG_PAGER="${GIT_PAGER:-}"
> > TG_PAGER="${TG_PAGER:-$PAGER}"
> >
> > ?
>
> I find it confusing too, but this is needed because they usually do
> something like this
>
> $ GIT_PAGER='' <some-git-command>
>
> to force it to be pagerless.
[...]
> So I think it would be better to preserve the same semantics for `tg
> patch` callers, though it's a pity that it's hard (maybe I'm wrong ?) to
> see whether an env var is unset.
Admittedly I haven't really studied your patch, but the ${} constructs
can in fact tell empty from unset:
$ EMPTY=
$ unset UNDEFINED
$ echo ${UNDEFINED-foo}
foo
$ echo ${UNDEFINED:-foo}
foo
$ echo ${EMPTY-foo}
$ echo ${EMPTY:-foo}
foo
'man bash' indeed says
When not performing substring expansion, bash tests for a parameter
that is unset or null; omitting the colon results in a test only for
a parameter that is unset.
So I suppose you could use
${GIT_PAGER-${PAGER-less}}
or similar.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git
From: Kirill Smelkov @ 2009-01-07 11:27 UTC (permalink / raw)
To: martin f krafft; +Cc: git, pasky
In-Reply-To: <20090106203203.GA11274@lapse.rw.madduck.net>
Martin, thanks for your review.
I'll too reply inline:
On Tue, Jan 06, 2009 at 09:32:03PM +0100, martin f krafft wrote:
> Thanks, Kirill, for the patches. A couple of comments inline. I hope
> Petr has a chance to look too.
>
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.06.1616 +0100]:
> > +# isatty FD
> > +isatty()
> > +{
> > + tty -s 0<&$1 || return 1
> > + return 0
> > +}
>
> You don't need any of the return statements. Functions' return
> values are the return values of the last commands they execute.
Agree, I'll rework isatty to be just
isatty()
{
tty -s 0<&$1
}
> Since we are not using set -e, just "tty -s 0<&$1" in the body will
> have the same effect.
We _are_ using `set -e` in tg.sh:
http://repo.or.cz/w/topgit.git?a=blob;f=tg.sh;h=8c23d26b9a62ddcc1869bb70299862c32edd4403;hb=HEAD#l254
And also I think `tty -s 0<&$1` is a bit obscure, so maybe it is still
better to put it into a function with well-known name such as isatty?
>
> > + # TG_PAGER = GIT_PAGER | PAGER
> > + # http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html
> > + case ${GIT_PAGER+XXX} in
> > + '')
> > + case ${PAGER+XXX} in
> > + '')
> > + # both GIT_PAGER & PAGER unset
>
> I find this very confusing. Why not simply
>
> TG_PAGER="${GIT_PAGER:-}"
> TG_PAGER="${TG_PAGER:-$PAGER}"
>
> ?
I find it confusing too, but this is needed because they usually do
something like this
$ GIT_PAGER='' <some-git-command>
to force it to be pagerless.
For example in git-rebase.sh:
http://repo.or.cz/w/git.git?a=blob;f=git-rebase.sh;h=ebd4df3a0e821ddcfd1eabfcaac17f854e172a85;hb=HEAD#l415
And other places.
So I think it would be better to preserve the same semantics for `tg
patch` callers, though it's a pity that it's hard (maybe I'm wrong ?) to
see whether an env var is unset.
I'll add additional note about why this is needed.
>
> > + [ -z "$TG_PAGER" -o "$TG_PAGER" = "cat" ] && return 0
>
> What if I set my pager to /bin/cat? But I suppose then there's just
> a wasted FIFO and process, nothing big.
Yes. I'd drop "cat" from here completely, but since I was mimicing
pager.c I left it:
http://repo.or.cz/w/git.git?a=blob;f=pager.c;h=f19ddbc87df04f117cd5e39189c8322fd5f29d68;hb=HEAD#l55
I'm ok with dropping cat. Should we?
> > + _pager_fifo="$(mktemp -t tg-pager-fifo.XXXXXX)"
> > + rm "$_pager_fifo" && mkfifo -m 600 "$_pager_fifo"
>
> There's a race condition here. I can't see a real problem with it,
> though, nor do I know of a better way.
Yes I know and agree here is a race.
But netheir did I knew how to overcome this (I though we'll need mkfifo
to support creating fifos with temp names, but mkfifo lacks support for
this)
The real problem here is that in bash, it's hard to get such constructs
going without fifos at all -- just using pipes like we would do in C,
because:
exec >file
redirects the rest of current process output to file, but
exec |less
does _not_ redirect the rest of the current process output through pipe
to less.
Likewise
exec > >(less)
works (process redirection), but is a bashishm, so = not an option for
us.
Fortunately Adeodato suggested to use `mktemp -d`, so this is reworked
to:
_pager_fifo_dir="$(mktemp -t -d tg-pager-fifo.XXXXXX)"
_pager_fifo="$_pager_fifo_dir/0"
mkfifo -m 600 "$_pager_fifo"
+ appopriate cleanup.
> > + "$TG_PAGER" < "$_pager_fifo" &
> > + exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
> > +
> > + # this is needed so e.g. `git diff` will still colorize it's output if
> > + # requested in ~/.gitconfig with color.diff=auto
> > + export GIT_PAGER_IN_USE=1
> > +
> > + # atexit(close(1); wait pager)
> > + trap "exec >&-; rm $_pager_fifo; wait" EXIT
>
> Consistency: $_pager_fifo is not passed as a quoted string to rm
> here. In the unlikely event that $TMPDIR contains a space, this
> would fail.
Thanks, corrected.
> I definitely want Petr's opinion on this before I integrate it.
Ok, let's wait what Petr says. Here is improved patch:
Changes since v1:
o isatty() simplified
o added a note about different meaning of GIT_PAGER='' and unset
GIT_PAGER
o fixed race wrt to mkfifo creation
o fixed potential whitespace problem in "$_pager_fifo" cleanup
(interdiff)
diff --git a/tg.sh b/tg.sh
index 51a82af..bf9cf5c 100644
--- a/tg.sh
+++ b/tg.sh
@@ -248,8 +248,7 @@ do_help()
# isatty FD
isatty()
{
- tty -s 0<&$1 || return 1
- return 0
+ tty -s 0<&$1
}
# setup_pager
@@ -259,6 +258,7 @@ setup_pager()
isatty 1 || return 0
# TG_PAGER = GIT_PAGER | PAGER
+ # (but differentiate between GIT_PAGER='' and unset variables)
# http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html
case ${GIT_PAGER+XXX} in
'')
@@ -283,8 +283,9 @@ setup_pager()
# now spawn pager
export LESS=${LESS:-FRSX} # as in pager.c:pager_preexec()
- _pager_fifo="$(mktemp -t tg-pager-fifo.XXXXXX)"
- rm "$_pager_fifo" && mkfifo -m 600 "$_pager_fifo"
+ _pager_fifo_dir="$(mktemp -t -d tg-pager-fifo.XXXXXX)"
+ _pager_fifo="$_pager_fifo_dir/0"
+ mkfifo -m 600 "$_pager_fifo"
"$TG_PAGER" < "$_pager_fifo" &
exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
@@ -294,7 +295,7 @@ setup_pager()
export GIT_PAGER_IN_USE=1
# atexit(close(1); wait pager)
- trap "exec >&-; rm $_pager_fifo; wait" EXIT
+ trap "exec >&-; rm "$_pager_fifo"; rmdir "$_pager_fifo_dir"; wait" EXIT
}
## Startup
>From 2193b7c703c2d31c8739eec617b8c0e8c1d09b79 Mon Sep 17 00:00:00 2001
From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Date: Tue, 6 Jan 2009 17:56:37 +0300
Subject: [PATCH (topgit) v2] Implement setup_pager just like in git
setup_pager() spawns a pager process and redirect the rest of our output
to it.
This will be needed to fix `tg patch` output in the next commit.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
---
tg.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/tg.sh b/tg.sh
index 8c23d26..bf9cf5c 100644
--- a/tg.sh
+++ b/tg.sh
@@ -243,6 +243,60 @@ do_help()
fi
}
+## Pager stuff
+
+# isatty FD
+isatty()
+{
+ tty -s 0<&$1
+}
+
+# setup_pager
+# Spawn pager process and redirect the rest of our output to it
+setup_pager()
+{
+ isatty 1 || return 0
+
+ # TG_PAGER = GIT_PAGER | PAGER
+ # (but differentiate between GIT_PAGER='' and unset variables)
+ # http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html
+ case ${GIT_PAGER+XXX} in
+ '')
+ case ${PAGER+XXX} in
+ '')
+ # both GIT_PAGER & PAGER unset
+ TG_PAGER=''
+ ;;
+ *)
+ TG_PAGER="$PAGER"
+ ;;
+ esac
+ ;;
+ *)
+ TG_PAGER="$GIT_PAGER"
+ ;;
+ esac
+
+ [ -z "$TG_PAGER" -o "$TG_PAGER" = "cat" ] && return 0
+
+
+ # now spawn pager
+ export LESS=${LESS:-FRSX} # as in pager.c:pager_preexec()
+
+ _pager_fifo_dir="$(mktemp -t -d tg-pager-fifo.XXXXXX)"
+ _pager_fifo="$_pager_fifo_dir/0"
+ mkfifo -m 600 "$_pager_fifo"
+
+ "$TG_PAGER" < "$_pager_fifo" &
+ exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
+
+ # this is needed so e.g. `git diff` will still colorize it's output if
+ # requested in ~/.gitconfig with color.diff=auto
+ export GIT_PAGER_IN_USE=1
+
+ # atexit(close(1); wait pager)
+ trap "exec >&-; rm "$_pager_fifo"; rmdir "$_pager_fifo_dir"; wait" EXIT
+}
## Startup
--
1.6.1.48.ge9b8
Thanks,
Kirill
^ permalink raw reply related
* [PATCH] diff --no-index -q: fix endless loop
From: Thomas Rast @ 2009-01-07 11:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7veizfbnuw.fsf@gitster.siamese.dyndns.org>
We forgot to move to the next argument when parsing -q, getting stuck
in an endless loop.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Junio C Hamano wrote:
> I'll queue the "--" fix, "-q" fix and this pager fix. Thanks.
Seems the after-midnight rule indeed has some value. I pointed out
the argv[i] bug, which I see you have squashed into the -- fix, but
failed to notice that the -q parsing is also missing an i++.
I'm still not convinced of the option's value, but this patch at least
fixes the bug.
diff-no-index.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 12ff1f1..60ed174 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -207,8 +207,10 @@ void diff_no_index(struct rev_info *revs,
int j;
if (!strcmp(argv[i], "--no-index"))
i++;
- else if (!strcmp(argv[i], "-q"))
+ else if (!strcmp(argv[i], "-q")) {
options |= DIFF_SILENT_ON_REMOVED;
+ i++;
+ }
else if (!strcmp(argv[i], "--"))
i++;
else {
--
tg: (3bbe36c..) t/diff-q-endless (depends on: next)
^ permalink raw reply related
* Re: BUG?? INSTALL MAKEFILE
From: Lars Sadau @ 2009-01-07 10:44 UTC (permalink / raw)
To: git
In-Reply-To: <vpqzli45p6q.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:
>
> Ted Pavlic <ted <at> tedpavlic.com> writes:
>
> > According to the INSTALL doc, the default prefix should be ~.
I am the same opinion
> I didn't read that in INSTALL. What I read is that if I only run "make
> install", the prefix is $HOME, which is true. Now, ./configure uses a
> default value which is not the one of the Makefile, but that's another
> point.
>
May be not, confuses newbies.
------
Lars
^ permalink raw reply
* Re: Error: unable to unlink ... when using "git gc"
From: Sitaram Chamarty @ 2009-01-07 10:55 UTC (permalink / raw)
To: git
In-Reply-To: <200901070027.21721.bss@iguanasuicide.net>
On 2009-01-07, Boyd Stephen Smith Jr. <bss@iguanasuicide.net> wrote:
> On Tuesday 06 January 2009, Sitaram Chamarty <sitaramc@gmail.com> wrote=20
>> chmod g+ws .git
>>
>>Now set the group to something (I use "gitpushers" ;-)
>>
>> chgrp -R gitpushers .git
> ISTR this breaking here when someone on the team had a umask like 077 and=20
> was using file:// or ssh:// to push. I tended up "fixing" things with a=20
> cronjob, (which is a bit of a hack) IIRC.
That doesn't sound right. "git help init" says:
- 0xxx: 0xxx is an octal number and each file will have mode 0xxx
- 0xxx will override users umask(2) value, and thus, users
with a safe umask (0077) can use this option
- 0660 is equivalent to group.
So when you say "group", you're saying "0660", and when you
say "0660", you're overriding users umask value.
I sorta-kinda tested it (some output of the "find|xargs ls"
is snipped for brevity):
$ mkdir umt;cd umt
$ umask
0022
$ git init --shared=group
Initialized empty shared Git repository in /home/sitaram/t/umt/.git/
(reverse-i-search)`find': sfind f|ack .git|map -x du -sk |sort -n
$ find . -type d|xargs ls -ald
drwxr-xr-x 3 sitaram sitaram 4096 2009-01-07 15:59 .
drwxrwsr-x 7 sitaram sitaram 4096 2009-01-07 15:59 ./.git
$ dummy_commit
Created initial commit afb2645: file-30824 at Wed Jan 7 15:59:23 IST 2009
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 file-30824
$ find . -type d|xargs ls -ald
drwxr-xr-x 3 sitaram sitaram 4096 2009-01-07 15:59 .
drwxrwsr-x 9 sitaram sitaram 4096 2009-01-07 15:59 ./.git
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/a7
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/af
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/ca
$ umask 077
$ jeeves dummy_commit
Created commit 232f157: file-3025 at Wed Jan 7 15:59:36 IST 2009
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 file-3025
$ find . -type d|xargs ls -ald
drwxr-xr-x 3 sitaram sitaram 4096 2009-01-07 15:59 .
drwxrwsr-x 9 sitaram sitaram 4096 2009-01-07 15:59 ./.git
drwxrws--- 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/23
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/a7
drwxrws--- 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/a8
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/af
drwxrws--- 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/c7
drwxrwsr-x 2 sitaram sitaram 4096 2009-01-07 15:59 ./.git/objects/ca
^ permalink raw reply
* Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git
From: Adeodato Simó @ 2009-01-07 10:35 UTC (permalink / raw)
To: martin f krafft; +Cc: Kirill Smelkov, git, pasky
In-Reply-To: <20090106203203.GA11274@lapse.rw.madduck.net>
* martin f krafft [Tue, 06 Jan 2009 21:32:03 +0100]:
> > + _pager_fifo="$(mktemp -t tg-pager-fifo.XXXXXX)"
> > + rm "$_pager_fifo" && mkfifo -m 600 "$_pager_fifo"
> There's a race condition here. I can't see a real problem with it,
> though, nor do I know of a better way.
mktemp -d, and create the fifo inside the directory created by mktemp.
--
Adeodato Simó dato at net.com.org.es
Debian Developer adeodato at debian.org
A hacker does for love what other would not do for money.
^ permalink raw reply
* Re: [PATCH (topgit) 1/2] Implement setup_pager just like in git
From: martin f krafft @ 2009-01-06 20:32 UTC (permalink / raw)
To: Kirill Smelkov, git; +Cc: pasky
In-Reply-To: <acaae74f79d385014e726b97f8258b2a0caa3dd0.1231254832.git.kirr@landau.phys.spbu.ru>
[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]
Thanks, Kirill, for the patches. A couple of comments inline. I hope
Petr has a chance to look too.
also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.06.1616 +0100]:
> +# isatty FD
> +isatty()
> +{
> + tty -s 0<&$1 || return 1
> + return 0
> +}
You don't need any of the return statements. Functions' return
values are the return values of the last commands they execute.
Since we are not using set -e, just "tty -s 0<&$1" in the body will
have the same effect.
> + # TG_PAGER = GIT_PAGER | PAGER
> + # http://unix.derkeiler.com/Newsgroups/comp.unix.shell/2004-03/0792.html
> + case ${GIT_PAGER+XXX} in
> + '')
> + case ${PAGER+XXX} in
> + '')
> + # both GIT_PAGER & PAGER unset
I find this very confusing. Why not simply
TG_PAGER="${GIT_PAGER:-}"
TG_PAGER="${TG_PAGER:-$PAGER}"
?
> + [ -z "$TG_PAGER" -o "$TG_PAGER" = "cat" ] && return 0
What if I set my pager to /bin/cat? But I suppose then there's just
a wasted FIFO and process, nothing big.
> + _pager_fifo="$(mktemp -t tg-pager-fifo.XXXXXX)"
> + rm "$_pager_fifo" && mkfifo -m 600 "$_pager_fifo"
There's a race condition here. I can't see a real problem with it,
though, nor do I know of a better way.
> + "$TG_PAGER" < "$_pager_fifo" &
> + exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
> +
> + # this is needed so e.g. `git diff` will still colorize it's output if
> + # requested in ~/.gitconfig with color.diff=auto
> + export GIT_PAGER_IN_USE=1
> +
> + # atexit(close(1); wait pager)
> + trap "exec >&-; rm $_pager_fifo; wait" EXIT
Consistency: $_pager_fifo is not passed as a quoted string to rm
here. In the unlikely event that $TMPDIR contains a space, this
would fail.
I definitely want Petr's opinion on this before I integrate it.
--
martin | http://madduck.net/ | http://two.sentenc.es/
if you see an onion ring -- answer it!
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Problems getting rid of large files using git-filter-branch
From: Øyvind Harboe @ 2009-01-07 10:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901071101480.7496@intel-tinevez-2-302>
On Wed, Jan 7, 2009 at 11:07 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Tue, 6 Jan 2009, Øyvind Harboe wrote:
>
>> On Tue, Jan 6, 2009 at 11:20 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>>
>> > $ git verify-pack -v $PACK | grep -v "^chain " | sort -n -k 4
>>
>> I have never used the git verify-pack command, but I'm pretty sure the
>> "Terminated" string isn't the normal output :-)
>>
>> $ git verify-pack -v
>> .git/objects/pack/pack-1e039b82d8ae53ef5ec3614a3021466663cc70a4
>> Terminated
>
> I did
>
> $ git grep Terminated
>
> and came up empty :-)
>
> Seriously, I guess this could be some OOM thing. We _should_ handle this
> more gracefully, but it is possible that some uncatchable condition hits
> you, such as out-of-stack-space.
>
> I'd try running the command either with strace or with gdb, and I'd look
> at $? after the command returns, to find out what is actually happening.
After some investigation it turns out that my server has 228mByte of RAM
available. It is a virtual server running CentOS, hence the strange number
and ridiciulously tiny amount of memory(these days).
Now the strange thing is that I'm not getting this error message this morning...
How would git behave if it ran out of memory?
--
Øyvind Harboe
http://www.zylin.com/zy1000.html
ARM7 ARM9 XScale Cortex
JTAG debugger and flash programmer
^ permalink raw reply
* Re: Problems getting rid of large files using git-filter-branch
From: Johannes Schindelin @ 2009-01-07 10:07 UTC (permalink / raw)
To: Øyvind Harboe; +Cc: git
In-Reply-To: <c09652430901061436w36c013ep938e9cfba43140c9@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 836 bytes --]
Hi,
On Tue, 6 Jan 2009, Øyvind Harboe wrote:
> On Tue, Jan 6, 2009 at 11:20 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > $ git verify-pack -v $PACK | grep -v "^chain " | sort -n -k 4
>
> I have never used the git verify-pack command, but I'm pretty sure the
> "Terminated" string isn't the normal output :-)
>
> $ git verify-pack -v
> .git/objects/pack/pack-1e039b82d8ae53ef5ec3614a3021466663cc70a4
> Terminated
I did
$ git grep Terminated
and came up empty :-)
Seriously, I guess this could be some OOM thing. We _should_ handle this
more gracefully, but it is possible that some uncatchable condition hits
you, such as out-of-stack-space.
I'd try running the command either with strace or with gdb, and I'd look
at $? after the command returns, to find out what is actually happening.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: Junio C Hamano @ 2009-01-07 9:42 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: Linus Torvalds, Nicolas Pitre, Jan Krüger, Git ML
In-Reply-To: <1231317131.8870.471.camel@starfruit>
"R. Tyler Ballance" <tyler@slide.com> writes:
> On Wed, 2009-01-07 at 00:16 -0800, Junio C Hamano wrote:
>> "R. Tyler Ballance" <tyler@slide.com> writes:
>>
>> > Unfortunately it doesn't, what I did notice was this when I did a `git
>> > status` in the directory right after untarring:
>> > tyler@grapefruit:~/jburgess_main> git status
>> > #
>> > # ---impressive amount of file names fly by---
>> > # ----snip---
>> > ...
>> > Basically, somehow Git thinks that *every* file in the repository is
>> > deleted at this point.
>>
>> That makes me suspect that your .git/index file is corrupt.
>
> Would this be tied to the corrupted pack file issue, or separate.
If you have perfectly good set of packs, if your index is corrupt you may
see "everything deleted", so in that sense it is independent.
As Linus's earlier conjecture was that this is related to some sort of
disk/cache corruption, I wouldn't be surprised if such a failure hit packs
and the index file indiscriminatingly. So in that sense they are
related.
I think "git ls-files" (before doing anything else, such as resetting, of
course) would report that the index is corrupt, if that is indeed the case.
^ permalink raw reply
* Re: [JGIT RFC] How read versions of a specific object
From: Imran M Yousuf @ 2009-01-07 9:23 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <20090107040417.GA10790@spearce.org>
On Wed, Jan 7, 2009 at 10:04 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Imran M Yousuf <imyousuf@gmail.com> wrote:
>> I am trying to read all or n-th version of an object. Currently to do
>> this I am using the following piece of code, which has to walk to
>> every commit is present and from there prepare a set of its object id,
>> it is definitely expensive if the commit history is huge, is there a
>> faster/better way to achieve it?
>
> Not really. You can more efficiently use JGit and reduce some of
> the overheads, but that's about it.
>
Thanks Shawn, for pointing it out and it actually does improve the
performance, for every lookup its like 200ms.
Best regards,
Imran
>> for (int i = 0; i < App.OBJECT_COUNT;
>> ++i) {
>> ObjectWalk objectWalk = new ObjectWalk(repo);
>
> Don't use ObjectWalk, use a RevWalk. You don't need it to keep
> track of tree or blob identities. The ObjectWalk code has more
> overhead to do that bookkeeping.
>
>> Commit revision = repo.mapCommit(revObject.getId());
>> Tree versionTree = repo.mapTree(revision.getTreeId());
>> if (versionTree.existsBlob(isbn)) {
>> revisions.add(versionTree.findBlobMember(isbn).getId());
>
> Use a TreeWalk to do this. Its quicker because it doesn't
> have to parse as much data to come up with the same result.
>
> More specifically there's a static factory method that sets up for
> a path limited walk and returns the TreeWalk pointing at that entry.
>
> You can use the fact that RevWalk.next() returns a RevCommit to get
> you the RevTree, which is the tree you need to give to the TreeWalk
> constructor (its the root level tree of the commit).
>
>
> But if App.OBJECT_COUNT is quite large and covers most of your
> objects, you are probably better off using a loop over the commits
> and diff'ing against the ancestor:
>
> final HashMap<String, Set<ObjectId>> versions = ...;
> final RevWalk rw = new RevWalk(repo);
> final TreeWalk tw = new TreeWalk(repo);
> rw.markStart(rw.parseCommit(repo.parse(HEAD)));
> tw.setFilter(TreeFilter.ANY_DIFF);
>
> RevCommit c;
> while ((c = rw.next()) != null) {
> final ObjectId[] p = new ObjectId[c.getParentCount() + 1];
> for (int i = 0; i < c.getParentCount(); i++) {
> rw.parse(c.getParent(i));
> p[i] = c.getParent(i).getTree();
> }
> final int me = p.length -1;
> p[me] = c.getTree();
> tw.reset(p);
> while (tw.next()) {
> if (tw.getFileMode(me).getObjectType() == Constants.OBJ_BLOB) {
> // This path was modified relative to the ancestor(s).
> //
> String s = tw.getPathString();
> Set<ObjectId> i = versions.get(s);
> if (i == null)
> versions.put(s, i = new HashSet<ObjectId>());
> i.add(tw.getObjectId(me));
> }
>
> if (tw.isSubtree()) {
> // make sure we recurse into modified directories
> tw.enterSubtree();
> }
> }
> }
>
> --
> Shawn.
>
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Blog: http://imyousuf-tech.blogs.smartitengineering.com/
Mobile: +880-1711402557
^ permalink raw reply
* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: R. Tyler Ballance @ 2009-01-07 9:05 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, Jan Krüger, Git ML
In-Reply-To: <1231314099.8870.415.camel@starfruit>
[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]
On Tue, 2009-01-06 at 23:41 -0800, R. Tyler Ballance wrote:
> I did try to do a git-fsck(1), and this is what I got:
> tyler@grapefruit:~/jburgess_main> /usr/local/bin/git fsck --full
> [1] 19381 segmentation fault /usr/local/bin/git fsck --full
> tyler@grapefruit:~/jburgess_main>
> >
Disregard this comment, Jan reminded me in IRC of the issue I had with
this repository and the stack-size ulimit. A healthy 32M stack-size
allows for the command to complete:
tyler@grapefruit:~/jburgess_main> /usr/local/bin/git fsck --full
dangling blob 6e58a06cd0f027fce1fc8a923a8c81d6b55f1705
dangling blob 5e87e049f69ee06af1f4f92a3d4ddcd912f8535e
dangling blob acb8e0937cea3f4068c5c67f60d3b97952654fa1
dangling blob 8bd540d657027ab12228e0522de86a97d3f8a7a9
dangling blob 90d93096369df4b2151df3e289952297c5f390dd
dangling blob aaa4a1bf9e6d39991503b7908ff71605d6632fef
dangling blob b003e20f06de7d0a7e11a1047fbb39e2deb84899
dangling blob bf207363786f08e888a725cfb8b2fe4bea11ceab
dangling blob 642493a1bcf09f74551f0ce5b4c3ccc23acaf3c9
dangling blob ff80b45e48795d4544d0a5b2f18714123ab21746
dangling blob e3c0e67e23ec588acab733e2069fb09fa38a235d
dangling blob 62efa65bcbb6d94f26be9b91dc98d7a7f6fbf602
dangling blob a6a2c717c230fe26263b9ce002f3f82fdab6f727
dangling blob d32988875aa90ef728dc1af6b71c130f2c8e8b94
dangling blob c8aad9fc37a27872bae18af78b1623bfb8f9a9f7
dangling blob 4cdbe97301e333d89037dc9f4a8440dab9e62049
dangling blob 2819bbcda3b0efe828709f5b22624712cb9ebdae
dangling blob b156cba3ee89e1506d02fbb845e8dc0889ff4090
dangling blob ed658b8beca69ebf6e25ab1ed6217881e27d4e9a
dangling blob d581cb2fb1cf2b4d15f8f9b90b08a3ebc619414f
dangling blob a85c2e55d27c9fb1f5874f8bd81c65e597327b4f
dangling blob 548f3e70c06f9306d71f6b8d11ed24ade581fa31
dangling blob 7a1c8fa8bc59c4e8fb347426cc11fabf8b0d1639
tyler@grapefruit:~/jburgess_main>
Cheers
--
-R. Tyler Ballance
Slide, Inc.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC/PATCH 2/3] replace_object: add mechanism to replace objects found in "refs/replace/"
From: Junio C Hamano @ 2009-01-07 8:41 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <20090107084341.1554d8cd.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> In "parse_commit_buffer", the parent sha1s from the original commit
> or from a commit graft that match a ref name in "refs/replace/" are
> replaced by the commit sha1 that has been read in the ref.
I may be reading this wrong, but if you replace the parent of the commit,
that won't improve anything over the graft mechanism, which we already
have.
What I was hoping to see was to give replacing commit back when a commit
is asked for by normal callers, while not replacing the objects when
reachability traversal (prune, pack transfer and fsck) wants to read the
commit. That way:
(1) Normal callers will see the rewritten history (which is the same as
graft); and
(2) We will kill the design bug of the current graft mechanism that graft
information cannot be transferred to the other end. By using object
replacement, you can fetch objects reachable from refs/replace/ at
the other end and place them in the same refs/replace/ hierarchy
locally, if you choose to use the same altered history, or you can
choose not to use the replacement yourself. The resulting repository
is fully connected either way.
(3) We will also kill the design bug of the current graft mechanism that
graft information hides repository corruption to fsck. The problem
with this is that if you and then remove the grafts, you will end up
with a corrupt repository, because these operations do look at grafts
(this is justified only partly because otherwise you will lose
objects that are reachable only via grafts, but is broken at the same
time because you can lose the true parents by letting graft hide them
from a reachable commit).
By doing fsck and prune always using the true reachability, and using
refs in the refs/replace/ hierarchy for protecting objects that are
involved in this new way of grafting, you will ensure the integrity
of the repository. Removal of a ref from the refs/replace/ hierarchy
won't result in such a corruption we can have today.
And that is exactly the reason why I was hoping the hook will be at
read_sha1_file() that gives you a rewritten object contents when you ask
for the original object, not at parse_commit_buffer which does not give
you a rewritten object, but makes you follow to a rewritten object when
parsing a commit (which itself is not replaced if it is the starting
point).
Doing replacement at parse_commit_buffer() time is one step too late. For
example, if you have replacement information for the commit that sits at
the tip of 'master' branch, I think your "git log master" will ignore that
replacement information. Creating one new commit on top of it and saying
"git log master" then will show the new commit, and suddenly starts
showing the replacement commit for the one you used to have at the tip of
the branch.
^ permalink raw reply
* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: R. Tyler Ballance @ 2009-01-07 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Nicolas Pitre, Jan Krüger, Git ML
In-Reply-To: <7vaba3bken.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 823 bytes --]
On Wed, 2009-01-07 at 00:16 -0800, Junio C Hamano wrote:
> "R. Tyler Ballance" <tyler@slide.com> writes:
>
> > Unfortunately it doesn't, what I did notice was this when I did a `git
> > status` in the directory right after untarring:
> > tyler@grapefruit:~/jburgess_main> git status
> > #
> > # ---impressive amount of file names fly by---
> > # ----snip---
> > ...
> > Basically, somehow Git thinks that *every* file in the repository is
> > deleted at this point.
>
> That makes me suspect that your .git/index file is corrupt.
Would this be tied to the corrupted pack file issue, or separate.
Either way, how could I verify your assumptions? (i'll be lurking in
#git for a while if you want to interactively help ;))
Cheers
--
-R. Tyler Ballance
Slide, Inc.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Problems getting rid of large files using git-filter-branch
From: Øyvind Harboe @ 2009-01-07 8:26 UTC (permalink / raw)
To: git
In-Reply-To: <c09652430901061359q7a02291fk656ab23e54b19f5e@mail.gmail.com>
Here is a summary of the solution I used. I'm a beginner in git
and just summarizing what others told me and what I did. Use at
your own risk!
1. Remove anything you know should be removed, e.g.:
git filter-branch --tree-filter 'find . -regex ".*toolchain\..*" -exec
rm -f {} \;' HEAD
2. Expire the log:
git reflog expire --all
3. Delete stuff from .git that should be manually "verified" to be
correct. I don't actually
know how to "verify" that at this point... Use backups Luke!
rm -rf .git/refs/original
# delete lines w/"refs/original" from .git/packed-refs
vi .git/packed-refs
# for good measure...
git reflog expire --all
git gc
4. Your repository is still huge. By creating a new repository and pulling from
this one, the garbage will stay in the old one...
mkdir newrep
cd newrep
git init
git pull file:///oldrep
5. Check size of .git. If it is still too big, try figuring out which
files that are big by looking at the packs(.git/objects/pack/xxx):
$ git verify-pack -v $PACK | grep -v "^chain " | sort -n -k 4
and then for the last few lines do a
$ git rev-list --all --objects | grep $SHA1
6. Go back to #1 until done.
Your repository should now be of reasonable size...
I've found some great scripts for converting from svn/cvs, but really
the above procedure
is necessary to run when converting nasty old repositories...
--
Øyvind Harboe
http://www.zylin.com/zy1000.html
ARM7 ARM9 XScale Cortex
JTAG debugger and flash programmer
^ permalink raw reply
* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: Junio C Hamano @ 2009-01-07 8:16 UTC (permalink / raw)
To: R. Tyler Ballance; +Cc: Linus Torvalds, Nicolas Pitre, Jan Krüger, Git ML
In-Reply-To: <1231314099.8870.415.camel@starfruit>
"R. Tyler Ballance" <tyler@slide.com> writes:
> Unfortunately it doesn't, what I did notice was this when I did a `git
> status` in the directory right after untarring:
> tyler@grapefruit:~/jburgess_main> git status
> #
> # ---impressive amount of file names fly by---
> # ----snip---
> ...
> Basically, somehow Git thinks that *every* file in the repository is
> deleted at this point.
That makes me suspect that your .git/index file is corrupt.
^ permalink raw reply
* [RFC/PATCH 3/3] replace_object: add a test case
From: Christian Couder @ 2009-01-07 7:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In this patch the setup code is very big, but this will be used in
test cases that will be added later.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t6050-replace.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 75 insertions(+), 0 deletions(-)
create mode 100755 t/t6050-replace.sh
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
new file mode 100755
index 0000000..0412659
--- /dev/null
+++ b/t/t6050-replace.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Christian Couder
+#
+test_description='Tests replace refs functionality'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+add_and_commit_file()
+{
+ _file="$1"
+ _msg="$2"
+
+ git add $_file || return $?
+ test_tick || return $?
+ git commit --quiet -m "$_file: $_msg"
+}
+
+HASH1=
+HASH2=
+HASH3=
+HASH4=
+HASH5=
+HASH6=
+HASH7=
+
+test_expect_success 'set up buggy branch' '
+ echo "line 1" >> hello &&
+ echo "line 2" >> hello &&
+ echo "line 3" >> hello &&
+ echo "line 4" >> hello &&
+ add_and_commit_file hello "4 lines" &&
+ HASH1=$(git rev-parse --verify HEAD) &&
+ echo "line BUG" >> hello &&
+ echo "line 6" >> hello &&
+ echo "line 7" >> hello &&
+ echo "line 8" >> hello &&
+ add_and_commit_file hello "4 more lines with a BUG" &&
+ HASH2=$(git rev-parse --verify HEAD) &&
+ echo "line 9" >> hello &&
+ echo "line 10" >> hello &&
+ add_and_commit_file hello "2 more lines" &&
+ HASH3=$(git rev-parse --verify HEAD) &&
+ echo "line 11" >> hello &&
+ add_and_commit_file hello "1 more line" &&
+ HASH4=$(git rev-parse --verify HEAD) &&
+ sed -e "s/BUG/5/" hello > hello.new &&
+ mv hello.new hello &&
+ add_and_commit_file hello "BUG fixed" &&
+ HASH5=$(git rev-parse --verify HEAD) &&
+ echo "line 12" >> hello &&
+ echo "line 13" >> hello &&
+ add_and_commit_file hello "2 more lines" &&
+ HASH6=$(git rev-parse --verify HEAD)
+ echo "line 14" >> hello &&
+ echo "line 15" >> hello &&
+ echo "line 16" >> hello &&
+ add_and_commit_file hello "again 3 more lines" &&
+ HASH7=$(git rev-parse --verify HEAD)
+'
+
+test_expect_success 'replace the author' '
+ git cat-file commit $HASH2 | grep "author A U Thor" &&
+ R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
+ git cat-file commit $R | grep "author O Thor" &&
+ git update-ref refs/replace/$HASH2 $R &&
+ git show HEAD~5 | grep "O Thor" &&
+ git show $HASH2 | grep "A U Thor"
+'
+
+#
+#
+test_done
--
1.6.1.162.g1cd53
^ permalink raw reply related
* [RFC/PATCH 2/3] replace_object: add mechanism to replace objects found in "refs/replace/"
From: Christian Couder @ 2009-01-07 7:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The code of this mechanism has been copied from the commit graft code.
Currently this mechanism is only used from the "parse_commit_buffer"
function in "commit.c". It should probably be used from "fsck.c" too.
(For information, grafts are looked up only from "parse_commit_buffer"
function in "commit.c" and from "fsck_commit" in "fsck.c".)
In "parse_commit_buffer", the parent sha1s from the original commit
or from a commit graft that match a ref name in "refs/replace/" are
replaced by the commit sha1 that has been read in the ref.
This means that for example "git show <original commit sha1>" will
display information about the original commit. If the mechanism
had been called from "read_sha1_file" instead of when parents
are read, then "git show <original commit sha1>" would display
information about the commit that replaces the original one.
This may be seen as a feature or as a bug depending on the point
of view.
Anyway this implementation makes sure that the mechanism is
triggered only when commit graft could be triggered, so hopefully the
object reachability traverser will ignore this mechanism as it
ignores the graft mechanism.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Makefile | 1 +
commit.c | 7 +++-
commit.h | 2 +
replace_object.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 110 insertions(+), 2 deletions(-)
create mode 100644 replace_object.c
diff --git a/Makefile b/Makefile
index aabf013..f355e63 100644
--- a/Makefile
+++ b/Makefile
@@ -471,6 +471,7 @@ LIB_OBJS += read-cache.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
LIB_OBJS += remote.o
+LIB_OBJS += replace_object.o
LIB_OBJS += rerere.o
LIB_OBJS += revision.o
LIB_OBJS += run-command.o
diff --git a/commit.c b/commit.c
index c99db16..0014174 100644
--- a/commit.c
+++ b/commit.c
@@ -241,6 +241,7 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size)
char *tail = buffer;
char *bufptr = buffer;
unsigned char parent[20];
+ const unsigned char *parent_sha1;
struct commit_list **pptr;
struct commit_graft *graft;
@@ -268,7 +269,8 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size)
bufptr += 48;
if (graft)
continue;
- new_parent = lookup_commit(parent);
+ parent_sha1 = lookup_replace_object(parent);
+ new_parent = lookup_commit(parent_sha1);
if (new_parent)
pptr = &commit_list_insert(new_parent, pptr)->next;
}
@@ -276,7 +278,8 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size)
int i;
struct commit *new_parent;
for (i = 0; i < graft->nr_parent; i++) {
- new_parent = lookup_commit(graft->parent[i]);
+ parent_sha1 = lookup_replace_object(graft->parent[i]);
+ new_parent = lookup_commit(parent_sha1);
if (!new_parent)
continue;
pptr = &commit_list_insert(new_parent, pptr)->next;
diff --git a/commit.h b/commit.h
index 3a7b06a..37aa763 100644
--- a/commit.h
+++ b/commit.h
@@ -122,6 +122,8 @@ struct commit_graft *read_graft_line(char *buf, int len);
int register_commit_graft(struct commit_graft *, int);
struct commit_graft *lookup_commit_graft(const unsigned char *sha1);
+const unsigned char *lookup_replace_object(const unsigned char *sha1);
+
extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup);
extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos, int cleanup);
extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);
diff --git a/replace_object.c b/replace_object.c
new file mode 100644
index 0000000..b50890d
--- /dev/null
+++ b/replace_object.c
@@ -0,0 +1,102 @@
+#include "cache.h"
+#include "refs.h"
+
+static struct replace_object {
+ unsigned char sha1[2][20];
+} **replace_object;
+
+static int replace_object_alloc, replace_object_nr;
+
+static int replace_object_pos(const unsigned char *sha1)
+{
+ int lo, hi;
+ lo = 0;
+ hi = replace_object_nr;
+ while (lo < hi) {
+ int mi = (lo + hi) / 2;
+ struct replace_object *rep = replace_object[mi];
+ int cmp = hashcmp(sha1, rep->sha1[0]);
+ if (!cmp)
+ return mi;
+ if (cmp < 0)
+ hi = mi;
+ else
+ lo = mi + 1;
+ }
+ return -lo - 1;
+}
+
+static int register_replace_object(struct replace_object *replace,
+ int ignore_dups)
+{
+ int pos = replace_object_pos(replace->sha1[0]);
+
+ if (0 <= pos) {
+ if (ignore_dups)
+ free(replace);
+ else {
+ free(replace_object[pos]);
+ replace_object[pos] = replace;
+ }
+ return 1;
+ }
+ pos = -pos - 1;
+ if (replace_object_alloc <= ++replace_object_nr) {
+ replace_object_alloc = alloc_nr(replace_object_alloc);
+ replace_object = xrealloc(replace_object,
+ sizeof(*replace_object) *
+ replace_object_alloc);
+ }
+ if (pos < replace_object_nr)
+ memmove(replace_object + pos + 1,
+ replace_object + pos,
+ (replace_object_nr - pos - 1) *
+ sizeof(*replace_object));
+ replace_object[pos] = replace;
+ return 0;
+}
+
+static int register_replace_ref(const char *refname,
+ const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ /* Get sha1 from refname */
+ const char *slash = strrchr(refname, '/');
+ const char *hash = slash ? slash + 1 : refname;
+ struct replace_object * repl_obj = xmalloc(sizeof(*repl_obj));
+
+ if (strlen(hash) != 40 || get_sha1_hex(hash, repl_obj->sha1[0])) {
+ free(repl_obj);
+ warning("bad replace ref name: %s", refname);
+ }
+
+ /* Copy sha1 from the read ref */
+ hashcpy(repl_obj->sha1[1], sha1);
+
+ /* Register new object */
+ if (register_replace_object(repl_obj, 1))
+ warning("duplicate replace ref: %s", refname);
+
+ return 0;
+}
+
+static void prepare_replace_object(void)
+{
+ static int replace_object_prepared;
+
+ if (replace_object_prepared)
+ return;
+
+ for_each_replace_ref(register_replace_ref, NULL);
+ replace_object_prepared = 1;
+}
+
+const unsigned char *lookup_replace_object(const unsigned char *sha1)
+{
+ int pos;
+
+ prepare_replace_object();
+ pos = replace_object_pos(sha1);
+
+ return (0 <= pos) ? replace_object[pos]->sha1[1] : sha1;
+}
--
1.6.1.162.g1cd53
^ permalink raw reply related
* [RFC/PATCH 1/3] refs: add a "for_each_replace_ref" function
From: Christian Couder @ 2009-01-07 7:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This is some preparation work for the following patches that are using
the "refs/replace/" ref namespace.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
refs.c | 5 +++++
refs.h | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
Junio wrote:
> What I thought we
> discussed during GitTogether was to write out the object name of the
> replacement object in refs/replace/<sha1>.
>
> When the caller asks read_sha1_file() for an object whose object name is
> <sha1>, you see if there is refs/replace/<sha1> in the repository, and
> read the ref to learn the object name of the object that replaces it.
> And you return that as if it is the original object.
Patch 2/3 in this series implements the new mechanism. As you can see I
prefered it to be called when reading parent commits than from
"read_sha1_file", because it seems to simplify things. I hope you still like
it.
Regards,
Christian.
diff --git a/refs.c b/refs.c
index 33ced65..042106d 100644
--- a/refs.c
+++ b/refs.c
@@ -632,6 +632,11 @@ int for_each_remote_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref("refs/remotes/", fn, 13, cb_data);
}
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+{
+ return do_for_each_ref("refs/replace/", fn, 13, cb_data);
+}
+
/*
* Make sure "ref" is something reasonable to have under ".git/refs/";
* We do not like it if:
diff --git a/refs.h b/refs.h
index 06ad260..8d2ee5a 100644
--- a/refs.h
+++ b/refs.h
@@ -23,6 +23,7 @@ extern int for_each_ref(each_ref_fn, void *);
extern int for_each_tag_ref(each_ref_fn, void *);
extern int for_each_branch_ref(each_ref_fn, void *);
extern int for_each_remote_ref(each_ref_fn, void *);
+extern int for_each_replace_ref(each_ref_fn, void *);
/*
* Extra refs will be listed by for_each_ref() before any actual refs
--
1.6.1.162.g1cd53
^ permalink raw reply related
* Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
From: R. Tyler Ballance @ 2009-01-07 7:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, Jan Krüger, Git ML
In-Reply-To: <alpine.LFD.2.00.0901062026500.3057@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 5231 bytes --]
On Tue, 2009-01-06 at 20:54 -0800, Linus Torvalds wrote:
>
> On Tue, 6 Jan 2009, R. Tyler Ballance wrote:
> >
> > I'll back the patch out and redeploy, it's worth mentioning that a
> > coworker of mine just got the issue as well (on 1.6.1). He was able to
> > `git pull` and the error went away, but I doubt that it "magically fixed
> > itself"
>
> Quite frankly, that behaviour sounds like a disk _cache_ corruption issue.
> The fact that some corruption "comes and goes" and sometimes magically
> heals itself sounds very much like some disk cache problem, and then that
> particular part of the cache gets replaced and then when re-populated it
> is magically correct.
>
> We had that in one case with a Linux NFS client, where a rename across
> directories caused problems.
>
> This was a networked filesystem on OS X, right? File caching is much more
> "interesting" in networked filesystems than it is in normal private
> on-disk ones.
Not quite, what I meant was that some users (not all) who've experienced
this issue are using Samba to copy files over directly into the Git
repository. I was mentioning this in case somewhere between Finder,
Samba, ext3 and Git, some file system change events were pissing Git off
and causing it. I don't think this is the case as the coworker that I
mentioned earlier doesn't use Samba and neither do I (we both experience
the issue today, mine disappeared by upgrading to 1.6.1, his by `git
pull`).
>
> > I've tarred one of the repositories that had it in a reproducible state
> > so I can create a build and extract the tar and run against that to
> > verify any patches anybody might have, but unfortunately at 7GB of
> > company code and assets, I can't exactly share ;)
>
> The thing to do is
>
> - untar it on some trusted machine with a local disk and a known-good
> filesystem.
>
> IOW, not that networked samba share.
>
> - verify that it really does happen on that machine, with that untarred
> image. Because maybe it doesn't.
Unfortunately it doesn't, what I did notice was this when I did a `git
status` in the directory right after untarring:
tyler@grapefruit:~/jburgess_main> git status
#
# ---impressive amount of file names fly by---
# ----snip---
#
# Untracked files:
# (use "git add <file>..." to include in what will be
committed)
#
# artwork/
# bt/
# flash/
tyler@grapefruit:~/jburgess_main>
Basically, somehow Git thinks that *every* file in the repository is
deleted at this point. I went ahead and performed a `git reset --hard`
to see if the issue would manifest itself thereafter, but it did not.
I did try to do a git-fsck(1), and this is what I got:
tyler@grapefruit:~/jburgess_main> /usr/local/bin/git fsck --full
[1] 19381 segmentation fault /usr/local/bin/git fsck --full
tyler@grapefruit:~/jburgess_main>
>
> The hope is that you caught the corruption in the cache, and it
> actually got written out to the tar-file. But if it _is_ a disk cache
> (well, network cache) issue, maybe the IO required to tar everything up
> was enough to flush it, and the tar-file actually _works_ because it
> got repopulated correctly.
When I was working through this with Jan, one of the things that we did
was move the actual object file in .git/objects, they existed so maybe I
could look into those to check?
>
> So that's why you should double-check that it really ends up being
> corrupt after being untarred again.
>
> - go back and test the original git repo on the network share, preferably
> on another client. See if the error has gone away.
Unfortunately the repository is being used by the original developer I
tarred from with our 1.6.1 build, he hasn't reported any issues, but I
can't exactly steal it back (that's why I made the tar)
> The fact that you seem to get a _lot_ of these errors really does make
> it
> sound like something in your environment. It's actually really hard to get
> git to corrupt anything. Especially objects that got packed. They've been
> quiescent for a long time, they got repacked in a very simple way, they
> are totally read-only.
I checked with our operations team, and contrary to my suspicion (your
NFS comment piqued my curiosity), these disks that are actually on the
machines are not NFS mounts but rather local disk arrays.
--> is it NFSd? or all local storage
<== all local
<== df -h
<== mount
<== /dev/sda5 705G 247G 423G 37% /nail
--> hm, there goes that theory
<== git corruption?
--> yeah, looking into it
<== sucks
--> Linus had a theory about NFS/etc corruption of the disk
cache
<== when the company folds we can all blame you... and your
silly git games
<== (think positive, joel)
--> thanks
;)
Any thing else I can do to help debug this? :-/
Cheers
--
-R. Tyler Ballance
Slide, Inc.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Comments on Presentation Notes Request.
From: david @ 2009-01-07 8:33 UTC (permalink / raw)
To: Tim Visher; +Cc: git
In-Reply-To: <c115fd3c0901061433i78bf3b26v77e5981aada6728e@mail.gmail.com>
On Tue, 6 Jan 2009, Tim Visher wrote:
> *** Natural Backup
>
> Because every developer has a copy of the repository, every developer
> you add adds an extra failure point. The more developers you have,
> the more backups you have of the repository.
this needs to be re-worded. 'extra failure point' can be read to mean
redundancy in what would otherwide be a single point of failure, but it
can also mean another point where things can fail.
something like 'every developer adds an extra layer of redundancy' would
be much less ambiguous.
David Lang
^ permalink raw reply
* Re: [PATCH] Fix sourcing "test-lib.sh" using dash shell in "t3003-ls-files-narrow-match.sh"
From: Christian Couder @ 2009-01-07 7:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vzli4kftt.fsf@gitster.siamese.dyndns.org>
Le mardi 6 janvier 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > dash barfs, on my old Ubuntu box, when "test-lib.sh" is sourced
> > without "./".
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
> > t/t3003-ls-files-narrow-match.sh | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > This patch applies to "pu".
>
> Thanks; I hope you don't mind squashing this in to 'Introduce "sparse
> patterns"'.
No problem.
Thanks,
Christian.
^ permalink raw reply
* Re: [RFC PATCH] diff --no-index: test for pager after option parsing
From: Junio C Hamano @ 2009-01-07 7:02 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Thomas Rast, git
In-Reply-To: <20090107032013.GO21154@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Jan 06, 2009 at 04:09:18PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>> But I wonder if it still makes a difference in real life.idn't we stop
>> reporting the exit status from the pager some time ago?
>
> I just wanted to write this, I think that code could be just removed
> since ea27a18 (spawn pager via run_command interface, 2008-07-22).
I think we shouldn't.
People may already have got used to "git diff --exit-code" to disable the
pager, and doing the same for "git diff --exit-code --no-index" should be
with less surprises.
I'll queue the "--" fix, "-q" fix and this pager fix. Thanks.
^ permalink raw reply
* Re: How make "git checkout <commit> <file>" *not* alter index?
From: chris @ 2009-01-07 6:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk599ne6a.fsf@gitster.siamese.dyndns.org>
On Mon, Jan 05, 2009 at 10:26:05PM -0800, Junio C Hamano wrote:
> $ git checkout HEAD~43 Makefile
> $ git reset Makefile
Thank you very much. git reset looks that just what I need.
cs
^ permalink raw reply
* Re: [RFC PATCH] diff --no-index: test for pager after option parsing
From: Jeff King @ 2009-01-07 6:42 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, Thomas Rast, git
In-Reply-To: <20090107032013.GO21154@genesis.frugalware.org>
On Wed, Jan 07, 2009 at 04:20:13AM +0100, Miklos Vajna wrote:
> On Tue, Jan 06, 2009 at 04:09:18PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> > But I wonder if it still makes a difference in real life.idn't we stop
> > reporting the exit status from the pager some time ago?
>
> I just wanted to write this, I think that code could be just removed
> since ea27a18 (spawn pager via run_command interface, 2008-07-22).
I don't think just removing it is right. You would also need to put
SETUP_PAGER into the flags for calling cmd_diff.
We do pass along the error code properly these days, but I think it is
nice that --exit-code always just suppresses the pager. Otherwise a
script like this:
if git diff --exit-code $x $y; then
do something
fi
will invoke the pager (and not everybody's setup immediately exits if
there is no output, either because they have different LESS options or
because they use a different pager). Of course one might argue that the
script should not be using "git diff" porcelain at all, but I don't
think there is another way to get a --no-index diff.
-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