* [PATCH] dir: do all size checks before seeking back and fix file closing
From: Jonas Fonseca @ 2006-08-26 14:17 UTC (permalink / raw)
To: git
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
dir.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c
index d53d48f..ff8a2fb 100644
--- a/dir.c
+++ b/dir.c
@@ -122,11 +122,11 @@ static int add_excludes_from_file_1(cons
size = lseek(fd, 0, SEEK_END);
if (size < 0)
goto err;
- lseek(fd, 0, SEEK_SET);
if (size == 0) {
close(fd);
return 0;
}
+ lseek(fd, 0, SEEK_SET);
buf = xmalloc(size+1);
if (read(fd, buf, size) != size)
goto err;
@@ -146,7 +146,7 @@ static int add_excludes_from_file_1(cons
return 0;
err:
- if (0 <= fd)
+ if (0 >= fd)
close(fd);
return -1;
}
--
1.4.2.GIT
--
Jonas Fonseca
^ permalink raw reply related
* [PATCH] git-svn: stop repeatedly reusing the first commit message with dcommit
From: Eric Wong @ 2006-08-26 16:52 UTC (permalink / raw)
To: Junio C Hamano, seth; +Cc: git, Eric Wong
In-Reply-To: <20060826070123.GA10801@localdomain>
Excessive use of global variables got me into trouble.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 9382a15..0290850 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -819,6 +819,7 @@ sub commit_diff {
} else {
$ed->close_edit;
}
+ $_message = $_file = undef;
}
########################### utility functions #########################
--
1.4.2.g7c9b
^ permalink raw reply related
* [PATCH 0/7] gitweb: Cleanups, fixes and small improvements
From: Jakub Narebski @ 2006-08-26 17:13 UTC (permalink / raw)
To: git
This series contains fixes, improvements and cleanups.
Fixes are patch 2 and 4, with 4 more important.
This series is based on next
(885a89a077c2c79e1fac81c57711566ec30f9bc1)
---
gitweb/gitweb.perl | 75 +++++++++++++++++++++++++++++++++++++---------------
1 files changed, 54 insertions(+), 21 deletions(-)
Shortlog:
gitweb: Restore old git_blame using git-annotate under "annotate"
gitweb: Remove workaround for git-diff bug fixed in f82cd3c
gitweb: Improve comments about gitweb features configuration
gitweb: Fix typo in git_patchset_body
gitweb: Use 'local $/ = undef;' before 'print <$fd>;'
gitweb: blobs defined by non-textual hash ids can be cached
gitweb: Always link to plain version of the blob
^ permalink raw reply
* [PATCH 3/7] gitweb: Improve comments about gitweb features configuration
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 25 ++++++++++++++++---------
1 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index df2f9e5..c29ffa8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -67,9 +67,16 @@ our $mimetypes_file = undef;
# You define site-wide feature defaults here; override them with
# $GITWEB_CONFIG as necessary.
our %feature = (
- # feature => {'sub' => feature-sub, 'override' => allow-override, 'default' => [ default options...]
- # if feature is overridable, feature-sub will be called with default options;
- # return value indicates if to enable specified feature
+ # feature => {
+ # 'sub' => feature-sub (subroutine),
+ # 'override' => allow-override (boolean),
+ # 'default' => [ default options...] (array reference)}
+ #
+ # if feature is overridable (it means that allow-override has true value,
+ # then feature-sub will be called with default options as parameters;
+ # return value of feature-sub indicates if to enable specified feature
+ #
+ # use gitweb_check_feature(<feature>) to check if <feature> is enabled
'blame' => {
'sub' => \&feature_blame,
@@ -95,9 +102,9 @@ sub gitweb_check_feature {
}
# To enable system wide have in $GITWEB_CONFIG
-# $feature{'blame'}{'default'} = [1];
-# To have project specific config enable override in $GITWEB_CONFIG
-# $feature{'blame'}{'override'} = 1;
+# $feature{'blame'}{'default'} = [1];
+# To have project specific config enable override in $GITWEB_CONFIG
+# $feature{'blame'}{'override'} = 1;
# and in project config gitweb.blame = 0|1;
sub feature_blame {
@@ -113,9 +120,9 @@ sub feature_blame {
}
# To disable system wide have in $GITWEB_CONFIG
-# $feature{'snapshot'}{'default'} = [undef];
-# To have project specific config enable override in $GITWEB_CONFIG
-# $feature{'blame'}{'override'} = 1;
+# $feature{'snapshot'}{'default'} = [undef];
+# To have project specific config enable override in $GITWEB_CONFIG
+# $feature{'blame'}{'override'} = 1;
# and in project config gitweb.snapshot = none|gzip|bzip2
sub feature_snapshot {
--
1.4.1.1
^ permalink raw reply related
* [PATCH 2/7] gitweb: Remove workaround for git-diff bug fixed in f82cd3c
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Remove workaround in git_blobdiff for error in git-diff (showing
reversed diff for diff of blobs), corrected in commit
f82cd3c Fix "git diff blob1 blob2" showing the diff in reverse.
which is post 1.4.2-rc2 commit.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abd0f0d..df2f9e5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2841,8 +2841,7 @@ sub git_blobdiff {
}
# open patch output
- #open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash
- open $fd, "-|", $GIT, "diff", '-p', $hash, $hash_parent
+ open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash
or die_error(undef, "Open git-diff failed");
} else {
die_error('404 Not Found', "Missing one of the blob diff parameters")
--
1.4.1.1
^ permalink raw reply related
* [PATCH 4/7] gitweb: Fix typo in git_patchset_body
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c29ffa8..093deab 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1657,7 +1657,7 @@ sub git_patchset_body {
print "<div class=\"patchset\">\n";
LINE:
- while (my $patch_line @$fd>) {
+ while (my $patch_line = <$fd>) {
chomp $patch_line;
if ($patch_line =~ m/^diff /) { # "git diff" header
--
1.4.1.1
^ permalink raw reply related
* [PATCH 7/7] gitweb: Always link to plain version of the blob in git_blob.
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Link to plain version of the blob. even if no valid hash_base is given
in "blob" view (in git_blob).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f6175bb..02d327c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2461,8 +2461,13 @@ sub git_blob {
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
} else {
+ $formats_nav .=
+ $cgi->a({-href => href(action=>"blob_plain",
+ hash=>$hash, file_name=>$file_name)},
+ "plain") .
+
print "<div class=\"page_nav\">\n" .
- "<br/><br/></div>\n" .
+ "<br/>$formats_nav<br/></div>\n" .
"<div class=\"title\">$hash</div>\n";
}
git_print_page_path($file_name, "blob", $hash_base);
--
1.4.1.1
^ permalink raw reply related
* [PATCH 5/7] gitweb: Use 'local $/ = undef;' before 'print <$fd>;'
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 093deab..eae83e6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2017,6 +2017,7 @@ sub git_project_list {
if (-f $home_text) {
print "<div class=\"index_include\">\n";
open (my $fd, $home_text);
+ local $/ = undef;
print <$fd>;
close $fd;
print "</div>\n";
@@ -2395,11 +2396,10 @@ sub git_blob_plain {
print $cgi->header(-type => "$type",
-content_disposition => "inline; filename=\"$save_as\"");
- undef $/;
binmode STDOUT, ':raw';
+ local $/ = undef;
print <$fd>;
binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
- $/ = "\n";
close $fd;
}
@@ -2585,6 +2585,7 @@ sub git_snapshot {
open my $fd, "-|", "$GIT tar-tree $hash \'$project\' | $command" or
die_error(undef, "Execute git-tar-tree failed.");
binmode STDOUT, ':raw';
+ local $/ = undef;
print <$fd>;
binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
close $fd;
--
1.4.1.1
^ permalink raw reply related
* [PATCH 6/7] gitweb: blobs defined by non-textual hash ids can be cached
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index eae83e6..f6175bb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2371,6 +2371,12 @@ sub git_heads {
}
sub git_blob_plain {
+ # blobs defined by non-textual hash id's can be cached
+ my $expires;
+ if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
+ $expires = "+1d";
+ }
+
if (!defined $hash) {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
@@ -2394,8 +2400,10 @@ sub git_blob_plain {
$save_as .= '.txt';
}
- print $cgi->header(-type => "$type",
- -content_disposition => "inline; filename=\"$save_as\"");
+ print $cgi->header(
+ -type => "$type",
+ -expires=>$expires,
+ -content_disposition => "inline; filename=\"$save_as\"");
binmode STDOUT, ':raw';
local $/ = undef;
print <$fd>;
@@ -2404,6 +2412,12 @@ sub git_blob_plain {
}
sub git_blob {
+ # blobs defined by non-textual hash id's can be cached
+ my $expires;
+ if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
+ $expires = "+1d";
+ }
+
if (!defined $hash) {
if (defined $file_name) {
my $base = $hash_base || git_get_head_hash($project);
@@ -2421,7 +2435,7 @@ sub git_blob {
close $fd;
return git_blob_plain($mimetype);
}
- git_header_html();
+ git_header_html(undef, $expires);
my $formats_nav = '';
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
if (defined $file_name) {
--
1.4.1.1
^ permalink raw reply related
* [PATCH 1/7] gitweb: Restore old git_blame using git-annotate under "annotate"
From: Jakub Narebski @ 2006-08-26 17:14 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Rename git_blame to git_annotate, and git_blame2 to git_blame.
Link git_annotate under "annotate" action. Add link to "blame
in git_annotate, and to "annotate" in git_blame.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e00a6ed..abd0f0d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -229,7 +229,8 @@ if (defined $searchtext) {
# dispatch
my %actions = (
- "blame" => \&git_blame2,
+ "blame" => \&git_blame,
+ "annotate" => \&git_annotate,
"blobdiff" => \&git_blobdiff,
"blobdiff_plain" => \&git_blobdiff_plain,
"blob" => \&git_blob,
@@ -2166,7 +2167,7 @@ sub git_tag {
git_footer_html();
}
-sub git_blame2 {
+sub git_blame {
my $fd;
my $ftype;
@@ -2193,6 +2194,9 @@ sub git_blame2 {
$cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
"blob") .
" | " .
+ $cgi->a({-href => href(action=>"annotate", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
+ "annotate") .
+ " | " .
$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
"head");
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
@@ -2236,7 +2240,7 @@ HTML
git_footer_html();
}
-sub git_blame {
+sub git_annotate {
my $fd;
if (!gitweb_check_feature('blame')) {
@@ -2258,6 +2262,9 @@ sub git_blame {
$cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
"blob") .
" | " .
+ $cgi->a({-href => href(action=>"blame", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)},
+ "blame") .
+ " | " .
$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
"head");
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
--
1.4.1.1
^ permalink raw reply related
* Re: Problem with pack
From: Linus Torvalds @ 2006-08-26 18:20 UTC (permalink / raw)
To: Sergio Callegari; +Cc: git
In-Reply-To: <44EEED9C.1010000@arces.unibo.it>
On Fri, 25 Aug 2006, Sergio Callegari wrote:
>
> If I try to verify the pack I get:
>
> git verify-pack -v pack-ebcdfbbda07e5a3e4136aa1f499990b35685bab4.idx
> fatal: failed to read delta-pack base object 2849bd2bd8a76bbca37df2a4c8e8b990811d01a7
>
> the package length seems reasonable, however... (no evident sign of
> truncation, but I haven't looked inside the index to check the exact positions
> of objects)...
Can you make the corrupt pack-file and index available publically (or
perhaps at least to a few git people?)
The fact that verify-pack is happy with the SHA1 checksum is interesting,
because it means that the pack-file at least didn't get corrupted on-disk
(or through the sync operation). Iow, it must have gotten corrupted at
write-out itself somehow, and it would be interesting to see what the
pack-file looks like.
> and git unpack-object dies with error code -3 in inflate...
That's Z_DATA_ERROR, which is what you get if the input to inflate is bad.
Linus
^ permalink raw reply
* Re: [PATCH] dir: do all size checks before seeking back and fix file closing
From: Mitchell Blank Jr @ 2006-08-26 18:43 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060826141709.GC11601@diku.dk>
Jonas Fonseca wrote:
> err:
> - if (0 <= fd)
> + if (0 >= fd)
> close(fd);
Could you explain that piece? You now only close "fd" if it's zero (stdin)
or negative (invalid). The old code (close if its >=0) make more sense.
-Mitch
^ permalink raw reply
* Re: Problem with pack
From: Sergio Callegari @ 2006-08-26 18:49 UTC (permalink / raw)
To: git
>
> Junio C Hamano <junkio@cox.net> writes:
>
> > Earlier you said "unpack-objects <$that-pack.pack" fails with
> > "error code -3 in inflate..." What exact error do you get?
> > I am guessing that it is get_data() which says:
> >
> > "inflate returned %d\n"
> >
> > (side note: we should not say \n there).
> > ...
> > This pattern appears practically everywhere...
> > ... I've been
> > wondering if it is possible for inflate to eat some input but
> > that was not enough to produce one byte of output, and what [it]
> > would return in such a case...
>
> I do not think this fear does not apply to this particular case;
> return value -3 is Z_DATA_ERROR, so the deflated stream is
> corrupt.
>
> > So there are only a few ways you can get that error message.
> > ...
>
> I just realized there is another not so inplausible explanation.
>
> When the problematic pack was made on the mothership,
> csum-file.c::sha1write_compressed() gave the data for the base
> object to zlib, an alpha particle hit a memory cell that
> contained zlib output buffer (resulting in a corrupt deflated
> stream in variable "out"), and sha1write() wrote it out while
> computing the right checksum.
>
> Is the memory on your mothership reliable (I do not want to make
> this message sound like one on the kernel list, but memtest86
> might be in order)?
>
For sure you can never say, but so far I have never had any problem with
that machine nor I had any after this incident...
Sergio
>
>
>
>
^ permalink raw reply
* Re: Problem with pack
From: Sergio Callegari @ 2006-08-26 18:53 UTC (permalink / raw)
To: git
> Earlier you said that the mothership has 1.4.2, and the note has
> 1.4.0. The sequence of events as I understand are:
>
> - repack -a -d on the mothership with 1.4.2; no problems
> observed.
>
> - transfer the results to note; this was done behind git so
> no problems observed.
>
> - tried to repack on note with 1.4.0; got "failed to
> read delta-pack base object" error.
>
Yes... only before that I had a few more iterations PC<->notebook always
syncing with unison.
> Can you make the pack/idx available to the public for
> postmortem?
>
Yes... I can make them available... the pack/idx actually do not contain
anything extremely confidential (just a bunch of LaTeX files).
Only, being that there is conference data and stuff by people who
professionally organize conferences, I'd prefer to make it available
directly to some specific people that I can trust not re-distributing it
rather than putting it in the general public.
> Also I wonder if the pack can be read by 1.4.2.
>
No it cannot.
> Earlier you said "unpack-objects <$that-pack.pack" fails with
> "error code -3 in inflate..." What exact error do you get?
> I am guessing that it is get_data() which says:
>
> "inflate returned %d\n"
>
This is right...
>
Just a question...
Might the problem have come out of a scenario like the following...
1) I use unison to sync my documents (rather than using the git tools...
silly me!)
2) I get things wrong in controlling unison (without realizing that I
do) and the result is that I lose some blobs.
3) I repack an unclean tree (missing some objects)
Can this be the case?
Thanks
Sergio
^ permalink raw reply
* Re: Problem with pack
From: Linus Torvalds @ 2006-08-26 19:24 UTC (permalink / raw)
To: Sergio Callegari; +Cc: git
In-Reply-To: <44F098C0.8000202@arces.unibo.it>
On Sat, 26 Aug 2006, Sergio Callegari wrote:
>
> Might the problem have come out of a scenario like the following...
>
> 1) I use unison to sync my documents (rather than using the git tools...
> silly me!)
> 2) I get things wrong in controlling unison (without realizing that I
> do) and the result is that I lose some blobs.
> 3) I repack an unclean tree (missing some objects)
>
> Can this be the case?
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.
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.
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.
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.
(Which is not to say that the native protocol might not miss something
too, but it should be _much_ harder to trigger: for anything but the
initial close, the native protocol will unpack all objects and recompute
their SHA1 hashes from first principles on the receiving side, rather than
trust the sender implicitly, so it's fundamentally safer. But maybe we
could be even _more_ anal somewhere).
So as a suggestion if you want to be careful:
- only use "git fetch/pull" to synchronize two git repos, because that's
inherently safer than any non-native synchronization.
- if you repack reasonably often, do "git fsck-objects" (which is very
cheap when there aren't a lot of unpacked objects) to verify the
archive before "git repack -a -d" to repack it.
- the "fsck-objects" thing won't catch a corrupt pack (unless you ask for
it with "--full", which is expensive for bigger projects), but at least
with "git fetch/pull", such corruption should not be able to replicate
to another repository.
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).
Linus
^ permalink raw reply
* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Junio C Hamano @ 2006-08-26 19:25 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200608252135.27894.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Remove $git_temp variable which held location for temporary files
> needed by git_diff_print, and removed creating $git_temp directory.
Very good. Not writing into the filesystem even in the
temporary location is a very good thing.
Other things I noticed in this 19 series (note that I've applied
them more or less intact already, expecting that any issues will
be fixed in-tree):
* Overall
Your MUA configuration seems to have improved and stabilized
quite a bit. I did not see any corruption this time around.
* 01/19 gitweb: Use git-diff-tree patch output for commitdiff
This was a bit large for me to review during work-day, and
seemed a bit scary. It initially did not give me a good
feeling to see that a large series that followed depended on
something that was marked RFC, but it turned out mostly Ok.
* 02/19 gitweb: Replace git_commitdiff_plain by anonymous subroutine
03/19 gitweb: Show information about incomplete lines in commitdiff
Revert "gitweb: Replace git_commitdiff_plain by anonymous
subroutine"
I've commented on these.
* 06/19 gitweb: Add git_get_{following,preceding}_references functions
07/19 gitweb: Return on first ref found when
git_get_preceding_references is called in scalar context
This looks *VERY* expensive. Does git_get_references()
cache and reuse its result? How many times during a single
invocation are these subs called?
Also I am not sure about the correctness of "get-following".
B------D------F
/ base
--A------C------E
hash
You read from "rev-list $base", stop when you see $hash, and
grab all the refs that point at the rev you have seen before
stopping as "following". But in the above picture, you will
follow from F down to the very initial commit without
stopping and there actually is _no_ rev that follows E so
your result would contain B D A (if they are tagged) but none
of them follows E. There is something wrong here.
At least you should read from "rev-list $hash..$base"; then
traversal would go F D B and stop at A; you probably would
want --boundary to force showing of A as well, but even then
I am not sure how well the result would work.
"get-preceding" also wants to go down to the initial commit.
"get-following" is inherently a very expensive operation, so
I would suggest not doing this. It seems that nobody uses
these two subs yet, so probably it is better to yank them
before they cause damages.
* 08/19 gitweb: Add git_get_rev_name_tags function
09/19 gitweb: Use git_get_name_rev_tags for commitdiff_plain
X-Git-Tag: header
I suspect these make the generation of the header extremely
expensive. I'd suggest reverting them to the original.
* 13/19 gitweb: Add invisible hyperlink to from-file/to-file diff header
You seem to have forgotten esc_html() on the patch-line
before sending it to the browser. Careful.
* 14/19 gitweb: Always display link to blobdiff_plain in git_blobdiff
Need justification why this change is needed (or why previous
logic to avoid showing it in certain cases is wrong).
* 16/19 gitweb: Use git-diff-tree or git-diff patch output for blobdiff
Is git_to_hash sub always called with object names and
nothing else? "git rev-parse no-such" would die with an
error message, and "git rev-parse Makefile" in populated
working tree would say "Makefile" without complaints.
Perhaps you want --revs-only --no-flags here.
I think it is a bad style to return [] or $ in scalar context
depending on the number of results. It forces the caller to
do a conditional depending on the type of the stuff returned.
I would suggest just removing if (wantarray) and always
return @hashes. A caller who is interested in a single
element can say "($it) = your_sub(...)", a caller who wants
the number of elements can say "$cnt = your_sub(...)", and a
caller who wants to know all can say "(@them) = your_sub(...)".
I think that is the usual thing to do in Perl. Unless there
is a compelling reason that is so important that it is worth
to deviate from that norm and confusing the programmer, that
is.
I think "# try to find filename from $hash" part would
misbehave if $hash returned by git_to_hash($hash) becomes
undef.
You seem to spell out '-M', '-C' everywhere. I suspect
fixing them all to just '-C' (or perhaps '-B', '-C') would be
tedious but probably is a good idea.
* 17/19 gitweb: git_blobdiff_plain is git_blobdiff('plain')
Needs justification why commitdiff and blobdiff plain needs
to behave differently.
^ permalink raw reply
* Re: [PATCH 19/19] gitweb: Remove creating directory for temporary files
From: Jakub Narebski @ 2006-08-26 20:18 UTC (permalink / raw)
To: git
In-Reply-To: <7vpsen1eq3.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> Remove $git_temp variable which held location for temporary files
>> needed by git_diff_print, and removed creating $git_temp directory.
>
> Very good. Not writing into the filesystem even in the
> temporary location is a very good thing.
>
> Other things I noticed in this 19 series (note that I've applied
> them more or less intact already, expecting that any issues will
> be fixed in-tree):
>
[...]
>
> * 06/19 gitweb: Add git_get_{following,preceding}_references functions
> 07/19 gitweb: Return on first ref found when
> git_get_preceding_references is called in scalar context
>
> This looks *VERY* expensive. Does git_get_references()
> cache and reuse its result? How many times during a single
> invocation are these subs called?
>
> Also I am not sure about the correctness of "get-following".
>
> B------D------F
> / base
> --A------C------E
> hash
>
> You read from "rev-list $base", stop when you see $hash, and
> grab all the refs that point at the rev you have seen before
> stopping as "following". But in the above picture, you will
> follow from F down to the very initial commit without
> stopping and there actually is _no_ rev that follows E so
> your result would contain B D A (if they are tagged) but none
> of them follows E. There is something wrong here.
>
> At least you should read from "rev-list $hash..$base"; then
> traversal would go F D B and stop at A; you probably would
> want --boundary to force showing of A as well, but even then
> I am not sure how well the result would work.
>
> "get-preceding" also wants to go down to the initial commit.
>
> "get-following" is inherently a very expensive operation, so
> I would suggest not doing this. It seems that nobody uses
> these two subs yet, so probably it is better to yank them
> before they cause damages.
I agree.
> * 08/19 gitweb: Add git_get_rev_name_tags function
> 09/19 gitweb: Use git_get_name_rev_tags for commitdiff_plain
> X-Git-Tag: header
>
> I suspect these make the generation of the header extremely
> expensive. I'd suggest reverting them to the original.
The git_get_following_references is copied almost verbatim from the original
(i.e. before this series) git_commitdiff_plain implementation, modified
only to allow for changed output of git_get_references (formerly
read_info_ref), and with "HEAD" changed to $hash_base || "HEAD".
git_get_preceding_references was made to be companion to
git_get_following_references, so of course it shares it's warts, errors
and disadvantages.
First patch in series changed X-Git-Tag: header to show only the tag that
points _directly_ to 'hash' commit, similarly to the ref marker in HTML
output (in git_commitdiff for example).
There was mentioned in discussion that it is nice to know what version you
need to have feature introduced by commitdiff. Hence writing code
generating X-Git-Tag: header into subroutine, writing companion
subroutine... then deciding that as there is native git command for that,
namely 'git name-rev --tags', why not use it? The git_get_following_refs
and git_get_preceding_refs didn't get used.
git name-rev is quite fast, perhaps up to half a second, or a second.
And is called only once per commitdiff_plain
I guess that generating (properly!) X-Git-Follows:, X-Git-Branch: and
X-Git-Precedes: (similarly to what gitk and qgit do) should be made into
feature, as it is time and resource consuming.
> * 13/19 gitweb: Add invisible hyperlink to from-file/to-file diff header
>
> You seem to have forgotten esc_html() on the patch-line
> before sending it to the browser. Careful.
Cannot esc_html() line with HTML code, namely the hyperlink. I know that the
line is "+++ a/<filename>" or "--- b/<filename>", and we replace <filename>
with hyperlink, which has esc_html($filename) as contents. There is no need
to escape "+++ a/" or "--- b/".
I guess I rely on git-diff/git-diff-tree to have "+++ /dev/null" and
"--- /dev/null" (both HTML safe), if there is no "a/" and "b/" in
from-file/to-file unified diff header.
> * 14/19 gitweb: Always display link to blobdiff_plain in git_blobdiff
>
> Need justification why this change is needed (or why previous
> logic to avoid showing it in certain cases is wrong).
Why we didn't display it before? I though it was a bug (oversimplification
in case we don't have $hash_base or it is not a commit). If we can display
"blob" view, we can display "blob_plain" view...
> * 16/19 gitweb: Use git-diff-tree or git-diff patch output for blobdiff
>
> Is git_to_hash sub always called with object names and
> nothing else? "git rev-parse no-such" would die with an
> error message, and "git rev-parse Makefile" in populated
> working tree would say "Makefile" without complaints.
> Perhaps you want --revs-only --no-flags here.
>
> I think it is a bad style to return [] or $ in scalar context
> depending on the number of results. It forces the caller to
> do a conditional depending on the type of the stuff returned.
>
> I would suggest just removing if (wantarray) and always
> return @hashes. A caller who is interested in a single
> element can say "($it) = your_sub(...)", a caller who wants
> the number of elements can say "$cnt = your_sub(...)", and a
> caller who wants to know all can say "(@them) = your_sub(...)".
>
> I think that is the usual thing to do in Perl. Unless there
> is a compelling reason that is so important that it is worth
> to deviate from that norm and confusing the programmer, that
> is.
>
> I think "# try to find filename from $hash" part would
> misbehave if $hash returned by git_to_hash($hash) becomes
> undef.
Gaaah, perhaps we should stop to try to be too nice, and just barf or use
legacy output if user didn't provide filename, and hash is not sha1 of blob
(don't try too hard to find filename if it is not provided).
And remove git_to_hash function until the correct implementation is written.
> You seem to spell out '-M', '-C' everywhere. I suspect
> fixing them all to just '-C' (or perhaps '-B', '-C') would be
> tedious but probably is a good idea.
Does '-C' imply '-M'?
> * 17/19 gitweb: git_blobdiff_plain is git_blobdiff('plain')
>
> Needs justification why commitdiff and blobdiff plain needs
> to behave differently.
First, if we have blobdiff generated with renames/copying detection (and
"commit" view uses it, and provides link to blobdiffs using it), there not
always is single diff (blobdiff) without renames/copying detection. So to
have blobdiff and blobdiff_plain equivalent, and blobdiff_plain without
renames detection, then blobdiff_plain view would have sometimes _two_
patches.
This isn't the case with commitdiff. The diff with and without renames
detection are equivalent for diff of whole commit.
Originally comittdiff and comitdiff_plain both were without renames
detection, while blobdiff and blobdiff_plain were called from commit with
renames detection, and from history with no need for renames detection.
The idea behind changing comittdiff (HTML version) to including rename
detection was that it gives shorter and better to understand patches. The
idea (perhaps wrong) behind leaving comitdiff_plain output without renames
detection was that this output can be applied directly by non-git-aware
tools. It can be easily changed to include renames/copying detection (put
'-C' in one place).
The idea behind having both blobdiff and blobdiff_plain have renames
detection was that commit view used rename detection, the two views should
be equivalent and one exist always for the other, and that it was easier on
implementation ;-)
P.S. I have most problems with having legacy blobdiff URL (without 'hpb'
i.e. hash_parent_base parameter) working correctly without making use of
external diff.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] dir: do all size checks before seeking back and fix file closing
From: Jonas Fonseca @ 2006-08-26 20:44 UTC (permalink / raw)
To: Mitchell Blank Jr; +Cc: git
In-Reply-To: <20060826184330.GA34439@gaz.sfgoth.com>
Mitchell Blank Jr <mitch@sfgoth.com> wrote Sat, Aug 26, 2006:
> Jonas Fonseca wrote:
> > err:
> > - if (0 <= fd)
> > + if (0 >= fd)
> > close(fd);
>
> Could you explain that piece? You now only close "fd" if it's zero (stdin)
> or negative (invalid). The old code (close if its >=0) make more sense.
Ah, yes, sorry for the noise. I read the old code as (fd <= 0).
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements
From: Junio C Hamano @ 2006-08-26 20:58 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
* gitweb: Restore old git_blame using git-annotate under "annotate"
I actually was hoping to see a patch to remove the git_blame
which is not used as far as I can see.
Although we carried annotate and blame in git.git tree for
quite a while, the intention was always to deprecate one over
the other once pros-and-cons of each implementation become
clear (and I think people would not miss annotate if we remove
annotate and make it an alias for blame -c anymore). What's
the reason we would want to have both?
* 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.
* gitweb: Use 'local $/ = undef;' before 'print <$fd>;'
You changed:
$/ = undef;
print <$fd>;
... hope that nobody depends on standard value of $/
... around here, which may still break if you did sub
... calls, the sub did not localize $/ (who would?),
... and depended to have a sane $/.
$/ = "\n";
to
local $/ = undef;
print <$fd>;
... hope that nobody depends on standard value of $/
... until the end of scope, and whoever changes this
... sub is careful enough in the future
which I think is worse. Introducing an extra scope explicitly
delimit the part you want to use localized $/ like this
{ local $/; print <$fd>; }
might have been more palatable. Am I guessing the reason of
your change wrong?
* gitweb: Always link to plain version of the blob
Needs a better description to justify the change by describing
in what way the current implementation is broken.
The ones above I won't apply just yet (I first applied, merged
into "next" and then commented on the earlier 19 series, but I'm
not going to repeat that mistake again this time).
The rest look OK so I'll apply them; [6/7] depended on [5/7] so
I manually adjusted the context.
^ permalink raw reply
* Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements
From: Jakub Narebski @ 2006-08-26 21:18 UTC (permalink / raw)
To: git
In-Reply-To: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> * gitweb: Restore old git_blame using git-annotate under "annotate"
>
> I actually was hoping to see a patch to remove the git_blame
> which is not used as far as I can see.
>
> Although we carried annotate and blame in git.git tree for
> quite a while, the intention was always to deprecate one over
> the other once pros-and-cons of each implementation become
> clear (and I think people would not miss annotate if we remove
> annotate and make it an alias for blame -c anymore). What's
> the reason we would want to have both?
Well, removing it completely might be not a best idea for now,
as git_annotate has for example two more columns ("Age" and "Author")
which might be usefull.
I guess the intention is to provide patch for those who want
to improve git_blame, to have comparison with git_annotate,
i.e. patch not applied but available.
> * gitweb: Use 'local $/ = undef;' before 'print <$fd>;'
>
> You changed:
>
> $/ = undef;
> print <$fd>;
> ... hope that nobody depends on standard value of $/
> ... around here, which may still break if you did sub
> ... calls, the sub did not localize $/ (who would?),
> ... and depended to have a sane $/.
> $/ = "\n";
>
> to
>
> local $/ = undef;
> print <$fd>;
> ... hope that nobody depends on standard value of $/
> ... until the end of scope, and whoever changes this
> ... sub is careful enough in the future
>
> which I think is worse. Introducing an extra scope explicitly
> delimit the part you want to use localized $/ like this
>
> { local $/; print <$fd>; }
>
> might have been more palatable. Am I guessing the reason of
> your change wrong?
Actually only one change was from '$/ = undef; print <$fd>; $/ = "\n"',
namely at the end of git_blob_plain. I think nobody will print anything
to the end of the sub.
Two other chunks actually _introduced_ 'local $/ = undef', both in the
fairly short 'if' body. First to read $home_text (there change is not that
important, as $home_text should be fairly short), next more important in
git_snapshot.
IMVHO 'local $/ = undef' in all commands that get the rest of the output
from filehandle (and outputting nothing later), like all 'plain' outputs
should be the idiom to use.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* [PATCH] Add git-zip-tree
From: Rene Scharfe @ 2006-08-26 21:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In the Windows world ZIP files are better supported than tar files.
Windows even includes built-in support for ZIP files nowadays.
git-zip-tree is similar to git-tar-tree; it creates ZIP files out of
git trees. It stores the commit ID (if available) in a ZIP file comment
which can be extracted by unzip.
There's still quite some room for improvement: this initial version
supports no symlinks, calls write() way too often (three times per file)
and there is no unit test.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Documentation/git-zip-tree.txt | 67 +++++++
Makefile | 3
builtin-zip-tree.c | 352 +++++++++++++++++++++++++++++++++++++++++
builtin.h | 1
git.c | 1
5 files changed, 423 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-zip-tree.txt b/Documentation/git-zip-tree.txt
new file mode 100644
index 0000000..2e9d981
--- /dev/null
+++ b/Documentation/git-zip-tree.txt
@@ -0,0 +1,67 @@
+git-zip-tree(1)
+===============
+
+NAME
+----
+git-zip-tree - Creates a ZIP archive of the files in the named tree
+
+
+SYNOPSIS
+--------
+'git-zip-tree' [-0|...|-9] <tree-ish> [ <base> ]
+
+DESCRIPTION
+-----------
+Creates a ZIP archive containing the tree structure for the named tree.
+When <base> is specified it is added as a leading path to the files in the
+generated ZIP archive.
+
+git-zip-tree behaves differently when given a tree ID versus when given
+a commit ID or tag ID. In the first case the current time is used as
+modification time of each file in the archive. In the latter case the
+commit time as recorded in the referenced commit object is used instead.
+Additionally the commit ID is stored as an archive comment.
+
+Currently git-zip-tree can handle only files and directories, symbolic
+links are not supported.
+
+OPTIONS
+-------
+
+-0::
+ Store the files instead of deflating them.
+
+-9::
+ Highest and slowest compression level. You can specify any
+ number from 1 to 9 to adjust compression speed and ratio.
+
+<tree-ish>::
+ The tree or commit to produce ZIP archive for. If it is
+ the object name of a commit object.
+
+<base>::
+ Leading path to the files in the resulting ZIP archive.
+
+EXAMPLES
+--------
+git zip-tree v1.4.0 git-1.4.0 >git-1.4.0.zip::
+
+ Create a ZIP file for v1.4.0 release.
+
+git zip-tree HEAD:Documentation/ git-docs >docs.zip::
+
+ Put everything in the current head's Documentation/ directory
+ into 'docs.zip', with the prefix 'git-docs/'.
+
+Author
+------
+Written by Rene Scharfe.
+
+Documentation
+--------------
+Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/Makefile b/Makefile
index b15b420..d9741f9 100644
--- a/Makefile
+++ b/Makefile
@@ -292,7 +292,8 @@ BUILTIN_OBJS = \
builtin-update-ref.o \
builtin-upload-tar.o \
builtin-verify-pack.o \
- builtin-write-tree.o
+ builtin-write-tree.o \
+ builtin-zip-tree.o
GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
LIBS = $(GITLIBS) -lz
diff --git a/builtin-zip-tree.c b/builtin-zip-tree.c
new file mode 100644
index 0000000..8dfe421
--- /dev/null
+++ b/builtin-zip-tree.c
@@ -0,0 +1,352 @@
+/*
+ * Copyright (c) 2006 Rene Scharfe
+ */
+#include <time.h>
+#include "cache.h"
+#include "commit.h"
+#include "blob.h"
+#include "tree.h"
+#include "quote.h"
+#include "builtin.h"
+
+static const char zip_tree_usage[] =
+"git-zip-tree [-0|...|-9] <tree-ish> [ <base> ]";
+
+static int zip_date;
+static int zip_time;
+
+static unsigned char *zip_dir;
+static unsigned int zip_dir_size;
+
+static unsigned int zip_offset;
+static unsigned int zip_dir_offset;
+static unsigned int zip_dir_entries;
+
+#define ZIP_DIRECTORY_MIN_SIZE (1024 * 1024)
+
+struct zip_local_header {
+ unsigned char magic[4];
+ unsigned char version[2];
+ unsigned char flags[2];
+ unsigned char compression_method[2];
+ unsigned char mtime[2];
+ unsigned char mdate[2];
+ unsigned char crc32[4];
+ unsigned char compressed_size[4];
+ unsigned char size[4];
+ unsigned char filename_length[2];
+ unsigned char extra_length[2];
+};
+
+struct zip_dir_header {
+ unsigned char magic[4];
+ unsigned char creator_version[2];
+ unsigned char version[2];
+ unsigned char flags[2];
+ unsigned char compression_method[2];
+ unsigned char mtime[2];
+ unsigned char mdate[2];
+ unsigned char crc32[4];
+ unsigned char compressed_size[4];
+ unsigned char size[4];
+ unsigned char filename_length[2];
+ unsigned char extra_length[2];
+ unsigned char comment_length[2];
+ unsigned char disk[2];
+ unsigned char attr1[2];
+ unsigned char attr2[4];
+ unsigned char offset[4];
+};
+
+struct zip_dir_trailer {
+ unsigned char magic[4];
+ unsigned char disk[2];
+ unsigned char directory_start_disk[2];
+ unsigned char entries_on_this_disk[2];
+ unsigned char entries[2];
+ unsigned char size[4];
+ unsigned char offset[4];
+ unsigned char comment_length[2];
+};
+
+static void copy_le16(unsigned char *dest, unsigned int n)
+{
+ dest[0] = 0xff & n;
+ dest[1] = 0xff & (n >> 010);
+}
+
+static void copy_le32(unsigned char *dest, unsigned int n)
+{
+ dest[0] = 0xff & n;
+ dest[1] = 0xff & (n >> 010);
+ dest[2] = 0xff & (n >> 020);
+ dest[3] = 0xff & (n >> 030);
+}
+
+static void *zlib_deflate(void *data, unsigned long size,
+ unsigned long *compressed_size)
+{
+ z_stream stream;
+ unsigned long maxsize;
+ void *buffer;
+ int result;
+
+ memset(&stream, 0, sizeof(stream));
+ deflateInit(&stream, zlib_compression_level);
+ maxsize = deflateBound(&stream, size);
+ buffer = xmalloc(maxsize);
+
+ stream.next_in = data;
+ stream.avail_in = size;
+ stream.next_out = buffer;
+ stream.avail_out = maxsize;
+
+ do {
+ result = deflate(&stream, Z_FINISH);
+ } while (result == Z_OK);
+
+ if (result != Z_STREAM_END) {
+ free(buffer);
+ return NULL;
+ }
+
+ deflateEnd(&stream);
+ *compressed_size = stream.total_out;
+
+ return buffer;
+}
+
+static char *construct_path(const char *base, int baselen,
+ const char *filename, int isdir, int *pathlen)
+{
+ int filenamelen = strlen(filename);
+ int len = baselen + filenamelen;
+ char *path, *p;
+
+ if (isdir)
+ len++;
+ p = path = xmalloc(len + 1);
+
+ memcpy(p, base, baselen);
+ p += baselen;
+ memcpy(p, filename, filenamelen);
+ p += filenamelen;
+ if (isdir)
+ *p++ = '/';
+ *p = '\0';
+
+ *pathlen = len;
+
+ return path;
+}
+
+static int write_zip_entry(const unsigned char *sha1,
+ const char *base, int baselen,
+ const char *filename, unsigned mode, int stage)
+{
+ struct zip_local_header header;
+ struct zip_dir_header dirent;
+ unsigned long compressed_size;
+ unsigned long uncompressed_size;
+ unsigned long crc;
+ unsigned long direntsize;
+ unsigned long size;
+ int method;
+ int result = -1;
+ int pathlen;
+ unsigned char *out;
+ char *path;
+ char type[20];
+ void *buffer = NULL;
+ void *deflated = NULL;
+
+ crc = crc32(0, Z_NULL, 0);
+
+ path = construct_path(base, baselen, filename, S_ISDIR(mode), &pathlen);
+ if (pathlen > 0xffff) {
+ error("path too long (%d chars, SHA1: %s): %s", pathlen,
+ sha1_to_hex(sha1), path);
+ goto out;
+ }
+
+ if (S_ISDIR(mode)) {
+ method = 0;
+ result = READ_TREE_RECURSIVE;
+ out = NULL;
+ uncompressed_size = 0;
+ compressed_size = 0;
+ } else if (S_ISREG(mode)) {
+ method = zlib_compression_level == 0 ? 0 : 8;
+ result = 0;
+ buffer = read_sha1_file(sha1, type, &size);
+ if (!buffer)
+ die("cannot read %s", sha1_to_hex(sha1));
+ crc = crc32(crc, buffer, size);
+ out = buffer;
+ uncompressed_size = size;
+ compressed_size = size;
+ } else {
+ error("unsupported file mode: 0%o (SHA1: %s)", mode,
+ sha1_to_hex(sha1));
+ goto out;
+ }
+
+ if (method == 8) {
+ deflated = zlib_deflate(buffer, size, &compressed_size);
+ if (deflated && compressed_size - 6 < size) {
+ /* ZLIB --> raw compressed data (see RFC 1950) */
+ out = deflated + 2; /* CMF and FLG ... */
+ compressed_size -= 6; /* ... and ADLER32 */
+ } else {
+ method = 0;
+ compressed_size = size;
+ }
+ }
+
+ /* make sure we have enough free space in the dictionary */
+ direntsize = sizeof(struct zip_dir_header) + pathlen;
+ while (zip_dir_size < zip_dir_offset + direntsize) {
+ zip_dir_size += ZIP_DIRECTORY_MIN_SIZE;
+ zip_dir = xrealloc(zip_dir, zip_dir_size);
+ }
+
+ copy_le32(dirent.magic, 0x02014b50);
+ copy_le16(dirent.creator_version, 0);
+ copy_le16(dirent.version, 20);
+ copy_le16(dirent.flags, 0);
+ copy_le16(dirent.compression_method, method);
+ copy_le16(dirent.mtime, zip_time);
+ copy_le16(dirent.mdate, zip_date);
+ copy_le32(dirent.crc32, crc);
+ copy_le32(dirent.compressed_size, compressed_size);
+ copy_le32(dirent.size, uncompressed_size);
+ copy_le16(dirent.filename_length, pathlen);
+ copy_le16(dirent.extra_length, 0);
+ copy_le16(dirent.comment_length, 0);
+ copy_le16(dirent.disk, 0);
+ copy_le16(dirent.attr1, 0);
+ copy_le32(dirent.attr2, 0);
+ copy_le32(dirent.offset, zip_offset);
+ memcpy(zip_dir + zip_dir_offset, &dirent, sizeof(struct zip_dir_header));
+ zip_dir_offset += sizeof(struct zip_dir_header);
+ memcpy(zip_dir + zip_dir_offset, path, pathlen);
+ zip_dir_offset += pathlen;
+ zip_dir_entries++;
+
+ copy_le32(header.magic, 0x04034b50);
+ copy_le16(header.version, 20);
+ copy_le16(header.flags, 0);
+ copy_le16(header.compression_method, method);
+ copy_le16(header.mtime, zip_time);
+ copy_le16(header.mdate, zip_date);
+ copy_le32(header.crc32, crc);
+ copy_le32(header.compressed_size, compressed_size);
+ copy_le32(header.size, uncompressed_size);
+ copy_le16(header.filename_length, pathlen);
+ copy_le16(header.extra_length, 0);
+ write_or_die(1, &header, sizeof(struct zip_local_header));
+ zip_offset += sizeof(struct zip_local_header);
+ write_or_die(1, path, pathlen);
+ zip_offset += pathlen;
+ if (compressed_size > 0) {
+ write_or_die(1, out, compressed_size);
+ zip_offset += compressed_size;
+ }
+
+out:
+ free(buffer);
+ free(deflated);
+ free(path);
+
+ return result;
+}
+
+static void write_zip_trailer(const unsigned char *sha1)
+{
+ struct zip_dir_trailer trailer;
+
+ copy_le32(trailer.magic, 0x06054b50);
+ copy_le16(trailer.disk, 0);
+ copy_le16(trailer.directory_start_disk, 0);
+ copy_le16(trailer.entries_on_this_disk, zip_dir_entries);
+ copy_le16(trailer.entries, zip_dir_entries);
+ copy_le32(trailer.size, zip_dir_offset);
+ copy_le32(trailer.offset, zip_offset);
+ copy_le16(trailer.comment_length, sha1 ? 40 : 0);
+
+ write_or_die(1, zip_dir, zip_dir_offset);
+ write_or_die(1, &trailer, sizeof(struct zip_dir_trailer));
+ if (sha1)
+ write_or_die(1, sha1_to_hex(sha1), 40);
+}
+
+static void dos_time(time_t *time, int *dos_date, int *dos_time)
+{
+ struct tm *t = localtime(time);
+
+ *dos_date = t->tm_mday + (t->tm_mon + 1) * 32 +
+ (t->tm_year + 1900 - 1980) * 512;
+ *dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;
+}
+
+int cmd_zip_tree(int argc, const char **argv, const char *prefix)
+{
+ unsigned char sha1[20];
+ struct tree *tree;
+ struct commit *commit;
+ time_t archive_time;
+ char *base;
+ int baselen;
+
+ git_config(git_default_config);
+
+ if (argc > 1 && argv[1][0] == '-') {
+ if (isdigit(argv[1][1]) && argv[1][2] == '\0') {
+ zlib_compression_level = argv[1][1] - '0';
+ argc--;
+ argv++;
+ }
+ }
+
+ switch (argc) {
+ case 3:
+ base = strdup(argv[2]);
+ baselen = strlen(base);
+ break;
+ case 2:
+ base = strdup("");
+ baselen = 0;
+ break;
+ default:
+ usage(zip_tree_usage);
+ }
+
+ if (get_sha1(argv[1], sha1))
+ die("Not a valid object name %s", argv[1]);
+
+ commit = lookup_commit_reference_gently(sha1, 1);
+ archive_time = commit ? commit->date : time(NULL);
+ dos_time(&archive_time, &zip_date, &zip_time);
+
+ zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
+ zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
+
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die("not a tree object");
+
+ if (baselen > 0) {
+ write_zip_entry(tree->object.sha1, "", 0, base, 040777, 0);
+ base = xrealloc(base, baselen + 1);
+ base[baselen] = '/';
+ baselen++;
+ base[baselen] = '\0';
+ }
+ read_tree_recursive(tree, base, baselen, 0, NULL, write_zip_entry);
+ write_zip_trailer(commit ? commit->object.sha1 : NULL);
+
+ free(zip_dir);
+ free(base);
+
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index ade58c4..25431d7 100644
--- a/builtin.h
+++ b/builtin.h
@@ -52,6 +52,7 @@ extern int cmd_show(int argc, const char
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_zip_tree(int argc, const char **argv, const char *prefix);
extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
extern int cmd_update_index(int argc, const char **argv, const char *prefix);
extern int cmd_update_ref(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index a01d195..ded89fb 100644
--- a/git.c
+++ b/git.c
@@ -263,6 +263,7 @@ static void handle_internal_command(int
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tar-tree", cmd_tar_tree, RUN_SETUP },
+ { "zip-tree", cmd_zip_tree, RUN_SETUP },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
{ "update-index", cmd_update_index, RUN_SETUP },
{ "update-ref", cmd_update_ref, RUN_SETUP },
^ permalink raw reply related
* [PATCH] gitweb: Fix typo in git_difftree_body
From: Jakub Narebski @ 2006-08-26 21:33 UTC (permalink / raw)
To: git
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 02d327c..9cf2b78 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1557,7 +1557,7 @@ sub git_difftree_body {
"blob") .
" | " .
$cgi->a({-href => href(action=>"history", hash_base=>$parent,
- file_name=>$diff{'file'})},\
+ file_name=>$diff{'file'})},
"history") .
"</td>\n";
--
1.4.1.1
^ permalink raw reply related
* Re: [PATCH] dir: do all size checks before seeking back and fix file closing
From: Linus Torvalds @ 2006-08-26 22:13 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060826141709.GC11601@diku.dk>
On Sat, 26 Aug 2006, Jonas Fonseca wrote:
>
> diff --git a/dir.c b/dir.c
> index d53d48f..ff8a2fb 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -122,11 +122,11 @@ static int add_excludes_from_file_1(cons
> size = lseek(fd, 0, SEEK_END);
> if (size < 0)
> goto err;
> - lseek(fd, 0, SEEK_SET);
> if (size == 0) {
> close(fd);
> return 0;
> }
> + lseek(fd, 0, SEEK_SET);
I really think you'd be better off rewriting that to use "fstat()"
instead. I don't know why it uses two lseek's, but it's wrong, and looks
like some bad habit Junio picked up at some point.
> @@ -146,7 +146,7 @@ static int add_excludes_from_file_1(cons
> return 0;
>
> err:
> - if (0 <= fd)
> + if (0 >= fd)
> close(fd);
That's wrong.
Now, admittedly it's wrong because another bad habit Junio picked up
(doing comparisons with constants in the wrong order), so please write it
as
if (fd >= 0)
close(fd);
instead.
Junio: I realize that you claim that you learnt that syntax from an
authorative source, but he was _wrong_. Doing the constant first is more
likely to cause bugs, rather than less. Compilers will warn about the
if (x = 0)
..
kind of bug, and putting the constant first just confuses humans.
It's more important to _not_ confuse humans than it is to try to avoid an
uncommon error that compilers can and do warn about anyway.
Linus
^ permalink raw reply
* Re: [PATCH] dir: do all size checks before seeking back and fix file closing
From: Jakub Narebski @ 2006-08-26 22:35 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0608261509290.11811@g5.osdl.org>
Linus Torvalds wrote:
> Now, admittedly it's wrong because another bad habit Junio picked up
> (doing comparisons with constants in the wrong order), so please write it
> as
>
> if (fd >= 0)
> close(fd);
>
> instead.
>
> Junio: I realize that you claim that you learnt that syntax from an
> authorative source, but he was _wrong_. Doing the constant first is more
> likely to cause bugs, rather than less. Compilers will warn about the
>
> if (x = 0)
> ..
>
> kind of bug, and putting the constant first just confuses humans.
Well, perhaps except checking if variable is in given range, e.g.
if (0 <= x && x <= 5)
> It's more important to _not_ confuse humans than it is to try to avoid an
> uncommon error that compilers can and do warn about anyway.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Relative timestamps in git log
From: Linus Torvalds @ 2006-08-26 22:45 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
I noticed that I was looking at the kernel gitweb output at some point
rather than just do "git log", simply because I liked seeing the
simplified date-format, ie the "5 days ago" rather than a full date.
This adds infrastructure to do that for "git log" too. It does NOT add the
actual flag to enable it, though, so right now this patch is a no-op, but
it should now be easy to add a command line flag (and possibly a config
file option) to just turn on the "relative" date format.
The exact cut-off points when it switches from days to weeks etc are
totally arbitrary, but are picked somewhat to avoid the "1 weeks ago"
thing (by making it show "10 days ago" rather than "1 week", or "70
minutes ago" rather than "1 hour ago").
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
With this, and changing the ", 0" for CMIT_FMT_MEDIUM into a ", 1", I get
output like:
commit a7f051987c5f020e60da1e5d6ddefc3d443d3299
Merge: 030b520... a8e0d16...
Author: Junio C Hamano <junkio@cox.net>
Date: 22 hours ago
Merge branch 'gl/cleanup'
* gl/cleanup:
Convert memset(hash,0,20) to hashclr(hash).
Convert memcpy(a,b,20) to hashcpy(a,b).
commit 030b52087f0a4b7b1178d34839868ce438eb2f0e
Author: Jakub Narebski <jnareb@gmail.com>
Date: 20 hours ago
gitweb: git_annotate didn't expect negative numeric timezone
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
commit b22d449721b22f6ec090f22c418ae6b0a560f78d
Author: Eric Wong <normalperson@yhbt.net>
Date: 23 hours ago
git-svn: add the 'dcommit' command
This is a high-level wrapper around the 'commit-diff' command
and used to produce cleaner history against the mirrored repository
through rebase/reset usage.
It's basically a more polished version of this:
for i in `git rev-list --no-merges remotes/git-svn..HEAD | tac`; do
git-svn commit-diff $i~1 $i
done
git reset --hard remotes/git-svn
Signed-off-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Junio C Hamano <junkio@cox.net>
which looks quite readable..
diff --git a/builtin-cat-file.c b/builtin-cat-file.c
index 7a6fa56..6c16bfa 100644
--- a/builtin-cat-file.c
+++ b/builtin-cat-file.c
@@ -54,7 +54,7 @@ static void pprint_tag(const unsigned ch
write_or_die(1, tagger, sp - tagger);
date = strtoul(sp, &ep, 10);
tz = strtol(ep, NULL, 10);
- sp = show_date(date, tz);
+ sp = show_date(date, tz, 0);
write_or_die(1, sp, strlen(sp));
xwrite(1, "\n", 1);
break;
diff --git a/cache.h b/cache.h
index 1f212d7..ccb83a1 100644
--- a/cache.h
+++ b/cache.h
@@ -286,7 +286,7 @@ extern void *read_object_with_reference(
unsigned long *size,
unsigned char *sha1_ret);
-const char *show_date(unsigned long time, int timezone);
+const char *show_date(unsigned long time, int timezone, int relative);
const char *show_rfc2822_date(unsigned long time, int timezone);
int parse_date(const char *date, char *buf, int bufsize);
void datestamp(char *buf, int bufsize);
diff --git a/commit.c b/commit.c
index 00bc3de..c3ff9b4 100644
--- a/commit.c
+++ b/commit.c
@@ -507,14 +507,14 @@ static int add_user_info(const char *wha
}
switch (fmt) {
case CMIT_FMT_MEDIUM:
- ret += sprintf(buf + ret, "Date: %s\n", show_date(time, tz));
+ ret += sprintf(buf + ret, "Date: %s\n", show_date(time, tz, 0));
break;
case CMIT_FMT_EMAIL:
ret += sprintf(buf + ret, "Date: %s\n",
show_rfc2822_date(time, tz));
break;
case CMIT_FMT_FULLER:
- ret += sprintf(buf + ret, "%sDate: %s\n", what, show_date(time, tz));
+ ret += sprintf(buf + ret, "%sDate: %s\n", what, show_date(time, tz, 0));
break;
default:
/* notin' */
diff --git a/date.c b/date.c
index d780846..92e3f6e 100644
--- a/date.c
+++ b/date.c
@@ -37,6 +37,16 @@ static const char *weekday_names[] = {
"Sundays", "Mondays", "Tuesdays", "Wednesdays", "Thursdays", "Fridays", "Saturdays"
};
+static time_t gm_time_t(unsigned long time, int tz)
+{
+ int minutes;
+
+ minutes = tz < 0 ? -tz : tz;
+ minutes = (minutes / 100)*60 + (minutes % 100);
+ minutes = tz < 0 ? -minutes : minutes;
+ return time + minutes * 60;
+}
+
/*
* The "tz" thing is passed in as this strange "decimal parse of tz"
* thing, which means that tz -0100 is passed in as the integer -100,
@@ -44,21 +54,56 @@ static const char *weekday_names[] = {
*/
static struct tm *time_to_tm(unsigned long time, int tz)
{
- time_t t;
- int minutes;
-
- minutes = tz < 0 ? -tz : tz;
- minutes = (minutes / 100)*60 + (minutes % 100);
- minutes = tz < 0 ? -minutes : minutes;
- t = time + minutes * 60;
+ time_t t = gm_time_t(time, tz);
return gmtime(&t);
}
-const char *show_date(unsigned long time, int tz)
+const char *show_date(unsigned long time, int tz, int relative)
{
struct tm *tm;
static char timebuf[200];
+ if (relative) {
+ unsigned long diff;
+ time_t t = gm_time_t(time, tz);
+ struct timeval now;
+ gettimeofday(&now, NULL);
+ 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;
+ }
+ /* Turn it into days */
+ diff = (diff + 12) / 24;
+ if (diff < 14) {
+ snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
+ return timebuf;
+ }
+ if (diff < 53) {
+ snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
+ return timebuf;
+ }
+ if (diff < 365) {
+ snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
+ return timebuf;
+ }
+ /* Else fall back on absolute format.. */
+ }
+
tm = time_to_tm(time, tz);
if (!tm)
return NULL;
^ 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