Git development
 help / color / mirror / Atom feed
* Re: Spam: Re: git branch performance problem?
From: Johannes Schindelin @ 2007-10-11 10:58 UTC (permalink / raw)
  To: Mike Ralphson; +Cc: Brandon Casey, Lars Hjemli, J. Bruce Fields, hanwen, git
In-Reply-To: <e2b179460710110241i445bc61ep8ae60e421c19c0f0@mail.gmail.com>

Hi,

On Thu, 11 Oct 2007, Mike Ralphson wrote:

> It's not something I've really looked into, but there seems to be a
> reflogs mechanism which can temporarily pin an otherwise unreferenced
> object so it doesn't get deleted. Would it be possible to populate the
> remote's view of referenced objects into this, at the point of clone,
> push or pull, which would seem to be the points at which this might be
> changing.
> 
> Obviously this is of no use if you're 'anonymously' poncing off a
> third repo to save clone time, but if you're in control of both repo's
> it might be useful.

I cannot really allege that I understood what you were trying to say, but 
I guess you want to use clone to get rid of objects you just threw out by 
either filter-branch or deleting a branch.

The answer is that the file:// as well as the git:// protocol will do 
that.  For local clones, they are not the default, since they are slower 
than hardlinking.

Hth,
Dscho

^ permalink raw reply

* Re: git branch performance problem?
From: Johannes Schindelin @ 2007-10-11 10:46 UTC (permalink / raw)
  To: hanwen; +Cc: Linus Torvalds, Lars Hjemli, git
In-Reply-To: <f329bf540710101926vedf8b19p52e3eeb193203d03@mail.gmail.com>

Hi,

On Wed, 10 Oct 2007, Han-Wen Nienhuys wrote:

> For cherrypicking convenience, I would still appreciate it if there was 
> a mechanism similar to alternates that would allow me to view objects 
> from an alternate repo; objects found through this mechanism should 
> never be assumed to be present in the database, of course.

Silly question: why don't you just

	git remote add -f other <url>

and then review the changes with "git log", "git diff" and "git show"?

Ciao,
Dscho

^ permalink raw reply

* Re: Spam: Re: git branch performance problem?
From: Mike Ralphson @ 2007-10-11  9:41 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Lars Hjemli, J. Bruce Fields, hanwen, git
In-Reply-To: <470D585D.1030808@nrlssc.navy.mil>

On 10/10/07, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> No, this is not the case, unless something has changed very recently
> in git-gc or git-repack. Even git-gc with no arguments is unsafe if
> the repository being gc'ed is listed in another's alternates.
>
> git-gc calls repack with -a and -d. which causes a new pack to be
> created which only contains the objects required by the local repository.
> The other packs are then deleted. Objects contained in those packs and
> required by a "sharing" repository (one using the alternates mechanism)
> will be deleted if the local repository no longer references them.

It's not something I've really looked into, but there seems to be a
reflogs mechanism which can temporarily pin an otherwise unreferenced
object so it doesn't get deleted. Would it be possible to populate the
remote's view of referenced objects into this, at the point of clone,
push or pull, which would seem to be the points at which this might be
changing.

Obviously this is of no use if you're 'anonymously' poncing off a
third repo to save clone time, but if you're in control of both repo's
it might be useful.

Mike

^ permalink raw reply

* Re: [FEATURE REQUEST] git clone, just clone selected branches?
From: Andreas Ericsson @ 2007-10-11  9:18 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: 'Linus Torvalds', git
In-Reply-To: <011c01c80b85$a0d8d560$04ac10ac@Jocke>

Joakim Tjernlund wrote:
>> -----Original Message-----
>> From: Linus Torvalds [mailto:torvalds@linux-foundation.org] 
>> Sent: den 10 oktober 2007 21:38
>> To: Joakim Tjernlund
>> Cc: git@vger.kernel.org
>> Subject: Re: [FEATURE REQUEST] git clone, just clone selected 
>> branches?
>>
>>
>>
>> On Wed, 10 Oct 2007, Joakim Tjernlund wrote:
>>> I know I can use git remote to do this, but it is a bit clumsy 
>>> when starting a new repo.
>> How about just
>>
>> 	git init
>> 	.. set up remote tracking info in .git/config ..
>> 	git fetch remote
>>
>> which should do what you want.
>>
>> 		Linus
> 
> From git remote man page:
>  o  Imitate git clone but track only selected branches
> 
>           $ mkdir project.git
>           $ cd project.git
>           $ git init
>           $ git remote add -f -t master -m master origin git://example.com/git.git/
>           $ git merge origin
> 
> yes, this does the trick too but that is more to type so I thought
> it would be simpler if I could just tell git clone which branches I want.
> Now, this isn't a killer feature to me so if you don't like it, I good
> with that too.
> 

Well, that's how git-clone should probably look, now that git-remote exists.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] git-cvsserver: added support for update -p
From: Andreas Ericsson @ 2007-10-11  8:45 UTC (permalink / raw)
  To: Frank Lichtenheld; +Cc: Jan Wielemaker, Git Mailing List
In-Reply-To: <20071010200029.GK31659@planck.djpig.de>

Frank Lichtenheld wrote:
> On Wed, Oct 10, 2007 at 01:16:03PM +0200, Jan Wielemaker wrote:
>> +	# if we have a -p we should just send the file
>> +        if ( exists ( $state->{opt}{p} ) )
>> +	{
>> +	    if ( open my $fh, '-|', "git-cat-file", "blob", $meta->{filehash} )
>> +	    {   while ( <$fh> )
>> +		{ print "M " . $_;
>> +		}
>> +		close $fh or die ("Couldn't close filehandle for transmitfile(): $!");
>> +	    } else
>> +	    { die("Couldn't execute git-cat-file");
>> +	    }
>> +
>> +	    next;
>> +	}
> 
> 
> There seems to be inconsistent whitespace in the patch.
> And please never do that else\n{ again, it hurts my eye ;)
> 

That cuddled opening brace hurts mine more.

{ while()\n{ print()...

It's usually a good idea to pick some indentation style that at least *some* tool
can create, and when contributing to a project it's usually considered good form
to stick to the style already used.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH 0/2] Add --dry-run option to git-push
From: Lars Hjemli @ 2007-10-11  7:57 UTC (permalink / raw)
  To: Baz; +Cc: git, Junio C Hamano
In-Reply-To: <2faad3050710101634s5ed39bcbn723184810bc265d5@mail.gmail.com>

On 10/11/07, Baz <brian.ewins@gmail.com> wrote:
> there was discussion recently about the default behaviour of git-push
> having the potential to confuse, making it easy to push commits that
> are still cooking in other branches.
>
> Its also possible for newbies to make mistakes with refspecs, but
> there's no way to know if you've got the syntax right without actually
> pushing. Steffan suggested a dry-run flag, which I've always wanted
> too, so here's an attempt. Patches git-send-pack, git-push, their
> docs, and adds a test for git-push.

Maybe some of this description could be reused in the commit messages?

> first patch to the list and the mailer will
> probably mangle it (sigh)

Yeah, it did.

Would it be possible for you to resend with git-send-email after
adding proper commit messages? If git-send-email is not an option,
attaching the patches are preferable to the current mangling.

--
larsh

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: Jonathan del Strother @ 2007-10-11  7:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: David Kastrup, git, Junio C Hamano
In-Reply-To: <470DCC76.7070809@viscovery.net>


On 11 Oct 2007, at 08:10, Johannes Sixt wrote:

> David Kastrup schrieb:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>>> IMHO, it's not too much of a burden for developers to require "sane"
>>> build directory paths.
>> For a normal user, the only writable directories might be of the
>> "C:\Programs and Data\User settings\Karl"
>> variety.
>
> I whole-heartedly agree about the part of the patch that fixes git- 
> rebase.sh. This should be a separate patch.
>
> But the reset of the patch is about running the test suite, and it  
> is much more difficult to fix because of the 'eval' that is going  
> on. And, yes, I do think that we can expect that contributors,  
> including this handful of people on Windows, have a "sane" build  
> directory.

How are you going to test that git works on paths with spaces if the  
test suite doesn't run there?

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: Johannes Sixt @ 2007-10-11  7:41 UTC (permalink / raw)
  To: Jonathan del Strother; +Cc: David Kastrup, git, Junio C Hamano
In-Reply-To: <81156EED-7AC0-4C8B-98B1-8338262459A6@bestbefore.tv>

Jonathan del Strother schrieb:
> How are you going to test that git works on paths with spaces if the 
> test suite doesn't run there?

By writing a specific test?

-- Hannes

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: Johannes Sixt @ 2007-10-11  7:10 UTC (permalink / raw)
  To: David Kastrup; +Cc: Jonathan del Strother, git, Junio C Hamano
In-Reply-To: <854pgytafi.fsf@lola.goethe.zz>

David Kastrup schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>> IMHO, it's not too much of a burden for developers to require "sane"
>> build directory paths.
> 
> For a normal user, the only writable directories might be of the
> "C:\Programs and Data\User settings\Karl"
> variety.

I whole-heartedly agree about the part of the patch that fixes 
git-rebase.sh. This should be a separate patch.

But the reset of the patch is about running the test suite, and it is much 
more difficult to fix because of the 'eval' that is going on. And, yes, I do 
think that we can expect that contributors, including this handful of people 
on Windows, have a "sane" build directory.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: David Kastrup @ 2007-10-11  6:47 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan del Strother, git, Junio C Hamano
In-Reply-To: <470DC05A.8020209@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Jonathan del Strother schrieb:
>> +	cmt=`cat "$dotest/current"`
>
> This is ok, but...
>
>> +			prev_head="`cat \"$dotest/prev_head\"`"
>
> ... there are shells out there in the wild that will get badly
> confused by this sort of quoting and escaping. Butter use
>
> 	prev_head=$(cat "$dotest/prev_head")
>
>
>> -VISUAL="$(pwd)/fake-editor.sh"
>> +VISUAL="'$(pwd)/fake-editor.sh'"
>
> Huh? This looks very wrong.

It is correct, modulo breaking when there are single quotes in the
current path name.

> What are the extra quotes needed for?

Spaces in the current path name.

> If they are really needed, isn't this a bug in
> git-rebase--interactive.sh?

No.

> I question the usefulness of this patch. Why only fix breakage due
> to spaces in the path? What about single-quotes, double-quotes?

Double quotes would work.  Single quotes wouldn't.  You can do
something like

visualpath="$(pwd)"
export visualpath
VISUAL='"$visualpath/fake-editor.sh"'

and this should work in all circumstances where VISUAL is interpreted
as intended (which at the current point of time does not include git's
C callers).

> IMHO, it's not too much of a burden for developers to require "sane"
> build directory paths.

For a normal user, the only writable directories might be of the
"C:\Programs and Data\User settings\Karl"
variety.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: git branch performance problem?
From: Alex Riesen @ 2007-10-11  6:41 UTC (permalink / raw)
  To: hanwen; +Cc: Linus Torvalds, Lars Hjemli, git
In-Reply-To: <f329bf540710101926vedf8b19p52e3eeb193203d03@mail.gmail.com>

Han-Wen Nienhuys, Thu, Oct 11, 2007 04:26:24 +0200:
> > So the rule really is: never *ever* do anything but fast-forward in a repo
> >[..]
> 
> Methinks this is all too difficult. I will use clone -l henceforth.

It is current default for local clones

^ permalink raw reply

* Re: [PATCH] fetch: if not fetching from default remote, ignore default merge
From: Johannes Sixt @ 2007-10-11  6:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, hjemli, gitster
In-Reply-To: <Pine.LNX.4.64.0710110146200.4174@racer.site>

Johannes Schindelin schrieb:
> +test_expect_success 'fetch with a non-applying branch.<name>.merge' '
> +	git config branch.master.remote yeti &&
> +	git config branch.master.merge refs/heads/bigfoot &&
> +	git config remote.blub.url one &&
> +	git config remote.blub.fetch 'refs/heads/*:refs/remotes/one/*' &&

Better use double-quotes around the refspecs.

> +	git fetch blub
> +'
> +

-- Hannes

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: Johannes Sixt @ 2007-10-11  6:19 UTC (permalink / raw)
  To: Jonathan del Strother; +Cc: git, Junio C Hamano
In-Reply-To: <11920508172434-git-send-email-jon.delStrother@bestbefore.tv>

Jonathan del Strother schrieb:
> +	cmt=`cat "$dotest/current"`

This is ok, but...

> +			prev_head="`cat \"$dotest/prev_head\"`"

... there are shells out there in the wild that will get badly confused by 
this sort of quoting and escaping. Butter use

	prev_head=$(cat "$dotest/prev_head")


> -VISUAL="$(pwd)/fake-editor.sh"
> +VISUAL="'$(pwd)/fake-editor.sh'"

Huh? This looks very wrong. What are the extra quotes needed for? If they 
are really needed, isn't this a bug in git-rebase--interactive.sh?

> -	 git-commit -F msg -m amending ."
> +	git-commit -F msg -m amending ."

You fix whitespace...

>  test_expect_success \
> -	"using message from other commit" \
> -	"git-commit -C HEAD^ ."
> +	 "using message from other commit" \
> +	 "git-commit -C HEAD^ ."

... and you break it. More of these follow. Don't do that, it makes patch 
review unnecessarily hard.

I question the usefulness of this patch. Why only fix breakage due to spaces 
in the path? What about single-quotes, double-quotes? IMHO, it's not too 
much of a burden for developers to require "sane" build directory paths.

-- Hannes

^ permalink raw reply

* Re: git branch performance problem?
From: Han-Wen Nienhuys @ 2007-10-11  2:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Lars Hjemli, git
In-Reply-To: <alpine.LFD.0.999.0710101610230.20690@woody.linux-foundation.org>

2007/10/10, Linus Torvalds <torvalds@linux-foundation.org>:

> IOW: alternates are dangerous. A shared object directory is dangerous. You
> should basically only do it under very controlled circumstances, and
> otherwise you should use either hardlinks or if you want added safety,
> totally separate repositories.

I recall reading a few months ago that it was "clone -l" that gave you
the jeebies, rather than "clone -s".


> So the rule really is: never *ever* do anything but fast-forward in a repo
>[..]

Methinks this is all too difficult. I will use clone -l henceforth. Is
there any reason to prefer -s over -l? Given your lengthy exposition
on the dangers of alternates, I would say this is a features that
deserves to be buried or at least deemphasized in the documentation.

For cherrypicking convenience, I would still appreciate it if there
was a mechanism similar to alternates that would allow me to view
objects from an alternate repo; objects found through this mechanism
should never be assumed to be present in the database, of course.


-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

^ permalink raw reply

* [PATCH] git-gc --auto: simplify "repack" command line building
From: Brandon Casey @ 2007-10-11  1:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0710110149100.4174@racer.site>

Since "-a" is removed from the base repack command line,
we can simplify how we add additional options to this
command line when using --auto.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-gc.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 6323e0d..956c32d 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -143,8 +143,6 @@ static int too_many_packs(void)
 
 static int need_to_gc(void)
 {
-	int ac = 0;
-
 	/*
 	 * Setting gc.auto and gc.autopacklimit to 0 or negative can
 	 * disable the automatic gc.
@@ -158,14 +156,10 @@ static int need_to_gc(void)
 	 * we run "repack -A -d -l".  Otherwise we tell the caller
 	 * there is no need.
 	 */
-	argv_repack[ac++] = "repack";
 	if (too_many_packs())
-		argv_repack[ac++] = "-A";
+		append_option(argv_repack, "-A", MAX_ADD);
 	else if (!too_many_loose_objects())
 		return 0;
-	argv_repack[ac++] = "-d";
-	argv_repack[ac++] = "-l";
-	argv_repack[ac++] = NULL;
 	return 1;
 }
 
-- 
1.5.3.4.208.g729c0

^ permalink raw reply related

* Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
From: Brandon Casey @ 2007-10-11  0:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0710110149100.4174@racer.site>

This makes use of repack's new "-A" option which does not drop packed
unreachable objects. This makes git-gc safe to call at any time,
particularly when a repository is referenced as an alternate by
another repository.

git-gc --prune will use the "-a" option to repack instead of "-A", so
that packed unreachable objects will be removed.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-gc.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 23ad2b6..6323e0d 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,7 +26,7 @@ static int gc_auto_pack_limit = 20;
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
-static const char *argv_repack[MAX_ADD] = {"repack", "-a", "-d", "-l", NULL};
+static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
 static const char *argv_prune[] = {"prune", NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
@@ -211,6 +211,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		prune = 0;
 		if (!need_to_gc())
 			return 0;
+	} else {
+		/*
+		 * Use safer (for shared repos) "-A" option to
+		 * repack when not pruning. Auto-gc makes its
+		 * own decision.
+		 */
+		if (prune)
+			append_option(argv_repack, "-a", MAX_ADD);
+		else
+			append_option(argv_repack, "-A", MAX_ADD);
 	}
 
 	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
-- 
1.5.3.4.208.g729c0

^ permalink raw reply related

* Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
From: Johannes Schindelin @ 2007-10-11  0:49 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0710101918060.30020@torch.nrlssc.navy.mil>

Hi,

On Wed, 10 Oct 2007, Brandon Casey wrote:

> On Thu, 11 Oct 2007, Johannes Schindelin wrote:
> 
> > > @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char
> > > *prefix)
> > >  		prune = 0;
> > >  		if (!need_to_gc())
> > >  			return 0;
> > > +	} else {
> > > +		/*
> > > +		 * Use safer (for shared repos) "-A" option to
> > > +		 * repack when not pruning. Auto-gc makes its
> > > +                 * own decision.
> > > +		 */
> > > +		if (prune)
> > > +			append_option(argv_repack, "-a", MAX_ADD);
> > > +		else
> > > +			append_option(argv_repack, "-A", MAX_ADD);
> > >  	}
> > > 
> > >  	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
> > 
> > To use the append_option() function, you need to make sure that the 
> > argv has enough space, so I suggest adding another NULL to 
> > argv_repack[].  This is enough, since you only ever add one option.  
> > But you should mention it somewhere, probably above the definition of 
> > argv_repack[].
> 
> Shouldn't argv_repack have space for MAX_ADD (10) items?

