* Re: An alternate model for preparing partial commits
From: Junio C Hamano @ 2008-06-28 21:53 UTC (permalink / raw)
To: Jeff King; +Cc: Robert Anderson, Johannes Schindelin, Git Mailing List
In-Reply-To: <20080628085359.GA29619@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Actually, I oversimplified a little bit in my "buckets" description.
> Stash actually stashes two things: the current index and the current
> worktree. So in that sense, it is not just another bucket as I
> described. For the purposes of the workflow we're discussing, I think
> that is how we want it to function. But the implementation will be a bit
> trickier than it might otherwise be because of this. I just didn't want
> to have to introduce another, slightly different type of stash.
You do *not* have to use stash that has different index and work tree
components and then your buckets description makes sense.
The index is to hold good changes verified to be fine to make commits, the
work tree is to test and verify outstanding changes, and the stash is to
hold the remainder (i.e. further changes that does not belong to what you
are currently looking at in your work tree).
When your workflow is to "verify and immediately commit", then the need
for buckets to your particular workflow can degenerate to a one that does
not need the index. That is essentially Robert's workflow (but it does
not mean it is the only valid one).
What I do these days is this:
* Fork a new topic from the commit I pushed out the last time to the
public ("git checkout -b jc/topic ko/master").
* Think, hack, commit, think, hack, commit, lather, rinse, repeat.
* Make sure everything is worthy for the final state. There can be (and
need to be to use the current set of tools) some uncommitted changes.
Make a stash, so that the work tree component records the final tree,
and mentally name it the "commit goal".
* I have never grew comfortable operating "edit" insn in "rebase -i", so
the workflow from this point does not use it. Instead, I detech the
HEAD to the root of the series ("git checkout ko/master^0") at this
point. Now, none of my change is in the work tree.
* Repeat the following:
* Recreate the work tree state for the next round to be built on HEAD
and make a commit, after verifying what I have in the commit is good.
Examples of the tools at my disposal are:
* "cherry-pick jc/topic~$N" to get the necessary changes from my
earlier "snapshots", which can possibly be followed by a "git
commit --amend". This "going forward" is easiest especially in the
early part of the sequence.
"format-patch --stdout jc/topic~$N..jc/topic~$M | git am" is a
slight variant of the above when I already had a good logical
sequence (someday we probably will have "cherry-pick A..B").
* "read-tree -m -u stash" to read the final state of the tree,
selectively _remove_ the parts I do not want in this round, and
make a commit ("git add -i && git commit && git reset --hard").
This "going backward" is easier near the end of the sequence than
other method that goes forward.
* If I find some issues to be fixed in the state that was stashed
(which I earlier thought was perfect) during the above:
* "read-tree -m -u stash" to read the (previous) final state, fix it
up, "stash drop" and "stash save" to update our "commit goal".
The above is repeated until "git diff HEAD stash" does not have
anything I need in the final series.
The lower-level "read-tree -m -u" probably can be replaced with "stash
apply" in real life, but I tend to try to ask for the final tree
explicitly, because there is no reason to perform three-way merge dance
"stash apply" does for these steps.
* Final re-review:
* "git diff jc/topic HEAD" to see if I did not miss anything (and review
the "oops, the earlier commit goal was faulty" fixes).
* "git log --reverse -p ko/master.." to see the final shape of the
series.
* "git branch -f jc/topic" to finish it off.
The commits from the session are merely convenient snapshot points that I
can use during the clean-up phase for series of cherry-picks to prepare
bulk of each of the logical change in the final series. If somebody
subscribes to a dogma not to make commits of unproven changes, that is
fine, and such a person may not have any commits during "think, hack,
lather, rinse, repeat" phase, and that is fine. But fortunately I don't.
^ permalink raw reply
* Re: [q] git-diff --reverse 7def2be1..7def2be1^
From: Junio C Hamano @ 2008-06-28 22:08 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Ingo Molnar, Jakub Narebski, Matthieu Moy, git
In-Reply-To: <20080626093726.GA24577@elte.hu>
Ingo Molnar <mingo@elte.hu> writes:
> * Jakub Narebski <jnareb@gmail.com> wrote:
> ...
>> contrib/completion/git-completion.bash in git repository.
>
> btw., i had to turn this off - it made certain types of file completions
> almost unusable, by adding a 2-3 seconds delay (during which bash would
> just spin around burning CPU time calculating its completion guesses).
>
> and that was on a 3 GHz dual-core box ...
>
> so please do not remove the git-* commands, they are really useful.
The slowness of the completion script (especially the time it takes before
it decides to complete pathnames) has been bothering me for some time, so
I share Ingo's frustration, but it hasn't got to the point to disable it.
After typing:
$ git checkout master -- Docu
and then hitting the "Tab" key to complete before I see "mentation/", it
takes about 2-3 seconds for the first time, and after that it becomes
torelable. Is it the command table bootstrapping that is taking this
long?
<offtopic> I'll be mostly offline for the rest of the day, so you do not
have to respond in a hurry. </offtopic>
^ permalink raw reply
* Re: [PATCH] cmd_reset: don't trash uncommitted changes unless told to
From: しらいしななこ @ 2008-06-28 22:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <20080628090642.6117@nanako3.lavabit.com>
Quoting myself:
>>> Don't you also want to talk about distinction between --cached and
>>> --index that new people are often confused about? These options are
>>> defined consistently across commands but people who do not know it bring
>>> up discussions to rename --cached to some commands to --index to make it
>>> inconsistent and waste your time every once in a while.
>
> Do you have any comment on the --index/--cached issue?
Junio, I haven't heard back from you yet and I take it you mean you are not interested in a vague suggestion but in a concrete patch, so here it is.
-- cut here -- 8< -- cut here --
Subject: [PATCH] gitcli: Document meaning of --cached and --index
We saw this explanation repeated on the git mailing list a
few times. Even though the description of individual options
to particular commands are explained in their manual pages,
the reason behind choosing which is which has been clearly
explained in none of the documentation.
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
Documentation/gitcli.txt | 38 +++++++++++++++++++++++++++++++++++++-
1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
index 2316049..281a987 100644
--- a/Documentation/gitcli.txt
+++ b/Documentation/gitcli.txt
@@ -133,9 +133,45 @@ $ git describe --abbrev 10 HEAD # NOT WHAT YOU MEANT
----------------------------
+NOTES ON FREQUENTLY CONFUSED OPTIONS
+------------------------------------
+
+Many commands that can work on files in the working tree
+and/or in the index can take `--cached` and/or `--index`
+options. Sometimes people incorrectly think that, because
+the index was originally called cache, these two are
+synonyms. They are _not_ --- these two options mean very
+different things.
+
+ * The `--cached` option is used to ask a command that
+ usually works on files in the working tree to _only_ work
+ with the index. For example, `git grep`, when used
+ without a commit to specify from which commit to look for
+ strings in, usually works on files in the working tree,
+ but with the `--cached` option, it looks for strings in
+ the index.
+
+ * The `--index` option is used to ask a command that
+ usually works on files in the working tree to _also_
+ affect the index. For example, `git stash apply` usually
+ merges changes recorded in a stash to the working tree,
+ but with the `--index` option, it also merges changes to
+ the index as well.
+
+`git apply` command can be used with `--cached` and
+`--index` (but not at the same time). Usually the command
+only affects the files in the working tree, but with
+`--index`, it patches both the files and their index
+entries, and with `--cached`, it modifies only the index
+entries.
+
+See also http://marc.info/?l=git&m=116563135620359 and
+http://marc.info/?l=git&m=119150393620273 for further
+information.
+
Documentation
-------------
-Documentation by Pierre Habouzit.
+Documentation by Pierre Habouzit and others.
GIT
---
--
1.5.6
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* [PATCH v2] git-svn: avoid filling up the disk with temp files.
From: Avery Pennarun @ 2008-06-28 23:33 UTC (permalink / raw)
To: git, Eric Wong, Adam Roben, Samuel Bronson, B.Steinbrink, gitster
Cc: Avery Pennarun
In-Reply-To: <1214686673-28099-1-git-send-email-apenwarr@gmail.com>
Commit ffe256f9bac8a40ff751a9341a5869d98f72c285 ("git-svn: Speed up fetch")
introduced changes that create a temporary file for each object fetched by
svn. These files should be deleted automatically, but perl apparently
doesn't do this until the process exits (or perhaps when its garbage
collector runs).
This means that on a large fetch, especially with lots of branches, we
sometimes fill up /tmp completely, which prevents the next temp file from
being written completely. This is aggravated by the fact that a new temp
file is created for each updated file, even if that update produces a file
identical to one already in git. Thus, it can happen even if there's lots
of disk space to store the finished repository.
We weren't adequately checking for write errors, so this would result in an
invalid file getting committed, which caused git-svn to fail later with an
invalid checksum.
This patch adds a check to syswrite() so similar problems don't lead to
corruption in the future. It also unlink()'s each temp file explicitly
when we're done with it, so the disk doesn't need to fill up.
Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
Please use this in favour of the "Revert "git-svn: Speed up fetch" I sent
earlier. I ended up having a surprise inspiration that led to a real fix :)
git-svn.perl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 263d66c..0011387 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3243,7 +3243,9 @@ sub close_file {
my ($tmp_fh, $tmp_filename) = File::Temp::tempfile(UNLINK => 1);
my $result;
while ($result = sysread($fh, my $string, 1024)) {
- syswrite($tmp_fh, $string, $result);
+ my $wrote = syswrite($tmp_fh, $string, $result);
+ defined($wrote) && $wrote == $result
+ or croak("write $tmp_filename: $!\n");
}
defined $result or croak $!;
close $tmp_fh or croak $!;
@@ -3251,6 +3253,7 @@ sub close_file {
close $fh or croak $!;
$hash = $::_repository->hash_and_insert_object($tmp_filename);
+ unlink($tmp_filename);
$hash =~ /^[a-f\d]{40}$/ or die "not a sha1: $hash\n";
close $fb->{base} or croak $!;
} else {
--
1.5.4.3
^ permalink raw reply related
* Re: [PATCH] Revert "git-svn: Speed up fetch"
From: Mikael Magnusson @ 2008-06-28 23:51 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Eric Wong, Adam Roben, Samuel Bronson, B.Steinbrink, git
In-Reply-To: <1214686673-28099-1-git-send-email-apenwarr@gmail.com>
2008/6/28 Avery Pennarun <apenwarr@gmail.com>:
> This reverts commit ffe256f9bac8a40ff751a9341a5869d98f72c285, because it
> was causing errors of the form:
>
> Checksum mismatch: trunk/.depend 16e748c219f9f95bf3d05c6b2af5444290bc8471
> expected: 05fb5edb8c8057be006c7e913ae0c764
> got: 763b9a426c5bd61e0a85252459d37cfa
>
> Note that the exact failing file and checksum seems to vary if you clear
> the repository and try again.
>
> Conflicts:
>
> git-svn.perl
Wasn't this the problem that was fixed by
d683a0e00cd4734b4fab704baef1ee76205722be[1]?
--
Mikael Magnusson
[1]
commit d683a0e00cd4734b4fab704baef1ee76205722be
Author: Junio C Hamano <gitster@pobox.com>
Date: Tue May 27 23:33:22 2008 -0700
Git::cat_blob: allow using an empty blob to fix git-svn breakage
Recent "git-svn optimization" series introduced Git::cat_blob() subroutine
whose interface was broken in that it returned the size of the blob but
signalled an error by returning 0. You can never use an empty blob with
such an interface.
This fixes the interface to return a negative value to signal an error.
Reported by Björn Steinbrink.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH] Revert "git-svn: Speed up fetch"
From: Avery Pennarun @ 2008-06-28 23:58 UTC (permalink / raw)
To: Mikael Magnusson; +Cc: Eric Wong, Adam Roben, Samuel Bronson, B.Steinbrink, git
In-Reply-To: <237967ef0806281651p3abf6ae4j9169cdc5bbf8f85d@mail.gmail.com>
On 6/28/08, Mikael Magnusson <mikachu@gmail.com> wrote:
> Wasn't this the problem that was fixed by
> d683a0e00cd4734b4fab704baef1ee76205722be?
>
> commit d683a0e00cd4734b4fab704baef1ee76205722be
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Tue May 27 23:33:22 2008 -0700
>
> Git::cat_blob: allow using an empty blob to fix git-svn breakage
> [...]
No, I have that patch and the problem still occurred.
Avery
^ permalink raw reply
* [PATCH] t7701-repack-unpack-unreachable.sh: check timestamp of unpacked objects
From: drafnel @ 2008-06-29 0:25 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey
From: Brandon Casey <drafnel@gmail.com>
Unpacked objects should receive the timestamp of the pack they were
unpacked from. Check.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
t/t7701-repack-unpack-unreachable.sh | 34 ++++++++++++++++++++++++++++++++++
1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 6a5211f..63485c1 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -4,6 +4,10 @@ test_description='git-repack works correctly'
. ./test-lib.sh
+fsha1=
+csha1=
+tsha1=
+
test_expect_success '-A option leaves unreachable objects unpacked' '
echo content > file1 &&
git add . &&
@@ -44,4 +48,34 @@ test_expect_success '-A option leaves unreachable objects unpacked' '
git show $tsha1
'
+compare_mtimes ()
+{
+ perl -e 'my $reference = shift;
+ foreach my $file (@ARGV) {
+ exit(1) unless(-f $file && -M $file == -M $reference);
+ }
+ exit(0);
+ ' -- "$@"
+}
+
+test_expect_success 'unpacked objects receive timestamp of pack file' '
+ fsha1path=$(echo "$fsha1" | sed -e "s/\(..\)\(.*\)/\1\/\2/") &&
+ fsha1path=".git/objects/$fsha1path" &&
+ csha1path=$(echo "$csha1" | sed -e "s/\(..\)\(.*\)/\1\/\2/") &&
+ csha1path=".git/objects/$csha1path" &&
+ tsha1path=$(echo "$tsha1" | sed -e "s/\(..\)\(.*\)/\1\/\2/") &&
+ tsha1path=".git/objects/$tsha1path" &&
+ git branch transient_branch $csha1 &&
+ git repack -a -d -l &&
+ test ! -f "$fsha1path" &&
+ test ! -f "$csha1path" &&
+ test ! -f "$tsha1path" &&
+ test 1 = $(ls -1 .git/objects/pack/pack-*.pack | wc -l) &&
+ packfile=$(ls .git/objects/pack/pack-*.pack) &&
+ git branch -D transient_branch &&
+ sleep 1 &&
+ git repack -A -l &&
+ compare_mtimes "$packfile" "$fsha1path" "$csha1path" "$tsha1path"
+'
+
test_done
--
1.5.6.1
^ permalink raw reply related
* Re: [PATCH v2] git-svn: avoid filling up the disk with temp files.
From: Björn Steinbrink @ 2008-06-29 0:58 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git, Eric Wong, Adam Roben, Samuel Bronson, gitster
In-Reply-To: <1214696036-8294-1-git-send-email-apenwarr@gmail.com>
On 2008.06.28 19:33:56 -0400, Avery Pennarun wrote:
> Commit ffe256f9bac8a40ff751a9341a5869d98f72c285 ("git-svn: Speed up fetch")
> introduced changes that create a temporary file for each object fetched by
> svn. These files should be deleted automatically, but perl apparently
> doesn't do this until the process exits (or perhaps when its garbage
> collector runs).
>
> This means that on a large fetch, especially with lots of branches, we
> sometimes fill up /tmp completely, which prevents the next temp file from
> being written completely. This is aggravated by the fact that a new temp
> file is created for each updated file, even if that update produces a file
> identical to one already in git. Thus, it can happen even if there's lots
> of disk space to store the finished repository.
>
> We weren't adequately checking for write errors, so this would result in an
> invalid file getting committed, which caused git-svn to fail later with an
> invalid checksum.
>
> This patch adds a check to syswrite() so similar problems don't lead to
> corruption in the future. It also unlink()'s each temp file explicitly
> when we're done with it, so the disk doesn't need to fill up.
Oh sweet! That also means that I don't have to fear about the repos I
already track with git-svn being corrupted (at least I hope so). :-)
And yeah, looking at /tmp, there are still 58K temp files belonging to
the failed import. Temporarily mounting a 8k tmpfs there triggers the
bug immediately, while with this patch I get a error message.
I'm running another import of the coq repo that failed earlier, just to
make sure, but I also just noticed that cat-file --batch is leaking
memory. For example this one shoots up to about 700M RSS usage with
git.git:
git rev-list --objects origin/master | \
sed 's/ .*//' | \
git cat-file --batch > /dev/null
I'll follow-up with a patch that at least fixes the worst part of that,
getting the RSS usage for the above test down to about 40M.
Thanks!
Björn
^ permalink raw reply
* [PATCH] git cat-file: Fix memory leak in batch mode
From: Björn Steinbrink @ 2008-06-29 1:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Avery Pennarun, Eric Wong, Adam Roben, Samuel Bronson, git
In-Reply-To: <20080629005858.GA2036@atjola.homenet>
When run in batch mode, git cat-file never frees the memory for the blob
contents it is printing. This quickly adds up and causes git-svn to be
hardly usable for imports of large svn repos, because it uses cat-file in
batch mode and cat-file's memory usage easily reaches several hundred MB
without any good reason.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
builtin-cat-file.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin-cat-file.c b/builtin-cat-file.c
index bd343ef..f966dcb 100644
--- a/builtin-cat-file.c
+++ b/builtin-cat-file.c
@@ -183,6 +183,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
fflush(stdout);
}
+ free(contents);
return 0;
}
--
1.5.6.1.94.gd3899.dirty
^ permalink raw reply related
* Re: [PATCH v2] git-svn: avoid filling up the disk with temp files.
From: Björn Steinbrink @ 2008-06-29 2:24 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git, Eric Wong, Adam Roben, Samuel Bronson, gitster
In-Reply-To: <20080629005858.GA2036@atjola.homenet>
On 2008.06.29 02:58:58 +0200, Björn Steinbrink wrote:
> On 2008.06.28 19:33:56 -0400, Avery Pennarun wrote:
> > Commit ffe256f9bac8a40ff751a9341a5869d98f72c285 ("git-svn: Speed up fetch")
> > introduced changes that create a temporary file for each object fetched by
> > svn. These files should be deleted automatically, but perl apparently
> > doesn't do this until the process exits (or perhaps when its garbage
> > collector runs).
> >
> > This means that on a large fetch, especially with lots of branches, we
> > sometimes fill up /tmp completely, which prevents the next temp file from
> > being written completely. This is aggravated by the fact that a new temp
> > file is created for each updated file, even if that update produces a file
> > identical to one already in git. Thus, it can happen even if there's lots
> > of disk space to store the finished repository.
> >
> > We weren't adequately checking for write errors, so this would result in an
> > invalid file getting committed, which caused git-svn to fail later with an
> > invalid checksum.
> >
> > This patch adds a check to syswrite() so similar problems don't lead to
> > corruption in the future. It also unlink()'s each temp file explicitly
> > when we're done with it, so the disk doesn't need to fill up.
>
> I'm running another import of the coq repo that failed earlier, just to
> make sure, ...
Import completed successfully, so:
Tested-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Thanks,
Björn
^ permalink raw reply
* Re: [PATCH v2] git-svn: avoid filling up the disk with temp files.
From: Eric Wong @ 2008-06-29 2:38 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git, Adam Roben, Samuel Bronson, B.Steinbrink, gitster
In-Reply-To: <1214696036-8294-1-git-send-email-apenwarr@gmail.com>
Avery Pennarun <apenwarr@gmail.com> wrote:
> Commit ffe256f9bac8a40ff751a9341a5869d98f72c285 ("git-svn: Speed up fetch")
> introduced changes that create a temporary file for each object fetched by
> svn. These files should be deleted automatically, but perl apparently
> doesn't do this until the process exits (or perhaps when its garbage
> collector runs).
>
> This means that on a large fetch, especially with lots of branches, we
> sometimes fill up /tmp completely, which prevents the next temp file from
> being written completely. This is aggravated by the fact that a new temp
> file is created for each updated file, even if that update produces a file
> identical to one already in git. Thus, it can happen even if there's lots
> of disk space to store the finished repository.
>
> We weren't adequately checking for write errors, so this would result in an
> invalid file getting committed, which caused git-svn to fail later with an
> invalid checksum.
>
> This patch adds a check to syswrite() so similar problems don't lead to
> corruption in the future. It also unlink()'s each temp file explicitly
> when we're done with it, so the disk doesn't need to fill up.
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
> ---
> Please use this in favour of the "Revert "git-svn: Speed up fetch" I sent
> earlier. I ended up having a surprise inspiration that led to a real fix :)
Ouch, I didn't noticed these unchecked syscalls :x
Very graciously
Acked-by: Eric Wong <normalperson@yhbt.net>
Apologies to all users who were bitten by this bug.
> git-svn.perl | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 263d66c..0011387 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3243,7 +3243,9 @@ sub close_file {
> my ($tmp_fh, $tmp_filename) = File::Temp::tempfile(UNLINK => 1);
> my $result;
> while ($result = sysread($fh, my $string, 1024)) {
> - syswrite($tmp_fh, $string, $result);
> + my $wrote = syswrite($tmp_fh, $string, $result);
> + defined($wrote) && $wrote == $result
> + or croak("write $tmp_filename: $!\n");
> }
> defined $result or croak $!;
> close $tmp_fh or croak $!;
> @@ -3251,6 +3253,7 @@ sub close_file {
> close $fh or croak $!;
>
> $hash = $::_repository->hash_and_insert_object($tmp_filename);
> + unlink($tmp_filename);
> $hash =~ /^[a-f\d]{40}$/ or die "not a sha1: $hash\n";
> close $fb->{base} or croak $!;
> } else {
> --
> 1.5.4.3
^ permalink raw reply
* Re: BUG (v1.5.6.1): ./configure missing check for zlib.h
From: Ian Hilt @ 2008-06-29 3:13 UTC (permalink / raw)
To: tom fogal; +Cc: Jakub Narebski, git
In-Reply-To: <20080627150732.D88F64B4002@artemis.sr.unh.edu>
On Fri, 27 Jun 2008 at 11:07am -0000, tom fogal wrote:
> I should note, however, that I disagree with that macro's logic in that
> it `searches' for zlib if the user does not specify it. IMO, if the
> user does not give a --with option, and it doesn't work `out of the
> box' (without hacking FLAGS), macros should die with an error rather
> than retry with changed FLAGS.
>
> -tom
Or, one could read the INSTALL file included with git,
- Git is reasonably self-sufficient, but does depend on a few external
programs and libraries:
- "zlib", the compression library. Git won't build without it.
--
Ian Hilt
Ian.Hilt (at) gmx.com
GnuPG key: 0x4AFC1EE3
^ permalink raw reply
* Re: [PATCH] git cat-file: Fix memory leak in batch mode
From: Junio C Hamano @ 2008-06-29 3:36 UTC (permalink / raw)
To: Björn Steinbrink
Cc: Avery Pennarun, Eric Wong, Adam Roben, Samuel Bronson, git
In-Reply-To: <20080629012125.GA1722@atjola.homenet>
Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> When run in batch mode, git cat-file never frees the memory for the blob
> contents it is printing. This quickly adds up and causes git-svn to be
> hardly usable for imports of large svn repos, because it uses cat-file in
> batch mode and cat-file's memory usage easily reaches several hundred MB
> without any good reason.
>
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> ---
> builtin-cat-file.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-cat-file.c b/builtin-cat-file.c
> index bd343ef..f966dcb 100644
> --- a/builtin-cat-file.c
> +++ b/builtin-cat-file.c
> @@ -183,6 +183,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
> fflush(stdout);
> }
>
> + free(contents);
> return 0;
> }
Thanks, except that it should go inside the "if (print_contents == BATCH)"
block to avoid freeing an uninitialized pointer.
^ permalink raw reply
* [PATCH] git-svn: don't sanitize remote names in config
From: Eric Wong @ 2008-06-29 3:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Avery Pennarun
In-Reply-To: <32541b130806250801p1508d15axc610f335b8d235ef@mail.gmail.com>
The original sanitization code was just taken from the
remotes2config.sh shell script in contrib.
Credit to Avery Pennarun for noticing this mistake, and Junio
for clarifying the rules for config section names:
Junio C Hamano wrote <7vfxr23s6m.fsf@gitster.siamese.dyndns.org>:
> In
>
> [foo "bar"] baz = value
>
> foo and baz must be config.c::iskeychar() (and baz must be isalpha()), but
> "bar" can be almost anything.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
Avery Pennarun <apenwarr@gmail.com> wrote:
> On 6/25/08, Eric Wong <normalperson@yhbt.net> wrote:
> > No, nothing to do with DNS hostnames in the remote names. I think I
> > just looked at remotes2config.sh one day and used it as a reference :x
> >
> > It's late and I've had a rough few days, but shouldn't
> > sanitize_remote_name() just escape . and "? Right now it's converting
> > stuff to . which has me very confused...
>
> I think there might be higher-level problems here: what is it
> sanitizing anyway, and why? If it found my D2007_Win32 svn-remote
> entry in the config (as it seems to have done when trying to locate
> its parent branch during fetch), and *then* it sanitized it to
> D2007.Win32, that doesn't even make any sense. Clearly something
> straight from the config file doesn't need to be sanitized.
>
> However, I don't understand the code well enough to be able to say a)
> whether that's exactly what happened, or b) other places where
> sanitize_remote_name() *is* important, or c) whether
> sanitize_remote_name() is even correct.
Nope. It's not important anywhere from what I can tell.
--
git-svn.perl | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 50ace22..f789a6e 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1462,13 +1462,6 @@ sub verify_remotes_sanity {
}
}
-# we allow more chars than remotes2config.sh...
-sub sanitize_remote_name {
- my ($name) = @_;
- $name =~ tr{A-Za-z0-9:,/+-}{.}c;
- $name;
-}
-
sub find_existing_remote {
my ($url, $remotes) = @_;
return undef if $no_reuse_existing;
@@ -2853,7 +2846,7 @@ sub _new {
unless (defined $ref_id && length $ref_id) {
$_[2] = $ref_id = $Git::SVN::default_ref_id;
}
- $_[1] = $repo_id = sanitize_remote_name($repo_id);
+ $_[1] = $repo_id;
my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
$_[3] = $path = '' unless (defined $path);
mkpath(["$ENV{GIT_DIR}/svn"]);
@@ -4707,8 +4700,7 @@ sub minimize_connections {
# skip existing cases where we already connect to the root
if (($ra->{url} eq $ra->{repos_root}) ||
- (Git::SVN::sanitize_remote_name($ra->{repos_root}) eq
- $repo_id)) {
+ ($ra->{repos_root} eq $repo_id)) {
$root_repos->{$ra->{url}} = $repo_id;
next;
}
@@ -4747,8 +4739,7 @@ sub minimize_connections {
foreach my $url (keys %$new_urls) {
# see if we can re-use an existing [svn-remote "repo_id"]
# instead of creating a(n ugly) new section:
- my $repo_id = $root_repos->{$url} ||
- Git::SVN::sanitize_remote_name($url);
+ my $repo_id = $root_repos->{$url} || $url;
my $fetch = $new_urls->{$url};
foreach my $path (keys %$fetch) {
--
Eric Wong
^ permalink raw reply related
* Re: Adding a 'version' command to the server
From: Shawn O. Pearce @ 2008-06-29 4:08 UTC (permalink / raw)
To: Lea Wiemann; +Cc: Alex Riesen, Junio C Hamano, git
In-Reply-To: <48662DB0.3010608@gmail.com>
Lea Wiemann <lewiemann@gmail.com> wrote:
> Alex Riesen wrote:
> >And exactly how are the incompatibility problems avoided? By denying
> >the service if the versions don't match?
>
> No, by talking to older servers in a way they understand it. So for
> instance it would call "git-upload-pack" instead of "git upload-pack".
>
> (See also the SSH protocol with two versions, and I think Subversion's
> svnserve also somehow identifies its own capabilities and/or version.)
We already have both sides of a git native connection select
functions to be used in the protocol based upon both sides having
the necessary capabilities. This is in heavy use today for things
like thin packs, sideband progress messages and remote deletion of
branches through git-push.
So we're doing what you suggest.
The problem we are faced with right now is when invoking Git over
SSH we need to pass some string to the remote shell and expect
the remote shell to start up the program we need to talk to.
Without that program running there is nothing to ask the other side
its version. :-(
This should be a one-time transition pain, as the original git
developers used git-foo-command style, but users today want just
"git foo-command" style, to reduce the number of commands they
see with tab completion. Its the price we/they pay for making
that sort of change this late in the project.
I'm not sure when SVN start supporting "svn serve" over SSH. It may
have been quite a while after they were already self-hosting, as
they started out with HTTP WebDAV as the only protocol. Thus they
may have already been heavily into the "svn foo" style of command
invocation and never made the "misstep" that Git made.
Of course SVN has made _many_ other "missteps" that Git has done
right since day 1. Like merge support. :-)
Nobody is perfect. Not even the Git developers.
--
Shawn.
^ permalink raw reply
* Re: [RFC/PATCH] Documentation: Don't assume git-sh-setup and git-parse-remote are in the PATH
From: Jeff King @ 2008-06-29 5:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Riesen, Jonathan Nieder, git
In-Reply-To: <7vvdztmfrk.fsf@gitster.siamese.dyndns.org>
On Sat, Jun 28, 2008 at 02:05:51PM -0700, Junio C Hamano wrote:
> > What do you need exec-path in PATH for?
>
> When git-parse-remote is not installed in $(bindir) anymore, the shell
> script library won't be found on user's $PATH in general.
I think the right question is:
PATH=$PATH:$(git --exec-path)
. git-sh-setup
or
. "$(git --exec-path)/git-sh-setup"
?
Generally, I would prefer the latter because it has no side effects. For
now at least, though, shell scripts probably want git's exec-path in
their PATH so "git-foo" invocations don't break.
-Peff
^ permalink raw reply
* Re: [PATCH 13/13] Build in merge
From: Junio C Hamano @ 2008-06-29 7:46 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Johannes Schindelin, Olivier Marin
In-Reply-To: <ed4b7f44474d6b9398fb0752740ede264bea25b2.1214620551.git.vmiklos@frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Sat, Jun 28, 2008 at 04:33:55AM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> OK, I'll send the main patch without the strbuf_initf() calls
>
> Here it is.
Sorry, this file is huge and very time consuming to review. Here are some
pieces. The parts I did not comment I haven't looked at.
> +/* Get the name for the merge commit's message. */
> +static void merge_name(const char *remote, struct strbuf *msg)
> +{
> + struct object *remote_head;
> + unsigned char branch_head[20], buf_sha[20];
> + struct strbuf buf;
> + char *ptr;
> + int len = 0;
> +
> + memset(branch_head, 0, sizeof(branch_head));
> + remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
> + if (!remote_head)
> + return;
Hmm. This is a faithful translation of scripted version, but I wonder
what should happen when we got a non-commit here...
> +
> + strbuf_init(&buf, 0);
> + strbuf_addstr(&buf, "refs/heads/");
> + strbuf_addstr(&buf, remote);
> + get_sha1(buf.buf, branch_head);
This does not correspond to the computation of $bh in the scripted version
that makes sure "remote" is actually a bare name of branch, e.g. "master",
without any adornment like "master~5^3~8. Your code would succeed and
leave the same object name in branch_head[] as remote_head->sha1, wouldn't
it?
> + if (!hashcmp(remote_head->sha1, branch_head)) {
> + strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
> + sha1_to_hex(branch_head), remote);
> + return;
> + }
> + /* See if remote matches <name>~<number>, or <name>^ */
The scripted version did not handle <name>^, so this is an extension.
Don't you want also handle <name>^^^ if we are extending it?
> + ptr = strrchr(remote, '^');
> + if (ptr && ptr[1] == '\0')
> + len = ptr-remote;
> + else {
> + ptr = strrchr(remote, '~');
> + if (ptr && ptr[1] != '0' && isdigit(ptr[1])) {
> + len = ptr-remote;
> + ptr++;
> + for (ptr++; *ptr; ptr++)
> + if (!isdigit(*ptr)) {
> + len = 0;
> + break;
> + }
> + }
> + }
> + if (len) {
> + struct strbuf truname = STRBUF_INIT;
> + strbuf_addstr(&truname, remote);
> + strbuf_setlen(&truname, len);
> + if (!get_sha1(truname.buf, buf_sha)) {
Again, isn't this wrong? You are not making sure truname is the name of
existing local branch. HEAD@{7}~23 will pass get_sha1() but you are not
merging an early part of HEAD@{7} branch.
> + strbuf_addf(msg,
> + "%s\t\tbranch '%s' (early part) of .\n",
> + sha1_to_hex(remote_head->sha1), truname.buf);
> + return;
> + }
> + }
> +int cmd_merge(int argc, const char **argv, const char *prefix)
> +{
This is an ultra-huge function. I wonder if it can further split up to
make it easier to maintain.
> + unsigned char sha1[20], result_tree[20];
> + struct object *second_token = NULL;
> + struct strbuf buf, head_arg;
> + int flag, head_invalid, i, single_strategy;
> + int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
> + struct commit_list *common = NULL;
> + struct path_list_item *best_strategy = NULL, *wt_strategy = NULL;
> + struct commit_list **remotes = &remoteheads;
> +
> + setup_work_tree();
> + if (unmerged_cache())
> + die("You are in the middle of a conflicted merge.");
> +
> + /*
> + * Check if we are _not_ on a detached HEAD, i.e. if there is a
> + * current branch.
> + */
> + branch = resolve_ref("HEAD", sha1, 0, &flag);
You are resolving for non-reading, so before an initial commit, you won't
get any error from this call, which is good. And sha1[] will be cleared.
Otherwise you learn where the HEAD is in sha1[] at this point.
> + if (branch && flag & REF_ISSYMREF) {
> + const char *ptr = skip_prefix(branch, "refs/heads/");
> + if (ptr)
> + branch = ptr;
> + }
> +
> + git_config(git_merge_config, NULL);
> +
> + /* for color.diff and diff.color */
> + git_config(git_diff_ui_config, NULL);
> +
> + /* for color.ui */
> + if (diff_use_color_default == -1)
> + diff_use_color_default = git_use_color_default;
> +
> + argc = parse_options(argc, argv, builtin_merge_options,
> + builtin_merge_usage, 0);
> +
> + if (squash) {
> + if (!allow_fast_forward)
> + die("You cannot combine --squash with --no-ff.");
> + option_commit = 0;
> + }
> +
> + if (!argc)
> + usage_with_options(builtin_merge_usage,
> + builtin_merge_options);
> +
> + /*
> + * This could be traditional "merge <msg> HEAD <commit>..." and
> + * the way we can tell it is to see if the second token is HEAD,
> + * but some people might have misused the interface and used a
> + * committish that is the same as HEAD there instead.
> + * Traditional format never would have "-m" so it is an
> + * additional safety measure to check for it.
> + */
> + strbuf_init(&buf, 0);
> + strbuf_init(&head_arg, 0);
> + if (argc > 1)
> + second_token = peel_to_type(argv[1], 0, NULL, OBJ_COMMIT);
If the second token was a string that could resolve to an object name that
does not peel to commit (say "merge -m 'HEAD^{tree}' other"), you will get
a complaint fro mpeel-to-type "I expected a commit but you gave something
else". You (or more likely Dscho) might have said "that won't matter in
practice", but I think you should really do get_sha1() followed by
lookup_commit_reference_gently() here to avoid the errors.
> + head_invalid = get_sha1("HEAD", head);
You've already done this earlier with resolve_ref() haven't you?
> + if (!have_message && second_token &&
> + !hashcmp(second_token->sha1, head)) {
Isn't this wrong if head_invalid is true?
> + strbuf_addstr(&merge_msg, argv[0]);
> + strbuf_addstr(&head_arg, argv[1]);
> + argv += 2;
> + argc -= 2;
I do not think there is any point using strbuf for head_arg. Shouldn't it
simply be a "const char *"?
> + } else if (head_invalid) {
> + struct object *remote_head;
> + /*
> + * If the merged head is a valid one there is no reason
> + * to forbid "git merge" into a branch yet to be born.
> + * We do the same for "git pull".
> + */
> + if (argc != 1)
> + die("Can merge only exactly one commit into "
> + "empty head");
> + remote_head = peel_to_type(argv[0], 0, NULL, OBJ_COMMIT);
Here we _do_ expect it to be a commit, so peel_to_type() is fine.
> + if (!remote_head)
> + die("%s - not something we can merge", argv[0]);
> + update_ref("initial pull", "HEAD", remote_head->sha1, NULL, 0,
> + DIE_ON_ERR);
> + reset_hard(remote_head->sha1, 0);
> + return 0;
Makes one wonder reset_hard() (aka "read-tree --reset -u HEAD") ever fail
and return here (iow, without calling die()). The answer is luckily no
in this case, but it is somewhat unnerving to reviewers.
> + } else {
> + /* We are invoked directly as the first-class UI. */
> + strbuf_addstr(&head_arg, "HEAD");
> + /*
> + * All the rest are the commits being merged;
> + * prepare the standard merge summary message to
> + * be appended to the given message. If remote
> + * is invalid we will die later in the common
> + * codepath so we discard the error in this
> + * loop.
> + */
> + struct strbuf msg;
Decl-after-statement.
> + strbuf_init(&msg, 0);
> + for (i = 0; i < argc; i++)
> + merge_name(argv[i], &msg);
> + fmt_merge_msg(option_log, &msg, &merge_msg);
> + if (merge_msg.len)
> + strbuf_setlen(&merge_msg, merge_msg.len-1);
> + }
> +
> + if (head_invalid || !argc)
> + usage_with_options(builtin_merge_usage,
> + builtin_merge_options);
> + strbuf_addstr(&buf, "merge");
> + for (i = 0; i < argc; i++)
> + strbuf_addf(&buf, " %s", argv[i]);
> + setenv("GIT_REFLOG_ACTION", buf.buf, 0);
> + strbuf_reset(&buf);
> +
> + for (i = 0; i < argc; i++) {
> + struct object *o;
> +
> + o = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);
> + if (!o)
> + die("%s - not something we can merge", argv[i]);
> + remotes = &commit_list_insert(lookup_commit(o->sha1),
> + remotes)->next;
> +
> + strbuf_addf(&buf, "GITHEAD_%s", sha1_to_hex(o->sha1));
> + setenv(buf.buf, argv[i], 1);
> + strbuf_reset(&buf);
> + }
> +
> + if (!use_strategies.nr) {
> + if (!remoteheads->next)
> + add_strategies(pull_twohead, DEFAULT_TWOHEAD);
> + else
> + add_strategies(pull_octopus, DEFAULT_OCTOPUS);
> + }
> +
> + for (i = 0; i < use_strategies.nr; i++) {
> + if ((unsigned int)use_strategies.items[i].util &
> + NO_FAST_FORWARD)
> + allow_fast_forward = 0;
> + if ((unsigned int)use_strategies.items[i].util & NO_TRIVIAL)
> + allow_trivial = 0;
Can we abstract out these ugly casts? Any code that use path_list to
store anything but list of paths (i.e. some value keyed with string) tends
to have this readability issue.
> + }
> +
> + if (!remoteheads->next)
> + common = get_merge_bases(lookup_commit(head),
> + remoteheads->item, 1);
> + else {
> + struct commit_list *list = remoteheads;
> + commit_list_insert(lookup_commit(head), &list);
> + common = get_octopus_merge_bases(list);
> + free(list);
> + }
> +
> + update_ref("updating ORIG_HEAD", "ORIG_HEAD", head, NULL, 0,
> + DIE_ON_ERR);
> +
> + if (!common)
> + ; /* No common ancestors found. We need a real merge. */
> + else if (!remoteheads->next &&
> + !hashcmp(common->item->object.sha1,
> + remoteheads->item->object.sha1)) {
Wouldn't the latter be "common->item == remoteheads->item" simply?
You do not have the check to make sure there is only one common ancestor
(scripted version compares $common and $1 textually to achieve this), and
checking only the first one of them. Is this correct?
> + /*
> + * If head can reach all the remote heads then we are up
> + * to date.
> + */
The comment is wrong --- you are doing "... but first the most common case
of merging one remote" here.
> + finish_up_to_date("Already up-to-date.");
> + return 0;
> + } else if (allow_fast_forward && !remoteheads->next &&
> + !hashcmp(common->item->object.sha1, head)) {
> + /* Again the most common case of merging one remote. */
Here again you are not checking there is only one common, and checking
only the first one of them.
> + struct strbuf msg;
> + struct object *o;
> + char hex[41];
> +
> + strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
> +
> + printf("Updating %s..%s\n",
> + hex,
> + find_unique_abbrev(remoteheads->item->object.sha1,
> + DEFAULT_ABBREV));
> + refresh_cache(REFRESH_QUIET);
> + strbuf_init(&msg, 0);
> + strbuf_addstr(&msg, "Fast forward");
> + if (have_message)
> + strbuf_addstr(&msg,
> + " (no commit created; -m option ignored)");
> + o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
> + 0, NULL, OBJ_COMMIT);
> + if (!o)
> + return 0;
> +
> + if (merge_one_remote(head, remoteheads->item->object.sha1))
> + return 0;
Isn't "merge_one_remote()" just a "git checkout" after fast-forward? The
function feels misnamed.
> + finish(o->sha1, msg.buf);
> + dropsave();
> + return 0;
> + } else if (!remoteheads->next && common->next)
> + ;
Here you are checking common->next but earlier if/elseif chain didn't so
it is too late.
> + /*
> + * We are not doing octopus and not fast forward. Need
> + * a real merge.
> + */
> + else if (!remoteheads->next && option_commit) {
> + /*
> + * We are not doing octopus, not fast forward, and have
> + * only one common.
Here again you did not check "have only one common" did you?
> + */
> + refresh_cache(REFRESH_QUIET);
> + if (allow_trivial) {
> + /* See if it is really trivial. */
> + git_committer_info(IDENT_ERROR_ON_NO_NAME);
> + printf("Trying really trivial in-index merge...\n");
> + if (!read_tree_trivial(common->item->object.sha1,
> + head, remoteheads->item->object.sha1)) {
> + unsigned char result_tree[20],
> + result_commit[20];
> + struct commit_list parent;
> +
> + write_tree_trivial(result_tree);
> + printf("Wonderful.\n");
> + parent.item = remoteheads->item;
> + parent.next = NULL;
> + commit_tree_trivial(merge_msg.buf,
> + result_tree, &parent,
> + result_commit);
> + finish(result_commit, "In-index merge");
> + dropsave();
> + return 0;
> + }
> + printf("Nope.\n");
> + }
There weren't any good way to squelch error messages selectively from the
trivial one in the scripted version and that is the only reason we
surround read-tree with "Trying..." and "Wonderful/Nope.". Literal
translation to make sure you get identical output in the first round of
this series is good, but after the code stabilizes, we may want to squelch
these messages. Something to keep in mind but not now.
^ permalink raw reply
* [JGIT PATCH 00/21] Push support over SFTP and (encrypted) Amazon S3
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
People have often asked on the mailing list if Git can push to
a remote server directly over SFTP, without needing to have Git
installed on the remote system. This mode of operation can be
useful if the remote server is an inexpensive hosting account
and the user wants to publish over HTTP.
With this series jgit can now push directly over sftp:// style
URI without needing Git to be installed on the remote system.
Both the real refs and the dumb transport support files (such
as info/refs) are updated during the push.
A transport for Amazon S3 (http://aws.amazon.com/s3) is also
included. S3 is an inexpensive network based storage system
provided as a commerical service by Amazon. Public data stored
in S3 is available over standard HTTP, making it an inexpensive
hosting provider.
Git repositories pushed to S3 may optionally be transparently
encrypted with an encryption key of the user's choosing, hiding the
repository content from Amazon. An encrypted repository may only be
accessed by jgit, or by downloading it through jets3t, and accessing
the local copy. (This is because all encryption/decryption occurs
on the client.)
I wanted the S3 support just so I could backup my repositories as
easily as I can backup through native Git. Its as simple as:
touch ~/.s3_ident
chmod 600 ~/.s3_ident
cat >~/.s3_ident
accesskey: <AWSAccessKeyId>
secretkey: <AWSSecretAccessKey>
password: <secretpassphrasetoseedencryption>
^D
jgit push amazon-s3://.s3_ident@bucket/repo.git refs/heads/master
The bucket must have already been created with another S3 client.
I consider it outside of the scope of jgit to register buckets.
However the repository name can be any string of your choosing and
the repository will be created on S3 during the first push.
You can also clone this branch off S3 using anonymous HTTP:
git clone http://gitney.s3.amazonaws.com/projects/egit.git
I pushed the above repository with:
touch ~/.s3_pub
chmod 600 ~/.s3_pub
cat >~/.s3_pub
accesskey: <AWSAccessKeyId>
secretkey: <AWSSecretAccessKey>
acl: public
^D
git remote add s3 amazon-s3://.s3_pub@gitney/projects/egit.git
jgit push s3 refs/heads/dumb-push
After writing the full S3 client from scratch and implementing an
encryption scheme that is compatible with jets3t (a popular Java
based S3 client) I've realized that jets3t's encryption scheme is
not as strong as it could be, especially if you can recognize a
pattern in the plain text (such as the format of info/refs, or even
of a pack and pack index). As such the encryption used by jgit is
"eh, ok". This may be an area of improvement in future versions,
but at present should at least stop any sort of casual snooping.
This series is based on `pu` as it requires both Marek's push topic
and my index-v2 topic.
----
Robert Harder (1):
Add Robert Harder's public domain Base64 encoding utility
Shawn O. Pearce (20):
Remove unused index files when WalkFetchConnection closes
Do not show URIish passwords in TransportExceptions
Use PackedObjectInfo as a base class for PackWriter's ObjectToPack
Refactor PackWriter to hold onto the sorted object list
Save the pack checksum after computing it in PackWriter
Allow PackIndexWriter to use any subclass of PackedObjectInfo
Allow PackWriter to create a corresponding index file
Allow PackWriter to prepare object list and compute name before
writing
Remember how a Ref was read in from disk and created
Simplify walker transport ref advertisement setup
Indicate the protocol jgit doesn't support push over
WalkTransport must allow subclasses to implement openPush
Support push over the sftp:// dumb transport
Extract readPackedRefs from TransportSftp for reuse
Specialized byte array output stream for large files
Misc. documentation fixes to Base64 utility
Extract the basic HTTP proxy support to its own class
Create a really simple Amazon S3 REST client
Add client side encryption to Amazon S3 client library
Bidirectional protocol support for Amazon S3
.../tst/org/spearce/jgit/lib/PackWriterTest.java | 8 +-
.../spearce/jgit/transport/PushProcessTest.java | 94 +-
.../spearce/jgit/transport/RefSpecTestCase.java | 26 +-
.../spearce/jgit/errors/TransportException.java | 4 +-
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 6 +-
.../src/org/spearce/jgit/lib/PackWriter.java | 216 ++--
org.spearce.jgit/src/org/spearce/jgit/lib/Ref.java | 91 ++-
.../src/org/spearce/jgit/lib/RefDatabase.java | 23 +-
.../src/org/spearce/jgit/pgm/Main.java | 36 +-
.../src/org/spearce/jgit/transport/AmazonS3.java | 770 ++++++++++
.../spearce/jgit/transport/BasePackConnection.java | 6 +-
.../jgit/transport/BasePackPushConnection.java | 5 +-
.../src/org/spearce/jgit/transport/Transport.java | 3 +
.../spearce/jgit/transport/TransportAmazonS3.java | 319 +++++
.../spearce/jgit/transport/TransportBundle.java | 3 +-
.../org/spearce/jgit/transport/TransportHttp.java | 64 +-
.../org/spearce/jgit/transport/TransportSftp.java | 162 ++-
.../src/org/spearce/jgit/transport/URIish.java | 24 +-
.../org/spearce/jgit/transport/WalkEncryption.java | 188 +++
.../jgit/transport/WalkFetchConnection.java | 2 +
.../spearce/jgit/transport/WalkPushConnection.java | 296 ++++
.../jgit/transport/WalkRemoteObjectDatabase.java | 301 ++++
.../org/spearce/jgit/transport/WalkTransport.java | 8 +-
.../src/org/spearce/jgit/util/Base64.java | 1465 ++++++++++++++++++++
.../src/org/spearce/jgit/util/HttpSupport.java | 165 +++
.../src/org/spearce/jgit/util/TemporaryBuffer.java | 260 ++++
26 files changed, 4250 insertions(+), 295 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/TransportAmazonS3.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/WalkEncryption.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/Base64.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/HttpSupport.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java
^ permalink raw reply
* [JGIT PATCH 01/21] Remove unused index files when WalkFetchConnection closes
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-1-git-send-email-spearce@spearce.org>
If we downloaded an index but then didn't download the corresponding
pack file we never deleted the index from disk. We should clear any
unused indexes that are left when we terminate the connection.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../jgit/transport/WalkFetchConnection.java | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 78116b2..5a21d24 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -201,6 +201,8 @@ class WalkFetchConnection extends BaseFetchConnection {
@Override
public void close() {
+ for (final RemotePack p : unfetchedPacks)
+ p.tmpIdx.delete();
for (final WalkRemoteObjectDatabase r : remotes)
r.close();
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 02/21] Do not show URIish passwords in TransportExceptions
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-2-git-send-email-spearce@spearce.org>
When construction a transport exception the message may be shown
on screen. If a password was in the URIish then we may wind up
showing the user's password, perhaps while there is someone else
looking over the user's shoulder and reading their monitor. By
setting the field to null we avoid displaying it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/errors/TransportException.java | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/errors/TransportException.java b/org.spearce.jgit/src/org/spearce/jgit/errors/TransportException.java
index 4a8e37c..7fbbc5a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/errors/TransportException.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/errors/TransportException.java
@@ -58,7 +58,7 @@ public class TransportException extends IOException {
* message
*/
public TransportException(final URIish uri, final String s) {
- super(uri + ": " + s);
+ super(uri.setPass(null) + ": " + s);
}
/**
@@ -74,7 +74,7 @@ public class TransportException extends IOException {
*/
public TransportException(final URIish uri, final String s,
final Throwable cause) {
- this(uri + ": " + s, cause);
+ this(uri.setPass(null) + ": " + s, cause);
}
/**
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 03/21] Use PackedObjectInfo as a base class for PackWriter's ObjectToPack
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-3-git-send-email-spearce@spearce.org>
The ObjectId and offset portions of PackedObjectInfo are also
needed by ObjectToPack. By sharing the same base class with
IndexPack we can later abstract the index writing function out
and use it inside of both IndexPack and PackWriter.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackWriter.java | 28 +++-----------------
1 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index cec2ab0..ccc6cfe 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -56,6 +56,7 @@ import org.spearce.jgit.revwalk.ObjectWalk;
import org.spearce.jgit.revwalk.RevFlag;
import org.spearce.jgit.revwalk.RevObject;
import org.spearce.jgit.revwalk.RevSort;
+import org.spearce.jgit.transport.PackedObjectInfo;
import org.spearce.jgit.util.CountingOutputStream;
import org.spearce.jgit.util.NB;
@@ -617,7 +618,7 @@ public class PackWriter {
assert !otp.isWritten();
- otp.markWritten(countingOut.getCount());
+ otp.setOffset(countingOut.getCount());
if (otp.isDeltaRepresentation())
writeDeltaObject(otp);
else
@@ -762,13 +763,11 @@ public class PackWriter {
* pack-file and object status.
*
*/
- static class ObjectToPack extends ObjectId {
+ static class ObjectToPack extends PackedObjectInfo {
private ObjectId deltaBase;
private PackedObjectLoader reuseLoader;
- private long offset = -1;
-
private int deltaDepth;
private boolean wantWrite;
@@ -838,26 +837,7 @@ public class PackWriter {
* @return true if object is already written; false otherwise.
*/
boolean isWritten() {
- return offset != -1;
- }
-
- /**
- * @return offset in pack when object has been already written, or -1 if
- * it has not been written yet
- */
- long getOffset() {
- return offset;
- }
-
- /**
- * Mark object as written. This information is used to achieve
- * delta-base precedence in a pack file.
- *
- * @param offset
- * offset where written object starts
- */
- void markWritten(long offset) {
- this.offset = offset;
+ return getOffset() != 0;
}
PackedObjectLoader getReuseLoader() {
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 04/21] Refactor PackWriter to hold onto the sorted object list
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-4-git-send-email-spearce@spearce.org>
When creating pack files we sometimes need the sorted object list
of all contents for two reasons. The first is to get the name of
the pack, which computeName() returns today. The other reason is
to generate a corresponding .idx file for this pack, to support
random access into the data.
Since not all uses of PackWriter require the sorted object name list
(for example streaming the pack to a network socket) the sorting is
done on demand, and cached to avoid needing to do it a second time.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackWriter.java | 24 ++++++++++++-------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index ccc6cfe..0f4cbb4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -180,6 +180,8 @@ public class PackWriter {
private final WindowCursor windowCursor = new WindowCursor();
+ private List<ObjectToPack> sortedByName;
+
private boolean reuseDeltas = DEFAULT_REUSE_DELTAS;
private boolean reuseObjects = DEFAULT_REUSE_OBJECTS;
@@ -470,22 +472,26 @@ public class PackWriter {
* @return ObjectId representing SHA-1 name of a pack that was created.
*/
public ObjectId computeName() {
- final ArrayList<ObjectToPack> sorted = new ArrayList<ObjectToPack>(
- objectsMap.size());
- for (List<ObjectToPack> list : objectsLists) {
- for (ObjectToPack otp : list)
- sorted.add(otp);
- }
-
final MessageDigest md = Constants.newMessageDigest();
- Collections.sort(sorted);
- for (ObjectToPack otp : sorted) {
+ for (ObjectToPack otp : sortByName()) {
otp.copyRawTo(buf, 0);
md.update(buf, 0, Constants.OBJECT_ID_LENGTH);
}
return ObjectId.fromRaw(md.digest());
}
+ private List<ObjectToPack> sortByName() {
+ if (sortedByName == null) {
+ sortedByName = new ArrayList<ObjectToPack>(objectsMap.size());
+ for (List<ObjectToPack> list : objectsLists) {
+ for (ObjectToPack otp : list)
+ sortedByName.add(otp);
+ }
+ Collections.sort(sortedByName);
+ }
+ return sortedByName;
+ }
+
private void writePackInternal() throws IOException {
if (reuseDeltas || reuseObjects)
searchForReuse();
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 05/21] Save the pack checksum after computing it in PackWriter
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-5-git-send-email-spearce@spearce.org>
In order to create a matching .idx file for the pack we have
written out we must retain the last 20 bytes of the pack file
so we can include it at the trailing end of the index file.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackWriter.java | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index 0f4cbb4..6adb629 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -182,6 +182,8 @@ public class PackWriter {
private List<ObjectToPack> sortedByName;
+ private byte packcsum[];
+
private boolean reuseDeltas = DEFAULT_REUSE_DELTAS;
private boolean reuseObjects = DEFAULT_REUSE_OBJECTS;
@@ -690,8 +692,8 @@ public class PackWriter {
private void writeChecksum() throws IOException {
out.on(false);
- final byte checksum[] = out.getMessageDigest().digest();
- out.write(checksum);
+ packcsum = out.getMessageDigest().digest();
+ out.write(packcsum);
}
private ObjectWalk setUpWalker(
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 06/21] Allow PackIndexWriter to use any subclass of PackedObjectInfo
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-6-git-send-email-spearce@spearce.org>
Some users of PackIndexWriter may have extended PackedObjectInfo
to store additional implementation specific details, but wish to
pass off their own subclass instances to avoid allocating even
more memory.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
index 3d0050d..567f099 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
@@ -86,7 +86,7 @@ public abstract class PackIndexWriter {
*/
@SuppressWarnings("fallthrough")
public static PackIndexWriter createOldestPossible(final OutputStream dst,
- final List<PackedObjectInfo> objs) {
+ final List<? extends PackedObjectInfo> objs) {
int version = 1;
LOOP: for (final PackedObjectInfo oe : objs) {
switch (version) {
@@ -137,7 +137,7 @@ public abstract class PackIndexWriter {
protected final byte[] tmp;
/** The entries this writer must pack. */
- protected List<PackedObjectInfo> entries;
+ protected List<? extends PackedObjectInfo> entries;
/** SHA-1 checksum for the entire pack data. */
protected byte[] packChecksum;
@@ -172,7 +172,7 @@ public abstract class PackIndexWriter {
* an error occurred while writing to the output stream, or this
* index format cannot store the object data supplied.
*/
- public void write(final List<PackedObjectInfo> toStore,
+ public void write(final List<? extends PackedObjectInfo> toStore,
final byte[] packDataChecksum) throws IOException {
entries = toStore;
packChecksum = packDataChecksum;
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 07/21] Allow PackWriter to create a corresponding index file
From: Shawn O. Pearce @ 2008-06-29 7:59 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git
In-Reply-To: <1214726371-93520-7-git-send-email-spearce@spearce.org>
If we are packing for local use, or are sending the pack file to
a dumb server we must also genrate the matching .idx file so Git
can use random access requests to read object data. Since all
of the necessary information is available in our ObjectToPack we
can just pass off the sorted list to PackIndexWriter.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackWriter.java | 38 ++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index 6adb629..e346668 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -192,6 +192,8 @@ public class PackWriter {
private int maxDeltaDepth = DEFAULT_MAX_DELTA_DEPTH;
+ private int outputVersion;
+
private boolean thin;
/**
@@ -348,6 +350,18 @@ public class PackWriter {
}
/**
+ * Set the pack index file format version this instance will create.
+ *
+ * @param version
+ * the version to write. The special version 0 designates the
+ * oldest (most compatible) format available for the objects.
+ * @see PackIndexWriter
+ */
+ public void setIndexVersion(final int version) {
+ outputVersion = version;
+ }
+
+ /**
* Returns objects number in a pack file that was created by this writer.
*
* @return number of objects in pack.
@@ -482,6 +496,30 @@ public class PackWriter {
return ObjectId.fromRaw(md.digest());
}
+ /**
+ * Create an index file to match the pack file just written.
+ * <p>
+ * This method can only be invoked after {@link #writePack(Iterator)} or
+ * {@link #writePack(Collection, Collection, boolean, boolean)} has been
+ * invoked and completed successfully. Writing a corresponding index is an
+ * optional feature that not all pack users may require.
+ *
+ * @param indexStream
+ * output for the index data. Caller is responsible for closing
+ * this stream.
+ * @throws IOException
+ * the index data could not be written to the supplied stream.
+ */
+ public void writeIndex(final OutputStream indexStream) throws IOException {
+ final List<ObjectToPack> list = sortByName();
+ final PackIndexWriter iw;
+ if (outputVersion <= 0)
+ iw = PackIndexWriter.createOldestPossible(indexStream, list);
+ else
+ iw = PackIndexWriter.createVersion(indexStream, outputVersion);
+ iw.write(list, packcsum);
+ }
+
private List<ObjectToPack> sortByName() {
if (sortedByName == null) {
sortedByName = new ArrayList<ObjectToPack>(objectsMap.size());
--
1.5.6.74.g8a5e
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox