Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Kyle Moffett @ 2008-10-31 11:38 UTC (permalink / raw)
  To: Stefan Karpinski; +Cc: Sam Vilain, git
In-Reply-To: <d4bc1a2a0810300355q42b35a35p2ba0e778691a0ab6@mail.gmail.com>

On Thu, Oct 30, 2008 at 6:55 AM, Stefan Karpinski
<stefan.karpinski@gmail.com> wrote:
> On Wed, Oct 29, 2008 at 8:48 PM, Sam Vilain <sam@vilain.net> wrote:
>> +  * 'git export' command that does what
>> +    'git archive --format=tar --prefix=dir | tar x' does now
>
> It would be nice if the "git export" command could "checkout" a
> non-repo copy of a remote repo at a specific version. This would be as
> simple as calling archive on the remote size and then unarchiving it
> locally. But would of course take care of all the plumbing.

I'm not sure whether the "git archive | tar" or the following is more efficient:

export GIT_INDEX_FILE="$(mktemp .git/export-index.XXXXXX)"
git read-tree -i "$1"
git checkout-index -f -a --prefix="$2/"

Cheers,
Kyle Moffett

^ permalink raw reply

* Re: why not TortoiseGit
From: Ian Hilt @ 2008-10-31 12:19 UTC (permalink / raw)
  To: Li Frank-B20596; +Cc: git
In-Reply-To: <7FD1F85C96D70C4A89DA1DF7667EAE96125890@zch01exm23.fsl.freescale.net>

On Fri, Oct 31, 2008 at 09:44:45AM +0800, Li Frank-B20596 wrote:
> There are TortoiseCVS, TortoiseSVN, TortoiseBzr, TortoiseHg
> Why not ToroiseGit

This is what Johannes Schindelin had to say,

	<http://code.google.com/p/msysgit/wiki/GitCheetah>

^ permalink raw reply

* Re: why not TortoiseGit
From: Andreas Ericsson @ 2008-10-31 12:35 UTC (permalink / raw)
  To: Ian Hilt; +Cc: Li Frank-B20596, git
In-Reply-To: <20081031121913.GE18221@sys-0.hiltweb.site>

Ian Hilt wrote:
> On Fri, Oct 31, 2008 at 09:44:45AM +0800, Li Frank-B20596 wrote:
>> There are TortoiseCVS, TortoiseSVN, TortoiseBzr, TortoiseHg
>> Why not ToroiseGit
> 
> This is what Johannes Schindelin had to say,
> 
> 	<http://code.google.com/p/msysgit/wiki/GitCheetah>

Noone's written TortoiseGit yet. I have no idea why, and I have
no reason to write it myself. If GitCheetah isn't working well,
I'm sure patches are welcome.

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

^ permalink raw reply

* [PATCH 3/3] git send-email: turn --compose on when more than one patch.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-3-git-send-email-madcoder@debian.org>

Automatically turn --compose on when there is more than one patch, and
that the output is a tty.

Do not print the list of files sent anymore in that case, as the list is
shown in the summary editor.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-send-email.perl |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5c189a7..5cebb40 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -49,7 +49,7 @@ git send-email [options] <file | directory | rev-list >
     --subject               <str>  * Email "Subject:"
     --in-reply-to           <str>  * Email "In-Reply-To:"
     --annotate                     * Review each patch that will be sent in an editor.
-    --compose                      * Open an editor for introduction.
+    --[no-]compose                 * Open an editor for introduction.
 
   Sending:
     --envelope-sender       <str>  * Email envelope sender.
@@ -237,7 +237,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-encryption=s" => \$smtp_encryption,
 		    "identity=s" => \$identity,
 		    "annotate" => \$annotate,
-		    "compose" => \$compose,
+		    "compose!" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
@@ -409,7 +409,11 @@ if ($validate) {
 }
 
 if (@files) {
-	unless ($quiet) {
+	if (!defined($compose) && -t STDOUT) {
+		# turn $compose on if there is more than one file
+		$compose = $#files;
+	}
+	unless ($quiet || $compose) {
 		print $_,"\n" for (@files);
 	}
 } else {
-- 
1.6.0.3.763.g0275.dirty

^ permalink raw reply related

* [PATCH 1/3] git send-email: make the message file name more specific.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-1-git-send-email-madcoder@debian.org>

This helps editors choosing their syntax hilighting properly.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 65c254d..4ca571f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -127,7 +127,7 @@ sub unique_email_list(@);
 sub cleanup_compose_files();
 
 # Constants (essentially)
-my $compose_filename = ".msg.$$";
+my $compose_filename = ".gitsendemail.msg.$$";
 
 # Variables we fill in automatically, or via prompting:
 my (@to,@cc,@initial_cc,@bcclist,@xh,
-- 
1.6.0.3.763.g0275.dirty

^ permalink raw reply related

* Further enhancement proposal for git-send-email
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
  To: git
In-Reply-To: <1225450632-7230-1-git-send-email-madcoder@debian.org>

Here is a three patch series (again).

[PATCH 1/3] git send-email: make the message file name more specific.
  -> quite independant, and should IMHO be taken.


[PATCH 2/3] git send-email: do not ask questions when --compose is used.
[PATCH 3/3] git send-email: turn --compose on when more than one patch.

  Those two patches enhance git-send-email by making ask less questions
  when --compose is used (as it can grab the subject, from and reply-to
  from the buffer).

  It also turns --compose on by default as soon as there is more than
  one patch, as I believe than commenting a patch series is more often
  done than not. It's is really trivial to "refuse" to comment the
  series by just erasing the full buffer content, which should not
  really be too anoying (or one can explicitely pass --no-compose for
  the same result).

  It's probable that those two changes may trigger some discussion
  though, but I just used that to send this series, and I can tell with
  this git-send-email is nearer what I would like it to be.

^ permalink raw reply

* [PATCH 2/3] git send-email: do not ask questions when --compose is used.
From: Pierre Habouzit @ 2008-10-31 12:36 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit
In-Reply-To: <1225456609-694-2-git-send-email-madcoder@debian.org>

When --compose is used, we can grab the From/Subject/In-Reply-To from the
edited summary, let it be so and don't ask the user silly questions.

The summary templates gets quite revamped, and includes the list of
patches subjects that are going to be sent with this batch.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-send-email.perl |  174 ++++++++++++++++++++++++++++++---------------------
 1 files changed, 102 insertions(+), 72 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 4ca571f..5c189a7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -417,6 +417,105 @@ if (@files) {
 	usage();
 }
 
+sub get_patch_subject($) {
+	my $fn = shift;
+	open (my $fh, '<', $fn);
+	while (my $line = <$fh>) {
+		next unless ($line =~ /^Subject: (.*)$/);
+		close $fh;
+		return "GIT: $1\n";
+	}
+	close $fh;
+	die "No subject line in $fn ?";
+}
+
+if ($compose) {
+	# Note that this does not need to be secure, but we will make a small
+	# effort to have it be unique
+	open(C,">",$compose_filename)
+		or die "Failed to open for writing $compose_filename: $!";
+
+
+	my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
+	my $tpl_subject = $initial_subject || '';
+	my $tpl_reply_to = $initial_reply_to || '';
+
+	print C <<EOT;
+From $tpl_sender # This line is ignored.
+GIT: Lines beginning in "GIT: " will be removed.
+GIT: Consider including an overall diffstat or table of contents
+GIT: for the patch you are writing.
+From: $tpl_sender
+Subject: $tpl_subject
+In-Reply-To: $tpl_reply_to
+
+GIT: Please enter your email below this line.
+
+EOT
+	for my $f (@files) {
+		print C get_patch_subject($f);
+	}
+	close(C);
+
+	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+
+	if ($annotate) {
+		do_edit($compose_filename, @files);
+	} else {
+		do_edit($compose_filename);
+	}
+
+	open(C2,">",$compose_filename . ".final")
+		or die "Failed to open $compose_filename.final : " . $!;
+
+	open(C,"<",$compose_filename)
+		or die "Failed to open $compose_filename : " . $!;
+
+	my $need_8bit_cte = file_has_nonascii($compose_filename);
+	my $in_body = 0;
+	my $summary_empty = 1;
+	while(<C>) {
+		next if m/^GIT: /;
+		if ($in_body) {
+		} elsif (/^\n$/) {
+			$in_body = 1;
+			if ($need_8bit_cte) {
+				print C2 "MIME-Version: 1.0\n",
+					 "Content-Type: text/plain; ",
+					   "charset=utf-8\n",
+					 "Content-Transfer-Encoding: 8bit\n";
+			}
+		} elsif (/^MIME-Version:/i) {
+			$need_8bit_cte = 0;
+		} elsif (/^Subject:\s*(.+)\s*$/i) {
+			$initial_subject = $1;
+			my $subject = $initial_subject;
+			$_ = "Subject: " .
+				($subject =~ /[^[:ascii:]]/ ?
+				 quote_rfc2047($subject) :
+				 $subject) .
+				"\n";
+		} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
+			$initial_reply_to = $1;
+			next;
+		} elsif (/^From:\s*(.+)\s*$/i) {
+			$sender = $1;
+			next;
+		}
+		$summary_empty = 0;
+		print C2 $_;
+	}
+	close(C);
+	close(C2);
+
+	if ($summary_empty) {
+		print "Summary email is empty, skpping it\n";
+		$compose = -1;
+	}
+} elsif ($annotate) {
+	do_edit(@files);
+}
+
 my $prompting = 0;
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
@@ -461,17 +560,6 @@ sub expand_aliases {
 @initial_cc = expand_aliases(@initial_cc);
 @bcclist = expand_aliases(@bcclist);
 
-if (!defined $initial_subject && $compose) {
-	while (1) {
-		$_ = $term->readline("What subject should the initial email start with? ", $initial_subject);
-		last if defined $_;
-		print "\n";
-	}
-
-	$initial_subject = $_;
-	$prompting++;
-}
-
 if ($thread && !defined $initial_reply_to && $prompting) {
 	while (1) {
 		$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ", $initial_reply_to);
@@ -498,64 +586,6 @@ if (!defined $smtp_server) {
 }
 
 if ($compose) {
-	# Note that this does not need to be secure, but we will make a small
-	# effort to have it be unique
-	open(C,">",$compose_filename)
-		or die "Failed to open for writing $compose_filename: $!";
-	print C "From $sender # This line is ignored.\n";
-	printf C "Subject: %s\n\n", $initial_subject;
-	printf C <<EOT;
-GIT: Please enter your email below.
-GIT: Lines beginning in "GIT: " will be removed.
-GIT: Consider including an overall diffstat or table of contents
-GIT: for the patch you are writing.
-
-EOT
-	close(C);
-
-	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
-
-	if ($annotate) {
-		do_edit($compose_filename, @files);
-	} else {
-		do_edit($compose_filename);
-	}
-
-	open(C2,">",$compose_filename . ".final")
-		or die "Failed to open $compose_filename.final : " . $!;
-
-	open(C,"<",$compose_filename)
-		or die "Failed to open $compose_filename : " . $!;
-
-	my $need_8bit_cte = file_has_nonascii($compose_filename);
-	my $in_body = 0;
-	while(<C>) {
-		next if m/^GIT: /;
-		if (!$in_body && /^\n$/) {
-			$in_body = 1;
-			if ($need_8bit_cte) {
-				print C2 "MIME-Version: 1.0\n",
-					 "Content-Type: text/plain; ",
-					   "charset=utf-8\n",
-					 "Content-Transfer-Encoding: 8bit\n";
-			}
-		}
-		if (!$in_body && /^MIME-Version:/i) {
-			$need_8bit_cte = 0;
-		}
-		if (!$in_body && /^Subject: ?(.*)/i) {
-			my $subject = $1;
-			$_ = "Subject: " .
-				($subject =~ /[^[:ascii:]]/ ?
-				 quote_rfc2047($subject) :
-				 $subject) .
-				"\n";
-		}
-		print C2 $_;
-	}
-	close(C);
-	close(C2);
-
 	while (1) {
 		$_ = $term->readline("Send this email? (y|n) ");
 		last if defined $_;
@@ -567,9 +597,9 @@ EOT
 		exit(0);
 	}
 
-	@files = ($compose_filename . ".final", @files);
-} elsif ($annotate) {
-	do_edit(@files);
+	if ($compose > 0) {
+		@files = ($compose_filename . ".final", @files);
+	}
 }
 
 # Variables we set as part of the loop over files
-- 
1.6.0.3.763.g0275.dirty

^ permalink raw reply related

* Re: Are binary xdeltas only used if you use git-gc?
From: Matthieu Moy @ 2008-10-31 12:42 UTC (permalink / raw)
  To: Thanassis Tsiodras; +Cc: git
In-Reply-To: <f1d2d9ca0810310243r669840bbj2c5ee7183e0caaed@mail.gmail.com>

"Thanassis Tsiodras" <ttsiodras@gmail.com> writes:

> If that is the case, I will create an alias to always git-gc after commits...

If you have a decent version of git, it already does "git gc --auto"
regularly. With --auto, git gc will do nothing if you don't have too
many unpacked objects, and will try to do the right thing otherwise
(incremental packs, see man git gc). The idea is that "git gc" is a
costly operation, and git prefers to waste a bit of disk space to make
most "commit" really fast, and to take time to optimize the repository
only when it grew too much.

If you're worried about repository size and you have a permanently
running machine, a good idea is to run git gc in a cron job, so that
you work fast in daytime, and your computer optimizes hard at night
time ;-) (I have gic gc + git fsck in a cron job, so I'll also know if
a repository gets corrupted).

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH] Use find instead of perl in t5000 to get file modification time
From: Peter Harris @ 2008-10-31 12:59 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Sam Vilain, Git Mailing List, Junio C Hamano, Jeff King,
	René Scharfe
In-Reply-To: <20081031070003.GA4458@blimp.localdomain>

On Fri, Oct 31, 2008 at 3:00 AM, Alex Riesen wrote:
> Sam Vilain, Thu, Oct 30, 2008 06:29:14 +0100:
>> On Wed, 2008-10-29 at 11:38 +0100, Alex Riesen wrote:
>> > I could not find what exactly does the ActiveState's Perl use for its stat
>> > implementation (and honestly, have no motivation to look harder).
>> > It seems to honor TZ, but the produced time does not seem to be either
>> > local or GMT.
>>
>> See, the difference is that the perl is portable and your patch isn't.
>
> ActiveState Perl on Windows is portable? To another windows, maybe.

Quite aside from missing the point (which is that Vanilla Perl runs
everywhere, including Windows[1]), this is also factually incorrect. A
quick check of the ActiveState website would reveal ActivePerl
downloads for AIX, Linux (x86 and x86-64), Solaris (x86, SPARC, and
SPARC64), MacOSX (x86 and PPC), and source code, in addition to
Windows.

Looks pretty portable to me.

Peter Harris

[1] http://vanillaperl.com/

^ permalink raw reply

* Re: [PATCH] Use find instead of perl in t5000 to get file modification time
From: Alex Riesen @ 2008-10-31 13:45 UTC (permalink / raw)
  To: Peter Harris
  Cc: Sam Vilain, Git Mailing List, Junio C Hamano, Jeff King,
	René Scharfe
In-Reply-To: <eaa105840810310559l29addd80i7a22c21e37231bb1@mail.gmail.com>

2008/10/31 Peter Harris <git@peter.is-a-geek.org>:
> On Fri, Oct 31, 2008 at 3:00 AM, Alex Riesen wrote:
>> Sam Vilain, Thu, Oct 30, 2008 06:29:14 +0100:
>>> On Wed, 2008-10-29 at 11:38 +0100, Alex Riesen wrote:
>>> > I could not find what exactly does the ActiveState's Perl use for its stat
>>> > implementation (and honestly, have no motivation to look harder).
>>> > It seems to honor TZ, but the produced time does not seem to be either
>>> > local or GMT.
>>>
>>> See, the difference is that the perl is portable and your patch isn't.
>>
>> ActiveState Perl on Windows is portable? To another windows, maybe.
>
> Quite aside from missing the point (which is that Vanilla Perl runs
> everywhere, including Windows[1]), this is also factually incorrect.

Ok, it is (almost: "Vanilla Perl is experimental and is not intended for
production purposes". Well, I need it exactly for production purposes!)

> A quick check of the ActiveState website would reveal ActivePerl
> downloads for AIX, Linux (x86 and x86-64), Solaris (x86, SPARC, and
> SPARC64), MacOSX (x86 and PPC), and source code, in addition to
> Windows.

what's the point of that, I wonder... Just to admit they broke the original
source beyond all repair on one platform (windows) so now they
have provide their poor customers with the same broken version
on all the other platforms, because they complained about incompatibilities.

So Perl is portable. ActiveState Perl may look portable. They just don't
seem to be compatible enough. Even then, the Git's test suite and some
scripts (I have no use for git-svn on Windows) do very good job to be usable
even if the portable dependency replaced with something else. I'm trying to
keep it at that because I personally have no choice.

^ permalink raw reply

* Re: Are binary xdeltas only used if you use git-gc?
From: Thanassis Tsiodras @ 2008-10-31 14:22 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqy705rl5u.fsf@bauges.imag.fr>

Actually, I am not so worried about disk size - I am far more worried
about how long it takes to git-push over my PSTN modem connection.

I'll try Jakub's suggestion (git-gc on both my machine and the remote
machine hosting the repos) and report back.

On Fri, Oct 31, 2008 at 2:42 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> If you're worried about repository size and you have a permanently
> running machine, a good idea is to run git gc in a cron job, so that
> you work fast in daytime, and your computer optimizes hard at night
> time ;-) (I have gic gc + git fsck in a cron job, so I'll also know if
> a repository gets corrupted).
>
> --
> Matthieu
>



-- 
What I gave, I have; what I spent, I had; what I kept, I lost. -Old Epitaph

^ permalink raw reply

* Re: [PATCH] Introduce receive.denyDeletes
From: Shawn O. Pearce @ 2008-10-31 14:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jan Krrrger, git
In-Reply-To: <7v63n99omx.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Jan Krüger" <jk@jk.gs> writes:
> 
> > Can I then delete the branch afterwards without lots of juggling (in
> > case the test fails due to a random other reason that the branch
> > accidentally getting deleted by receive-pack)? I'd expect I'd have to
> > save the exit code to a temporary variable and that's just as ugly.

If you want to delete the branch after the test is done, do it
outside of the test_expect_success's 3rd argument.  Then it will
run the branch deletion whether or not the test was successful.
 
> Although I agree that your attempt to allow the test continue even when
> this test fails is a very good practice, I personally do not find the
> alternative you mention ugly at all.  I actually find that "return 1"
> uglier because it feels like it knows too much about how
> test_expect_success is implemented.

Yea, I also found the "return 1" to be horribly difficult to read, and
knowing far too much about the test suite.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Avoid using non-portable `echo -n` in tests.
From: Shawn O. Pearce @ 2008-10-31 14:32 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List
In-Reply-To: <1225429753-70109-1-git-send-email-benji@silverinsanity.com>

Brian Gernhardt <benji@silverinsanity.com> wrote:
> Not all /bin/sh have a builtin echo that recognizes -n.  Using printf
> is far more portable.
 
> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index c1850d2..f6a2dbd 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -424,7 +424,7 @@ cd "$WORKDIR"
>  test_expect_success 'cvs update (-p)' '
>      touch really-empty &&
>      echo Line 1 > no-lf &&
> -    echo -n Line 2 >> no-lf &&
> +    printf Line 2 >> no-lf &&

That needs to be:

	printf 'Line 2'

to have the same result.  Fortunately I don't think it matters
in this test, but it does read odd.

-- 
Shawn.

^ permalink raw reply

* Re: jgit as a jira plugin
From: Shawn O. Pearce @ 2008-10-31 14:42 UTC (permalink / raw)
  To: J. Longman; +Cc: git
In-Reply-To: <C9B1B0D7-6F99-48DE-8593-A13F1187ADE2@xiplink.com>

"J. Longman" <longman@xiplink.com> wrote:
> Right, I see now, I thought there was going to be more detail.  Anyways, 
> I'm wondering if there is some action required to ensure it reads from 
> the database vs. the working directory?  I'm assuming it is still looking 
> at the workspace as this is what I'm seeing:
>
> ...RevisionIndexer] Latest indexed revision for repository=1 is :  
> 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503
> ...GitManagerImpl] Fetch...
> ...GitManagerImpl] From /Users/longman/workspace2/work/../masterRepo/
> ...GitManagerImpl]    c209b0f..594e8ff  master     -> origin/master
> ...GitManagerImpl] scan for repo changes...  
> repository.scanForRepoChanges();
> ...GitManagerImpl] scan for repo changes... complete
> ...RevisionIndexer] testing 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503 at 
> CORE-23 sdjskl
> ...RevisionIndexer] 	update latest as  
> 29ed4398d047ba5e0d6fbad9ebbf98304b0fc503 at Wed Oct 29 18:49:54 EDT 2008

Oh.

Your RevisionIndexer must be looking at HEAD, which is the current
branch data.  However the fetch process updated the remote tracking
refs, which are in a different namespace.

You should point your indexer at "origin/master", or use a bare
repository (one with no working directory) and fetch directly into
"refs/heads/*" instead of into "refs/remotes/origin/*", that way
the indexer can look at branch "master".

Fetch doesn't ever touch the working directory; it only updates
the database in the background and the refs/remotes/ namespace so
that other applications can see the data that was transferred and
choose how to process it.

In command line Git "git pull" uses "git fetch" to get the data
and then uses the received data to update the working directory.
But a lot of workflows also will just issue "git fetch" on their own
to get the data, then examine it with "gitk --all", or do nothing
at all because they are mirroring the data, or have to close their
laptop and catch a bus, etc...

If all you are doing is scanning the revision history and the
files via JGit you just have to run a Transport.fetch() call and
then look at the refs that were updated during that call.  See the
FetchResult object you get returned for the list; it is what the
pgm.Fetch class uses to display the output cited above.

-- 
Shawn.

^ permalink raw reply

* Re: [JGIT PATCH 0/3] Improved object validation
From: Shawn O. Pearce @ 2008-10-31 14:55 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810310101.12241.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> torsdagen den 30 oktober 2008 18.46.22 skrev Shawn O. Pearce:
> > This is mostly a resend as I haven't heard anything on the series.
> > One new patch at the end, to handle '.' and '..' cases.
> Ack, pushed now.  I never got to the push action, because I wanted
> to do some testing on the bundlewriter (which you did not resubmit) and
> the I went to a warmer place for a few days.
> 
> These patches and the bundlewrite + a small unit test for it now pushed.