Ouch... I overlooked that.  So please strike this objection.

Thanks,
Dscho

^ permalink raw reply

* [PATCH] fetch: if not fetching from default remote, ignore default merge
From: Johannes Schindelin @ 2007-10-11  0:47 UTC (permalink / raw)
  To: git, hjemli, gitster


When doing "git fetch <remote>" on a remote that does not have the
branch referenced in branch.<current-branch>.merge, git fetch failed.
It failed because it tried to add the "merge" ref to the refs to be
fetched.

Fix that.  And add a test case.

Incidentally, this unconvered a bug in our own test suite, where
"git pull <some-path>" was expected to merge the ref given in the
defaults, even if not pulling from the default remote.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-fetch.c            |    8 +++++++-
 t/t5510-fetch.sh           |    8 ++++++++
 t/t5700-clone-reference.sh |    4 ++--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index cf7498b..ca4de9f 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -105,7 +105,13 @@ static struct ref *get_ref_map(struct transport *transport,
 				    !remote->fetch[0].pattern)
 					ref_map->merge = 1;
 			}
-			if (has_merge)
+			/*
+			 * if the remote we're fetching from is the same
+			 * as given in branch.<name>.remote, we add the
+			 * ref given in branch.<name>.merge, too.
+			 */
+			if (has_merge && !strcmp(branch->remote_name,
+						remote->name))
 				add_merge_config(&ref_map, remote_refs, branch, &tail);
 		} else {
 			ref_map = get_remote_ref(remote_refs, "HEAD");
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 73a4e3c..755d809 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -188,4 +188,12 @@ test_expect_success 'push via rsync' '
 '
 }
 
