* [PATCH v7] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Lea Wiemann @ 2008-06-24 2:18 UTC (permalink / raw)
To: git; +Cc: Lea Wiemann, Jakub Narebski
In-Reply-To: <4860556F.5060206@gmail.com>
This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Changes since v6:
- Removed GITPERL again.
- Added some basic tests for the page contents of blob and blob_plain
views.
- Incorporated Jakub's suggestions (see parent posts); in particular
blame and feed views *do* get spidered now in --long-tests mode, but
the spider tests are marked as TODO and thus don't cause the test
suite to fail.
- Some minor (internal) changes: @files only contains blobs, not
trees; improved the logic that avoids checking pages twice; die if
test.pl is run directly rather than from the shell script.
t/t9503-gitweb-Mechanize.sh | 132 +++++++++++++++
t/t9503/test.pl | 378 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 510 insertions(+), 0 deletions(-)
create mode 100755 t/t9503-gitweb-Mechanize.sh
create mode 100755 t/t9503/test.pl
diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..6f7168e
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+ chmod "$1" "$2" &&
+ if [ "$(git config --get core.filemode)" = false ]
+ then
+ git update-index --chmod="$1" "$2"
+ fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, perl version is too old' :
+ test_done
+ exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+ test_expect_success \
+ 'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+ test_done
+ exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+ echo "Not an empty file." > file &&
+ git add file &&
+ test_tick && git commit -a -m "Initial commit." &&
+ git branch b &&
+
+ echo "New file" > new_file &&
+ git add new_file &&
+ test_tick && git commit -a -m "File added." &&
+
+ safe_chmod +x new_file &&
+ test_tick && git commit -a -m "Mode changed." &&
+
+ git mv new_file renamed_file &&
+ test_tick && git commit -a -m "File renamed." &&
+
+ rm renamed_file &&
+ ln -s file renamed_file &&
+ test_tick && git commit -a -m "File to symlink." &&
+ git tag with-symlink &&
+
+ git rm renamed_file &&
+ rm -f renamed_file &&
+ test_tick && git commit -a -m "File removed." &&
+
+ cp file file2 &&
+ git add file2 &&
+ test_tick && git commit -a -m "File copied." &&
+
+ echo "New line" >> file2 &&
+ safe_chmod +x file2 &&
+ test_tick && git commit -a -m "Mode change and modification." &&
+
+ git checkout b &&
+ echo "Branch" >> b &&
+ git add b &&
+ test_tick && git commit -a -m "On branch" &&
+ git checkout master &&
+ test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+ 'test gitweb output' \
+ perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..872dbfd
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,378 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+die "this must be run by calling the t/t*.sh shell script(s)\n"
+ if Cwd->cwd !~ /trash directory$/;
+
+our $long_tests = $ENV{GIT_TEST_LONG}; # "our" so we can use "local $long_tests"
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+ or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+ or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+ or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+sub rev_parse {
+ my $name = shift;
+ chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+ $hash or undef;
+}
+
+sub get_type {
+ my $name = shift;
+ chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+ $type or undef;
+}
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);
+my @files = grep { get_type("HEAD:$_") eq 'blob' } @root_entries or die;
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# This subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+# http://rt.cpan.org/Ticket/Display.html?id=36654
+my $cgi = sub {
+ # Use exec, not the shell, to support embedded whitespace in
+ # the path to gitweb.cgi.
+ my $status = system $gitweb $gitweb;
+ my $value = $status >> 8;
+
+ croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+ if ( $status == -1 );
+ croak( qq/Application '$gitweb' exited with value: $value/ )
+ if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+ GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+ SCRIPT_FILENAME => $gitweb,
+ $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+ or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+ my ($uri, $fragment) = split '#', $mech->uri;
+ $mech->content_like(qr/(name|id)="$fragment"/,
+ "[auto] fragment #$fragment exists ($uri)")
+ if $fragment;
+
+ return 1 if $verified_uris{$uri};
+ $verified_uris{$uri} = 1;
+
+ # Internal errors yield 200 but cause gitweb.cgi to exit with
+ # non-zero exit code, which Mechanize::CGI translates to 500,
+ # so we don't really need to check for "Software error" here,
+ # provided that the test cases always check the status code.
+ #$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+ # Validate. This is fast, so we can do it even without
+ # $long_tests.
+ $mech->html_lint_ok('[auto] validate HTML') or return 0
+ if $html_lint_installed && $mech->is_html;
+ my $content_type = $mech->response->header('Content-Type')
+ or die "$uri does not have a Content-Type header";
+ if ($xml_parser_installed && $content_type =~ /xml/) {
+ eval { XML::Parser->new->parse($mech->content); };
+ ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+ }
+ if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+ my $snapshot_file = File::Temp->new;
+ print $snapshot_file $mech->content;
+ close $snapshot_file;
+ my $t = Archive::Tar->new;
+ $t->read($snapshot_file->filename, 1);
+ ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+ }
+ # WebService::Validator::Feed::W3C would be nice to
+ # use, but it doesn't support direct input (as opposed
+ # to URIs) long enough for our feeds.
+
+ return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given. Do not check the status code of the current page.
+my %spidered_uris; # pages whose links have been checked
+my %status_checked_uris; # verified pages whose status is known to be 2xx
+sub check_page {
+ _verify_page or return 0;
+ my $orig_url = $mech->uri;
+ if ($long_tests && !$spidered_uris{$mech->uri} ) {
+ $spidered_uris{$mech->uri} = 1;
+ for my $url (map { $_->url_abs } $mech->followable_links) {
+ if (!$status_checked_uris{$url}) {
+ $status_checked_uris{$url} = 1;
+ $mech->get_ok($url, "[auto] check link ($url)")
+ or diag("broken link to $url on $orig_url");
+ _verify_page;
+ $mech->back;
+ }
+ }
+ }
+ return 1;
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+# $mech->...;
+# $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure. Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages). You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+ ($params, $pagedesc, $status) = @_;
+ $pagedesc = $pagedesc ? " -- $pagedesc" : '';
+ if($params =~ /^$baseurl/) {
+ $url = "$params";
+ } else {
+ $url = "$baseurl$params";
+ }
+ if ($status) {
+ $mech->get($url);
+ } else {
+ $mech->get_ok($url, "get $url$pagedesc") or return 0;
+ }
+ check_page or return 0;
+ if ($status) {
+ return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+ } else {
+ return 1;
+ }
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+ (my $parms, $pagedesc) = @_;
+ unless ($mech->follow_link_ok($parms, "follow link: $pagedesc")) {
+ diag("cannot follow link ($pagedesc) on page " . $mech->uri);
+ return 0;
+ }
+ $url = $mech->uri;
+ return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+ $mech->title_like(qr!$site_name!,
+ "title contains $site_name");
+ $mech->content_contains('./t9503-gitweb-Mechanize.sh test repository',
+ 'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+ if (test_page $sumparams, 'repository summary') {
+ $mech->title_like(qr!$site_name.*\.git/summary!,
+ "title contains $site_name and \".git/summary\"");
+ }
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+ fields => { 's' => 'Initial' }
+ }, "submit search form (default)")) {
+ check_page;
+ $mech->content_contains('Initial commit',
+ 'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log. To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+ for my $link_text qw( commit commitdiff tree snapshot ) {
+ ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+ }
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog. We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+ 'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+ "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads. (This should be extracted as well.)
+for my $head (@heads) {
+ for my $link_text qw( shortlog log tree ) {
+ ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+ }
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+ my $commit = rev_parse("$tag^{commit}");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+ "shortlog link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+ "log link for tag '$tag'");
+ ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+ "commit link for tag '$tag'");
+ # To do: Test tag link for tag objects.
+ # Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view
+# Simply retrieve and verify well-formedness, but don't spider.
+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
+TODO: {
+ # Now spider -- but there are broken links.
+ # http://mid.gmane.org/485EB333.5070108@gmail.com
+ local $TODO = "fix broken links in Atom/RSS feeds";
+ test_page('?p=.git;a=atom', 'Atom feed');
+ test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+ ok($mech->find_link(url_abs_regex => qr/a=tree/),
+ "tree link on commit page ($url)");
+ $mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+ $mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+ $mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'first tree link on page')) {
+ for my $file (@files) {
+ my $file_hash = rev_parse("HEAD:$file");
+ ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+ for my $link_text qw( blob blame history raw ) {
+ my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+ text => $link_text);
+ ok($link, "'$file' file has $link_text link in tree view ($url)");
+ }
+ }
+ # To do: write tests for subtrees. (Need to set up subtrees
+ # in t9503-gitweb-Mechanize.sh.)
+}
+
+
+# Blame view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'first tree link on page')) {
+ for my $blame_link ($mech->find_all_links(text => 'blame')) {
+ my $url = $blame_link->url;
+ $mech->get_ok($url, "get $url -- blame link on tree view")
+ and _verify_page;
+ $mech->content_like(qr/A U Thor/,
+ "author mentioned on blame page");
+ TODO: {
+ # Now spider -- but there are broken links.
+ # http://mid.gmane.org/485EC621.7090101@gmail.com
+ local $TODO = "fix broken links in certain blame views";
+ check_page;
+ }
+ }
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'first tree link on page')) {
+ for my $history_link ($mech->find_all_links(text => 'history')) {
+ test_page($history_link->url, "follow history link from tree view");
+ # To do: Expand.
+ }
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'first tree link on page')) {
+ for my $file (@files) {
+ if (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },
+ "\"$file\" (blob) entry on tree view")) {
+ chomp(my $first_line_regex = (`cat "$file"`)[0]);
+ $first_line_regex =~ s/ / | /g;
+ # Hope that the first line doesn't contain any
+ # HTML-escapable character.
+ $mech->content_like(qr/$first_line_regex/,
+ "blob view contains first line of file ($url)");
+ $mech->back;
+ }
+ }
+}
+
+
+# Raw (blob_plain) view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+ follow_link( { text => 'tree' }, 'first tree link on page')) {
+ for my $file (@files) {
+ if (follow_link( { text => 'raw', url_abs_regex => qr/[^a-z]f=$file(;|$)/ },
+ "raw (blob_plain) entry for \"$file\" in tree view")) {
+ chomp(my $last_line = (`cat "$file"`)[-1]);
+ $mech->content_contains(
+ $last_line, "blob_plain view contains last line of file");
+ $mech->back;
+ }
+ }
+}
+
+
+1;
+__END__
--
1.5.6.109.g0c85.dirty
^ permalink raw reply related
* Re: why is git destructive by default? (i suggest it not be!)
From: Nicolas Pitre @ 2008-06-24 2:17 UTC (permalink / raw)
To: David Jeske; +Cc: git
In-Reply-To: <willow-jeske-01l5cKsCFEDjC=91MX@videotron.ca>
On Tue, 24 Jun 2008, David Jeske wrote:
> I've heard from a couple users that the solution to these problems is to "go
> dig what you need out of the log, it's still in there". However, it's only in
> there until the log is garbage collected. This either means they are
> destructive operations, or we expect "running without ever collecting the log"
> to be a valid mode of operation... which I doubt is the case.
Why not?
> Question: How about assuring ALL operations can be done non-destructivly by
> default?
git config --global gc.reflogexpire "2 years"
Nicolas
^ permalink raw reply
* [JGIT PATCH 10/10] Default IndexPack to honor pack.indexversion configuration
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-10-git-send-email-spearce@spearce.org>
Users may already desire to create only v2 pack index files, as
the extra CRC code makes repacking faster due to quicker delta
reuse code paths being available. However we still must default
to version 0 to select oldest version available to improve our
changes of being compatible with really old Git executables.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/CoreConfig.java | 11 +++++++++++
.../src/org/spearce/jgit/transport/IndexPack.java | 4 +++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
index 01d4210..2dd8aea 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/CoreConfig.java
@@ -48,8 +48,11 @@ public class CoreConfig {
private final int compression;
+ private final int packIndexVersion;
+
CoreConfig(final RepositoryConfig rc) {
compression = rc.getInt("core", "compression", DEFAULT_COMPRESSION);
+ packIndexVersion = rc.getInt("pack", "indexversion", 0);
}
/**
@@ -59,4 +62,12 @@ public class CoreConfig {
public int getCompression() {
return compression;
}
+
+ /**
+ * @return the preferred pack index file format; 0 for oldest possible.
+ * @see org.spearce.jgit.transport.IndexPack
+ */
+ public int getPackIndexVersion() {
+ return packIndexVersion;
+ }
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 06ef7cc..8083cc8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -102,7 +102,9 @@ public class IndexPack {
final File base;
base = new File(objdir, n.substring(0, n.length() - suffix.length()));
- return new IndexPack(db, is, base);
+ final IndexPack ip = new IndexPack(db, is, base);
+ ip.setIndexVersion(db.getConfig().getCore().getPackIndexVersion());
+ return ip;
}
private final Repository repo;
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 09/10] Add support for writing pack index v2 files
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-9-git-send-email-spearce@spearce.org>
The v2 format is more robust for delta reuse as it has a CRC
element that covers the entire packed representation, permitting
more efficient delta-reuse during packing. It also can address
objects in pack files larger than 4 GB, making it a better format
for the future.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 21 ++++
.../org/spearce/jgit/lib/PackIndexWriterV2.java | 101 ++++++++++++++++++++
.../src/org/spearce/jgit/pgm/IndexPack.java | 4 +
.../src/org/spearce/jgit/transport/IndexPack.java | 22 ++++-
4 files changed, 146 insertions(+), 2 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV2.java
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 c9b27d2..2d9d822 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
@@ -122,6 +122,8 @@ public abstract class PackIndexWriter {
switch (version) {
case 1:
return new PackIndexWriterV1(dst);
+ case 2:
+ return new PackIndexWriterV2(dst);
default:
throw new IllegalArgumentException(
"Unsupported pack index version " + version);
@@ -203,6 +205,25 @@ public abstract class PackIndexWriter {
protected abstract void writeImpl() throws IOException;
/**
+ * Output the version 2 (and later) TOC header, with version number.
+ * <p>
+ * Post version 1 all index files start with a TOC header that makes the
+ * file an invalid version 1 file, and then includes the version number.
+ * This header is necessary to recognize a version 1 from a version 2
+ * formatted index.
+ *
+ * @param version
+ * version number of this index format being written.
+ * @throws IOException
+ * an error occurred while writing to the output stream.
+ */
+ protected void writeTOC(final int version) throws IOException {
+ out.write(TOC);
+ NB.encodeInt32(tmp, 0, version);
+ out.write(tmp, 0, 4);
+ }
+
+ /**
* Output the standard 256 entry first-level fan-out table.
* <p>
* The fan-out table is 4 KB in size, holding 256 32-bit unsigned integer
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV2.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV2.java
new file mode 100644
index 0000000..8fa4d1a
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV2.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+import org.spearce.jgit.transport.PackedObjectInfo;
+import org.spearce.jgit.util.NB;
+
+/**
+ * Creates the version 2 pack table of contents files.
+ *
+ * @see PackIndexWriter
+ * @see PackIndexV2
+ */
+class PackIndexWriterV2 extends PackIndexWriter {
+ PackIndexWriterV2(final OutputStream dst) {
+ super(dst);
+ }
+
+ @Override
+ protected void writeImpl() throws IOException {
+ writeTOC(2);
+ writeFanOutTable();
+ writeObjectNames();
+ writeCRCs();
+ writeOffset32();
+ writeOffset64();
+ writeChecksumFooter();
+ }
+
+ private void writeObjectNames() throws IOException {
+ for (final PackedObjectInfo oe : entries)
+ oe.copyRawTo(out);
+ }
+
+ private void writeCRCs() throws IOException {
+ for (final PackedObjectInfo oe : entries) {
+ NB.encodeInt32(tmp, 0, oe.getCRC());
+ out.write(tmp, 0, 4);
+ }
+ }
+
+ private void writeOffset32() throws IOException {
+ int o64 = 0;
+ for (final PackedObjectInfo oe : entries) {
+ final long o = oe.getOffset();
+ if (o < Integer.MAX_VALUE)
+ NB.encodeInt32(tmp, 0, (int) o);
+ else
+ NB.encodeInt32(tmp, 0, (1 << 31) | o64++);
+ out.write(tmp, 0, 4);
+ }
+ }
+
+ private void writeOffset64() throws IOException {
+ for (final PackedObjectInfo oe : entries) {
+ final long o = oe.getOffset();
+ if (o > Integer.MAX_VALUE) {
+ NB.encodeInt64(tmp, 0, o);
+ out.write(tmp, 0, 8);
+ }
+ }
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/IndexPack.java
index 5a82a35..60926c1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/IndexPack.java
@@ -47,10 +47,13 @@ class IndexPack extends TextBuiltin {
void execute(final String[] args) throws Exception {
boolean fixThin = false;
int argi = 0;
+ int version = 0;
for (; argi < args.length; argi++) {
final String a = args[argi];
if ("--fix-thin".equals(a))
fixThin = true;
+ else if (a.startsWith("--index-version="))
+ version = Integer.parseInt(a.substring(a.indexOf('=') + 1));
else if ("--".equals(a)) {
argi++;
break;
@@ -69,6 +72,7 @@ class IndexPack extends TextBuiltin {
in = new BufferedInputStream(System.in);
ip = new org.spearce.jgit.transport.IndexPack(db, in, base);
ip.setFixThin(fixThin);
+ ip.setIndexVersion(version);
ip.index(new TextProgressMonitor());
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 047f0dc..06ef7cc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -125,6 +125,8 @@ public class IndexPack {
private boolean fixThin;
+ private int outputVersion;
+
private final File dstPack;
private final File dstIdx;
@@ -185,6 +187,18 @@ public class IndexPack {
}
/**
+ * 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;
+ }
+
+ /**
* Configure this index pack instance to make a thin pack complete.
* <p>
* Thin packs are sometimes used during network transfers to allow a delta
@@ -466,8 +480,12 @@ public class IndexPack {
final FileOutputStream os = new FileOutputStream(dstIdx);
try {
- PackIndexWriter.createOldestPossible(os, list)
- .write(list, packcsum);
+ final PackIndexWriter iw;
+ if (outputVersion <= 0)
+ iw = PackIndexWriter.createOldestPossible(os, list);
+ else
+ iw = PackIndexWriter.createVersion(os, outputVersion);
+ iw.write(list, packcsum);
} finally {
os.close();
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 08/10] Compute packed object entry CRC32 data during IndexPack
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-8-git-send-email-spearce@spearce.org>
In order to create a pack index using the v2 file format we must
have the CRC32 data available for every object we discovered in
that pack stream. We now compute this on the fly as we read the
object entries in from the input stream. Always running the CRC
computation isn't that expensive and we can be certain we would
always have the information necessary to create a v2 index file.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/transport/IndexPack.java | 135 ++++++++++++++------
.../spearce/jgit/transport/PackedObjectInfo.java | 25 ++++-
2 files changed, 117 insertions(+), 43 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 60e0bce..047f0dc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -49,6 +49,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
+import java.util.zip.CRC32;
import java.util.zip.DataFormatException;
import java.util.zip.Deflater;
import java.util.zip.Inflater;
@@ -136,6 +137,8 @@ public class IndexPack {
private int entryCount;
+ private final CRC32 crc = new CRC32();
+
private ObjectIdMap<ArrayList<UnresolvedDelta>> baseById;
private HashMap<Long, ArrayList<UnresolvedDelta>> baseByPos;
@@ -282,13 +285,15 @@ public class IndexPack {
}
private void resolveDeltas(final PackedObjectInfo oe) throws IOException {
+ final int oldCRC = oe.getCRC();
if (baseById.containsKey(oe)
|| baseByPos.containsKey(new Long(oe.getOffset())))
- resolveDeltas(oe.getOffset(), Constants.OBJ_BAD, null, oe);
+ resolveDeltas(oe.getOffset(), oldCRC, Constants.OBJ_BAD, null, oe);
}
- private void resolveDeltas(final long pos, int type, byte[] data,
- PackedObjectInfo oe) throws IOException {
+ private void resolveDeltas(final long pos, final int oldCRC, int type,
+ byte[] data, PackedObjectInfo oe) throws IOException {
+ crc.reset();
position(pos);
int c = readFromFile();
final int typeCode = (c >> 4) & 7;
@@ -309,14 +314,14 @@ public class IndexPack {
data = inflateFromFile((int) sz);
break;
case Constants.OBJ_OFS_DELTA: {
- c = readFromInput() & 0xff;
+ c = readFromFile() & 0xff;
while ((c & 128) != 0)
- c = readFromInput() & 0xff;
+ c = readFromFile() & 0xff;
data = BinaryDelta.apply(data, inflateFromFile((int) sz));
break;
}
case Constants.OBJ_REF_DELTA: {
- fillFromInput(20);
+ crc.update(buf, fillFromFile(20), 20);
use(20);
data = BinaryDelta.apply(data, inflateFromFile((int) sz));
break;
@@ -325,6 +330,9 @@ public class IndexPack {
throw new IOException("Unknown object type " + typeCode + ".");
}
+ final int crc32 = (int) crc.getValue();
+ if (oldCRC != crc32)
+ throw new IOException("Corruption detected re-reading at " + pos);
if (oe == null) {
objectDigest.update(Constants.encodedTypeString(type));
objectDigest.update((byte) ' ');
@@ -332,7 +340,8 @@ public class IndexPack {
objectDigest.update((byte) 0);
objectDigest.update(data);
tempObjectId.fromRaw(objectDigest.digest(), 0);
- oe = new PackedObjectInfo(pos, tempObjectId);
+
+ oe = new PackedObjectInfo(pos, crc32, tempObjectId);
entries[entryCount++] = oe;
}
@@ -349,20 +358,24 @@ public class IndexPack {
final UnresolvedDelta ad = a.get(ai);
final UnresolvedDelta bd = b.get(bi);
if (ad.position < bd.position) {
- resolveDeltas(ad.position, type, data, null);
+ resolveDeltas(ad.position, ad.crc, type, data, null);
ai++;
} else {
- resolveDeltas(bd.position, type, data, null);
+ resolveDeltas(bd.position, bd.crc, type, data, null);
bi++;
}
}
}
if (a != null)
- while (ai < a.size())
- resolveDeltas(a.get(ai++).position, type, data, null);
+ while (ai < a.size()) {
+ final UnresolvedDelta ad = a.get(ai++);
+ resolveDeltas(ad.position, ad.crc, type, data, null);
+ }
if (b != null)
- while (bi < b.size())
- resolveDeltas(b.get(bi++).position, type, data, null);
+ while (bi < b.size()) {
+ final UnresolvedDelta bd = b.get(bi++);
+ resolveDeltas(bd.position, bd.crc, type, data, null);
+ }
}
private void fixThinPack(final ProgressMonitor progress) throws IOException {
@@ -377,10 +390,11 @@ public class IndexPack {
final int typeCode = ldr.getType();
final PackedObjectInfo oe;
- oe = new PackedObjectInfo(end, baseId);
+ crc.reset();
+ writeWhole(def, typeCode, data);
+ oe = new PackedObjectInfo(end, (int) crc.getValue(), baseId);
entries[entryCount++] = oe;
packOut.seek(end);
- writeWhole(def, typeCode, data);
end = packOut.getFilePointer();
resolveChildDeltas(oe.getOffset(), typeCode, data, oe);
@@ -403,12 +417,16 @@ public class IndexPack {
buf[hdrlen++] = (byte) (sz & 0x7f);
sz >>>= 7;
}
+ crc.update(buf, 0, hdrlen);
packOut.write(buf, 0, hdrlen);
def.reset();
def.setInput(data);
def.finish();
- while (!def.finished())
- packOut.write(buf, 0, def.deflate(buf));
+ while (!def.finished()) {
+ final int datlen = def.deflate(buf);
+ crc.update(buf, 0, datlen);
+ packOut.write(buf, 0, datlen);
+ }
}
private void fixHeaderFooter() throws IOException {
@@ -493,6 +511,7 @@ public class IndexPack {
private void indexOneObject() throws IOException {
final long pos = position();
+ crc.reset();
int c = readFromInput();
final int typeCode = (c >> 4) & 7;
long sz = c & 15;
@@ -511,11 +530,11 @@ public class IndexPack {
whole(typeCode, pos, sz);
break;
case Constants.OBJ_OFS_DELTA: {
- c = readFromInput() & 0xff;
+ c = readFromInput();
long ofs = c & 127;
while ((c & 128) != 0) {
ofs += 1;
- c = readFromInput() & 0xff;
+ c = readFromInput();
ofs <<= 7;
ofs += (c & 127);
}
@@ -525,25 +544,24 @@ public class IndexPack {
r = new ArrayList<UnresolvedDelta>(8);
baseByPos.put(base, r);
}
- r.add(new UnresolvedDelta(pos));
- deltaCount++;
inflateFromInput(false);
+ r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
+ deltaCount++;
break;
}
case Constants.OBJ_REF_DELTA: {
c = fillFromInput(20);
- final byte[] ref = new byte[20];
- System.arraycopy(buf, c, ref, 0, 20);
+ crc.update(buf, c, 20);
+ final ObjectId base = ObjectId.fromRaw(buf, c);
use(20);
- final ObjectId base = ObjectId.fromRaw(ref);
ArrayList<UnresolvedDelta> r = baseById.get(base);
if (r == null) {
r = new ArrayList<UnresolvedDelta>(8);
baseById.put(base, r);
}
- r.add(new UnresolvedDelta(pos));
- deltaCount++;
inflateFromInput(false);
+ r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
+ deltaCount++;
break;
}
default:
@@ -559,7 +577,9 @@ public class IndexPack {
objectDigest.update((byte) 0);
inflateFromInput(true);
tempObjectId.fromRaw(objectDigest.digest(), 0);
- entries[entryCount++] = new PackedObjectInfo(pos, tempObjectId);
+
+ final int crc32 = (int) crc.getValue();
+ entries[entryCount++] = new PackedObjectInfo(pos, crc32, tempObjectId);
}
// Current position of {@link #bOffset} within the entire file.
@@ -579,15 +599,19 @@ public class IndexPack {
if (bAvail == 0)
fillFromInput(1);
bAvail--;
- return buf[bOffset++] & 0xff;
+ final int b = buf[bOffset++] & 0xff;
+ crc.update(b);
+ return b;
}
// Consume exactly one byte from the buffer and return it.
private int readFromFile() throws IOException {
if (bAvail == 0)
- fillFromFile();
+ fillFromFile(1);
bAvail--;
- return buf[bOffset++] & 0xff;
+ final int b = buf[bOffset++] & 0xff;
+ crc.update(b);
+ return b;
}
// Consume cnt bytes from the buffer.
@@ -615,13 +639,21 @@ public class IndexPack {
}
// Ensure at least need bytes are available in in {@link #buf}.
- private int fillFromFile() throws IOException {
- if (bAvail == 0) {
- final int next = packOut.read(buf, 0, buf.length);
+ private int fillFromFile(final int need) throws IOException {
+ if (bAvail < need) {
+ int next = bOffset + bAvail;
+ int free = buf.length - next;
+ if (free + bAvail < need) {
+ if (bAvail > 0)
+ System.arraycopy(buf, bOffset, buf, 0, bAvail);
+ bOffset = 0;
+ next = bAvail;
+ free = buf.length - next;
+ }
+ next = packOut.read(buf, next, free);
if (next <= 0)
throw new EOFException("Packfile is truncated.");
- bAvail = next;
- bOffset = 0;
+ bAvail += next;
}
return bOffset;
}
@@ -642,11 +674,15 @@ public class IndexPack {
try {
final byte[] dst = objectData;
int n = 0;
+ int p = -1;
while (!inf.finished()) {
if (inf.needsInput()) {
- final int p = fillFromInput(1);
+ if (p >= 0) {
+ crc.update(buf, p, bAvail);
+ use(bAvail);
+ }
+ p = fillFromInput(1);
inf.setInput(buf, p, bAvail);
- use(bAvail);
}
int free = dst.length - n;
@@ -661,7 +697,11 @@ public class IndexPack {
}
if (digest)
objectDigest.update(dst, 0, n);
- use(-inf.getRemaining());
+ n = bAvail - inf.getRemaining();
+ if (n > 0) {
+ crc.update(buf, p, n);
+ use(n);
+ }
} catch (DataFormatException dfe) {
throw corrupt(dfe);
} finally {
@@ -674,15 +714,23 @@ public class IndexPack {
try {
final byte[] dst = new byte[sz];
int n = 0;
+ int p = -1;
while (!inf.finished()) {
if (inf.needsInput()) {
- final int p = fillFromFile();
+ if (p >= 0) {
+ crc.update(buf, p, bAvail);
+ use(bAvail);
+ }
+ p = fillFromFile(1);
inf.setInput(buf, p, bAvail);
- use(bAvail);
}
n += inf.inflate(dst, n, sz - n);
}
- use(-inf.getRemaining());
+ n = bAvail - inf.getRemaining();
+ if (n > 0) {
+ crc.update(buf, p, n);
+ use(n);
+ }
return dst;
} catch (DataFormatException dfe) {
throw corrupt(dfe);
@@ -699,8 +747,11 @@ public class IndexPack {
private static class UnresolvedDelta {
final long position;
- UnresolvedDelta(final long headerOffset) {
+ final int crc;
+
+ UnresolvedDelta(final long headerOffset, final int crc32) {
position = headerOffset;
+ crc = crc32;
}
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
index 58feada..eae34df 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
@@ -51,9 +51,13 @@ import org.spearce.jgit.lib.ObjectId;
public class PackedObjectInfo extends ObjectId {
private long offset;
- PackedObjectInfo(final long headerOffset, final AnyObjectId id) {
+ private int crc;
+
+ PackedObjectInfo(final long headerOffset, final int packedCRC,
+ final AnyObjectId id) {
super(id);
offset = headerOffset;
+ crc = packedCRC;
}
/**
@@ -83,4 +87,23 @@ public class PackedObjectInfo extends ObjectId {
public void setOffset(final long offset) {
this.offset = offset;
}
+
+ /**
+ * @return the 32 bit CRC checksum for the packed data.
+ */
+ public int getCRC() {
+ return crc;
+ }
+
+ /**
+ * Record the 32 bit CRC checksum for the packed data.
+ *
+ * @param crc
+ * checksum of all packed data (including object type code,
+ * inflated length and delta base reference) as computed by
+ * {@link java.util.zip.CRC32}.
+ */
+ public void setCRC(final int crc) {
+ this.crc = crc;
+ }
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 07/10] Add 64 bit network byte order encoding to NB
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-7-git-send-email-spearce@spearce.org>
The pack index v2 file format uses a 64 bit network byte order
element for the 64 bit offset table. We already have code to
read these elements, but we do not yet have code to write such
an element to an output file.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
org.spearce.jgit/src/org/spearce/jgit/util/NB.java | 37 ++++++++++++++++++++
1 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
index 61877b8..6bb65d4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
@@ -172,6 +172,43 @@ public final class NB {
intbuf[offset] = (byte) v;
}
+ /**
+ * Write a 64 bit integer as a sequence of 8 bytes (network byte order).
+ *
+ * @param intbuf
+ * buffer to write the 48bytes of data into.
+ * @param offset
+ * position within the buffer to begin writing to. This position
+ * and the next 7 bytes after it (for a total of 8 bytes) will be
+ * replaced.
+ * @param v
+ * the value to write.
+ */
+ public static void encodeInt64(final byte[] intbuf, final int offset, long v) {
+ intbuf[offset + 7] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 6] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 5] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 4] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 3] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 2] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset + 1] = (byte) v;
+ v >>>= 8;
+
+ intbuf[offset] = (byte) v;
+ }
+
private NB() {
// Don't create instances of a static only utility.
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 06/10] Reuse the magic tOc constant for pack index headers
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-6-git-send-email-spearce@spearce.org>
We need this constant to detect version 2 index files at read time,
but we also need it to create version 2 index files.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackIndex.java | 3 ++-
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 3 +++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
index 3935d4f..ef4b13d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndex.java
@@ -42,6 +42,7 @@ import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Iterator;
import org.spearce.jgit.util.NB;
@@ -104,7 +105,7 @@ public abstract class PackIndex implements Iterable<PackIndex.MutableEntry> {
}
private static boolean isTOC(final byte[] h) {
- return h[0] == -1 && h[1] == 't' && h[2] == 'O' && h[3] == 'c';
+ return Arrays.equals(h, PackIndexWriter.TOC);
}
/**
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 473e6cf..c9b27d2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
@@ -55,6 +55,9 @@ import org.spearce.jgit.util.NB;
* to the byte offset within the pack where the object's data can be read.
*/
public abstract class PackIndexWriter {
+ /** Magic constant indicating post-version 1 format. */
+ protected static final byte[] TOC = { -1, 't', 'O', 'c' };
+
/**
* Create a new writer for the oldest (most widely understood) format.
* <p>
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 05/10] Refactor pack index writing to a common API
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-5-git-send-email-spearce@spearce.org>
To repack a repository or to push a pack file over a dumb transport
we need to create a .idx file to match the .pack. This means
the index writing features of transport.IndexPack are needed by
PackWriter, and/or dumb protocol transports.
This refactoring creates a PackIndexWriter class structure that
mirrors the PackIndex (reader) structure we already have in place
for PackIndexV1 and PackIndexV2.
At present only PackIndexV1 is supported as we do not have access
to the CRC data necessary to write PackIndexV2 style files.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 243 ++++++++++++++++++++
.../org/spearce/jgit/lib/PackIndexWriterV1.java | 78 +++++++
.../src/org/spearce/jgit/transport/IndexPack.java | 37 +---
3 files changed, 330 insertions(+), 28 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV1.java
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
new file mode 100644
index 0000000..473e6cf
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
@@ -0,0 +1,243 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.BufferedOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.security.DigestOutputStream;
+import java.util.List;
+
+import org.spearce.jgit.transport.PackedObjectInfo;
+import org.spearce.jgit.util.NB;
+
+/**
+ * Creates a table of contents to support random access by {@link PackFile}.
+ * <p>
+ * Pack index files (the <code>.idx</code> suffix in a pack file pair)
+ * provides random access to any object in the pack by associating an ObjectId
+ * to the byte offset within the pack where the object's data can be read.
+ */
+public abstract class PackIndexWriter {
+ /**
+ * Create a new writer for the oldest (most widely understood) format.
+ * <p>
+ * This method selects an index format that can accurate describe the
+ * supplied objects and that will be the most compatible format with older
+ * Git implementations.
+ * <p>
+ * Index version 1 is widely recognized by all Git implementations, but
+ * index version 2 (and later) is not as well recognized as it was
+ * introduced more than a year later. Index version 1 can only be used if
+ * the resulting pack file is under 4 gigabytes in size; packs larger than
+ * that limit must use index version 2.
+ *
+ * @param dst
+ * the stream the index data will be written to. If not already
+ * buffered it will be automatically wrapped in a buffered
+ * stream. Callers are always responsible for closing the stream.
+ * @param objs
+ * the objects the caller needs to store in the index. Entries
+ * will be examined until a format can be conclusively selected.
+ * @return a new writer to output an index file of the requested format to
+ * the supplied stream.
+ * @throws IllegalArgumentException
+ * no recognized pack index version can support the supplied
+ * objects. This is likely a bug in the implementation.
+ */
+ @SuppressWarnings("fallthrough")
+ public static PackIndexWriter createOldestPossible(final OutputStream dst,
+ final List<PackedObjectInfo> objs) {
+ int version = 1;
+ LOOP: for (final PackedObjectInfo oe : objs) {
+ switch (version) {
+ case 1:
+ if (PackIndexWriterV1.canStore(oe))
+ continue;
+ version = 2;
+ case 2:
+ break LOOP;
+ }
+ }
+ return createVersion(dst, version);
+ }
+
+ /**
+ * Create a new writer instance for a specific index format version.
+ *
+ * @param dst
+ * the stream the index data will be written to. If not already
+ * buffered it will be automatically wrapped in a buffered
+ * stream. Callers are always responsible for closing the stream.
+ * @param version
+ * index format version number required by the caller. Exactly
+ * this formatted version will be written.
+ * @return a new writer to output an index file of the requested format to
+ * the supplied stream.
+ * @throws IllegalArgumentException
+ * the version requested is not supported by this
+ * implementation.
+ */
+ public static PackIndexWriter createVersion(final OutputStream dst,
+ final int version) {
+ switch (version) {
+ case 1:
+ return new PackIndexWriterV1(dst);
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported pack index version " + version);
+ }
+ }
+
+ /** The index data stream we are responsible for creating. */
+ protected final DigestOutputStream out;
+
+ /** A temporary buffer for use during IO to {link #out}. */
+ protected final byte[] tmp;
+
+ /** The entries this writer must pack. */
+ protected List<PackedObjectInfo> entries;
+
+ /** SHA-1 checksum for the entire pack data. */
+ protected byte[] packChecksum;
+
+ /**
+ * Create a new writer instance.
+ *
+ * @param dst
+ * the stream this instance outputs to. If not already buffered
+ * it will be automatically wrapped in a buffered stream.
+ */
+ protected PackIndexWriter(final OutputStream dst) {
+ out = new DigestOutputStream(dst instanceof BufferedOutputStream ? dst
+ : new BufferedOutputStream(dst), Constants.newMessageDigest());
+ tmp = new byte[4 + Constants.OBJECT_ID_LENGTH];
+ }
+
+ /**
+ * Write all object entries to the index stream.
+ * <p>
+ * After writing the stream passed to the factory is flushed but remains
+ * open. Callers are always responsible for closing the output stream.
+ *
+ * @param toStore
+ * sorted list of objects to store in the index. The caller must
+ * have previously sorted the list using {@link PackedObjectInfo}'s
+ * native {@link Comparable} implementation.
+ * @param packDataChecksum
+ * checksum signature of the entire pack data content. This is
+ * traditionally the last 20 bytes of the pack file's own stream.
+ * @throws IOException
+ * 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,
+ final byte[] packDataChecksum) throws IOException {
+ entries = toStore;
+ packChecksum = packDataChecksum;
+ writeImpl();
+ out.flush();
+ }
+
+ /**
+ * Writes the index file to {@link #out}.
+ * <p>
+ * Implementations should go something like:
+ *
+ * <pre>
+ * writeFanOutTable();
+ * for (final PackedObjectInfo po : entries)
+ * writeOneEntry(po);
+ * writeChecksumFooter();
+ * </pre>
+ *
+ * <p>
+ * Where the logic for <code>writeOneEntry</code> is specific to the index
+ * format in use. Additional headers/footers may be used if necessary and
+ * the {@link #entries} collection may be iterated over more than once if
+ * necessary. Implementors therefore have complete control over the data.
+ *
+ * @throws IOException
+ * an error occurred while writing to the output stream, or this
+ * index format cannot store the object data supplied.
+ */
+ protected abstract void writeImpl() throws IOException;
+
+ /**
+ * Output the standard 256 entry first-level fan-out table.
+ * <p>
+ * The fan-out table is 4 KB in size, holding 256 32-bit unsigned integer
+ * counts. Each count represents the number of objects within this index
+ * whose {@link ObjectId#getFirstByte()} matches the count's position in the
+ * fan-out table.
+ *
+ * @throws IOException
+ * an error occurred while writing to the output stream.
+ */
+ protected void writeFanOutTable() throws IOException {
+ final int[] fanout = new int[256];
+ for (final PackedObjectInfo po : entries)
+ fanout[po.getFirstByte() & 0xff]++;
+ for (int i = 1; i < 256; i++)
+ fanout[i] += fanout[i - 1];
+ for (final int n : fanout) {
+ NB.encodeInt32(tmp, 0, n);
+ out.write(tmp, 0, 4);
+ }
+ }
+
+ /**
+ * Output the standard two-checksum index footer.
+ * <p>
+ * The standard footer contains two checksums (20 byte SHA-1 values):
+ * <ol>
+ * <li>Pack data checksum - taken from the last 20 bytes of the pack file.</li>
+ * <li>Index data checksum - checksum of all index bytes written, including
+ * the pack data checksum above.</li>
+ * </ol>
+ *
+ * @throws IOException
+ * an error occurred while writing to the output stream.
+ */
+ protected void writeChecksumFooter() throws IOException {
+ out.write(packChecksum);
+ out.on(false);
+ out.write(out.getMessageDigest().digest());
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV1.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV1.java
new file mode 100644
index 0000000..b790a28
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV1.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+import org.spearce.jgit.transport.PackedObjectInfo;
+import org.spearce.jgit.util.NB;
+
+/**
+ * Creates the version 1 (old style) pack table of contents files.
+ *
+ * @see PackIndexWriter
+ * @see PackIndexV1
+ */
+class PackIndexWriterV1 extends PackIndexWriter {
+ static boolean canStore(final PackedObjectInfo oe) {
+ // We are limited to 4 GB per pack as offset is 32 bit unsigned int.
+ //
+ return oe.getOffset() >>> 1 < Integer.MAX_VALUE;
+ }
+
+ PackIndexWriterV1(final OutputStream dst) {
+ super(dst);
+ }
+
+ @Override
+ protected void writeImpl() throws IOException {
+ writeFanOutTable();
+
+ for (final PackedObjectInfo oe : entries) {
+ if (!canStore(oe))
+ throw new IOException("Pack too large for index version 1");
+ NB.encodeInt32(tmp, 0, (int) oe.getOffset());
+ oe.copyRawTo(tmp, 4);
+ out.write(tmp);
+ }
+
+ writeChecksumFooter();
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index ad4bcae..60e0bce 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -38,7 +38,6 @@
package org.spearce.jgit.transport;
-import java.io.BufferedOutputStream;
import java.io.EOFException;
import java.io.File;
import java.io.FileOutputStream;
@@ -49,6 +48,7 @@ import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
import java.util.zip.DataFormatException;
import java.util.zip.Deflater;
import java.util.zip.Inflater;
@@ -61,6 +61,7 @@ import org.spearce.jgit.lib.MutableObjectId;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.ObjectIdMap;
import org.spearce.jgit.lib.ObjectLoader;
+import org.spearce.jgit.lib.PackIndexWriter;
import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.util.NB;
@@ -441,34 +442,14 @@ public class IndexPack {
private void writeIdx() throws IOException {
Arrays.sort(entries, 0, entryCount);
- final int[] fanout = new int[256];
- for (int i = 0; i < entryCount; i++)
- fanout[entries[i].getFirstByte() & 0xff]++;
- for (int i = 1; i < 256; i++)
- fanout[i] += fanout[i - 1];
-
- final BufferedOutputStream os = new BufferedOutputStream(
- new FileOutputStream(dstIdx), BUFFER_SIZE);
+ List<PackedObjectInfo> list = Arrays.asList(entries);
+ if (entryCount < entries.length)
+ list = list.subList(0, entryCount);
+
+ final FileOutputStream os = new FileOutputStream(dstIdx);
try {
- final byte[] rawoe = new byte[4 + Constants.OBJECT_ID_LENGTH];
- final MessageDigest d = Constants.newMessageDigest();
- for (int i = 0; i < 256; i++) {
- NB.encodeInt32(rawoe, 0, fanout[i]);
- os.write(rawoe, 0, 4);
- d.update(rawoe, 0, 4);
- }
- for (int i = 0; i < entryCount; i++) {
- final PackedObjectInfo oe = entries[i];
- if (oe.getOffset() >>> 1 > Integer.MAX_VALUE)
- throw new IOException("Pack too large for index version 1");
- NB.encodeInt32(rawoe, 0, (int) oe.getOffset());
- oe.copyRawTo(rawoe, 4);
- os.write(rawoe);
- d.update(rawoe);
- }
- os.write(packcsum);
- d.update(packcsum);
- os.write(d.digest());
+ PackIndexWriter.createOldestPossible(os, list)
+ .write(list, packcsum);
} finally {
os.close();
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 04/10] Document PackedObjectInfo and make it public for reuse
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-4-git-send-email-spearce@spearce.org>
Classes outside of transport may wish to use this abstraction,
so we mark it public, expose the offset field, and document
as much of the API as possible.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/transport/PackedObjectInfo.java | 33 ++++++++++++++++++--
1 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
index eaedee9..58feada 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
@@ -41,7 +41,14 @@ package org.spearce.jgit.transport;
import org.spearce.jgit.lib.AnyObjectId;
import org.spearce.jgit.lib.ObjectId;
-class PackedObjectInfo extends ObjectId {
+/**
+ * Description of an object stored in a pack file, including offset.
+ * <p>
+ * When objects are stored in packs Git needs the ObjectId and the offset
+ * (starting position of the object data) to perform random-access reads of
+ * objects from the pack. This extension of ObjectId includes the offset.
+ */
+public class PackedObjectInfo extends ObjectId {
private long offset;
PackedObjectInfo(final long headerOffset, final AnyObjectId id) {
@@ -50,10 +57,30 @@ class PackedObjectInfo extends ObjectId {
}
/**
- * @return offset in pack when object has been already written, or -1 if it
+ * Create a new structure to remember information about an object.
+ *
+ * @param id
+ * the identity of the object the new instance tracks.
+ */
+ public PackedObjectInfo(final AnyObjectId id) {
+ super(id);
+ }
+
+ /**
+ * @return offset in pack when object has been already written, or 0 if it
* has not been written yet
*/
- long getOffset() {
+ public long getOffset() {
return offset;
}
+
+ /**
+ * Set the offset in pack when object has been written to.
+ *
+ * @param offset
+ * offset where written object starts
+ */
+ public void setOffset(final long offset) {
+ this.offset = offset;
+ }
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 03/10] Rename ObjectEntry to PackedObjectInfo
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-3-git-send-email-spearce@spearce.org>
Technically this class contains information about a packed object,
data which is necessary to access it or to create the index record
to support random access to the object's information. Calling it
just ObjectEntry is no longer a very sufficient name.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/transport/IndexPack.java | 26 ++++----
.../org/spearce/jgit/transport/ObjectEntry.java | 59 --------------------
.../spearce/jgit/transport/PackedObjectInfo.java | 59 ++++++++++++++++++++
3 files changed, 72 insertions(+), 72 deletions(-)
delete mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 19a4b7b..ad4bcae 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -129,7 +129,7 @@ public class IndexPack {
private long objectCount;
- private ObjectEntry[] entries;
+ private PackedObjectInfo[] entries;
private int deltaCount;
@@ -208,7 +208,7 @@ public class IndexPack {
try {
readPackHeader();
- entries = new ObjectEntry[(int) objectCount];
+ entries = new PackedObjectInfo[(int) objectCount];
baseById = new ObjectIdMap<ArrayList<UnresolvedDelta>>();
baseByPos = new HashMap<Long, ArrayList<UnresolvedDelta>>();
@@ -280,14 +280,14 @@ public class IndexPack {
progress.endTask();
}
- private void resolveDeltas(final ObjectEntry oe) throws IOException {
+ private void resolveDeltas(final PackedObjectInfo oe) throws IOException {
if (baseById.containsKey(oe)
|| baseByPos.containsKey(new Long(oe.getOffset())))
resolveDeltas(oe.getOffset(), Constants.OBJ_BAD, null, oe);
}
private void resolveDeltas(final long pos, int type, byte[] data,
- ObjectEntry oe) throws IOException {
+ PackedObjectInfo oe) throws IOException {
position(pos);
int c = readFromFile();
final int typeCode = (c >> 4) & 7;
@@ -331,7 +331,7 @@ public class IndexPack {
objectDigest.update((byte) 0);
objectDigest.update(data);
tempObjectId.fromRaw(objectDigest.digest(), 0);
- oe = new ObjectEntry(pos, tempObjectId);
+ oe = new PackedObjectInfo(pos, tempObjectId);
entries[entryCount++] = oe;
}
@@ -339,7 +339,7 @@ public class IndexPack {
}
private void resolveChildDeltas(final long pos, int type, byte[] data,
- ObjectEntry oe) throws IOException {
+ PackedObjectInfo oe) throws IOException {
final ArrayList<UnresolvedDelta> a = baseById.remove(oe);
final ArrayList<UnresolvedDelta> b = baseByPos.remove(new Long(pos));
int ai = 0, bi = 0;
@@ -374,9 +374,9 @@ public class IndexPack {
final ObjectLoader ldr = repo.openObject(baseId);
final byte[] data = ldr.getBytes();
final int typeCode = ldr.getType();
- final ObjectEntry oe;
+ final PackedObjectInfo oe;
- oe = new ObjectEntry(end, baseId);
+ oe = new PackedObjectInfo(end, baseId);
entries[entryCount++] = oe;
packOut.seek(end);
writeWhole(def, typeCode, data);
@@ -432,9 +432,9 @@ public class IndexPack {
}
private void growEntries() {
- final ObjectEntry[] ne;
+ final PackedObjectInfo[] ne;
- ne = new ObjectEntry[(int) objectCount + baseById.size()];
+ ne = new PackedObjectInfo[(int) objectCount + baseById.size()];
System.arraycopy(entries, 0, ne, 0, entryCount);
entries = ne;
}
@@ -458,7 +458,7 @@ public class IndexPack {
d.update(rawoe, 0, 4);
}
for (int i = 0; i < entryCount; i++) {
- final ObjectEntry oe = entries[i];
+ final PackedObjectInfo oe = entries[i];
if (oe.getOffset() >>> 1 > Integer.MAX_VALUE)
throw new IOException("Pack too large for index version 1");
NB.encodeInt32(rawoe, 0, (int) oe.getOffset());
@@ -578,7 +578,7 @@ public class IndexPack {
objectDigest.update((byte) 0);
inflateFromInput(true);
tempObjectId.fromRaw(objectDigest.digest(), 0);
- entries[entryCount++] = new ObjectEntry(pos, tempObjectId);
+ entries[entryCount++] = new PackedObjectInfo(pos, tempObjectId);
}
// Current position of {@link #bOffset} within the entire file.
@@ -739,7 +739,7 @@ public class IndexPack {
final MessageDigest d = Constants.newMessageDigest();
final byte[] oeBytes = new byte[Constants.OBJECT_ID_LENGTH];
for (int i = 0; i < entryCount; i++) {
- final ObjectEntry oe = entries[i];
+ final PackedObjectInfo oe = entries[i];
oe.copyRawTo(oeBytes, 0);
d.update(oeBytes);
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
deleted file mode 100644
index 10a8f4d..0000000
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
- * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
- *
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials provided
- * with the distribution.
- *
- * - Neither the name of the Git Development Community nor the
- * names of its contributors may be used to endorse or promote
- * products derived from this software without specific prior
- * written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
- * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
- * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-package org.spearce.jgit.transport;
-
-import org.spearce.jgit.lib.AnyObjectId;
-import org.spearce.jgit.lib.ObjectId;
-
-class ObjectEntry extends ObjectId {
- private long offset;
-
- ObjectEntry(final long headerOffset, final AnyObjectId id) {
- super(id);
- offset = headerOffset;
- }
-
- /**
- * @return offset in pack when object has been already written, or -1 if it
- * has not been written yet
- */
- long getOffset() {
- return offset;
- }
-}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
new file mode 100644
index 0000000..eaedee9
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.transport;
+
+import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.ObjectId;
+
+class PackedObjectInfo extends ObjectId {
+ private long offset;
+
+ PackedObjectInfo(final long headerOffset, final AnyObjectId id) {
+ super(id);
+ offset = headerOffset;
+ }
+
+ /**
+ * @return offset in pack when object has been already written, or -1 if it
+ * has not been written yet
+ */
+ long getOffset() {
+ return offset;
+ }
+}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 02/10] Make ObjectEntry's position field private
From: Shawn O. Pearce @ 2008-06-24 2:10 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-2-git-send-email-spearce@spearce.org>
PackWriter calls this getOffset() and hides the field as a private
field as part of its related ObjectToPack class. We do that here
as part of ObjectEntry's API so we can later replace that part of
ObjectToPack by inheriting from ObjectEntry.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/transport/IndexPack.java | 11 ++++++-----
.../org/spearce/jgit/transport/ObjectEntry.java | 12 ++++++++++--
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index e182cfc..19a4b7b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -281,8 +281,9 @@ public class IndexPack {
}
private void resolveDeltas(final ObjectEntry oe) throws IOException {
- if (baseById.containsKey(oe) || baseByPos.containsKey(new Long(oe.pos)))
- resolveDeltas(oe.pos, Constants.OBJ_BAD, null, oe);
+ if (baseById.containsKey(oe)
+ || baseByPos.containsKey(new Long(oe.getOffset())))
+ resolveDeltas(oe.getOffset(), Constants.OBJ_BAD, null, oe);
}
private void resolveDeltas(final long pos, int type, byte[] data,
@@ -381,7 +382,7 @@ public class IndexPack {
writeWhole(def, typeCode, data);
end = packOut.getFilePointer();
- resolveChildDeltas(oe.pos, typeCode, data, oe);
+ resolveChildDeltas(oe.getOffset(), typeCode, data, oe);
if (progress.isCancelled())
throw new IOException("Download cancelled during indexing");
}
@@ -458,9 +459,9 @@ public class IndexPack {
}
for (int i = 0; i < entryCount; i++) {
final ObjectEntry oe = entries[i];
- if (oe.pos >>> 1 > Integer.MAX_VALUE)
+ if (oe.getOffset() >>> 1 > Integer.MAX_VALUE)
throw new IOException("Pack too large for index version 1");
- NB.encodeInt32(rawoe, 0, (int) oe.pos);
+ NB.encodeInt32(rawoe, 0, (int) oe.getOffset());
oe.copyRawTo(rawoe, 4);
os.write(rawoe);
d.update(rawoe);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
index 58d2eb2..10a8f4d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
@@ -42,10 +42,18 @@ import org.spearce.jgit.lib.AnyObjectId;
import org.spearce.jgit.lib.ObjectId;
class ObjectEntry extends ObjectId {
- final long pos;
+ private long offset;
ObjectEntry(final long headerOffset, final AnyObjectId id) {
super(id);
- pos = headerOffset;
+ offset = headerOffset;
+ }
+
+ /**
+ * @return offset in pack when object has been already written, or -1 if it
+ * has not been written yet
+ */
+ long getOffset() {
+ return offset;
}
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 01/10] Extract inner ObjectEntry from IndexPack class
From: Shawn O. Pearce @ 2008-06-24 2:09 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
In-Reply-To: <1214273408-70793-1-git-send-email-spearce@spearce.org>
This type is useful not just for IndexPack but also for PackWriter
as it is essentially the index record (ObjectId and offset). Both
data items are necessary to compute a valid index record, so we can
unify the index writing code if we have a common type.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/transport/IndexPack.java | 10 ----
.../org/spearce/jgit/transport/ObjectEntry.java | 51 ++++++++++++++++++++
2 files changed, 51 insertions(+), 10 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index bec211c..e182cfc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -54,7 +54,6 @@ import java.util.zip.Deflater;
import java.util.zip.Inflater;
import org.spearce.jgit.errors.CorruptObjectException;
-import org.spearce.jgit.lib.AnyObjectId;
import org.spearce.jgit.lib.BinaryDelta;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.InflaterCache;
@@ -715,15 +714,6 @@ public class IndexPack {
+ dfe.getMessage());
}
- private static class ObjectEntry extends ObjectId {
- final long pos;
-
- ObjectEntry(final long headerOffset, final AnyObjectId id) {
- super(id);
- pos = headerOffset;
- }
- }
-
private static class UnresolvedDelta {
final long position;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
new file mode 100644
index 0000000..58d2eb2
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ObjectEntry.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.transport;
+
+import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.ObjectId;
+
+class ObjectEntry extends ObjectId {
+ final long pos;
+
+ ObjectEntry(final long headerOffset, final AnyObjectId id) {
+ super(id);
+ pos = headerOffset;
+ }
+}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* [JGIT PATCH 00/10] Support writing pack index version 2
From: Shawn O. Pearce @ 2008-06-24 2:09 UTC (permalink / raw)
To: Robin Rosenberg, Marek Zawirski; +Cc: git, Shawn O. Pearce
This series adds support for writing pack index v2 formatted files
out of IndexPack, and thus any pack we fetch over the network. It
also abstracts out the index formatting functions so we can reuse
them efficiently inside of PackWriter to support creating packs on
the local disk, or to upload packs directly over a dumb transport.
I started down this path because we're missing index v2 support and
that is likely to become a default in the near future for C Git,
and because I want to support push over sftp style URLs now that
Marek has a pack writing implementation.
----
The following changes since commit 535041bba0836a3488fbd465adb171a2c70c9415:
Florian Koeberle (1):
Added the package fnmatch and two exceptions.
are available in the git repository at:
repo.or.cz:/srv/git/egit/spearce.git index-v2
Shawn O. Pearce (10):
Extract inner ObjectEntry from IndexPack class
Make ObjectEntry's position field private
Rename ObjectEntry to PackedObjectInfo
Document PackedObjectInfo and make it public for reuse
Refactor pack index writing to a common API
Reuse the magic tOc constant for pack index headers
Add 64 bit network byte order encoding to NB
Compute packed object entry CRC32 data during IndexPack
Add support for writing pack index v2 files
Default IndexPack to honor pack.indexversion configuration
.../src/org/spearce/jgit/lib/CoreConfig.java | 11 +
.../src/org/spearce/jgit/lib/PackIndex.java | 3 +-
.../src/org/spearce/jgit/lib/PackIndexWriter.java | 267 ++++++++++++++++++++
.../org/spearce/jgit/lib/PackIndexWriterV1.java | 78 ++++++
.../org/spearce/jgit/lib/PackIndexWriterV2.java | 101 ++++++++
.../src/org/spearce/jgit/pgm/IndexPack.java | 4 +
.../src/org/spearce/jgit/transport/IndexPack.java | 225 ++++++++++-------
.../spearce/jgit/transport/PackedObjectInfo.java | 109 ++++++++
org.spearce.jgit/src/org/spearce/jgit/util/NB.java | 37 +++
9 files changed, 743 insertions(+), 92 deletions(-)
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriter.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV1.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/PackIndexWriterV2.java
create mode 100644 org.spearce.jgit/src/org/spearce/jgit/transport/PackedObjectInfo.java
^ permalink raw reply
* Re: git status bug problem.
From: Ben Bennett @ 2008-06-24 2:09 UTC (permalink / raw)
To: git
In-Reply-To: <237967ef0806231849k2aa0c1ebt9f4dee45e2b40896@mail.gmail.com>
Nope, it was an blank test repo .
Did git init
git add *
(which added a few text files)
did git commit -m "just a test"
Modified the file , did git status and I get nothing . git-gui
displays nothing . I think I hosed up something , either some dynamic
lib or something.
doing git status * gives me the files.
On Mon, Jun 23, 2008 at 8:49 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> 2008/6/24 Ben Bennett <benbennett@gmail.com>:
>> I made the initial commit. git-gui doesn't return anything either.
>> Something tells me it isn't getting the cwd. Add git status * it will
>> work correctly , but git-gui doesn't give anything.
>>
>> somewhere@something:~/git_test$ git status
>> # On branch master
>> nothing to commit (working directory clean)
>>
>> somewhere@something:~/git_test$ git status *
>> # On branch master
>> # Changes to be committed:
>> # (use "git reset HEAD <file>..." to unstage)
>> #
>> # modified: test/test2.txt
>> #
>> Thanks,
>> Ben
>
> The file isn't by chance matched by a git ignore rule somewhere? An easy
> way to check is to try and git add the file, it will say you have to use
> -f if it does.
>
> --
> Mikael Magnusson
>
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 1:47 UTC (permalink / raw)
To: git
In-Reply-To: <jeske@willow=01l5V7waFEDjChmh>
As a new user, I'm finding git difficult to trust, because there are operations
which are destructive by default and capable of inadvertently throwing hours or
days of work into the bit bucket.
More problematic, those commands have no discernible pattern that shows their
danger, and they need to be used to do typical everyday things. I'm starting to
feel like I need to use another source control system on top of the git
repository in case I make a mistake. My philosophy is simple, I never never
never want to throw away changes, you shouldn't either. Disks are cheaper than
programmer hours. I can understand wanting to keep things tidy, so I can
understand ways to correct the 'easily visible changes', and also avoid pushing
them to other trees, but I don't understand why git needs to delete things.
For example, the following commands seem capable of totally destroying hours or
days of work. Some of them need to be used regularly to do everyday things, and
there is no pattern among them spelling out danger.
git reset --hard : if another branch name hasn't been created
git rebase
git branch -D <branch> : if branch hasn't been merged
git branch -f <new> : if new exists and hasn't been merged
git branch -m <old> <new> : if new exists and hasn't been merged
I've heard from a couple users that the solution to these problems is to "go
dig what you need out of the log, it's still in there". However, it's only in
there until the log is garbage collected. This either means they are
destructive operations, or we expect "running without ever collecting the log"
to be a valid mode of operation... which I doubt is the case.
Question: How about assuring ALL operations can be done non-destructivly by
default? Then make destructive things require an explicit action that follows a
common pattern.
Suggestion Illustration
-----------------------
Below is one illustration of how these commands could be changed to be entirely
non-destructive, while retaining the current functionality. It also allows you
to destroy stuff if you have lawyers breathing down your neck, or really really
can't afford the hard drive space for a couple lines of text (though I'll
personally make a donation to anyone in this state!) :)
1) Require the "--destroy" flag for ANY git operation which is capable of
destroying data such that it is unrecoverable. A narrow view of this is to only
consider checked-in repository data, and not metadata, such as the location of
a branchname. However, the broad view would be to include all/most metadata.
2) Make a pattern for branch names which are kept in the local tree, not
included in push/pull, not modifiable without first renaming, and not shown by
default when viewing all branch history. For example, "local-<date>-*"
3) make 'git reset --hard <commit>' safe
Automatically commit working set and make a branch name (if necessary) to avoid
changes being thrown away. The branch name could be of the form
"local-<date>-reset-<user>-<date>". If the user really wants to destroy it,
they could use the dangerous version "git reset --hard --destroy", or they
could just "git branch -d --destroy <branchname>" afterwords. Most users would
do neither.
4) make 'git rebase' safe
'rebase' would make a branch name before performing its operation, assuring it
was easy to get back to the previous state. Currently, "git rebase" turns this:
A---B---C topic
/
D---E---F---G master
Into this:
A'--B'--C' topic
/
D---E---F---G master
.. and in turn destroys the original changes. It would instead create this:
A--B--C (x) A'--B'--C' (y)
/ /
D---E------F-------G master
(x) - local-<date>-rebase-topic-<commit for G>
(y) - topic
5) make 'git branch' follow rule 1 above (safe without --destroy)
Using any of the following commands without --destroy would cause them to
create a branch "local-<date>-rename-<old branch name>", to prevet the
destruction of the old branch location:
git branch -d <branchname>
git branch -M <old> <new>
git branch -f <branchname>
^ permalink raw reply
* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 1:47 UTC (permalink / raw)
To: git
In-Reply-To: <jeske@willow=01l5V7waFEDjChmh>
As a new user, I'm finding git difficult to trust, because there are operations
which are destructive by default and capable of inadvertently throwing hours or
days of work into the bit bucket.
More problematic, those commands have no discernible pattern that shows their
danger, and they need to be used to do typical everyday things. I'm starting to
feel like I need to use another source control system on top of the git
repository in case I make a mistake. My philosophy is simple, I never never
never want to throw away changes, you shouldn't either. Disks are cheaper than
programmer hours. I can understand wanting to keep things tidy, so I can
understand ways to correct the 'easily visible changes', and also avoid pushing
them to other trees, but I don't understand why git needs to delete things.
For example, the following commands seem capable of totally destroying hours or
days of work. Some of them need to be used regularly to do everyday things, and
there is no pattern among them spelling out danger.
git reset --hard : if another branch name hasn't been created
git rebase
git branch -D <branch> : if branch hasn't been merged
git branch -f <new> : if new exists and hasn't been merged
git branch -m <old> <new> : if new exists and hasn't been merged
I've heard from a couple users that the solution to these problems is to "go
dig what you need out of the log, it's still in there". However, it's only in
there until the log is garbage collected. This either means they are
destructive operations, or we expect "running without ever collecting the log"
to be a valid mode of operation... which I doubt is the case.
Question: How about assuring ALL operations can be done non-destructivly by
default? Then make destructive things require an explicit action that follows a
common pattern.
Suggestion Illustration
-----------------------
Below is one illustration of how these commands could be changed to be entirely
non-destructive, while retaining the current functionality. It also allows you
to destroy stuff if you have lawyers breathing down your neck, or really really
can't afford the hard drive space for a couple lines of text (though I'll
personally make a donation to anyone in this state!) :)
1) Require the "--destroy" flag for ANY git operation which is capable of
destroying data such that it is unrecoverable. A narrow view of this is to only
consider checked-in repository data, and not metadata, such as the location of
a branchname. However, the broad view would be to include all/most metadata.
2) Make a pattern for branch names which are kept in the local tree, not
included in push/pull, not modifiable without first renaming, and not shown by
default when viewing all branch history. For example, "local-<date>-*"
3) make 'git reset --hard <commit>' safe
Automatically commit working set and make a branch name (if necessary) to avoid
changes being thrown away. The branch name could be of the form
"local-<date>-reset-<user>-<date>". If the user really wants to destroy it,
they could use the dangerous version "git reset --hard --destroy", or they
could just "git branch -d --destroy <branchname>" afterwords. Most users would
do neither.
4) make 'git rebase' safe
'rebase' would make a branch name before performing its operation, assuring it
was easy to get back to the previous state. Currently, "git rebase" turns this:
A---B---C topic
/
D---E---F---G master
Into this:
A'--B'--C' topic
/
D---E---F---G master
.. and in turn destroys the original changes. It would instead create this:
A--B--C (x) A'--B'--C' (y)
/ /
D---E------F-------G master
(x) - local-<date>-rebase-topic-<commit for G>
(y) - topic
5) make 'git branch' follow rule 1 above (safe without --destroy)
Using any of the following commands without --destroy would cause them to
create a branch "local-<date>-rename-<old branch name>", to prevet the
destruction of the old branch location:
git branch -d <branchname>
git branch -M <old> <new>
git branch -f <branchname>
^ permalink raw reply
* Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Lea Wiemann @ 2008-06-24 2:01 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200806240018.20820.jnareb@gmail.com>
Jakub Narebski wrote:
> Although I guess it would be nice to have link to the ticket to the bug
> (report) in WWW::Mechanize::CGI, so we could use cgi_application()
> "again" if it gets fixed...
Sure, I've added a link to
http://rt.cpan.org/Ticket/Display.html?id=36654 for completeness, though
we won't be able to use it if it gets fixed, since we can't break
compatibility to older versions. Btw, you may want to add a reply to
the bug report (if that's possible -- haven't used CPAN's ticket system
before) that "system $application $application" would be a better
solution, since it doesn't suffer from quoting issues.
>> Right -- here's my new version (which still fails if the feeds die or
>> are not well-formed XML -- I'll want that when I change gitweb):
>>
>> [snip code]
>
> Does it make t9503-gitweb.Mechanize.sh fail?
No, my point was that it will fail *in case* it breaks (which is good).
Putting everything into a TODO block would silently ignore issues with
the feeds.
>>>> +GITPERL=${GITPERL:-perl}
>>>> +export GITPERL
Breaking the indecisiveness, I've ripped it out. ;-) We can still test
gitweb under different Perl versions by make'ing it with different
PERL_PATHs, so I think it'll be fine.
-- Lea
^ permalink raw reply
* Re: git status bug problem.
From: Mikael Magnusson @ 2008-06-24 1:49 UTC (permalink / raw)
To: Ben Bennett; +Cc: git
In-Reply-To: <970bc7c80806231837j19458b80x81063745a2ef40c4@mail.gmail.com>
2008/6/24 Ben Bennett <benbennett@gmail.com>:
> I made the initial commit. git-gui doesn't return anything either.
> Something tells me it isn't getting the cwd. Add git status * it will
> work correctly , but git-gui doesn't give anything.
>
> somewhere@something:~/git_test$ git status
> # On branch master
> nothing to commit (working directory clean)
>
> somewhere@something:~/git_test$ git status *
> # On branch master
> # Changes to be committed:
> # (use "git reset HEAD <file>..." to unstage)
> #
> # modified: test/test2.txt
> #
> Thanks,
> Ben
The file isn't by chance matched by a git ignore rule somewhere? An easy
way to check is to try and git add the file, it will say you have to use
-f if it does.
--
Mikael Magnusson
^ permalink raw reply
* Re: git status bug problem.
From: Ben Bennett @ 2008-06-24 1:37 UTC (permalink / raw)
To: git
In-Reply-To: <m3r6aol0u8.fsf@localhost.localdomain>
I made the initial commit. git-gui doesn't return anything either.
Something tells me it isn't getting the cwd. Add git status * it will
work correctly , but git-gui doesn't give anything.
somewhere@something:~/git_test$ git status
# On branch master
nothing to commit (working directory clean)
somewhere@something:~/git_test$ git status *
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: test/test2.txt
#
Thanks,
Ben
On Mon, Jun 23, 2008 at 8:47 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> "Ben Bennett" <benbennett@gmail.com> writes:
>
>> I can't seem to get git status to return that a file has changed. I
>> think my environment is hosed up, but I don't know for sure. My
>> machine is ubuntu 8.04 with kernel 2.6.24-17-generic #1 SMPx86_64
>> GNU/Linux . I set up a test repo and it added one file to it ,
>> modified the file and saved . Doing git status returns nothing, git
>> gui finds no modified files. Doing git add * finds the modified file
>> and stages it. I can get someone a strace for the git status it is
>> only about a 1 page.
>
> You should have got something like the following:
>
> # On branch master
> # Changes to be committed:
> # (use "git reset HEAD <file>..." to unstage)
> #
> # new file: bar
> #
> # Changed but not updated:
> # (use "git add <file>..." to update what will be committed)
> #
> # modified: bar
> #
>
> BTW. you have made initial commit, did you?
>
> --
> Jakub Narebski
> Poland
> ShadeHawk on #git
>
^ permalink raw reply
* Re: [PATCH 2/3] implement some resilience against pack corruptions
From: Shawn O. Pearce @ 2008-06-24 1:33 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.1.10.0806232122180.2979@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> We should be able to fall back to loose objects or alternative packs when
> a pack becomes corrupted. This is especially true when an object exists
> in one pack only as a delta but its base object is corrupted. Currently
> there is no way to retrieve the former object even if the later is
> available in another pack or loose.
Dang, nice timing Nico. We were bitten by something like this at
day-job a couple of weeks back. Adding this sort of support went
onto my internal todo-list. I'm glad you beat me to it. :)
> Yes I've been bitten by the corruption described above...
But sorry to hear about this.
As usual for your patches, the implementation looks quite sane.
Thanks for taking care of it.
--
Shawn.
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Jeff King @ 2008-06-24 1:27 UTC (permalink / raw)
To: Pierre Habouzit
Cc: Junio C Hamano, Linus Torvalds, Johannes Schindelin,
Git Mailing List
In-Reply-To: <20080623233146.GP13395@artemis.madism.org>
On Tue, Jun 24, 2008 at 01:31:46AM +0200, Pierre Habouzit wrote:
> Are we sure argv[argc] is NULL when those are main() arguments ? If
> yes there is no issue, and I should read posix more carefully, but I was
> under the impression that POSIX wasn't enforcing that.
It's not POSIX; it's actually in the C standard. 5.1.2.2.1, paragraph 2:
"argv[argc] shall be a null pointer"
-Peff
^ permalink raw reply
* [PATCH 3/3] test case for pack resilience against corruptions
From: Nicolas Pitre @ 2008-06-24 1:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
t/t5303-pack-corruption-resilience.sh | 194 +++++++++++++++++++++++++++++++++
1 files changed, 194 insertions(+), 0 deletions(-)
create mode 100755 t/t5303-pack-corruption-resilience.sh
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
new file mode 100755
index 0000000..b0f5693
--- /dev/null
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -0,0 +1,194 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Nicolas Pitre
+#
+
+test_description='resilience to pack corruptions with redundant objects'
+. ./test-lib.sh
+
+# Note: the test objects are created with knowledge of their pack encoding
+# to ensure good code path coverage, and to facilitate direct alteration
+# later on. The assumed characteristics are:
+#
+# 1) blob_2 is a delta with blob_1 for base and blob_3 is a delta with blob2
+# for base, such that blob_3 delta depth is 2;
+#
+# 2) the bulk of object data is uncompressible so the text part remains
+# visible;
+#
+# 3) object header is always 2 bytes.
+
+create_test_files() {
+ test-genrandom "foo" 2000 > file_1 &&
+ test-genrandom "foo" 1800 > file_2 &&
+ test-genrandom "foo" 1800 > file_3 &&
+ echo " base " >> file_1 &&
+ echo " delta1 " >> file_2 &&
+ echo " delta delta2 " >> file_3 &&
+ test-genrandom "bar" 150 >> file_2 &&
+ test-genrandom "baz" 100 >> file_3
+}
+
+create_new_pack() {
+ rm -rf .git &&
+ git init &&
+ blob_1=`git hash-object -t blob -w file_1` &&
+ blob_2=`git hash-object -t blob -w file_2` &&
+ blob_3=`git hash-object -t blob -w file_3` &&
+ pack=`printf "$blob_1\n$blob_2\n$blob_3\n" |
+ git pack-objects $@ .git/objects/pack/pack` &&
+ pack=".git/objects/pack/pack-${pack}" &&
+ git verify-pack -v ${pack}.pack
+}
+
+do_corrupt_object() {
+ ofs=`git show-index < ${pack}.idx | grep $1 | cut -f1 -d" "` &&
+ ofs=$(($ofs + $2)) &&
+ chmod +w ${pack}.pack &&
+ dd if=/dev/zero of=${pack}.pack count=1 bs=1 conv=notrunc seek=$ofs &&
+ test_must_fail git verify-pack ${pack}.pack
+}
+
+test_expect_success \
+ 'initial setup validation' \
+ 'create_test_files &&
+ create_new_pack &&
+ git prune-packed &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'create corruption in header of first object' \
+ 'do_corrupt_object $blob_1 0 &&
+ test_must_fail git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_1 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and loose copy of first delta allows for partial recovery' \
+ 'git prune-packed &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ test_must_fail git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'create corruption in data of first object' \
+ 'create_new_pack &&
+ git prune-packed &&
+ chmod +w ${pack}.pack &&
+ perl -i -pe "s/ base /abcdef/" ${pack}.pack &&
+ test_must_fail git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_1 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and loose copy of second object allows for partial recovery' \
+ 'git prune-packed &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ test_must_fail git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'create corruption in header of first delta' \
+ 'create_new_pack &&
+ git prune-packed &&
+ do_corrupt_object $blob_2 0 &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'create corruption in data of first delta' \
+ 'create_new_pack &&
+ git prune-packed &&
+ chmod +w ${pack}.pack &&
+ perl -i -pe "s/ delta1 /abcdefgh/" ${pack}.pack &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'corruption in delta base reference of first delta (OBJ_REF_DELTA)' \
+ 'create_new_pack &&
+ git prune-packed &&
+ do_corrupt_object $blob_2 2 &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... but having a loose copy allows for full recovery' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_2 &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ 'corruption in delta base reference of first delta (OBJ_OFS_DELTA)' \
+ 'create_new_pack --delta-base-offset &&
+ git prune-packed &&
+ do_corrupt_object $blob_2 2 &&
+ git cat-file blob $blob_1 > /dev/null &&
+ test_must_fail git cat-file blob $blob_2 > /dev/null &&
+ test_must_fail git cat-file blob $blob_3 > /dev/null'
+
+test_expect_success \
+ '... and a redundant pack allows for full recovery too' \
+ 'mv ${pack}.idx tmp &&
+ git hash-object -t blob -w file_1 &&
+ git hash-object -t blob -w file_2 &&
+ printf "$blob_1\n$blob_2\n" | git pack-objects .git/objects/pack/pack &&
+ git prune-packed &&
+ mv tmp ${pack}.idx &&
+ git cat-file blob $blob_1 > /dev/null &&
+ git cat-file blob $blob_2 > /dev/null &&
+ git cat-file blob $blob_3 > /dev/null'
+
+test_done
--
1.5.6.GIT
^ permalink raw reply related
* [PATCH 2/3] implement some resilience against pack corruptions
From: Nicolas Pitre @ 2008-06-24 1:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
We should be able to fall back to loose objects or alternative packs when
a pack becomes corrupted. This is especially true when an object exists
in one pack only as a delta but its base object is corrupted. Currently
there is no way to retrieve the former object even if the later is
available in another pack or loose.
This patch allows for a delta to be resolved (with a performance cost)
using a base object from a source other than the pack where that delta
is located. Same thing for non-delta objects: rather than failing
outright, a search is made in other packs or used loose when the
currently active pack has it but corrupted.
Of course git will become extremely noisy with error messages when that
happens. However, if the operation succeeds nevertheless, a simple
'git repack -a -f -d' will "fix" the corrupted repository given that all
corrupted objects have a good duplicate somewhere in the object store,
possibly manually copied from another source.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
Yes I've been bitten by the corruption described above...
cache.h | 2 +
sha1_file.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 78 insertions(+), 16 deletions(-)
diff --git a/cache.h b/cache.h
index a0688c6..8ad29e3 100644
--- a/cache.h
+++ b/cache.h
@@ -645,6 +645,8 @@ extern struct packed_git {
const void *index_data;
size_t index_size;
uint32_t num_objects;
+ uint32_t num_bad_objects;
+ unsigned char *bad_object_sha1;
int index_version;
time_t mtime;
int pack_fd;
diff --git a/sha1_file.c b/sha1_file.c
index fe4ee3e..ffd31c6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -818,6 +818,8 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
p->index_data = NULL;
p->index_size = 0;
p->num_objects = 0;
+ p->num_bad_objects = 0;
+ p->bad_object_sha1 = NULL;
p->pack_size = st.st_size;
p->next = NULL;
p->windows = NULL;
@@ -982,6 +984,18 @@ void reprepare_packed_git(void)
prepare_packed_git();
}
+static void mark_bad_packed_object(struct packed_git *p,
+ const unsigned char *sha1)
+{
+ unsigned i;
+ for (i = 0; i < p->num_bad_objects; i++)
+ if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
+ return;
+ p->bad_object_sha1 = xrealloc(p->bad_object_sha1, 20 * (p->num_bad_objects + 1));
+ hashcpy(p->bad_object_sha1 + 20 * p->num_bad_objects, sha1);
+ p->num_bad_objects++;
+}
+
int check_sha1_signature(const unsigned char *sha1, void *map, unsigned long size, const char *type)
{
unsigned char real_sha1[20];
@@ -1300,20 +1314,17 @@ static off_t get_delta_base(struct packed_git *p,
while (c & 128) {
base_offset += 1;
if (!base_offset || MSB(base_offset, 7))
- die("offset value overflow for delta base object");
+ return 0; /* overflow */
c = base_info[used++];
base_offset = (base_offset << 7) + (c & 127);
}
base_offset = delta_obj_offset - base_offset;
if (base_offset >= delta_obj_offset)
- die("delta base offset out of bound");
+ return 0; /* out of bound */
*curpos += used;
} else if (type == OBJ_REF_DELTA) {
/* The base entry _must_ be in the same pack */
base_offset = find_pack_entry_one(base_info, p);
- if (!base_offset)
- die("failed to find delta-pack base object %s",
- sha1_to_hex(base_info));
*curpos += 20;
} else
die("I am totally screwed");
@@ -1406,6 +1417,9 @@ const char *packed_object_info_detail(struct packed_git *p,
return typename(type);
case OBJ_OFS_DELTA:
obj_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
+ if (!obj_offset)
+ die("pack %s contains bad delta base reference",
+ p->pack_name, type);
if (*delta_chain_length == 0) {
revidx = find_pack_revindex(p, obj_offset);
hashcpy(base_sha1, nth_packed_object_sha1(p, revidx->nr));
@@ -1600,17 +1614,41 @@ static void *unpack_delta_entry(struct packed_git *p,
off_t base_offset;
base_offset = get_delta_base(p, w_curs, &curpos, *type, obj_offset);
+ if (!base_offset) {
+ error("failed to validate delta base reference "
+ "at offset %"PRIuMAX" from %s",
+ (uintmax_t)curpos, p->pack_name);
+ return NULL;
+ }
base = cache_or_unpack_entry(p, base_offset, &base_size, type, 0);
- if (!base)
- die("failed to read delta base object"
- " at %"PRIuMAX" from %s",
- (uintmax_t)base_offset, p->pack_name);
+ if (!base) {
+ /*
+ * We're probably in deep shit, but let's try to fetch
+ * the required base anyway from another pack or loose.
+ * This is costly but should happen only in the presence
+ * of a corrupted pack, and is better than failing outright.
+ */
+ struct revindex_entry *revidx = find_pack_revindex(p, base_offset);
+ const unsigned char *base_sha1 =
+ nth_packed_object_sha1(p, revidx->nr);
+ error("failed to read delta base object %s"
+ " at offset %"PRIuMAX" from %s",
+ sha1_to_hex(base_sha1), (uintmax_t)base_offset,
+ p->pack_name);
+ mark_bad_packed_object(p, base_sha1);
+ base = read_sha1_file(base_sha1, type, &base_size);
+ if (!base)
+ return NULL;
+ }
delta_data = unpack_compressed_entry(p, w_curs, curpos, delta_size);
- if (!delta_data)
- die("failed to unpack compressed delta"
- " at %"PRIuMAX" from %s",
- (uintmax_t)curpos, p->pack_name);
+ if (!delta_data) {
+ error("failed to unpack compressed delta "
+ "at offset %"PRIuMAX" from %s",
+ (uintmax_t)curpos, p->pack_name);
+ free(base);
+ return NULL;
+ }
result = patch_delta(base, base_size,
delta_data, delta_size,
sizep);
@@ -1642,7 +1680,9 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
data = unpack_compressed_entry(p, &w_curs, curpos, *sizep);
break;
default:
- die("unknown object type %i in %s", *type, p->pack_name);
+ data = NULL;
+ error("unknown object type %i at offset %"PRIuMAX" in %s",
+ *type, (uintmax_t)obj_offset, p->pack_name);
}
unuse_pack(&w_curs);
return data;
@@ -1788,6 +1828,13 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons
goto next;
}
+ if (p->num_bad_objects) {
+ unsigned i;
+ for (i = 0; i < p->num_bad_objects; i++)
+ if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
+ goto next;
+ }
+
offset = find_pack_entry_one(sha1, p);
if (offset) {
/*
@@ -1872,11 +1919,24 @@ static void *read_packed_sha1(const unsigned char *sha1,
enum object_type *type, unsigned long *size)
{
struct pack_entry e;
+ void *data;
if (!find_pack_entry(sha1, &e, NULL))
return NULL;
- else
- return cache_or_unpack_entry(e.p, e.offset, size, type, 1);
+ data = cache_or_unpack_entry(e.p, e.offset, size, type, 1);
+ if (!data) {
+ /*
+ * We're probably in deep shit, but let's try to fetch
+ * the required object anyway from another pack or loose.
+ * This should happen only in the presence of a corrupted
+ * pack, and is better than failing outright.
+ */
+ error("failed to read object %s at offset %"PRIuMAX" from %s",
+ sha1_to_hex(sha1), (uintmax_t)e.offset, e.p->pack_name);
+ mark_bad_packed_object(e.p, sha1);
+ data = read_sha1_file(sha1, type, size);
+ }
+ return data;
}
/*
--
1.5.6.GIT
^ permalink raw reply related
* [PATCH 1/3] call init_pack_revindex() lazily
From: Nicolas Pitre @ 2008-06-24 1:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This makes life much easier for next patch, as well as being more efficient
when the revindex is actually not used.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
builtin-pack-objects.c | 2 --
pack-check.c | 1 -
pack-revindex.c | 6 ++++--
pack-revindex.h | 1 -
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 447d492..827673c 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1148,8 +1148,6 @@ static void get_object_details(void)
sorted_by_offset[i] = objects + i;
qsort(sorted_by_offset, nr_objects, sizeof(*sorted_by_offset), pack_offset_sort);
- init_pack_revindex();
-
for (i = 0; i < nr_objects; i++)
check_object(sorted_by_offset[i]);
diff --git a/pack-check.c b/pack-check.c
index f489873..b99a917 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -107,7 +107,6 @@ static void show_pack_info(struct packed_git *p)
nr_objects = p->num_objects;
memset(chain_histogram, 0, sizeof(chain_histogram));
- init_pack_revindex();
for (i = 0; i < nr_objects; i++) {
const unsigned char *sha1;
diff --git a/pack-revindex.c b/pack-revindex.c
index a8aa2cd..cd300bd 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -40,7 +40,7 @@ static int pack_revindex_ix(struct packed_git *p)
return -1 - i;
}
-void init_pack_revindex(void)
+static void init_pack_revindex(void)
{
int num;
struct packed_git *p;
@@ -118,9 +118,11 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
struct pack_revindex *rix;
struct revindex_entry *revindex;
+ if (!pack_revindex_hashsz)
+ init_pack_revindex();
num = pack_revindex_ix(p);
if (num < 0)
- die("internal error: pack revindex uninitialized");
+ die("internal error: pack revindex fubar");
rix = &pack_revindex[num];
if (!rix->revindex)
diff --git a/pack-revindex.h b/pack-revindex.h
index c3527a7..36a514a 100644
--- a/pack-revindex.h
+++ b/pack-revindex.h
@@ -6,7 +6,6 @@ struct revindex_entry {
unsigned int nr;
};
-void init_pack_revindex(void);
struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs);
#endif
--
1.5.6.GIT
^ permalink raw reply related
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Junio C Hamano @ 2008-06-24 0:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Johannes Schindelin, Pierre Habouzit, Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0806231114180.2926@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, 23 Jun 2008, Linus Torvalds wrote:
>>
>> Umm. Helloo, reality.. There are actually very few options that take a
>> flag for their arguments. In particular, the option parsing we really
>> _care_ about (revision parsing - see builtin-blame.c which is exactly
>> where I wanted to convert things) very much DOES NOT.
>
> Actually, I guess "--default" does, but if you try to mix that up with (a)
> a default head that starts with a dash and (b) git-blame, you're doing
> something pretty odd.
>
> And yes, "-n" does too, but if you pass it negative numbers you get what
> you deserve.
>
> The point being, we really _do_ have a real-life existing case for
> PARSE_OPT_CONTINUE_ON_UNKNOWN, which is hard to handle any other way.
> Currently you can literally do
>
> git blame --since=April -b Makefile
>
> and while it's a totally made-up example, it's one I've picked to show
> exactly what does *not* work with my patch that started this whole thread.
>
> And guess what you need to fix it?
>
> If you guessed PARSE_OPT_CONTINUE_ON_UNKNOWN, you win a prize.
With this on top of Pierre's series, and adding PARSE_OPT_SKIP_UNKNOWN to
the obvious place in your patch, "blame --since=April -b Makefile" would
work.
-- >8 --
Subject: [PATCH] parse-options: PARSE_OPT_SKIP_UNKNOWN
---
parse-options.c | 14 ++++++++++----
parse-options.h | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 71a8056..9f1eb65 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -341,20 +341,26 @@ int parse_options(int argc, const char **argv, const struct option *options,
struct parse_opt_ctx_t ctx;
parse_options_start(&ctx, argc, argv, flags);
+
+ again:
switch (parse_options_step(&ctx, options, usagestr)) {
case PARSE_OPT_HELP:
exit(129);
case PARSE_OPT_DONE:
break;
default: /* PARSE_OPT_UNKNOWN */
- if (ctx.argv[0][1] == '-') {
+ if (flags & PARSE_OPT_KEEP_UNKNOWN) {
+ ctx.out[ctx.cpidx++] = ctx.argv[0];
+ ctx.argc--;
+ ctx.argv++;
+ goto again;
+ }
+ if (ctx.argv[0][1] == '-')
error("unknown option `%s'", ctx.argv[0] + 2);
- } else {
+ else
error("unknown switch `%c'", *ctx.opt);
- }
usage_with_options(usagestr, options);
}
-
return parse_options_end(&ctx);
}
diff --git a/parse-options.h b/parse-options.h
index 403794f..30fbf7e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,7 @@ enum parse_opt_type {
enum parse_opt_flags {
PARSE_OPT_KEEP_DASHDASH = 1,
PARSE_OPT_STOP_AT_NON_OPTION = 2,
+ PARSE_OPT_KEEP_UNKNOWN = 4,
};
enum parse_opt_option_flags {
--
1.5.6.49.g112db
^ 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