Thanks for putting together the bundle writer test.  I had to fix
it a bit to avoid the /home/me path it had hardcoded in there.
This patch was applied on top of master and pushed out.

I meant to work on a test for BundleWriter myself, but I got too busy
with other stuff and totally forgot about it.  I wasn't pushing to
get the BundleWriter included as it was non-critical to my current
needs, but I had it written so I posted it in case someone else
cared about it.

--8<--
[PATCH] Fix BundleWriter unit test to work for people who are not 'me'

My system lacks a /home/me as I do not call myself me.  My login is
usually based upon my email address, and nobody else on my system is
'me'.  So we cannot use hardcoded paths like /home/me in a test case.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/transport/BundleWriterTest.java   |   68 ++++++++++----------
 1 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
index 4e33108..3cfb8b1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/transport/BundleWriterTest.java
@@ -37,11 +37,13 @@
 
 package org.spearce.jgit.transport;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.util.Collections;
+import java.util.Set;
 
 import org.spearce.jgit.errors.MissingBundlePrerequisiteException;
 import org.spearce.jgit.errors.NotSupportedException;
@@ -58,15 +60,14 @@
 
 	public void testWrite0() throws Exception {
 		// Create a tiny bundle, (well one of) the first commits only
-		URIish bundleURI = new URIish("file:///home/me/tmp/foo");
-		makeBundle("refs/heads/firstcommit",
-				"42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", bundleURI, null);
+		final byte[] bundle = makeBundle("refs/heads/firstcommit",
+				"42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", null);
 
 		// Then we clone a new repo from that bundle and do a simple test. This
 		// makes sure
 		// we could read the bundle we created.
 		Repository newRepo = createNewEmptyRepo();
-		FetchResult fetchResult = fetchFromBundle(newRepo, bundleURI);
+		FetchResult fetchResult = fetchFromBundle(newRepo, bundle);
 		Ref advertisedRef = fetchResult
 				.getAdvertisedRef("refs/heads/firstcommit");
 
@@ -80,19 +81,20 @@ assertEquals("42e4e7c5e507e113ebbb7801b16b52cf867b7ce1", newRepo
 
 	/**
 	 * Incremental bundle test
-	 *
+	 * 
 	 * @throws Exception
 	 */
 	public void testWrite1() throws Exception {
+		byte[] bundle;
+
 		// Create a small bundle, an early commit
-		URIish bundleURI = new URIish("file:///home/me/tmp/foo");
-		makeBundle("refs/heads/aa", db.resolve("a").name(), bundleURI, null);
+		bundle = makeBundle("refs/heads/aa", db.resolve("a").name(), null);
 
 		// Then we clone a new repo from that bundle and do a simple test. This
 		// makes sure
 		// we could read the bundle we created.
 		Repository newRepo = createNewEmptyRepo();
-		FetchResult fetchResult = fetchFromBundle(newRepo, bundleURI);
+		FetchResult fetchResult = fetchFromBundle(newRepo, bundle);
 		Ref advertisedRef = fetchResult.getAdvertisedRef("refs/heads/aa");
 
 		assertEquals(db.resolve("a").name(), advertisedRef.getObjectId().name());
@@ -101,9 +103,9 @@ assertEquals(db.resolve("a").name(), newRepo.resolve("refs/heads/aa")
 		assertNull(newRepo.resolve("refs/heads/a"));
 
 		// Next an incremental bundle
-		makeBundle("refs/heads/cc", db.resolve("c").name(), bundleURI,
+		bundle = makeBundle("refs/heads/cc", db.resolve("c").name(),
 				new RevWalk(db).parseCommit(db.resolve("a").toObjectId()));
-		fetchResult = fetchFromBundle(newRepo, bundleURI);
+		fetchResult = fetchFromBundle(newRepo, bundle);
 		advertisedRef = fetchResult.getAdvertisedRef("refs/heads/cc");
 		assertEquals(db.resolve("c").name(), advertisedRef.getObjectId().name());
 		assertEquals(db.resolve("c").name(), newRepo.resolve("refs/heads/cc")
@@ -114,7 +116,7 @@ assertEquals(db.resolve("c").name(), newRepo.resolve("refs/heads/cc")
 		try {
 			// Check that we actually needed the first bundle
 			Repository newRepo2 = createNewEmptyRepo();
-			fetchResult = fetchFromBundle(newRepo2, bundleURI);
+			fetchResult = fetchFromBundle(newRepo2, bundle);
 			fail("We should not be able to fetch from bundle with prerequisistes that are not fulfilled");
 		} catch (MissingBundlePrerequisiteException e) {
 			assertTrue(e.getMessage()
@@ -122,31 +124,29 @@ assertTrue(e.getMessage()
 		}
 	}
 
-	private FetchResult fetchFromBundle(Repository newRepo, URIish uriish)
-			throws URISyntaxException, NotSupportedException,
-			TransportException {
-		RemoteConfig remoteConfig = new RemoteConfig(newRepo.getConfig(),
-				"origin");
-		remoteConfig.addURI(uriish);
-		RefSpec theRefSpec = new RefSpec("refs/heads/*:refs/heads/*");
-		remoteConfig.addFetchRefSpec(theRefSpec);
-		Transport transport = Transport.open(newRepo, remoteConfig);
-		FetchResult fetch = transport.fetch(new NullProgressMonitor(),
-				Collections.singleton(theRefSpec));
-		return fetch;
+	private FetchResult fetchFromBundle(final Repository newRepo,
+			final byte[] bundle) throws URISyntaxException,
+			NotSupportedException, TransportException {
+		final URIish uri = new URIish("in-memory://");
+		final ByteArrayInputStream in = new ByteArrayInputStream(bundle);
+		final RefSpec rs = new RefSpec("refs/heads/*:refs/heads/*");
+		final Set<RefSpec> refs = Collections.singleton(rs);
+		return new TransportBundleStream(newRepo, uri, in).fetch(
+				NullProgressMonitor.INSTANCE, refs);
 	}
 
-	private void makeBundle(String name, String anObjectToInclude,
-			URIish bundleName, RevCommit assume) throws FileNotFoundException,
-			IOException {
-		BundleWriter bundleWriter = new BundleWriter(db,
-				new NullProgressMonitor());
-		bundleWriter.include(name, ObjectId.fromString(anObjectToInclude));
+	private byte[] makeBundle(final String name,
+			final String anObjectToInclude, final RevCommit assume)
+			throws FileNotFoundException, IOException {
+		final BundleWriter bw;
+
+		bw = new BundleWriter(db, NullProgressMonitor.INSTANCE);
+		bw.include(name, ObjectId.fromString(anObjectToInclude));
 		if (assume != null)
-			bundleWriter.assume(assume);
-		FileOutputStream os = new FileOutputStream(bundleName.getPath());
-		bundleWriter.writeBundle(os);
-		os.close();
+			bw.assume(assume);
+		final ByteArrayOutputStream out = new ByteArrayOutputStream();
+		bw.writeBundle(out);
+		return out.toByteArray();
 	}
 
 }
-- 
1.6.0.3.756.gb776d

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH 1/9] close another possibility for propagating pack corruption
From: Nicolas Pitre @ 2008-10-31 15:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vod11884o.fsf@gitster.siamese.dyndns.org>

On Fri, 31 Oct 2008, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > Abstract
> > --------
> >
> > With index v2 we have a per object CRC to allow quick and safe reuse of
> > pack data when repacking.  this, however, doesn't currently prevent a
> 
> s/this/This/;

Thanks for proofreading.  ;-)

> Very nicely done.  I've never seen a commit message that needs its own
> Abstract ;-)

Well, maybe I could run for the best commit message award!  :-)

> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> > index 59c30d1..0366277 100644
> > --- a/builtin-pack-objects.c
> > +++ b/builtin-pack-objects.c
> > @@ -1689,6 +1689,8 @@ static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, vo
> >  	return 0;
> >  }
> >  
> > +extern int do_check_packed_object_crc;
> > +
> 
> This ought to belong to cache.h or some other header file.  Perhaps you
> did this to avoid unnecessary recompilation (we've discussed this at
> GitTogether)?

No, I just felt this wasn't a really public thing to expose at large.
But I don't feel really strongly about that.

Revised patch follows.

--------
From: Nicolas Pitre <nico@cam.org>
Date: Tue, 28 Oct 2008 20:58:42 -0400
Subject: [PATCH] close another possibility for propagating pack corruption

Abstract
--------

With index v2 we have a per object CRC to allow quick and safe reuse of
pack data when repacking.  This, however, doesn't currently prevent a
stealth corruption from being propagated into a new pack when _not_
reusing pack data as demonstrated by the modification to t5302 included
here.

The Context
-----------

The Git database is all checksummed with SHA1 hashes.  Any kind of
corruption can be confirmed by verifying this per object hash against
corresponding data.  However this can be costly to perform systematically
and therefore this check is often not performed at run time when
accessing the object database.

First, the loose object format is entirely compressed with zlib which
already provide a CRC verification of its own when inflating data.  Any
disk corruption would be caught already in this case.

Then, packed objects are also compressed with zlib but only for their
actual payload.  The object headers and delta base references are not
deflated for obvious performance reasons, however this leave them
vulnerable to potentially undetected disk corruptions.  Object types
are often validated against the expected type when they're requested,
and deflated size must always match the size recorded in the object header,
so those cases are pretty much covered as well.

Where corruptions could go unnoticed is in the delta base reference.
Of course, in the OBJ_REF_DELTA case,  the odds for a SHA1 reference to
get corrupted so it actually matches the SHA1 of another object with the
same size (the delta header stores the expected size of the base object
to apply against) are virtually zero.  In the OBJ_OFS_DELTA case, the
reference is a pack offset which would have to match the start boundary
of a different base object but still with the same size, and although this
is relatively much more "probable" than in the OBJ_REF_DELTA case, the
probability is also about zero in absolute terms.  Still, the possibility
exists as demonstrated in t5302 and is certainly greater than a SHA1
collision, especially in the OBJ_OFS_DELTA case which is now the default
when repacking.

Again, repacking by reusing existing pack data is OK since the per object
CRC provided by index v2 guards against any such corruptions. What t5302
failed to test is a full repack in such case.

The Solution
------------

As unlikely as this kind of stealth corruption can be in practice, it
certainly isn't acceptable to propagate it into a freshly created pack.
But, because this is so unlikely, we don't want to pay the run time cost
associated with extra validation checks all the time either.  Furthermore,
consequences of such corruption in anything but repacking should be rather
visible, and even if it could be quite unpleasant, it still has far less
severe consequences than actively creating bad packs.

So the best compromize is to check packed object CRC when unpacking
objects, and only during the compression/writing phase of a repack, and
only when not streaming the result.  The cost of this is minimal (less
than 1% CPU time), and visible only with a full repack.

Someone with a stats background could provide an objective evaluation of
this, but I suspect that it's bad RAM that has more potential for data
corruptions at this point, even in those cases where this extra check
is not performed.  Still, it is best to prevent a known hole for
corruption when recreating object data into a new pack.

What about the streamed pack case?  Well, any client receiving a pack
must always consider that pack as untrusty and perform full validation
anyway, hence no such stealth corruption could be propagated to remote
repositoryes already.  It is therefore worthless doing local validation
in that case.

Signed-off-by: Nicolas Pitre <nico@cam.org>

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 59c30d1..026b7ea 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1697,6 +1697,16 @@ static void prepare_pack(int window, int depth)
 
 	get_object_details();
 
+	/*
+	 * If we're locally repacking then we need to be doubly careful
+	 * from now on in order to make sure no stealth corruption gets
+	 * propagated to the new pack.  Clients receiving streamed packs
+	 * should validate everything they get anyway so no need to incur
+	 * the additional cost here in that case.
+	 */
+	if (!pack_to_stdout)
+		do_check_packed_object_crc = 1;
+
 	if (!nr_objects || !window || !depth)
 		return;
 
diff --git a/cache.h b/cache.h
index a3c77f0..0dc13cc 100644
--- a/cache.h
+++ b/cache.h
@@ -565,6 +565,9 @@ extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 /* just like read_sha1_file(), but non fatal in presence of bad objects */
 extern void *read_object(const unsigned char *sha1, enum object_type *type, unsigned long *size);
 
+/* global flag to enable extra checks when accessing packed objects */
+extern int do_check_packed_object_crc;
+
 extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned long size, const char *type);
 
 extern int move_temp_to_file(const char *tmpfile, const char *filename);
diff --git a/sha1_file.c b/sha1_file.c
index ab2b520..88d9cf3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1694,6 +1694,8 @@ static void *unpack_delta_entry(struct packed_git *p,
 	return result;
 }
 
+int do_check_packed_object_crc;
+
 void *unpack_entry(struct packed_git *p, off_t obj_offset,
 		   enum object_type *type, unsigned long *sizep)
 {
@@ -1701,6 +1703,19 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 	off_t curpos = obj_offset;
 	void *data;
 
+	if (do_check_packed_object_crc && p->index_version > 1) {
+		struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
+		unsigned long len = revidx[1].offset - obj_offset;
+		if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
+			const unsigned char *sha1 =
+				nth_packed_object_sha1(p, revidx->nr);
+			error("bad packed object CRC for %s",
+			      sha1_to_hex(sha1));
+			mark_bad_packed_object(p, sha1);
+			return NULL;
+		}
+	}
+
 	*type = unpack_object_header(p, &w_curs, &curpos, sizep);
 	switch (*type) {
 	case OBJ_OFS_DELTA:
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index b0b0fda..884e242 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -196,7 +196,8 @@ test_expect_success \
 
 test_expect_success \
     '[index v2] 5) pack-objects refuses to reuse corrupted data' \
-    'test_must_fail git pack-objects test-5 <obj-list'
+    'test_must_fail git pack-objects test-5 <obj-list &&
+     test_must_fail git pack-objects --no-reuse-object test-6 <obj-list'
 
 test_expect_success \
     '[index v2] 6) verify-pack detects CRC mismatch' \

^ permalink raw reply related

* Re: [PATCH] Avoid using non-portable `echo -n` in tests.
From: Brian Gernhardt @ 2008-10-31 15:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Shawn O Pearce
In-Reply-To: <7vy70589jc.fsf@gitster.siamese.dyndns.org>


On Oct 31, 2008, at 4:15 AM, Junio C Hamano wrote:

> Brian Gernhardt <benji@silverinsanity.com> writes:
>
>> Not all /bin/sh have a builtin echo that recognizes -n.  Using printf
>> is far more portable.
>
> This is much better (minor nit: we do not care if echo is built-in or
> /bin/echo); the point of t4030 is to produce an incomplete line ;-).

Well, yes.  The reason I phrased it that way was that my /bin/echo  
recognizes -n but /bin/sh does not.

~~ Brian

^ permalink raw reply

* [PATCH] prepare deprecation of git-revert
From: Pierre Habouzit @ 2008-10-31 15:55 UTC (permalink / raw)
  To: git; +Cc: Pierre Habouzit

* Rename builtin-revert.c into builtin-cherry-pick.c

* Add option -R/--revert to git-cherry-pick.
  Document it by taking the current content of git-revert manpage for the
  option.

* get rid of the no_replay initialization, just ignore it when we're in
  the revert case, it makes really no sense to error out.

* put the warning of deprecation in cmd_revert, #if 0-ed out for now.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

 I've not kept the auto-edit feature of git-revert for the git-cherry-pick -R
 case as I don't believe it makes a lot of sense. But if people are unhappy
 with that, I can easily "fix" it.

 Documentation/git-cherry-pick.txt         |   15 +++++++++++++++
 Makefile                                  |    6 +++---
 builtin-revert.c => builtin-cherry-pick.c |   10 ++++------
 3 files changed, 22 insertions(+), 9 deletions(-)
 rename builtin-revert.c => builtin-cherry-pick.c (98%)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 837fb08..2d92f2d 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -40,6 +40,21 @@ OPTIONS
 	development branch), adding this information can be
 	useful.
 
+-R::
+--revert::
+	Given one existing commit, revert the change the patch introduces, and
+	record a new commit that records it.  This requires your working tree
+	to be clean (no modifications from the HEAD commit).
++
+Note: 'git revert' is used to record a new commit to reverse the
+effect of an earlier commit (often a faulty one).  If you want to
+throw away all uncommitted changes in your working directory, you
+should see linkgit:git-reset[1], particularly the '--hard' option.  If
+you want to extract specific files as they were in another commit, you
+should see linkgit:git-checkout[1], specifically the 'git checkout
+<commit> -- <filename>' syntax.  Take care with these alternatives as
+both will discard uncommitted changes in your working directory.
+
 -r::
 	It used to be that the command defaulted to do `-x`
 	described above, and `-r` was to disable it.  Now the
diff --git a/Makefile b/Makefile
index d6f3695..43eb8e4 100644
--- a/Makefile
+++ b/Makefile
@@ -306,7 +306,7 @@ PROGRAMS += git-var$X
 # builtin-$C.o but is linked in as part of some other command.
 BUILT_INS += $(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS))
 
-BUILT_INS += git-cherry-pick$X
+BUILT_INS += git-revert$X
 BUILT_INS += git-cherry$X
 BUILT_INS += git-format-patch$X
 BUILT_INS += git-fsck-objects$X
@@ -508,6 +508,7 @@ BUILTIN_OBJS += builtin-check-attr.o
 BUILTIN_OBJS += builtin-check-ref-format.o
 BUILTIN_OBJS += builtin-checkout-index.o
 BUILTIN_OBJS += builtin-checkout.o
+BUILTIN_OBJS += builtin-cherry-pick.o
 BUILTIN_OBJS += builtin-clean.o
 BUILTIN_OBJS += builtin-clone.o
 BUILTIN_OBJS += builtin-commit-tree.o
@@ -556,7 +557,6 @@ BUILTIN_OBJS += builtin-rerere.o
 BUILTIN_OBJS += builtin-reset.o
 BUILTIN_OBJS += builtin-rev-list.o
 BUILTIN_OBJS += builtin-rev-parse.o
-BUILTIN_OBJS += builtin-revert.o
 BUILTIN_OBJS += builtin-rm.o
 BUILTIN_OBJS += builtin-send-pack.o
 BUILTIN_OBJS += builtin-shortlog.o
@@ -1261,7 +1261,7 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.h
+builtin-cherry-pick.o wt-status.o: wt-status.h
 
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
diff --git a/builtin-revert.c b/builtin-cherry-pick.c
similarity index 98%
rename from builtin-revert.c
rename to builtin-cherry-pick.c
index 4038b41..d1a7188 100644
--- a/builtin-revert.c
+++ b/builtin-cherry-pick.c
@@ -57,6 +57,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
+		OPT_SET_INT('R', "revert", &action, "cherry-pick a reverted patch", REVERT),
 		OPT_END(),
 	};
 