+test_expect_success 'fetch with a non-applying branch.<name>.merge' '
+	git config branch.master.remote yeti &&
+	git config branch.master.merge refs/heads/bigfoot &&
+	git config remote.blub.url one &&
+	git config remote.blub.fetch 'refs/heads/*:refs/remotes/one/*' &&
+	git fetch blub
+'
+
 test_done
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 4e93aaa..b6a5486 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -38,7 +38,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling from reference' \
 'cd C &&
-git pull ../B'
+git pull ../B master'
 
 cd "$base_dir"
 
@@ -61,7 +61,7 @@ test_expect_success 'existence of info/alternates' \
 cd "$base_dir"
 
 test_expect_success 'pulling from reference' \
-'cd D && git pull ../B'
+'cd D && git pull ../B master'
 
 cd "$base_dir"
 
-- 
1.5.3.4.1169.g5fb8d

^ permalink raw reply related

* Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
From: Brandon Casey @ 2007-10-11  0:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0710110106160.4174@racer.site>

On Thu, 11 Oct 2007, Johannes Schindelin wrote:

> Hi,
>
> On Wed, 10 Oct 2007, Brandon Casey wrote:

<snip>

> This is a nice cleanup, but it does not belong in this patch.

I was thinking the same thing right after I sent it off.

