* Re: [PATCH] Add git-zip-tree to .gitignore
From: Junio C Hamano @ 2006-08-27 22:04 UTC (permalink / raw)
To: Rene Scharfe; +Cc: git
In-Reply-To: <44F17FD5.2030000@lsrfire.ath.cx>
Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
>
> diff --git a/.gitignore b/.gitignore
> index 55cd984..58a7c92 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -124,6 +124,7 @@ git-verify-pack
> git-verify-tag
> git-whatchanged
> git-write-tree
> +git-zip-tree
> git-core-*/?*
> gitweb/gitweb.cgi
> test-date
That is sensible.
Although it was my fault applying it already to "master" without
asking this question first, I started to wonder how many more
archive format people would want, and if it might make more
sense to consolidate git-*-tree into a single frontend with an
option to specify the archiver.
We would obviously need to keep git-tar-tree as a backward
compatibility alias for "git archive-tree -f tar", but doing
things that way we do not have to introduce unbounded number of
new git-{zip,rar,...}-tree programs.
^ permalink raw reply
* Re: Starting to think about sha-256?
From: Johannes Schindelin @ 2006-08-27 22:02 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Krzysztof Halasa, Jeff Garzik, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0608271343120.27779@g5.osdl.org>
Hi,
On Sun, 27 Aug 2006, Linus Torvalds wrote:
> On Sun, 27 Aug 2006, Krzysztof Halasa wrote:
> >
> > > Maybe sha-256 could be considered for the next major-rev of git?
> >
> > Not sure, but _if_ we want it we should do it sooner rather than
> > later.
>
> Modifying git-convert-objects.c to rewrite the regular sha1 into a sha256
> should be fairly straightforward. It's never been used since the early
> days (and has limits like a maximum of a million objects etc that can need
> fixing), but it shouldn't be "fundamentally hard" per se.
But what about signed tags? (This issue has come up before, but never has
been adressed.)
I also thought about supporting hybrid hashes, i.e. that older objects
still can be hashed with SHA-1. Alas, a simple thought experiment
demonstrates how silly that idea is: most of the objects will not change
between two revisions, and they'd have to be rehashed with SHA-256 (or
whatever we decide upon) anyway, so hybrids would do no good.
A better idea would be to increment the repository version, and expect
SHA-1 for version 1, SHA-256 for version >= 2.
However, I could imagine that we do not need this huge change (it would
break _many_ setups). The breakthrough was announced last Tuesday, and it
involved 75% payload, i.e. to fake a new -- say -- git.c, one would need
to enlarge git.c by a factor 4, and you would see a lot of gibberish
inside some comment. (Note that I did not listen to the talk myself, this
is all deducted from the scarce information which is available via the
'net.)
Even if the breakthrough really comes to full SHA-1, you still have to add
_at least_ 20 bytes of gibberish. Which would be harder to spot, but it
would be spotted.
This made me think about the use of hashes in git. Why do we need a hash
here (in no particular order):
1) integrity checking,
2) fast lookup,
3) identifying objects (related to (2)),
4) trust.
Except for (4), I do not see why SHA-1 -- even if broken -- should not be
adequate. It is not like somebody found out that all JPGs tend to have
similar hashes so that collisions are more likely.
And thinking about trust: The hash is augmented by thinking persons. It is
not like you blindly trust a person forever. You build up trust, and once
you were failed, the trust is lost, and very hard to build up again. So,
you just would try to get all objects again from somebody you still trust,
and never pull from the loser^H^H^H^H^Huntrusted person again. Ever.
Besides, as has been pointed out several times, a dishonest person could
try to sneak bad code into your repository _regardless_ of a secure hash.
So: Do we really need a secure hash, or do we need an adequate hash, and
just happen to use one which was intended as a secure hash, but no longer
is?
Ciao,
Dscho
^ permalink raw reply
* [PATCH/RFC] gitweb: Make git_print_log generic; git_print_simplified_log uses it
From: Jakub Narebski @ 2006-08-27 21:55 UTC (permalink / raw)
To: git
Collapse git_print_log and git_print_simplified_log into one
subroutine git_print_log. git_print_simplified_log now simply calls
git_print_log with proper options.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch doesn't change output any, but it makes it easy
to add signoff lines (to not remove them) to commitdiff and log views
(as requested by Linus).
The RFC is about style of git_print_log function. Is it a good idea
and good implementation to pass miscelaneus options as hash values
instead of using fixed order of parameters, and passing 1 or undef?
Is it a good naming of parameters like '-remove_title', or would
'remove_title' or 'remove-title' be better?
gitweb/gitweb.perl | 63 ++++++++++++++++++++++++++--------------------------
1 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ae18c27..b6c1620 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1364,9 +1364,15 @@ sub git_print_page_path {
}
}
-sub git_print_log {
+# sub git_print_log (\@;%) {
+sub git_print_log ($;%) {
my $log = shift;
+ my %opts = @_;
+ if ($opts{'-remove_title'}) {
+ # remove title, i.e. first line of log
+ shift @$log;
+ }
# remove leading empty lines
while (defined $log->[0] && $log->[0] eq "") {
shift @$log;
@@ -1376,6 +1382,19 @@ sub git_print_log {
my $signoff = 0;
my $empty = 0;
foreach my $line (@$log) {
+ if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
+ $signoff = 1;
+ if (! $opts{'-remove_signoff'}) {
+ print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
+ next;
+ } else {
+ # remove signoff lines
+ next;
+ }
+ } else {
+ $signoff = 0;
+ }
+
# print only one empty line
# do not print empty line after signoff
if ($line eq "") {
@@ -1384,13 +1403,13 @@ sub git_print_log {
} else {
$empty = 0;
}
- if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
- $signoff = 1;
- print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
- } else {
- $signoff = 0;
- print format_log_line_html($line) . "<br/>\n";
- }
+
+ print format_log_line_html($line) . "<br/>\n";
+ }
+
+ if ($opts{'-final_empty_line'}) {
+ # end with single empty line
+ print "<br/>\n" unless $empty;
}
}
@@ -1398,30 +1417,10 @@ sub git_print_simplified_log {
my $log = shift;
my $remove_title = shift;
- shift @$log if $remove_title;
- # remove leading empty lines
- while (defined $log->[0] && $log->[0] eq "") {
- shift @$log;
- }
-
- # simplify and print log
- my $empty = 0;
- foreach my $line (@$log) {
- # remove signoff lines
- if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
- next;
- }
- # print only one empty line
- if ($line eq "") {
- next if $empty;
- $empty = 1;
- } else {
- $empty = 0;
- }
- print format_log_line_html($line) . "<br/>\n";
- }
- # end with single empty line
- print "<br/>\n" unless $empty;
+ git_print_log($log,
+ -final_empty_line=> 1,
+ -remove_signoff => 1,
+ -remove_title => $remove_title);
}
## ......................................................................
--
1.4.1.1
^ permalink raw reply related
* Re: Problem with pack
From: Junio C Hamano @ 2006-08-27 21:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0608271102450.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> NOTE! This is all assuming my theory that a packed entry was broken in the
> first place was correct. We obviously still don't _know_ what the problem
> was. So far it's just a theory.
But it is a good theory. More plausible than alpha particle
hitting the output buffer of zlib at the right moment, although
the effects are the same ;-).
> So it might well be the case that we should simply add an extra integrity
> check to the raw data copy in builtin-pack-objects.c: write_object().
I would agree that it is a sensible thing to do to insert check
at places shown in the attached. The revalidate_pack_piece()
would:
- decode object header to make sure it decodes to sensible
enum object_type value from the start of the buffer given as
its first argument;
- if it is of type OBJ_DELTA, skip 20-byte base object name;
- the second argument is the length of the piece -- make sure
the above steps did not require more than the length;
- make sure the remainder is sane by running inflate() into
void. When fed the remainder in full, inflate() should
return Z_OK.
For the first step we need to refactor unpack_object_header() in
sha1_file.c a tiny bit to reuse it.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 46f524d..0521cad 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -276,6 +282,7 @@ static unsigned long write_object(struct
map = map_sha1_file(entry->sha1, &mapsize);
if (map && !legacy_loose_object(map)) {
/* We can copy straight into the pack file */
+ revalidate_pack_piece(map, mapsize);
sha1write(f, map, mapsize);
munmap(map, mapsize);
written++;
@@ -319,6 +326,7 @@ static unsigned long write_object(struct
datalen = find_packed_object_size(p, entry->in_pack_offset);
buf = (char *) p->pack_base + entry->in_pack_offset;
+ revalidate_pack_piece(buf, datalen);
sha1write(f, buf, datalen);
unuse_packed_git(p);
hdrlen = 0; /* not really */
^ permalink raw reply related
* Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
From: Jon Loeliger @ 2006-08-27 21:49 UTC (permalink / raw)
To: git
The other day, Junio lamented:
> I have to admit that I kinda liked JDL's simpler one first (and
> it has been in production use for some time). We'll see.
I think the two aspects of my implementation that are
favorable are the slightly more general table-driven string
interplotion routine and the generalization of the interface
to the upload() call here:
@@ -310,8 +377,14 @@ #endif
if (len && line[len-1] == '\n')
line[--len] = 0;
- if (!strncmp("git-upload-pack ", line, 16))
- return upload(line+16);
+ if (len != pktlen) {
+ parse_extra_args(line + len + 1, pktlen - len - 1);
+ }
+
+ if (!strncmp("git-upload-pack ", line, 16)) {
+ interp_table[INTERP_SLOT_DIR].value = line+16;
+ return upload(interp_table);
+ }
Naturally, I only placed entries into the interpolation table
that I needed to get my code working, but it could easily be
extended and filled with additional entries such as the %u for
user paths and %IP for IP address, etc. I might even recommend
some form of lower-case-izing option too.
In any event, we should clearly attempt to unify my proposed
implementation with Pierre's proposal.
jdl
^ permalink raw reply
* [PATCH] gitweb: Use @diff_opts, default ('M'), as git-diff and git-diff-tree paramete
From: Jakub Narebski @ 2006-08-27 21:49 UTC (permalink / raw)
To: git
Added new global configuration variable @diff_opts, which holds
additional options (parameters) to git-diff and git-diff-tree, usually
dealing rename/copying detection. Default value is '-M', taken from
git_commit subroutine. Description of options and their approximate
cost by Junio C Hamano.
Changes:
* git_commitdiff, git_blobdiff and git_blobdiff_plain now use '-M'
instead of '-M', '-C'
* git-diff now uses the same options as git-diff-tree
* git_comittdiff_plain now uses '-M' instead of '-B'
and is now rename-aware
* git_rss uses now '-M' instead of ()
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
As requested, gitweb now uses '-M' without '-C' as option
to git-diff-tree thorough the script.
I'm not that sure about last chunk, changing git_rss. I didn't review
that function, and I'm not sure if we should add '-M' there...
I've added Junio to sob, because the comments are his.
gitweb/gitweb.perl | 33 ++++++++++++++++++++++++---------
1 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6918947..99c74f1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -141,6 +141,16 @@ sub feature_snapshot {
return ($ctype, $suffix, $command);
}
+# rename detection options for git-diff and git-diff-tree
+# - default is '-M', with the cost proportional to
+# (number of removed files) * (number of new files).
+# - more costly is '-C' (or '-C', '-M'), with the cost proportional to
+# (number of changed files + number of removed files) * (number of new files)
+# - even more costly is '-C', '--find-copies-harder' with cost
+# (number of files in the original tree) * (number of new files)
+# - one might want to include '-B' option, e.g. '-B', '-M'
+our @diff_opts = ('-M'); # taken from git_commit
+
our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
require $GITWEB_CONFIG if -e $GITWEB_CONFIG;
@@ -2613,7 +2623,7 @@ sub git_commit {
if (!defined $parent) {
$parent = "--root";
}
- open my $fd, "-|", $GIT, "diff-tree", '-r', '-M', $parent, $hash
+ open my $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts, $parent, $hash
or die_error(undef, "Open git-diff-tree failed");
my @difftree = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading git-diff-tree failed");
@@ -2720,7 +2730,7 @@ sub git_blobdiff {
if (defined $hash_base && defined $hash_parent_base) {
if (defined $file_name) {
# read raw output
- open $fd, "-|", $GIT, "diff-tree", '-r', '-M', '-C', $hash_parent_base, $hash_base,
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base,
"--", $file_name
or die_error(undef, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
@@ -2735,7 +2745,7 @@ sub git_blobdiff {
}
# read filtered raw output
- open $fd, "-|", $GIT, "diff-tree", '-r', '-M', '-C', $hash_parent_base, $hash_base
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base
or die_error(undef, "Open git-diff-tree failed");
@difftree =
# ':100644 100644 03b21826... 3b93d5e7... M ls-files.c'
@@ -2769,7 +2779,8 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", $GIT, "diff-tree", '-r', '-p', '-M', '-C', $hash_parent_base, $hash_base,
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts,
+ '-p', $hash_parent_base, $hash_base,
"--", $file_name
or die_error(undef, "Open git-diff-tree failed");
}
@@ -2804,7 +2815,7 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash
+ open $fd, "-|", $GIT, "diff", '-p', @diff_opts, $hash_parent, $hash
or die_error(undef, "Open git-diff failed");
} else {
die_error('404 Not Found', "Missing one of the blob diff parameters")
@@ -2889,7 +2900,7 @@ sub git_commitdiff {
my $fd;
my @difftree;
if ($format eq 'html') {
- open $fd, "-|", $GIT, "diff-tree", '-r', '-M', '-C',
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts,
"--patch-with-raw", "--full-index", $hash_parent, $hash
or die_error(undef, "Open git-diff-tree failed");
@@ -2900,7 +2911,8 @@ sub git_commitdiff {
}
} elsif ($format eq 'plain') {
- open $fd, "-|", $GIT, "diff-tree", '-r', '-p', '-B', $hash_parent, $hash
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts,
+ '-p', $hash_parent, $hash
or die_error(undef, "Open git-diff-tree failed");
} else {
@@ -3209,9 +3221,12 @@ XML
last;
}
my %cd = parse_date($co{'committer_epoch'});
- open $fd, "-|", $GIT, "diff-tree", '-r', $co{'parent'}, $co{'id'} or next;
+ open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts,
+ $co{'parent'}, $co{'id'}
+ or next;
my @difftree = map { chomp; $_ } <$fd>;
- close $fd or next;
+ close $fd
+ or next;
print "<item>\n" .
"<title>" .
sprintf("%d %s %02d:%02d", $cd{'mday'}, $cd{'month'}, $cd{'hour'}, $cd{'minute'}) . " - " . esc_html($co{'title'}) .
--
1.4.1.1
^ permalink raw reply related
* [PATCH 1/2] gitweb: Remove unused git_get_{preceding,following}_references
From: Jakub Narebski @ 2006-08-27 21:44 UTC (permalink / raw)
To: git
In-Reply-To: <200608272343.14653.jnareb@gmail.com>
Remove unused (and with errors in implementation)
git_get_{preceding,following}_references subroutines.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 51 ---------------------------------------------------
1 files changed, 0 insertions(+), 51 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ba5024a..6918947 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -784,57 +784,6 @@ sub git_get_references {
return \%refs;
}
-sub git_get_following_references {
- my $hash = shift || return undef;
- my $type = shift;
- my $base = shift || $hash_base || "HEAD";
-
- my $refs = git_get_references($type);
- open my $fd, "-|", $GIT, "rev-list", $base
- or return undef;
- my @commits = map { chomp; $_ } <$fd>;
- close $fd
- or return undef;
-
- my @reflist;
- my $lastref;
-
- foreach my $commit (@commits) {
- foreach my $ref (@{$refs->{$commit}}) {
- $lastref = $ref;
- push @reflist, $lastref;
- }
- if ($commit eq $hash) {
- last;
- }
- }
-
- return wantarray ? @reflist : $lastref;
-}
-
-sub git_get_preceding_references {
- my $hash = shift || return undef;
- my $type = shift;
-
- my $refs = git_get_references($type);
- open my $fd, "-|", $GIT, "rev-list", $hash
- or return undef;
- my @commits = map { chomp; $_ } <$fd>;
- close $fd
- or return undef;
-
- my @reflist;
-
- foreach my $commit (@commits) {
- foreach my $ref (@{$refs->{$commit}}) {
- return $ref unless wantarray;
- push @reflist, $ref;
- }
- }
-
- return @reflist;
-}
-
sub git_get_rev_name_tags {
my $hash = shift || return undef;
--
1.4.1.1
^ permalink raw reply related
* [PATCH 2/2] gitweb: Remove git_to_hash function
From: Jakub Narebski @ 2006-08-27 21:45 UTC (permalink / raw)
To: git
In-Reply-To: <200608272343.14653.jnareb@gmail.com>
Remove git_to_hash function, which was to translate symbolic reference
to hash, and it's use in git_blobdiff. We don't try so hard to guess
filename if it was not provided.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 27 +++------------------------
1 files changed, 3 insertions(+), 24 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 99c74f1..ae18c27 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -635,26 +635,6 @@ sub git_get_hash_by_path {
return $3;
}
-# converts symbolic name to hash
-sub git_to_hash {
- my @params = @_;
- return undef unless @params;
-
- open my $fd, "-|", $GIT, "rev-parse", @params
- or return undef;
- my @hashes = map { chomp; $_ } <$fd>;
- close $fd;
-
- if (wantarray) {
- return @hashes;
- } elsif (scalar(@hashes) == 1) {
- # single hash
- return $hashes[0];
- } else {
- return \@hashes;
- }
-}
-
## ......................................................................
## git utility functions, directly accessing git repository
@@ -2739,10 +2719,9 @@ sub git_blobdiff {
@difftree
or die_error('404 Not Found', "Blob diff not found");
- } elsif (defined $hash) { # try to find filename from $hash
- if ($hash !~ /[0-9a-fA-F]{40}/) {
- $hash = git_to_hash($hash);
- }
+ } elsif (defined $hash &&
+ $hash =~ /[0-9a-fA-F]{40}/) {
+ # try to find filename from $hash
# read filtered raw output
open $fd, "-|", $GIT, "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base
--
1.4.1.1
^ permalink raw reply related
* [PATCH 0/2] gitweb: Get rid of failed experiments ;-)
From: Jakub Narebski @ 2006-08-27 21:43 UTC (permalink / raw)
To: git
Remove unused subroutines or subroutines which don't work correctly from
gitweb.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Starting to think about sha-256?
From: Krzysztof Halasa @ 2006-08-27 21:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jeff Garzik, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0608271343120.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> Modifying git-convert-objects.c to rewrite the regular sha1 into a sha256
> should be fairly straightforward. It's never been used since the early
> days (and has limits like a maximum of a million objects etc that can need
> fixing), but it shouldn't be "fundamentally hard" per se.
Sure. I was rather thinking of rapidly increasing number of git
repositories, each with growing history.
--
Krzysztof Halasa
^ permalink raw reply
* Re: Starting to think about sha-256?
From: Linus Torvalds @ 2006-08-27 20:46 UTC (permalink / raw)
To: Krzysztof Halasa; +Cc: Jeff Garzik, Git Mailing List
In-Reply-To: <m31wr1exbf.fsf@defiant.localdomain>
On Sun, 27 Aug 2006, Krzysztof Halasa wrote:
>
> > Maybe sha-256 could be considered for the next major-rev of git?
>
> Not sure, but _if_ we want it we should do it sooner rather than
> later.
Modifying git-convert-objects.c to rewrite the regular sha1 into a sha256
should be fairly straightforward. It's never been used since the early
days (and has limits like a maximum of a million objects etc that can need
fixing), but it shouldn't be "fundamentally hard" per se.
Linus
^ permalink raw reply
* Re: Starting to think about sha-256?
From: Krzysztof Halasa @ 2006-08-27 20:30 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Git Mailing List
In-Reply-To: <44F1DCB7.6020804@garzik.org>
Jeff Garzik <jeff@garzik.org> writes:
> Downsides to sha-256:
> * git protocol/storage format change implications.
The only which really matters, I think.
> Maybe sha-256 could be considered for the next major-rev of git?
Not sure, but _if_ we want it we should do it sooner rather than
later.
--
Krzysztof Halasa
^ permalink raw reply
* Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements
From: Jakub Narebski @ 2006-08-27 20:21 UTC (permalink / raw)
To: git
In-Reply-To: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> * gitweb: Remove workaround for git-diff bug fixed in f82cd3c
> . gitweb: Fix typo in git_patchset_body
>
> I think you had separate patches for these; applied.
I think the second one, the one fixing typo ('@$fd>' instead of '= <$fd>')
was not applied. I don't know how this typo got into gitweb, as I have
tested gitweb after the 19-patch series, and this error looks was introduced
in 'next' branch, 157e43b4b0dd5a08eb7a6838192ac58bca62fa5b commit
gitweb: Streamify patch output in git_commitdiff
This error is critical!
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] Refactoring tracing code in "git.c" and "exec_cmd.c".
From: Christian Couder @ 2006-08-27 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmz9qybs5.fsf@assigned-by-dhcp.cox.net>
Le dimanche 27 août 2006 07:42, Junio C Hamano a écrit :
> > +char *sq_quote_argv(const char** argv, int count)
> > +{
> > + char *buf, *to;
> > + int i;
> > + size_t len;
> > +
> > + /* Count argv if needed. */
> > + if (count < 0) {
> > + char **p = (char **)argv;
> > + count = 0;
> > + while (*p++) count++;
> > + }
>
> Wouldn't this be easier to read?
>
> if (count < 0)
> for (count = 0; argv[count]; count++)
> ; /* just counting */
Yes, it looks better.
> > + /* Get destination buffer length. */
> > + len = count ? count : 1;
>
> This confused me quite a bit. Wouldn't it be simpler to special
> case the count==0 case and return xcalloc(1,1) here (this would
> allow you to lose "if (!count)" later as well)?
>
> > + /* Copy into destination buffer. */
> > + for (i = 0; i < count; ++i) {
> > + if (i) *to++ = ' ';
>
> (style)
> if (i)
> *to++ = ' ';
Ok, I will take care of this.
> > + to += sq_quote_buf(to, len, argv[i]);
> > + }
> > +
> > + if (!count)
> > + *buf = 0;
> > +
> > + return buf;
> > +}
> >
> > +/* Return a newly allocated copy of "format" where the
> > + * first occurence of "old" has been replaced by "new". */
> > +static char *str_subst(const char *format, const char *old, const char
> > *new) +{
>
> I do not think there is anything wrong with this function
> per-se, but...
>
> > +void sq_quote_argv_printf(FILE* out, const char **argv, int count,
> > + const char *format, ...)
> > +{
> > + /* Replace the string "ARGV" in format with the quoted arg values. */
> > + char *argv_str = sq_quote_argv(argv, count);
> > + char *new_format = str_subst(format, "ARGV", argv_str);
> > +
> > + /* Print into "out" using the new format. */
> > + va_list rest;
> > + va_start(rest, format);
> > + vfprintf(out, new_format, rest);
> > + va_end(rest);
>
> this feels wrong. What happens when the original argv had
> a per-cent in it?
You are right, I will rework this.
Thanks,
Christian.
^ permalink raw reply
* Re: Problem with pack
From: Nicolas Pitre @ 2006-08-27 19:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Sergio Callegari, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0608271102450.27779@g5.osdl.org>
On Sun, 27 Aug 2006, Linus Torvalds wrote:
> Now, "git repack -a" is obviously special in that the "-a" will mean that
> we will generally touch all of the old pack _anyway_, and thus verifying
> the signature is no longer at all as unreasonable as it is under other
> circumstances. And very arguably, if you _also_ do "-d", then since that
> is a fairly dangerous operation with the potential for real data loss, you
> could well argue that we should do it.
We definitely should do it with -d.
> However, since the data was _already_ corrupt in that situation, and since
> a "git-fsck-objects --full" _will_ pick up the corruption in that case
> both before and after, equally arguably it's also true that there's really
> not a huge advantage to checking it in "git repack -a -d".
There really is an advantage. Given that the absence of -d leaves old
objects/packs around, there is a greater chance for still finding an
early copy of the bad object.
> In other words, in your case, the reason you ended up with the corruption
> spreading was _not_ that "git repack -a -d" might have silently not
> noticed it, but really the fact that unison meant that the corruption
> would spread from one archive to another in the first place.
But -d would definitely delete old packs that could have had a non
corrupted copy of the desired object.
> Final note: a "git repack -a -d" normally actually _does_ do almost as
> much checking as a "git-fsck-objects". It's literally just the "copy the
> already packed object from an old pack to a new one" that it an
> optimization that short-circuits all the normal git sanity checks. All the
> other cases will effectively do a lot of integrity checking just by virtue
> of unpacking the data in the object that is packed, before re-packing it.
>
> So it might well be the case that we should simply add an extra integrity
> check to the raw data copy in builtin-pack-objects.c: write_object().
>
> Now, these days there is actually two cases of that: the pack-to-pack copy
> (which has existed for a long while) and the new "!legacy_loose_object()"
> case. They should perhaps both verify the integrity of what they copy.
I think that git-pack-object should grow another flag: --verify-src or
something. That flag would force the verification of the pack checksum
for any pack used for the repack operation. Then it should be used
anytime -d is provided to git-repack. When -d is not provided then we
can skip that --verify-src security measure since nothing gets deleted
and therefore no risk of loosing a good object over a corrupted one
would happen.
Nicolas
^ permalink raw reply
* Re: Problem with pack
From: Linus Torvalds @ 2006-08-27 18:27 UTC (permalink / raw)
To: Sergio Callegari, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <44F1DA25.3050403@arces.unibo.it>
On Sun, 27 Aug 2006, Sergio Callegari wrote:
>
> There is something that I still do not understand... (sorry if I ask stupid
> questions)...
> Since packs have an sha signature too, if there was a data corruption (disk or
> transfer), shouldn't that have been detected at the repack? I.e. doesn't
> repack -d verify the available data before cancelling anything?
The packs do have a SHA1 signature, but verifying it is too expensive for
normal operations. It's only verified when you explicitly ask for it, ie
by git-fsck-objects and git-verify-pack.
Now, for small projects we could easily verify the SHA1 csum when we load
the pack, but imagine doing the same thing when the pack is half a
gigabyte in size, and I think you see the problem.. Especially as most
normal operations wouldn't even otherwise touch more than a small small
fraction of the pack contents, so verifying the SHA1 would be relatively
very expensive indeed.
Now, "git repack -a" is obviously special in that the "-a" will mean that
we will generally touch all of the old pack _anyway_, and thus verifying
the signature is no longer at all as unreasonable as it is under other
circumstances. And very arguably, if you _also_ do "-d", then since that
is a fairly dangerous operation with the potential for real data loss, you
could well argue that we should do it.
However, since the data was _already_ corrupt in that situation, and since
a "git-fsck-objects --full" _will_ pick up the corruption in that case
both before and after, equally arguably it's also true that there's really
not a huge advantage to checking it in "git repack -a -d".
In other words, in your case, the reason you ended up with the corruption
spreading was _not_ that "git repack -a -d" might have silently not
noticed it, but really the fact that unison meant that the corruption
would spread from one archive to another in the first place.
NOTE! This is all assuming my theory that a packed entry was broken in the
first place was correct. We obviously still don't _know_ what the problem
was. So far it's just a theory.
Final note: a "git repack -a -d" normally actually _does_ do almost as
much checking as a "git-fsck-objects". It's literally just the "copy the
already packed object from an old pack to a new one" that it an
optimization that short-circuits all the normal git sanity checks. All the
other cases will effectively do a lot of integrity checking just by virtue
of unpacking the data in the object that is packed, before re-packing it.
So it might well be the case that we should simply add an extra integrity
check to the raw data copy in builtin-pack-objects.c: write_object().
Now, these days there is actually two cases of that: the pack-to-pack copy
(which has existed for a long while) and the new "!legacy_loose_object()"
case. They should perhaps both verify the integrity of what they copy.
Junio, comments?
Linus
^ permalink raw reply
* Starting to think about sha-256?
From: Jeff Garzik @ 2006-08-27 17:56 UTC (permalink / raw)
To: Git Mailing List
Recent press[1] is talking about sha-1 collisions again. Even though
the reported attack was against a weakened variant of sha-1 (64, not 80,
passes), it serves as a useful point to start talking about the future.
I argue that sha-256 is better suited to git's purposes, and to modern
machines, than sha-1.
Upsides to sha-256:
* not just a bit increase, but a stronger algorithm. there is more
mixing, doing a more-than-incrementally better job at avoiding collisions.
* the bit increase itself provides more hash space, theoretically
reducing collisions.
* properly aligned, a set of 32-byte hashes won't straddle CPU cachelines.
Downsides to sha-256:
* git protocol/storage format change implications.
* increase in storage size (20 to 32 bytes per hash).
* fewer hand-optimized algorithm variants have been implemented.
* likely more CPU cycles per hash, though I haven't measured.
Wikimedia page has lotsa info:
http://en.wikipedia.org/wiki/Secure_Hash_Algorithm
Maybe sha-256 could be considered for the next major-rev of git?
Jeff
[1] http://www.heise-security.co.uk/news/77244
^ permalink raw reply
* Re: Problem with pack
From: Sergio Callegari @ 2006-08-27 17:45 UTC (permalink / raw)
To: git
>
> I do think that your synchronization using unison is _somehow_ part of the
> reason why bad things happened, but I really can't see why it would cause
> problems, and perhaps more importantly, git should have noticed them
> earlier (and, in particular, failed the repack). So a git bug and/or
> misfeature is involved somehow.
>
Glad if my broken pack can help finding out!
> One thing that may have happened is that the use of unison somehow
> corrupted an older pack (or you had a disk corruption), and that it was
> missed because the corruption was in a delta of the old pack that was
> silently re-used for the new one.
>
> That would explain how the SHA1 of the pack-file matches - the repack
> would have re-computed the SHA1 properly, but since the source delta
> itself was corrupt, the resulting new pack is corrupt.
>
There is something that I still do not understand... (sorry if I ask
stupid questions)...
Since packs have an sha signature too, if there was a data corruption
(disk or transfer), shouldn't that have been detected at the repack?
I.e. doesn't repack -d verify the available data before cancelling anything?
> If you had used git itself to synchronize the two repositories, that
> corruption of one repo would have been noticed when it transfers the data
> over to the other side, which is one reason why the native git syncing
> tools are so superior to doing a filesystem-level synchronization.
>
I think I learnt the lesson!
> With a filesystem-level sync (unison or anything else - rsync, cp -r,
> etc), a problem introduced in one repository will be copied to another one
> without any sanity checking.
>
Idem!
> but in the meantime, when you find a place to put the corrupt pack/index
> file, please include me and Junio at a minimum into the group of people
> who you tell where to find it (and/or passwords to access it). I'll
> happily keep your data private (I've done it before for others).
>
>
Sure... I have already sent an email to Junio to arrange this.
Thanks,
Sergio
^ permalink raw reply
* Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
From: Pierre Habouzit @ 2006-08-27 16:26 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <ecsdqa$1pi$1@sea.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 967 bytes --]
Le dim 27 août 2006 17:30, Jakub Narebski a écrit :
> Pierre Habouzit wrote:
> > Le dim 27 ao?t 2006 12:52, Junio C Hamano a écrit :
> >> About vger potentially throwing things away, I use this script
> >> (called "taboo.perl") to check my messages before sending them
> >> out.
> >
> > that was not it, I was biten (again) by git-send-mail that uses
> > strftime (localized) to generate rfc822 dates, making them
> > unparseable :|
>
> Update your git, or use ./git-send-email.perl directly from git
> repository (from 'master' branch, as 'next' branch version uses
> Git.pm). Strftime was replaced by pure Perl to generate rfc2822 date
> some time ago...
I know that, I've done that on many of my system, but not the one where
I hack git ... how lame isn't it ? :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
From: Randal L. Schwartz @ 2006-08-27 16:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7v64gexxgl.fsf@assigned-by-dhcp.cox.net>
>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:
Junio> About vger potentially throwing things away, I use this script
Junio> (called "taboo.perl") to check my messages before sending them
Junio> out.
Junio> Obviously the taboo-word list itself is not attached here, but
Junio> the actual script should have a copy of it after the __DATA__
Junio> marker.
With "Inline::Files" from the CPAN, you could have a switch where the
script updates itself with the new list:
use Inline::Files;
if (@ARGV == 1 and $ARGV[0] eq "-update") {
use LWP::Simple;
my $list = get "http://example.com/foo/bar.txt";
open DATA, ">$DATA" or die "cannot write myself: $!";
print DATA $list;
close DATA;
exit 0;
}
... rest of your program here ...
... read using <DATA> as before ...
__DATA__
the list will magically go here.
--
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!
^ permalink raw reply
* Re: [PATCH 00/19] gitweb: Remove dependency on external diff and need for temporary files
From: Jakub Narebski @ 2006-08-27 15:37 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0608262026230.11811@g5.osdl.org>
Linus Torvalds wrote:
> On Fri, 25 Aug 2006, Jakub Narebski wrote:
>>
>> This series of patches (now finished) removes dependency on
>> external diff (/usr/bin/diff) to produce commitdiff and blobdiff
>> views, and the need for temporary files.
>
> Ok, can we now please fix my final annoyance, which is that gitweb from
> the very beginning has apparently believed that the "Signed-off-by:" etc
> lines are not important, and they get stripped away when looking at the
> "commit-diff".
This can be easily fixed.
> Also, "commit-diff" really should have some minimal authorship
> information. It's silly to have to go to "commit" and then separately ask
> for "diff" to see all these very basic things.
And this need some layout redesign for commitdiff and log views.
Currently it is specially formatted subject/first line, simplified
message (with empty lines collapsed), and without signoff.
Perhaps something similar to what "log" view uses?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: File archiver using git
From: Jakub Narebski @ 2006-08-27 15:34 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.63.0608271528130.8018@alpha.polcom.net>
Grzegorz Kulewski wrote:
> On Sun, 27 Aug 2006, Matt McCutchen wrote:
>> Dear git people,
>>
>> You might like the two attached scripts that I wrote around git to
>> pack file trees containing lots of redundancy into very small
>> packages. For example, if I have ten slightly different versions of a
>> piece of software because I didn't use version control, I can use
>> gitar to compress them together.
>
> Does it (and GIT in general) work ok with file permisions, ownership, soft
> and hard links, named sockets, device files and similar "strange"
> filesystem objects? Do I need any options to GIT to make it work with
> them?
Git in general only preserves executable bit, deals with symlinks,
hardlinks after a fashion (stored once, but unpacked/checked out as separate
files, not hardlinked), and does not deal with other "strange" filesystem
objects as far as I know.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] git-daemon: more powerful base-path/user-path settings, using formats.
From: Jakub Narebski @ 2006-08-27 15:30 UTC (permalink / raw)
To: git
In-Reply-To: <200608271340.32792.madcoder@debian.org>
Pierre Habouzit wrote:
> Le dim 27 ao?t 2006 12:52, Junio C Hamano a écrit :
>> About vger potentially throwing things away, I use this script
>> (called "taboo.perl") to check my messages before sending them
>> out.
>
> that was not it, I was biten (again) by git-send-mail that uses strftime
> (localized) to generate rfc822 dates, making them unparseable :|
Update your git, or use ./git-send-email.perl directly from git repository
(from 'master' branch, as 'next' branch version uses Git.pm). Strftime was
replaced by pure Perl to generate rfc2822 date some time ago...
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] Simplify code outputting relative timestamps in git log
From: Nikolai Weibull @ 2006-08-27 14:27 UTC (permalink / raw)
To: git; +Cc: Nikolai Weibull
In-Reply-To: <11566886081901-git-send-email-now@bitwi.se>
On 8/27/06, Nikolai Weibull <now@bitwi.se> wrote:
> From: Nikolai Weibull <now@puritan.pcp.ath.cx>
Hm...why is format-patch not using user.email?
> The code that outputs relative timestamps is repetitive and can be
> simplified by using an array to deal with the various cutoffs. This makes
> it easier to modify and remove the cutoffs if we in the future desire to do
> so.
I just realized that I forgot to update the cutoffs to those in the
repository. I was playing around with other cutoffs. I'll send a new
patch if it is of any interest.
nikolai
^ permalink raw reply
* [PATCH] Simplify code outputting relative timestamps in git log
From: Nikolai Weibull @ 2006-08-27 14:23 UTC (permalink / raw)
To: git; +Cc: Nikolai Weibull, Nikolai Weibull
From: Nikolai Weibull <now@puritan.pcp.ath.cx>
The code that outputs relative timestamps is repetitive and can be
simplified by using an array to deal with the various cutoffs. This makes
it easier to modify and remove the cutoffs if we in the future desire to do
so.
Signed-off-by: Nikolai Weibull <now@bitwi.se>
---
date.c | 63 +++++++++++++++++++++++++++++++--------------------------------
1 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/date.c b/date.c
index e387dcd..5891fa8 100644
--- a/date.c
+++ b/date.c
@@ -64,6 +64,29 @@ const char *show_date(unsigned long time
static char timebuf[200];
if (relative) {
+ static struct {
+ char name[8];
+ unsigned long cutoff;
+ unsigned long factor;
+ unsigned long term;
+ } cutoffs[] = {
+#define CUTOFF(name, cutoff, factor, ceiling) \
+ { (name), (cutoff) * (factor), (factor), (ceiling) }
+#define MINUTES(minutes) ((minutes) * 60)
+#define HOURS(hours) ((hours) * MINUTES(60))
+#define DAYS(days) ((days) * HOURS(24))
+ CUTOFF("seconds", 90, 1, 0),
+ CUTOFF("minutes", 90, MINUTES(1), 30),
+ CUTOFF("hours", 36, HOURS(1), MINUTES(30)),
+ CUTOFF("days", 14, DAYS(1), HOURS(12)),
+ CUTOFF("weeks", 12, DAYS(7), HOURS(12)),
+ CUTOFF("months", 12, DAYS(30), HOURS(12))
+#undef MINUTES
+#undef DAYS
+#undef HOURS
+#undef CUTOFF
+ };
+ int i;
unsigned long diff;
time_t t = gm_time_t(time, tz);
struct timeval now;
@@ -71,39 +94,15 @@ const char *show_date(unsigned long time
if (now.tv_sec < t)
return "in the future";
diff = now.tv_sec - t;
- if (diff < 90) {
- snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
- return timebuf;
- }
- /* Turn it into minutes */
- diff = (diff + 30) / 60;
- if (diff < 90) {
- snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
- return timebuf;
- }
- /* Turn it into hours */
- diff = (diff + 30) / 60;
- if (diff < 36) {
- snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
- return timebuf;
- }
- /* We deal with number of days from here on */
- diff = (diff + 12) / 24;
- if (diff < 14) {
- snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
- return timebuf;
- }
- /* Say weeks for the past 10 weeks or so */
- if (diff < 70) {
- snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
- return timebuf;
- }
- /* Say months for the past 12 months or so */
- if (diff < 360) {
- snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
- return timebuf;
+ for (i = 0; i < ARRAY_SIZE(cutoffs); i++) {
+ if (diff < cutoffs[i].cutoff) {
+ snprintf(timebuf, sizeof(timebuf), "%lu %s ago",
+ (diff + cutoffs[i].term) / cutoffs[i].factor,
+ cutoffs[i].name);
+ return timebuf;
+ }
}
- /* Else fall back on absolute format.. */
+ /* If we went beyond the month cutoff, use absolute format. */
}
tm = time_to_tm(time, tz);
--
1.4.2.GIT-dirty
^ 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