@@ -261,10 +262,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	setenv(GIT_REFLOG_ACTION, me, 0);
 	parse_args(argc, argv);
 
-	/* this is copied from the shell script, but it's never triggered... */
-	if (action == REVERT && !no_replay)
-		die("revert is incompatible with replay");
-
 	if (read_cache() < 0)
 		die("git %s: failed to read the index", me);
 	if (no_commit) {
@@ -439,16 +436,17 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
+#if 0
+	warning("git revert is deprecated, please use git cherry-pick --revert/-R instead");
+#endif
 	if (isatty(0))
 		edit = 1;
-	no_replay = 1;
 	action = REVERT;
 	return revert_or_cherry_pick(argc, argv);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-	no_replay = 0;
 	action = CHERRY_PICK;
 	return revert_or_cherry_pick(argc, argv);
 }
-- 
1.6.0.3.790.ga4dd7.dirty

^ permalink raw reply related

* Re: [PATCH] prepare deprecation of git-revert
From: Pierre Habouzit @ 2008-10-31 15:57 UTC (permalink / raw)
  To: git
In-Reply-To: <1225468527-29694-1-git-send-email-madcoder@debian.org>

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

On Fri, Oct 31, 2008 at 03:55:27PM +0000, Pierre Habouzit wrote:
> * Rename builtin-revert.c into builtin-cherry-pick.c
> 
> * Add option -R/--revert to git-cherry-pick.
>   Document it by taking the current content of git-revert manpage for the
>   option.
> 
> * get rid of the no_replay initialization, just ignore it when we're in
>   the revert case, it makes really no sense to error out.
> 
> * put the warning of deprecation in cmd_revert, #if 0-ed out for now.
> 
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> 
>  I've not kept the auto-edit feature of git-revert for the git-cherry-pick -R
>  case as I don't believe it makes a lot of sense. But if people are unhappy
>  with that, I can easily "fix" it.
> 
>  Documentation/git-cherry-pick.txt         |   15 +++++++++++++++
>  Makefile                                  |    6 +++---
>  builtin-revert.c => builtin-cherry-pick.c |   10 ++++------
>  3 files changed, 22 insertions(+), 9 deletions(-)
>  rename builtin-revert.c => builtin-cherry-pick.c (98%)
> 
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index 837fb08..2d92f2d 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -40,6 +40,21 @@ OPTIONS
>  	development branch), adding this information can be
>  	useful.
>  
> +-R::
> +--revert::
> +	Given one existing commit, revert the change the patch introduces, and
> +	record a new commit that records it.  This requires your working tree
> +	to be clean (no modifications from the HEAD commit).
> ++
> +Note: 'git revert' is used to record a new commit to reverse the
         ^ this was supposed to spell out 'git cherry-pick -R' of course :/
> +effect of an earlier commit (often a faulty one).  If you want to
> +throw away all uncommitted changes in your working directory, you
> +should see linkgit:git-reset[1], particularly the '--hard' option.  If
> +you want to extract specific files as they were in another commit, you
> +should see linkgit:git-checkout[1], specifically the 'git checkout
> +<commit> -- <filename>' syntax.  Take care with these alternatives as
> +both will discard uncommitted changes in your working directory.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: why not TortoiseGit
From: Scott Chacon @ 2008-10-31 15:57 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Ian Hilt, Li Frank-B20596, git
In-Reply-To: <490AFBA5.5090700@op5.se>

I'm trying to get this restarted - dscho and I talked about this at
the GitTogether, and I met some people (from the OpenAFS project that
also happened to be there, oddly) who were interested in working on
this with me.  I think the lack of a linkable library has greatly
hindered the development of projects like this, so that will likely be
part of the development process as well.

Scott

On Fri, Oct 31, 2008 at 5:35 AM, Andreas Ericsson <ae@op5.se> wrote:
> Ian Hilt wrote:
>>
>> On Fri, Oct 31, 2008 at 09:44:45AM +0800, Li Frank-B20596 wrote:
>>>
>>> There are TortoiseCVS, TortoiseSVN, TortoiseBzr, TortoiseHg
>>> Why not ToroiseGit
>>
>> This is what Johannes Schindelin had to say,
>>
>>        <http://code.google.com/p/msysgit/wiki/GitCheetah>
>
> Noone's written TortoiseGit yet. I have no idea why, and I have
> no reason to write it myself. If GitCheetah isn't working well,
> I'm sure patches are welcome.
>
> --
> Andreas Ericsson                   andreas.ericsson@op5.se
> OP5 AB                             www.op5.se
> Tel: +46 8-230225                  Fax: +46 8-230231
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: Are binary xdeltas only used if you use git-gc?
From: Jakub Narebski @ 2008-10-31 16:26 UTC (permalink / raw)
  To: Thanassis Tsiodras; +Cc: git
In-Reply-To: <f1d2d9ca0810310428o166dc075wbb43c00c1a555350@mail.gmail.com>

Thanassis Tsiodras wrote:
> On Fri, Oct 31, 2008 at 1:15 PM, Jakub Narebski <jnareb@gmail.com> wrote:
 
> > Git does deltification _only_ in packfiles. But when you push via SSH
> > git would generate a pack file with commits the other side doesn't
> > have, and those packs are thin packs, so they also have deltas... but
> > the remote side then adds bases to those thin packs making them
> > standalone: you would have to git-gc on remote.
> 
> So I have to git-gc on my side (after the commits), git-gc on the remote,
> and then git-push?

Perhaps I haven't made myself clear.

On the local side: git-commit creates loose (compressed, but not
deltified) objects. git-gc packs and deltifies.

On the remote side (for smart protocols, i.e. git and ssh): git
creates _thin_ pack, deltified; on the remote side git either makes
pack thick/self contained by adding base objects (object + deltas),
or explodes pack into loose object (object). You need git-gc on
remote server to fully deltify on remote side. But transfer is fully
deltified.

On the remote side (for dumb protocols, i.e. rsync and http): git
finds required packs and transfers them whole. So the situation is
like on local side, but git might transfer more than really needed
because it transfers packs in full.

HTH.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] prepare deprecation of git-revert
From: Jakub Narebski @ 2008-10-31 16:36 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <1225468527-29694-1-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

> * Rename builtin-revert.c into builtin-cherry-pick.c
> 
> * Add option -R/--revert to git-cherry-pick.
>   Document it by taking the current content of git-revert manpage for the
>   option.
> 
> * get rid of the no_replay initialization, just ignore it when we're in
>   the revert case, it makes really no sense to error out.
> 
> * put the warning of deprecation in cmd_revert, #if 0-ed out for now.

> +#if 0
> +	warning("git revert is deprecated, please use git cherry-pick --revert/-R instead");
> +#endif

By the way, Mercurial names this command IIRC 'hg backout'. 

But I think that adding '-R' option to git-cherry-pick is a good idea
even if we don't go deprecating git-revert.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Johannes Schindelin @ 2008-10-31 16:46 UTC (permalink / raw)
  To: Jeff King
  Cc: Sam Vilain, git, Scott Chacon, Tom Preston-Werner, J.H.,
	Christian Couder, Kai Blin
In-Reply-To: <20081031003154.GA5745@sigill.intra.peff.net>

Hi,

On Thu, 30 Oct 2008, Jeff King wrote:

> On Wed, Oct 29, 2008 at 05:22:00PM -0700, Sam Vilain wrote:
> 
> > +  * 'git branch --switch' : alternative to checkout
> 
> Blech. I think switching branches is the one thing that checkout does 
> unconfusedly. And this is much more typing. Not to mention that So I 
> would rather see "git switch" if checkout is somehow unpalatable.

You know, I asked for this because a _user_ told me "Guess how long it 
took me to find out how to check out a branch!".

I think if you are not confused by CVS/SVN, the name "checkout" is utterly 
unintuitive.

Ciao,
Dscho

^ permalink raw reply

* Re: Are binary xdeltas only used if you use git-gc?
From: Matthieu Moy @ 2008-10-31 16:42 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Thanassis Tsiodras, git
In-Reply-To: <200810311726.57122.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Thanassis Tsiodras wrote:
>  
>> So I have to git-gc on my side (after the commits), git-gc on the remote,
>> and then git-push?
>
> Perhaps I haven't made myself clear.
>
> On the local side: git-commit creates loose (compressed, but not
> deltified) objects. git-gc packs and deltifies.
>
> On the remote side (for smart protocols, i.e. git and ssh): git
> creates _thin_ pack, deltified; 

I don't understand this point: the OP talks about pushing, so isn't
the pack created on the _local_ machine (and then sent to the remote)?

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH] git show <tree>: show mode and hash, and handle -r
From: Johannes Schindelin @ 2008-10-31 16:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, schacon
In-Reply-To: <7vtzatdbtk.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 30 Oct 2008, Junio C Hamano wrote:

> I wonder if it would help breaking down cmd_log_init() a bit like this.

Sorry, I am quite busy (this is the first time I am able to check my mail 
since the GitTogether), so I cannot look at that in detail.

However, I strongly expect your suggestion not to help: for showing 
commits, we _want_ recursive to be the default.  And switching that on 
devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if the 
user said '-r' _explicitely_.

Ciao,
Dscho

^ permalink raw reply


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