>
>> @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>>  		prune = 0;
>>  		if (!need_to_gc())
>>  			return 0;
>> +	} else {
>> +		/*
>> +		 * Use safer (for shared repos) "-A" option to
>> +		 * repack when not pruning. Auto-gc makes its
>> +                 * own decision.
>> +		 */
>> +		if (prune)
>> +			append_option(argv_repack, "-a", MAX_ADD);
>> +		else
>> +			append_option(argv_repack, "-A", MAX_ADD);
>>  	}
>>
>>  	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
>
> To use the append_option() function, you need to make sure that the argv
> has enough space, so I suggest adding another NULL to argv_repack[].  This
> is enough, since you only ever add one option.  But you should mention it
> somewhere, probably above the definition of argv_repack[].

Shouldn't argv_repack have space for MAX_ADD (10) items?

append_option() is already used with argv_repack when --aggressive is
used.

> Other than that: I think your patch makes tons of sense; much better than
> what I tried to cobble together with the cyclic alternates detection.

Thanks.

-brandon

^ permalink raw reply

* Re: [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
From: Johannes Schindelin @ 2007-10-11  0:12 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <470D6509.5020607@nrlssc.navy.mil>

Hi,

On Wed, 10 Oct 2007, Brandon Casey wrote:

>  static int need_to_gc(void)
>  {
> -	int ac = 0;
> -
>  	/*
>  	 * Setting gc.auto and gc.autopacklimit to 0 or negative can
>  	 * disable the automatic gc.
> @@ -158,14 +156,10 @@ static int need_to_gc(void)
>  	 * we run "repack -A -d -l".  Otherwise we tell the caller
>  	 * there is no need.
>  	 */
> -	argv_repack[ac++] = "repack";
>  	if (too_many_packs())
> -		argv_repack[ac++] = "-A";
> +		append_option(argv_repack, "-A", MAX_ADD);
>  	else if (!too_many_loose_objects())
>  		return 0;
> -	argv_repack[ac++] = "-d";
> -	argv_repack[ac++] = "-l";
> -	argv_repack[ac++] = NULL;
>  	return 1;
>  }
>  

This is a nice cleanup, but it does not belong in this patch.

> @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		prune = 0;
>  		if (!need_to_gc())
>  			return 0;
> +	} else {
> +		/*
> +		 * Use safer (for shared repos) "-A" option to
> +		 * repack when not pruning. Auto-gc makes its
> +                 * own decision.
> +		 */
> +		if (prune)
> +			append_option(argv_repack, "-a", MAX_ADD);
> +		else
> +			append_option(argv_repack, "-A", MAX_ADD);
>  	}
>  
>  	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))

To use the append_option() function, you need to make sure that the argv 
has enough space, so I suggest adding another NULL to argv_repack[].  This 
is enough, since you only ever add one option.  But you should mention it 
somewhere, probably above the definition of argv_repack[].

Other than that: I think your patch makes tons of sense; much better than 
what I tried to cobble together with the cyclic alternates detection.

Ciao,
Dscho

^ permalink raw reply

* Re: inexplicable failure to merge recursively across cherry-picks
From: Miklos Vajna @ 2007-10-11  0:08 UTC (permalink / raw)
  To: David Brown; +Cc: Linus Torvalds, martin f krafft, git discussion list
In-Reply-To: <20071010154831.GA19226@old.davidb.org>

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

[ ehh, sorry for my previous mail, i must be doing something wrong.. ]

On Wed, Oct 10, 2007 at 08:48:31AM -0700, David Brown <git@davidb.org> wrote:
> There is a darcs-git project that tries to do the darcs things on top of
> git.

actually it's broken, according to its author:

http://www.mail-archive.com/darcs-users@darcs.net/msg03161.html

though i loved darcs, but yes - it scales horribly

- VMiklos

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] git-gc: by default use safer "-A" option to repack when not --prune'ing
From: Brandon Casey @ 2007-10-10 23:49 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano

This makes use of repack's new "-A" option which does not drop packed
unreachable objects. This makes git-gc safe to call at any time,
particularly when a repository is referenced as an alternate by
another repository.

git-gc --prune will use the "-a" option to repack instead of "-A", so
that packed unreachable objects will be removed.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-gc.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 23ad2b6..779245b 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -26,7 +26,7 @@ static int gc_auto_pack_limit = 20;
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};
 static const char *argv_reflog[] = {"reflog", "expire", "--all", NULL};
-static const char *argv_repack[MAX_ADD] = {"repack", "-a", "-d", "-l", NULL};
+static const char *argv_repack[MAX_ADD] = {"repack", "-d", "-l", NULL};
 static const char *argv_prune[] = {"prune", NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
@@ -143,8 +143,6 @@ static int too_many_packs(void)
 
 static int need_to_gc(void)
 {
-	int ac = 0;
-
 	/*
 	 * Setting gc.auto and gc.autopacklimit to 0 or negative can
 	 * disable the automatic gc.
@@ -158,14 +156,10 @@ static int need_to_gc(void)
 	 * we run "repack -A -d -l".  Otherwise we tell the caller
 	 * there is no need.
 	 */
-	argv_repack[ac++] = "repack";
 	if (too_many_packs())
-		argv_repack[ac++] = "-A";
+		append_option(argv_repack, "-A", MAX_ADD);
 	else if (!too_many_loose_objects())
 		return 0;
-	argv_repack[ac++] = "-d";
-	argv_repack[ac++] = "-l";
-	argv_repack[ac++] = NULL;
 	return 1;
 }
 
@@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		prune = 0;
 		if (!need_to_gc())
 			return 0;
+	} else {
+		/*
+		 * Use safer (for shared repos) "-A" option to
+		 * repack when not pruning. Auto-gc makes its
+                 * own decision.
+		 */
+		if (prune)
+			append_option(argv_repack, "-a", MAX_ADD);
+		else
+			append_option(argv_repack, "-A", MAX_ADD);
 	}
 
 	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
-- 
1.5.3.4.207.g00000

^ permalink raw reply related

* Re: git branch performance problem?
From: Linus Torvalds @ 2007-10-10 23:39 UTC (permalink / raw)
  To: hanwen; +Cc: Lars Hjemli, git
In-Reply-To: <f329bf540710101430i63926b25q7d55976af96b891d@mail.gmail.com>



On Wed, 10 Oct 2007, Han-Wen Nienhuys wrote:
> 
> The way I solved that, was to have both repositories pointing to each
> other, using alternates.

Ouch. Double un-good. Not a good idea. Especially not if you do 
development in both and pull and push between them. 

What will happen is that if you do alternates pointing both ways, you 
basically end up having a "shared pool of objects". So it's pretty much 
equivalent to just using a shared object directory, and it has *exactly* 
the same issues with object reachability and references: you have a shared 
pool of objects, but you only ever see *one* set of references, so garbage 
collection cannot work - because it will always see just a subset of the 
real references, while it sees essentially all objects.

> could it be that GC does not handle cyclic alternates correctly?

It's not about cyclic per se: it's about the fact that GC will do garbage 
collection based on reachability with the local references.

Which is normally fine. It's normally fine, because the object tree is 
"local" too. But when doing alternates:

 - the tree that is being used as an alternate *has* to be totally stable. 
   It must *never* have been re-based, or have any GC'able objects in the 
   first place. IOW, doing a "git gc" on it will be safe, because there is 
   no way any objects that the other alternate depends on could be pruned.

 - You definitely must *not* do a two-way alternate, because that violates 
   another rule: the rule that the "alternate base" (which is now *both*
   of the repositories) is self-sufficient. Since they both point to each 
   other, there's no way to know whether they are self-sufficient or not: 
   they may be re-using each others objects *and* packs!

And in the above, the "*and* packs" is important, and probably the cause 
of your problems. Because "git repack -a -d -l" (which is what "git gc" 
does) will always gather up any loose objects even from remote sites, but 
the "-l" means that it will not do so for alternate packed objects.

So what happens is that if one of the repositories can reach some object 
that is in a pack in the other repository, "git gc" will still *leave* it 
dependent on a pack in the other repository. But maybe that object isn't 
even reachable in the other repo any more (for whatever reason - a rebase, 
whatever), then when you repack the other repository, now all the packs 
will be replaced by one new pack - and the one new pack will only contain 
the objects reachable from the other repo.

IOW: alternates are dangerous. A shared object directory is dangerous. You 
should basically only do it under very controlled circumstances, and 
otherwise you should use either hardlinks or if you want added safety, 
totally separate repositories.

Basically, here's an example of badness, with A and B being repos that 
point to each other.

 - do something in A
 - pull it into B - this leaves the objects in A, because of the 
   alternates link.
 - rebase A
 - "git gc" in A: this removes unreachable objects from A, and now B is 
   screwed.

So the rule really is: never *ever* do anything but fast-forward in a repo 
that is an alternate for another one. If you do a circular link, I think 
it's still safe if you follow that rule, but now obviously the rule holds 
for *both* repos (and quite frankly, I'd worry so much that I'd never do 
it even then).

There should be another rule too: git on its own is not a backup system. 
You can use git *as* a backup system, but you need to do so by mirroring 
the whole repository, and not on the same disk.

(ie, for me, git *is* a backup system, but that's only because I push my 
repos to other sites - a single git repo on its own has zero redundancy)

		Linus

^ permalink raw reply

* [PATCH 2/2] Add a --dry-run option to git-push.
From: Brian Ewins @ 2007-10-10 23:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Signed-off-by: Brian Ewins <brian.ewins@gmail.com>
---
  Documentation/git-push.txt |    6 +++++-
  builtin-push.c             |   10 ++++++++--
  t/t5516-fetch-push.sh      |   10 ++++++++++
  3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 6bc559d..2dd95ba 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated  
objects
  SYNOPSIS
  --------
  [verse]
-'git-push' [--all] [--tags] [--receive-pack=<git-receive-pack>]
+'git-push' [--all] [--dry-run] [--tags] [--receive-pack=<git-receive- 
pack>]
             [--repo=all] [-f | --force] [-v] [<repository>  
<refspec>...]

  DESCRIPTION
@@ -63,6 +63,10 @@ the remote repository.
  	Instead of naming each ref to push, specifies that all
  	refs under `$GIT_DIR/refs/heads/` be pushed.

+\--dry-run::
+	Show what would have been updated, but do not perform
+	any updates.
+
  \--tags::
  	All refs under `$GIT_DIR/refs/tags` are pushed, in
  	addition to refspecs explicitly listed on the command
diff --git a/builtin-push.c b/builtin-push.c
index 88c5024..141380b 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -7,9 +7,9 @@
  #include "builtin.h"
  #include "remote.h"

-static const char push_usage[] = "git-push [--all] [--tags] [-- 
receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v]  
[<repository> <refspec>...]";
+static const char push_usage[] = "git-push [--all] [--dry-run] [-- 
tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force]  
[-v] [<repository> <refspec>...]";

-static int all, force, thin, verbose;
+static int all, dry_run, force, thin, verbose;
  static const char *receivepack;

  static const char **refspec;
@@ -69,6 +69,8 @@ static int do_push(const char *repo)
  	argc = 1;
  	if (all)
  		argv[argc++] = "--all";
+	if (dry_run)
+		argv[argc++] = "--dry-run";
  	if (force)
  		argv[argc++] = "--force";
  	if (receivepack)
@@ -147,6 +149,10 @@ int cmd_push(int argc, const char **argv, const  
char *prefix)
  			all = 1;
  			continue;
  		}
+		if (!strcmp(arg, "--dry-run")) {
+			dry_run = 1;
+			continue;
+		}
  		if (!strcmp(arg, "--tags")) {
  			add_refspec("refs/tags/*");
  			continue;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ca46aaf..4fbd5b1 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -244,4 +244,14 @@ test_expect_success 'push with colon-less  
refspec (4)' '

  '

+test_expect_success 'push with dry-run' '
+
+	mk_test heads/master &&
+	cd testrepo &&
+	old_commit=$(git show-ref -s --verify refs/heads/master) &&
+	cd .. &&
+	git push --dry-run testrepo &&
+	check_push_result $old_commit heads/master
+'
+
  test_done
-- 
1.5.2.5

^ permalink raw reply related

* [PATCH 1/2] Add a --dry-run option to git-send-pack.
From: Brian Ewins @ 2007-10-10 23:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Signed-off-by: Brian Ewins <brian.ewins@gmail.com>
---
  Documentation/git-send-pack.txt |    5 ++++-
  send-pack.c                     |   31 +++++++++++++++++++------------
  2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send- 
pack.txt
index 3271e88..9e2783c 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -8,7 +8,7 @@ git-send-pack - Push objects over git protocol to  
another repository

  SYNOPSIS
  --------
-'git-send-pack' [--all] [--force] [--receive-pack=<git-receive- 
pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
+'git-send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git- 
receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]

  DESCRIPTION
  -----------
@@ -34,6 +34,9 @@ OPTIONS
  	Instead of explicitly specifying which refs to update,
  	update all heads that locally exist.

+\--dry-run::
+	Show what would have been updated, but do not send any updates.
+
  \--force::
  	Usually, the command refuses to update a remote ref that
  	is not an ancestor of the local ref used to overwrite it.
diff --git a/send-pack.c b/send-pack.c
index f74e66a..e8bef4f 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -7,13 +7,14 @@
  #include "remote.h"

  static const char send_pack_usage[] =
-"git-send-pack [--all] [--force] [--receive-pack=<git-receive-pack>]  
[--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git-send-pack [--all] [--dry-run] [--force] [--receive-pack=<git- 
receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
  "  --all and explicit <ref> specification are mutually exclusive.";
  static const char *receivepack = "git-receive-pack";
  static int verbose;
  static int send_all;
  static int force_update;
  static int use_thin_pack;
+static int dry_run;

  /*
   * Make a pack stream and spit it out into file descriptor fd
@@ -282,16 +283,18 @@ static int send_pack(int in, int out, struct  
remote *remote, int nr_refspec, cha
  		strcpy(old_hex, sha1_to_hex(ref->old_sha1));
  		new_hex = sha1_to_hex(ref->new_sha1);

-		if (ask_for_status_report) {
-			packet_write(out, "%s %s %s%c%s",
-				     old_hex, new_hex, ref->name, 0,
-				     "report-status");
-			ask_for_status_report = 0;
-			expect_status_report = 1;
+		if (!dry_run) {
+			if (ask_for_status_report) {
+				packet_write(out, "%s %s %s%c%s",
+				     	old_hex, new_hex, ref->name, 0,
+						"report-status");
+				ask_for_status_report = 0;
+				expect_status_report = 1;
+			}
+			else
+				packet_write(out, "%s %s %s",
+					old_hex, new_hex, ref->name);
  		}
-		else
-			packet_write(out, "%s %s %s",
-				     old_hex, new_hex, ref->name);
  		if (will_delete_ref)
  			fprintf(stderr, "deleting '%s'\n", ref->name);
  		else {
@@ -302,7 +305,7 @@ static int send_pack(int in, int out, struct  
remote *remote, int nr_refspec, cha
  			fprintf(stderr, "\n  from %s\n  to   %s\n",
  				old_hex, new_hex);
  		}
-		if (remote) {
+		if (remote && !dry_run) {
  			struct refspec rs;
  			rs.src = ref->name;
  			rs.dst = NULL;
@@ -321,7 +324,7 @@ static int send_pack(int in, int out, struct  
remote *remote, int nr_refspec, cha
  	}

  	packet_flush(out);
-	if (new_refs)
+	if (new_refs && !dry_run)
  		ret = pack_objects(out, remote_refs);
  	close(out);

@@ -390,6 +393,10 @@ int main(int argc, char **argv)
  				send_all = 1;
  				continue;
  			}
+			if (!strcmp(arg, "--dry-run")) {
+				dry_run = 1;
+				continue;
+			}
  			if (!strcmp(arg, "--force")) {
  				force_update = 1;
  				continue;
-- 
1.5.2.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox