* Re: Change set based shallow clone
From: Jon Smirl @ 2006-09-09 22:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Marco Costalba, Paul Mackerras, linux@horizon.com,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609091433460.27779@g5.osdl.org>
On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
> However, it's just a heuristic. "Most recent date" is not well-defined in
> a distributed environment that doesn't have a global clock. If somebody
> does commits on a machine that has the clock just set wrong, "most recent"
> may well not be _really_ recent.
When a merge happens could git fix things up in the database by adding
a corrected, hidden time stamp that would keep things from having an
out of order time sequence? That way you wouldn't need to rediscover
the out of order commit each time the tree is generated.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: {RFC/PATCH] micro-optimize get_sha1_hex()
From: Jeff Garzik @ 2006-09-09 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vzmd8vh6q.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> int get_sha1_hex(const char *hex, unsigned char *sha1)
> {
> int i;
> for (i = 0; i < 20; i++) {
> - unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
> - if (val & ~0xff)
> + unsigned int v, w, val;
> + v = *hex++;
> + if ((v < '0') || ('f' < v) ||
> + ((v = hexval[v-'0']) == 255))
> + return -1;
> + w = *hex++;
> + if ((w < '0') || ('f' < w) ||
> + ((w = hexval[w-'0']) == 255))
> return -1;
> - *sha1++ = val;
> - hex += 2;
> + *sha1++ = (v << 4) | w;
Why not just make the table include the range of all possible
characters? That would eliminate some comparisons and subtractions
against '0'.
The end result would look more like the current (unpatched) form, except
with a function replaced by the table.
Jeff
^ permalink raw reply
* Re: [PATCH] gitweb: support perl 5.6
From: Jakub Narebski @ 2006-09-09 22:20 UTC (permalink / raw)
To: git
In-Reply-To: <20060909145914.GA25289@liacs.nl>
Sven Verdoolaege wrote:
> +# returns pipe reading from git command with given arguments
> +sub git_pipe {
> + open my $fd, "-|", join(' ', git_cmd(), @_) or return undef;
> + return $fd;
> +}
> +
I'm sorry, but this is not enough. For example the $file_name argument
should be quoted in shell, i.e. in three argument magic open "-|". So
either you return to your old patch, which changes each list form of open
"-|" into old three argument form (which adds penalty of additional shell
invocation, and is prone to shell interpretation of arguments), and
sometimes add quotes (e.g. "... \'$file_name\' ..."), or (what would be
better) to reimplement list form of open "-|" using two argument forking
open "-|", and doing exec in child, a la "Safe Pipe Opens" in perlipc(3pm).
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] gitweb: reset input record separator in parse_commit even on error
From: Jakub Narebski @ 2006-09-09 22:00 UTC (permalink / raw)
To: git
In-Reply-To: <20060909205159.GC16906@coredump.intra.peff.net>
Jeff King wrote:
> On Sat, Sep 09, 2006 at 05:12:36PM +0200, Sven Verdoolaege wrote:
>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 7afdf33..60dd598 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -897,8 +897,8 @@ sub parse_commit {
>> my $fd = git_pipe("rev-list", "--header", "--parents", "--max-count=1", $commit_id)
>> or return;
>> @commit_lines = split '\n', <$fd>;
>> - close $fd or return;
>> $/ = "\n";
>> + close $fd or return;
>> pop @commit_lines;
>> }
>> my $header = shift @commit_lines;
>
> You missed the other early return from git_pipe. However, I think the
> approach is wrong; this is a great opportunity to use the dynamic
> scoping offered by 'local':
>
> else {
> local $/ = "\0";
> # do stuff with $/ as "\0"
> }
> # $/ has automatically been reset at the end of the block
And this should be done consistently, for all plain formats
(blob_plain, blobdiff_plain, commitdiff_plain), and some other
places.
Sometimes it is
local $/ = "\0";
more often
local $/ = undef;
(or equivalently but slightly less human friendly, just 'local $/');
> Looking further at this bit of code, it seems even more confusing,
> though. We split the output of rev-list on NUL, grab presumably the
> entire thing (since there shouldn't be any NULs in the output, right?)
> and then split it on newline into a list. Why aren't we doing this:
> else {
> open my $fd, ...;
> @commit_lines = map { chomp; $_ } <$fd>;
> ...
That is because by default git-rev-list separates output for different
commits (when --header option is used) bu NULL... only because of
"--max-count=1" there is only _one_ record... nevertheless it ends with
"\0" (NULL) character.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* {RFC/PATCH] micro-optimize get_sha1_hex()
From: Junio C Hamano @ 2006-09-09 21:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
I was profiling 'git-rev-list v2.16.12..', because I suspected
insert_by_date() might be expensive (the function inserts into
singly-linked ordered list, so the data structure has to become
array based to allow optimization). But profiling showed it was
not the bottleneck. Probably because the kernel history is not
that bushy and we do not have too many "active" heads during
traversal.
But I noticed something else.
Flat profile:
Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls ms/call ms/call name
21.52 0.17 0.17 2800320 0.00 0.00 hexval
15.19 0.29 0.12 70008 0.00 0.00 get_sha1_hex
15.19 0.41 0.12 70008 0.00 0.00 lookup_object
15.19 0.53 0.12 68495 0.00 0.00 find_pack_entry_one
11.39 0.62 0.09 198667 0.00 0.00 insert_obj_hash
7.60 0.68 0.06 34675 0.00 0.00 unpack_object_header_gently
3.80 0.71 0.03 33819 0.00 0.02 parse_commit_buffer
1.27 0.72 0.01 103822 0.00 0.00 commit_list_insert
1.27 0.73 0.01 67640 0.00 0.00 prepare_packed_git
1.27 0.74 0.01 67611 0.00 0.00 created_object
1.27 0.75 0.01 33820 0.00 0.00 use_packed_git
1.27 0.76 0.01 33819 0.00 0.00 lookup_tree
1.27 0.77 0.01 1 10.00 10.00 prepare_packed_git_one
1.27 0.78 0.01 parse_tree_indirect
1.27 0.79 0.01 verify_filename
...
The attached brings get_sha1_hex() down from 15.19% to 5.41%,
but I feel we should be able to do better.
Is this barking up the wrong tree? Or did I pick a good target
but the shooter wasn't skilled enough?
diff --git a/sha1_file.c b/sha1_file.c
index 428d791..00aa364 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -26,26 +26,30 @@ const unsigned char null_sha1[20];
static unsigned int sha1_file_open_flag = O_NOATIME;
-static unsigned hexval(char c)
-{
- if (c >= '0' && c <= '9')
- return c - '0';
- if (c >= 'a' && c <= 'f')
- return c - 'a' + 10;
- if (c >= 'A' && c <= 'F')
- return c - 'A' + 10;
- return ~0;
-}
+static const unsigned char hexval[] = {
+ 0, 1, 2, 3, 4, 5, 6, 7, /* 30-37 */
+ 8, 9, 255, 255, 255, 255, 255, 255, /* 38-3F */
+ 255, 10, 11, 12, 13, 14, 15, 255, /* 40-47 */
+ 255, 255, 255, 255, 255, 255, 255, 255, /* 48-4F */
+ 255, 255, 255, 255, 255, 255, 255, 255, /* 50-57 */
+ 255, 255, 255, 255, 255, 255, 255, 255, /* 58-5F */
+ 255, 10, 11, 12, 13, 14, 15, 255, /* 60-67 */
+};
int get_sha1_hex(const char *hex, unsigned char *sha1)
{
int i;
for (i = 0; i < 20; i++) {
- unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
- if (val & ~0xff)
+ unsigned int v, w, val;
+ v = *hex++;
+ if ((v < '0') || ('f' < v) ||
+ ((v = hexval[v-'0']) == 255))
+ return -1;
+ w = *hex++;
+ if ((w < '0') || ('f' < w) ||
+ ((w = hexval[w-'0']) == 255))
return -1;
- *sha1++ = val;
- hex += 2;
+ *sha1++ = (v << 4) | w;
}
return 0;
}
^ permalink raw reply related
* (unknown),
From: Rajkumar S @ 2006-09-09 21:46 UTC (permalink / raw)
To: git
subscribe git
^ permalink raw reply
* Re: Change set based shallow clone
From: Linus Torvalds @ 2006-09-09 21:40 UTC (permalink / raw)
To: Jeff King
Cc: Marco Costalba, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <20060909204307.GB16906@coredump.intra.peff.net>
On Sat, 9 Sep 2006, Jeff King wrote:
>
> The problem you describe seems to come from doing a depth-first display
> of each branch
No, not at all.
We _don't_ do depth-first, or breadth-first. The thing is, neither would
really work, since the "distance" between commits is very variable.
Instead, the rule is always to try to traverse the tree in date order.
That's most likely the closest order to what you want - it ends up being
breadth-first if there is a lot of concurrent work, and if there is one
old branch and one new branch, we'll look at the new one first, and
approximate depth-first there.
> Why not look at the tip of each "active" branch
> simultaneously and pick the one with the most recent date?
Which is _exactly_ what we do.
However, it's just a heuristic. "Most recent date" is not well-defined in
a distributed environment that doesn't have a global clock. If somebody
does commits on a machine that has the clock just set wrong, "most recent"
may well not be _really_ recent.
So all the algorithms are actually very careful to just use the date as a
heuristic. They have real guarantees, but they aren't about the ordering.
The ordering is just used as a way to have _some_ non-random and likely
good order to traverse the DAG in. Doing it depth-first would suck (you'd
always reach the root before you reach a lot of much more interesting
commits), and breadth-first is not well-defined either, since one "level"
can be a year old for one parent, and a day old for another.
So you could kind of think of the ordering as "breadth-first, modified by
date" kind of thing.
Linus
^ permalink raw reply
* Re: Change set based shallow clone
From: Jeff King @ 2006-09-09 21:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4pvgwxrc.fsf@assigned-by-dhcp.cox.net>
On Sat, Sep 09, 2006 at 02:11:51PM -0700, Junio C Hamano wrote:
> The trouble Linus illustrated is that in a global project you
> cannot rely on timestamps always being correct. You can use
> them as HINT, but you need to be prepared to do sensible things
> when some people screw up the time.
Right, I forgot about the unreliability of the timestamps. Thanks for
the explanation!
-Peff
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-09 21:11 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20060909204307.GB16906@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm just coming into this discussion in the middle and know very little
> about the rev-list code, so please humor me and tell me why my
> suggestion is completely irrelevant.
Not irrelevant.
> The problem you describe seems to come from doing a depth-first display
> of each branch. Why not look at the tip of each "active" branch
> simultaneously and pick the one with the most recent date? Something
> like:
That's what we have been doing from day one.
The trouble Linus illustrated is that in a global project you
cannot rely on timestamps always being correct. You can use
them as HINT, but you need to be prepared to do sensible things
when some people screw up the time.
> On Sat, Sep 09, 2006 at 01:05:42PM -0700, Linus Torvalds wrote:
>
>> The example is
>>
>> A <--- tip of branch
>> / \
>> B E
>> | |
>> | F
>> | /
>> C
>> |
>> D
>> ...
>>
>> where the lettering is in "date order" (ie "A" is more recent than "B"
>> etc). In this situation, we'd start following the branch A->B->C->D->..
>> before we even start looking at E and F, because they all _look_ more
>> recent.
The ancestry graph, topologically, flows from bottom to top but
the timestamps are in F E D C B A order (A is closer to current,
F is in the most distant past). Somebody forked from C on a
machine with slow clock, made two commits with wrong (from the
point of view of the person who made commit C anyway) timestamps,
and that side branch ended up merged with B into A.
You start following A, read A and find B and E (now B and E are
"active" in your lingo), pop B because it is the most recent.
We look at B, find C is the parent, push C into the active list
(which is always sorted by timestamp order). Now "active" are C
and E, and C is most recent so we pop C.
In the past people suggested workarounds such as making commit-tree
to ensure that a child commit has timestamp no older than any of
its parents by either refusing to make such or automatically
adjusting. That would surely work around the problem, but if
somebody made a commit with a wrong timestamp far into the
future, every commit that is made after that will have
"adjusted" timestamp that pretty much is meaningless, so that
would not work well in practice.
If we had a commit generation number field in the commit object,
we could have used something like that. Each commit gets
generation number that is maximum of its parents' generation
number plus one, and we prime the recursive definition by giving
root commits generation number of 0, and rev-list would have
used the generation number not timestamp to do the above
"date-order" sort and we will always get topology right. Of
course fsck-objects needs to be taught to check and complain if
the generation numbers between parent and child are inverted.
But it's too late in the game now -- maybe in git version 47.
^ permalink raw reply
* Re: [PATCH] gitweb: reset input record separator in parse_commit even on error
From: Jeff King @ 2006-09-09 20:51 UTC (permalink / raw)
To: Sven Verdoolaege; +Cc: Jakub Narebski, git
In-Reply-To: <20060909151236.GA25518@liacs.nl>
On Sat, Sep 09, 2006 at 05:12:36PM +0200, Sven Verdoolaege wrote:
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7afdf33..60dd598 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -897,8 +897,8 @@ sub parse_commit {
> my $fd = git_pipe("rev-list", "--header", "--parents", "--max-count=1", $commit_id)
> or return;
> @commit_lines = split '\n', <$fd>;
> - close $fd or return;
> $/ = "\n";
> + close $fd or return;
> pop @commit_lines;
> }
> my $header = shift @commit_lines;
You missed the other early return from git_pipe. However, I think the
approach is wrong; this is a great opportunity to use the dynamic
scoping offered by 'local':
else {
local $/ = "\0";
# do stuff with $/ as "\0"
}
# $/ has automatically been reset at the end of the block
Looking further at this bit of code, it seems even more confusing,
though. We split the output of rev-list on NUL, grab presumably the
entire thing (since there shouldn't be any NULs in the output, right?)
and then split it on newline into a list. Why aren't we doing this:
else {
open my $fd, ...;
@commit_lines = map { chomp; $_ } <$fd>;
...
-Peff
^ permalink raw reply
* Re: Change set based shallow clone
From: Jeff King @ 2006-09-09 20:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Marco Costalba, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609091256110.27779@g5.osdl.org>
On Sat, Sep 09, 2006 at 01:05:42PM -0700, Linus Torvalds wrote:
> The example is
>
> A <--- tip of branch
> / \
> B E
> | |
> | F
> | /
> C
> |
> D
> ...
>
> where the lettering is in "date order" (ie "A" is more recent than "B"
> etc). In this situation, we'd start following the branch A->B->C->D->..
> before we even start looking at E and F, because they all _look_ more
> recent.
I'm just coming into this discussion in the middle and know very little
about the rev-list code, so please humor me and tell me why my
suggestion is completely irrelevant.
The problem you describe seems to come from doing a depth-first display
of each branch. Why not look at the tip of each "active" branch
simultaneously and pick the one with the most recent date? Something
like:
void show_all(commit_array start)
{
commit_array active;
copy_array(active, start);
sort_array(active);
while (active.size > 0) {
show_commit(active.data[0]);
push_back(active.data[0].parents);
pop_front(active);
sort_array(active);
}
}
Obviously you could use a specialized data structure to add nodes into
the right place instead of resorting constantly.
-Peff
^ permalink raw reply
* Re: Change set based shallow clone
From: Linus Torvalds @ 2006-09-09 20:05 UTC (permalink / raw)
To: Marco Costalba
Cc: Paul Mackerras, Jon Smirl, linux@horizon.com, Git Mailing List
In-Reply-To: <e5bfff550609091104s3709b82fld3057a07a84ae857@mail.gmail.com>
On Sat, 9 Sep 2006, Marco Costalba wrote:
>
> Sorry, but I don't understand why you should restart the whole thing
> instead of store away the FIXME commit and continue.
>
> If you read my previous e-mail in this thread perhaps it is better
> explained the whole idea.
>
> Anyhow the basic is:
>
> -git-rev-list starts outputting the data early (order is not guaranteed)
>
> -before to mark for output a revision check if it breaks --topo-order
>
> -if the above is true store the revision away and *do not send*
You don't seem to understand.
It's not that individual new commits might be "out of order", and you can
suppress them.
It's that individual commits end up showing that OTHER COMMITS, THAT YOU
HAVE ALREADY SEEN AND SHOWN, can now be shown to be "out of order".
So you're _not_ just suppressing and delaying the few commits that didn't
conform to the topological sorting. What you're asking for is impossible.
You can't just delay those to the end, because they implicitly are telling
you that the earlier commits were already shown in the wrong order.
The example is
A <--- tip of branch
/ \
B E
| |
| F
| /
C
|
D
...
where the lettering is in "date order" (ie "A" is more recent than "B"
etc). In this situation, we'd start following the branch A->B->C->D->..
before we even start looking at E and F, because they all _look_ more
recent.
But then, at some point, we see E and F, and suddenly when looking at F we
realize that C was printed out much much earlier, and was already shown as
having only one child. We can't just say "delay F to the end", because
we'd then have to draw the line _backwards_ to C, which we haven't even
left room for in the graph, not to mention that the end result would look
absolutely disgusting even if we had.
So we started out painting this as
A
/ \
B |
| |
C |
| |
D |
| |
| E
| |
| F
| |
before we notice how wrong it was, and now we have to go _back_, and
repaint both E and F as being above C, because only once we hit F do we
suddenly realize that yeah, it really _was_ younger than C, despite the
timestamp (which was off, because somebody had set his clock to be a week
in the past by mistake).
So we can't just delay F and "fix it up" at the end.
Linus
^ permalink raw reply
* Re: [PATCH 2/4] git-archive: wire up TAR format.
From: Junio C Hamano @ 2006-09-09 19:42 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: Junio C Hamano, Rene Scharfe, git
In-Reply-To: <cda58cb80609090810t6fdab535r761636e65205a0f@mail.gmail.com>
"Franck Bui-Huu" <vagabon.xyz@gmail.com> writes:
> Almost Acked by me, except you've missed some Rene's comments. And
> more important I fixed an "uninitialized variable" bug. See patch
> below.
Gaah, I swear I fixed all of these at one time in my tree but
somehow forgot to apply the fix-up patch while cleaning it up.
Big thanks for eyeballing.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-09 19:17 UTC (permalink / raw)
To: linux@horizon.com; +Cc: torvalds, git, jonsmirl, paulus
In-Reply-To: <20060909184441.23764.qmail@science.horizon.com>
On 9 Sep 2006 14:44:41 -0400, linux@horizon.com <linux@horizon.com> wrote:
> > Anyhow the basic is:
> >
> > -git-rev-list starts outputting the data early (order is not guaranteed)
> >
> > -before to mark for output a revision check if it breaks --topo-order
> >
> > -if the above is true store the revision away and *do not send*
> >
> > - at the end you get an early started steram of topological corrects
> > revisions without
> > preordering, just because you filter away the (few?) revisions that
> > are not in order.
> > The list you get is guaranteed to be in topo order although my not be complete.
> >
> > - _then_ you send the missing revisions that where previously
> > filtered out. At this stage the receiver has already drwan the graph,
> > indeed it has start drwaing as soon as the first revisons arrived and
> > *very important* receiver used old and fast topo-order parsing code.
> >
> > - finally the fixup routine at receiver's end updates the graph with
> > the info from the small set of out of order revisions filtered out
> > before and sent at the end (only this small set is sent at the end).
>
> The problem is that the gitk display code doesn't *like* input like this;
> it's only designed to append to a list. Handling insertions would be
> hard work for a rare corner case, and a perpetual source of bugs.
>
> Unless gitk does a complete second pass, or course, which would
> guarantee an annoying flicker a few seconds after startup.
> And Twice the work.
>
I think I need to add another argument here, I didn't mention before
for clarity (indeed I'm not very good at this it seems ;-) )
I don't know for gitk, perhaps Paul can better explain, but the usual
optimization of a git visualizer is simply to not draw any graph
until that part does became visibile on the screen.
So your arguments are true but the fact is that there is no graph
insertion handling at all in almost all cases, but only insertion in
loaded revs list ; when the user scrolls to the inserted commit only
_then_ the graph will be calculated and dispalyed (I'm not sure in
gitk, but in qgit it works that way). So there's no flickering too,
and not double work.
Indeed lazy/on demand graph drawing policy is very well supported by
the above schema, and the above schema is good also because of the
lazy graph drawing.
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: linux @ 2006-09-09 18:44 UTC (permalink / raw)
To: mcostalba, torvalds; +Cc: git, jonsmirl, linux, paulus
In-Reply-To: <e5bfff550609091104s3709b82fld3057a07a84ae857@mail.gmail.com>
> Anyhow the basic is:
>
> -git-rev-list starts outputting the data early (order is not guaranteed)
>
> -before to mark for output a revision check if it breaks --topo-order
>
> -if the above is true store the revision away and *do not send*
>
> - at the end you get an early started steram of topological corrects
> revisions without
> preordering, just because you filter away the (few?) revisions that
> are not in order.
> The list you get is guaranteed to be in topo order although my not be complete.
>
> - _then_ you send the missing revisions that where previously
> filtered out. At this stage the receiver has already drwan the graph,
> indeed it has start drwaing as soon as the first revisons arrived and
> *very important* receiver used old and fast topo-order parsing code.
>
> - finally the fixup routine at receiver's end updates the graph with
> the info from the small set of out of order revisions filtered out
> before and sent at the end (only this small set is sent at the end).
The problem is that the gitk display code doesn't *like* input like this;
it's only designed to append to a list. Handling insertions would be
hard work for a rare corner case, and a perpetual source of bugs.
Unless gitk does a complete second pass, or course, which would
guarantee an annoying flicker a few seconds after startup.
And Twice the work.
The original alternative was to note that out-of-order commits
usually aren't *very* out of order. So if gitk could checkpoint
the state of its display algorithm periodically, it could just, when
seeing an out-of-order commit, rewind to the last checkpoint
before the problem and replay while merging in the corrections.
While that is potentially O(n^2) (if you feed it all the commits
in reverse order), in practice it
- Is less than 2*n work, and
- Always maintains the longest possible correct prefix of
recent commits. It's the old stuff that takes a while to
fill in.
Either way, gitk has to have topo-sorting code added to it. I think
Linus' idea was to avoid that.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-09 18:04 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Mackerras, Jon Smirl, linux@horizon.com, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609091022360.27779@g5.osdl.org>
On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> - output all revisions in the normal non-topological ordering
>
> - when git-rev-list notices a topological sort error, it outputs a
> "FIXME" line, and restarts the whole thing - printing out the commits
> in the newly fixed ordering - and does it right the next time around
>
Sorry, but I don't understand why you should restart the whole thing
instead of store away the FIXME commit and continue.
If you read my previous e-mail in this thread perhaps it is better
explained the whole idea.
Anyhow the basic is:
-git-rev-list starts outputting the data early (order is not guaranteed)
-before to mark for output a revision check if it breaks --topo-order
-if the above is true store the revision away and *do not send*
- at the end you get an early started steram of topological corrects
revisions without
preordering, just because you filter away the (few?) revisions that
are not in order.
The list you get is guaranteed to be in topo order although my not be complete.
- _then_ you send the missing revisions that where previously
filtered out. At this stage the receiver has already drwan the graph,
indeed it has start drwaing as soon as the first revisons arrived and
*very important* receiver used old and fast topo-order parsing code.
- finally the fixup routine at receiver's end updates the graph with
the info from the small set of out of order revisions filtered out
before and sent at the end (only this small set is sent at the end).
Sorry if it is not still clear, in the previous my e-mail in this
thread there is also a small example that could be useful.
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: Linus Torvalds @ 2006-09-09 17:33 UTC (permalink / raw)
To: Marco Costalba
Cc: Paul Mackerras, Jon Smirl, linux@horizon.com, Git Mailing List
In-Reply-To: <e5bfff550609090147q37d61f37j9c3e8ae6d3a0cf35@mail.gmail.com>
On Sat, 9 Sep 2006, Marco Costalba wrote:
>
> Perhaps is total idiocy but why do not implement the fix-up logic
> directly in git-rev-list?
It's possible in theory, but in practice it's not worth it.
Why?
Because you really want the _asynchronous_ nature of having a separate
user, that only shows _partial_ results.
In other words, we could reasonably easily make git-rev-list do something
like
- output all revisions in the normal non-topological ordering
- when git-rev-list notices a topological sort error, it outputs a
"FIXME" line, and restarts the whole thing - printing out the commits
in the newly fixed ordering - and does it right the next time around
- it then continues doing this until it's totally exhausted the whole
commit list and has done one final output in the proper topological
ordering.
Possible? Yes.
BUT:
- as long as git-rev-list is entirely single-threaded (and trust me, it's
going to be that, because otherwise we'd be better off doing it in a
separate process - like gitk), this means that it will be _entirely_
unaware of what has actually been _shown_, so it will restart a LOT
more than the external interactive process would do. So it would be
much worse than doing it externally and knowing what you've actually
shown to the user (if you haven't shown the bad thing yet, there's no
reason to restart).
- Again, as long as it's single-threaded, git-rev-list will block once it
has filled up the pipeline between the processes, so instead of parsing
everything in parallel with the "show it all", if will synchronize with
the showing process all the time, and especially so when it needs to
re-show the stuff that it already sent once. So it's also fairly
inefficient.
However, what you seem to imply is something different:
> Where, while git-rev-list is working _whithout sorting the whole tree
> first_, when finds an out of order revision stores it in a fixup-list
> buffer and *at the end* of normal git-rev-lsit the buffer is flushed
> to receiver, so that the drawing logic does not change and the out of
> order revisions arrive at the end, already packed, sorted and prepared
> by git-rev-list.
But this is exactly what we already do. We flush things *at the end*
because that's when we actually know the ordering. And that's exactly why
"git-rev-list --topo-ordering" has a latency ranging from a few seconds to
a few minutes for large projects (depending on whether they are packed or
not).
The "wait for the end" is _not_ good, exactly because the end will take
some time to arrive. The whole point is to start outputting the data
early, and thet BY DEFINITION means that the order of revisions isn't
guaranteed to be in topological order.
Linus
^ permalink raw reply
* Re: [PATCH 1/4] Add git-archive
From: Franck Bui-Huu @ 2006-09-09 15:25 UTC (permalink / raw)
To: Rene Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <4502D78B.6000905@lsrfire.ath.cx>
2006/9/9, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
> Franck Bui-Huu schrieb:
> > 2006/9/8, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
> >> > + url = strdup(ar->remote);
> >>
> >> xstrdup()
> >>
> >
> > ok, but need to rebase...
>
> Why? On a similar note , can we use Junio's pu branch (and soon his
> next branch) as our base for further work?
>
Yes. I just noticed Junio's post right after my reply.
> >> > + if (list) {
> >> > + if (!remote) {
> >> > + for (i = 0; i < ARRAY_SIZE(archivers); i++)
> >> > + printf("%s\n", archivers[i].name);
> >> > + exit(0);
> >> > + }
> >> > + die("--list and --remote are mutually exclusive");
> >> > + }
> >>
> >> Not sure if we really need a list option. I guess it only really
> >> makes sense if we have more than five formats. I have no _strong_
> >> feelings against it, though. *shrug*
> >>
> >
> > well it's almost free to add it, and no need any maintenance if we add
> > a new archiver backend, so I would say let it.
>
> I thought a bit about it, and I can now see a good use case for --list:
> checking the capabilities of a remote site. Unfortunately this is
> currently forbidden. Why? git-archive --list writes to stdout, so the
> result can be transported the same way an archive would.
>
Yes that was the main goal for this option. But then we talked about
enable/disable formats on the server side, and adding side band
support...all that points are still dark for me and I don't know how
'--list --remote' will interact with them. So I prefered to make this
option simple for now and not make the user to believe that doing
git archive --list --remote=...
list the capabilites of the remote side.
--
Franck
^ permalink raw reply
* [PATCH] gitweb: reset input record separator in parse_commit even on error
From: Sven Verdoolaege @ 2006-09-09 15:12 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
---
This probably never happens, but just in case...
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7afdf33..60dd598 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -897,8 +897,8 @@ sub parse_commit {
my $fd = git_pipe("rev-list", "--header", "--parents", "--max-count=1", $commit_id)
or return;
@commit_lines = split '\n', <$fd>;
- close $fd or return;
$/ = "\n";
+ close $fd or return;
pop @commit_lines;
}
my $header = shift @commit_lines;
--
0.99.8c.g64e8-dirty
^ permalink raw reply related
* Re: [PATCH 2/4] git-archive: wire up TAR format.
From: Franck Bui-Huu @ 2006-09-09 15:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rene Scharfe, git
In-Reply-To: <7vk64derfd.fsf@assigned-by-dhcp.cox.net>
2006/9/9, Junio C Hamano <junkio@cox.net>:
> Junio C Hamano <junkio@cox.net> writes:
>
> > Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> >
> >> I did not sign off this exact patch. I wrote and submitted the
> >> builtin-tar-tree.c part, with memory leak and all, then sent a note
> >> on where the leak needs to be plugged. You put it together and
> >> converted it to struct archiver_args. I'd very much have liked to
> >> see a comment stating this. Or simply just say "based on code by
> >> Rene" or something. The same is true for patch 3/4.
> >>...
> >> Especially I would not have signed off this invisible comment. ;)
> >
> > I take your response is a mild NAK.
>
> Just to reduce everybody's pain, why don't I fix them up and
> push out the 4 series in "pu" with attribution clarification and
> review comments from Rene in mind, so that you two can Ack them?
> After that they will be placed on "next".
>
Almost Acked by me, except you've missed some Rene's comments. And
more important I fixed an "uninitialized variable" bug. See patch
below.
> I needed to apply small tweaks on 1/4 (ANSI-C pedantic did not
> like empty struct initializers) and 4/4 (the updated 1/1 needed
> the way struct archiver is initialized and used be different
> from the original one) as well.
>
thanks for that.
-- >8 --
diff --git a/builtin-archive.c b/builtin-archive.c
index 0a02519..9b90d87 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -28,7 +28,7 @@ static int run_remote_archiver(struct ar
sprintf(buf, "git-upload-archive");
- url = strdup(ar->remote);
+ url = xstrdup(ar->remote);
pid = git_connect(fd, url, buf);
if (pid < 0)
return pid;
@@ -101,6 +101,7 @@ void parse_treeish_arg(const char **argv
commit_sha1 = commit->object.sha1;
archive_time = commit->date;
} else {
+ commit_sha1 = NULL;
archive_time = time(NULL);
}
@@ -141,7 +142,7 @@ int parse_archive_args(int argc, const c
{
const char *extra_argv[MAX_EXTRA_ARGS];
int extra_argc = 0;
- const char *format = NULL; /* some default values */
+ const char *format = NULL;
const char *remote = NULL;
const char *base = "";
int list = 0;
@@ -171,6 +172,8 @@ int parse_archive_args(int argc, const c
break;
}
if (arg[0] == '-') {
+ if (extra_argc > MAX_EXTRA_ARGS - 1)
+ die("Too many extra options");
extra_argv[extra_argc++] = arg;
continue;
}
@@ -185,7 +188,7 @@ int parse_archive_args(int argc, const c
die("--list and --remote are mutually exclusive");
}
if (argc - i < 1) {
- die("%s", archive_usage);
+ usage(archive_usage);
}
if (!format){
die("You must specify an archive format");
^ permalink raw reply related
* [PATCH] git-archive: make compression level of ZIP archives configurable
From: Rene Scharfe @ 2006-09-09 15:02 UTC (permalink / raw)
To: Junio C Hamano, Franck Bui-Huu; +Cc: Git Mailing List
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
diff --git a/archive.h b/archive.h
index f3d344b..d8cca73 100644
--- a/archive.h
+++ b/archive.h
@@ -42,5 +42,6 @@ extern void parse_pathspec_arg(const cha
*/
extern int write_tar_archive(struct archiver_args *);
extern int write_zip_archive(struct archiver_args *);
+extern void *parse_extra_zip_args(int argc, const char **argv);
#endif /* ARCHIVE_H */
diff --git a/builtin-archive.c b/builtin-archive.c
index 6ef2c90..83e5589 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,8 +15,15 @@ static const char archive_usage[] =
"git-archive --format=<fmt> [--prefix=<prefix>/] [<extra>] <tree-ish> [path...]";
struct archiver archivers[] = {
- { .name = "tar", .write_archive = write_tar_archive },
- { .name = "zip", .write_archive = write_zip_archive },
+ {
+ .name = "tar",
+ .write_archive = write_tar_archive,
+ },
+ {
+ .name = "zip",
+ .write_archive = write_zip_archive,
+ .parse_extra = parse_extra_zip_args,
+ },
};
static int run_remote_archiver(struct archiver *ar, int argc, const char **argv)
diff --git a/builtin-zip-tree.c b/builtin-zip-tree.c
index 23b4de5..fdac2bd 100644
--- a/builtin-zip-tree.c
+++ b/builtin-zip-tree.c
@@ -379,3 +379,16 @@ int write_zip_archive(struct archiver_ar
return 0;
}
+
+void *parse_extra_zip_args(int argc, const char **argv)
+{
+ for (; argc > 0; argc--, argv++) {
+ const char *arg = argv[0];
+
+ if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0')
+ zlib_compression_level = arg[1] - '0';
+ else
+ die("Unknown argument for zip format: %s", arg);
+ }
+ return NULL;
+}
^ permalink raw reply related
* Re: [PATCH 1/4] Add git-archive
From: Rene Scharfe @ 2006-09-09 15:02 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: Junio C Hamano, git
In-Reply-To: <cda58cb80609090731w7c66dcfbrbababb6c38d29bf6@mail.gmail.com>
Franck Bui-Huu schrieb:
> 2006/9/8, Rene Scharfe <rene.scharfe@lsrfire.ath.cx>:
>> > + url = strdup(ar->remote);
>>
>> xstrdup()
>>
>
> ok, but need to rebase...
Why? On a similar note , can we use Junio's pu branch (and soon his
next branch) as our base for further work?
>> > + if (list) {
>> > + if (!remote) {
>> > + for (i = 0; i < ARRAY_SIZE(archivers); i++)
>> > + printf("%s\n", archivers[i].name);
>> > + exit(0);
>> > + }
>> > + die("--list and --remote are mutually exclusive");
>> > + }
>>
>> Not sure if we really need a list option. I guess it only really
>> makes sense if we have more than five formats. I have no _strong_
>> feelings against it, though. *shrug*
>>
>
> well it's almost free to add it, and no need any maintenance if we add
> a new archiver backend, so I would say let it.
I thought a bit about it, and I can now see a good use case for --list:
checking the capabilities of a remote site. Unfortunately this is
currently forbidden. Why? git-archive --list writes to stdout, so the
result can be transported the same way an archive would.
René
^ permalink raw reply
* Re: [PATCH 2/4] git-archive: wire up TAR format.
From: Rene Scharfe @ 2006-09-09 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Franck Bui-Huu, git
In-Reply-To: <7vk64derfd.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano schrieb:
> Just to reduce everybody's pain, why don't I fix them up and
> push out the 4 series in "pu" with attribution clarification and
> review comments from Rene in mind, so that you two can Ack them?
> After that they will be placed on "next".
This is an excellent idea. What you have in pu is a good base to
add the (few and small) missing pieces. Consider patches 1-3 ACKed;
I haven't looked at git-upload-archive, yet.
Thanks,
René
^ permalink raw reply
* [PATCH] gitweb: support perl 5.6
From: Sven Verdoolaege @ 2006-09-09 14:59 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Specifically, perl 5.6 doesn't have the Encode module and
doesn't support the open "-|" list form.
Signed-off-by: Sven Verdoolage <skimo@liacs.nl>
---
This is take three.
I had missed the other binmodes in the code.
gitweb/gitweb.perl | 102 +++++++++++++++++++++++++++++-----------------------
1 files changed, 57 insertions(+), 45 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d89f709..7afdf33 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -12,11 +12,12 @@ use warnings;
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser);
-use Encode;
+eval { require Encode; import Encode; };
+our $have_encode = !$@;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
-binmode STDOUT, ':utf8';
+binmode STDOUT, ':utf8' if $have_encode;
our $cgi = new CGI;
our $version = "++GIT_VERSION++";
@@ -347,10 +348,16 @@ sub esc_param {
return $str;
}
+sub utf8_decode {
+ my $str = shift;
+ $str = decode("utf8", $str, eval "Encode::FB_DEFAULT") if $have_encode;
+ return $str;
+}
+
# replace invalid utf8 character with SUBSTITUTION sequence
sub esc_html {
my $str = shift;
- $str = decode("utf8", $str, Encode::FB_DEFAULT);
+ $str = utf8_decode($str);
$str = escapeHTML($str);
$str =~ s/\014/^L/g; # escape FORM FEED (FF) character (e.g. in COPYING file)
return $str;
@@ -582,6 +589,12 @@ sub git_cmd {
return $GIT, '--git-dir='.$git_dir;
}
+# returns pipe reading from git command with given arguments
+sub git_pipe {
+ open my $fd, "-|", join(' ', git_cmd(), @_) or return undef;
+ return $fd;
+}
+
# returns path to the core git executable and the --git-dir parameter as string
sub git_cmd_str {
return join(' ', git_cmd());
@@ -593,7 +606,7 @@ sub git_get_head_hash {
my $o_git_dir = $git_dir;
my $retval = undef;
$git_dir = "$projectroot/$project";
- if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
+ if (my $fd = git_pipe("rev-parse", "--verify", "HEAD")) {
my $head = <$fd>;
close $fd;
if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
@@ -610,7 +623,7 @@ # get type of given object
sub git_get_type {
my $hash = shift;
- open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
+ my $fd = git_pipe("cat-file", '-t', $hash) or return;
my $type = <$fd>;
close $fd or return;
chomp $type;
@@ -640,7 +653,7 @@ sub git_get_hash_by_path {
my $tree = $base;
- open my $fd, "-|", git_cmd(), "ls-tree", $base, "--", $path
+ my $fd = git_pipe("ls-tree", $base, "--", $path)
or die_error(undef, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd or return undef;
@@ -719,7 +732,7 @@ sub git_get_projects_list {
if (-e "$projectroot/$path/HEAD") {
my $pr = {
path => $path,
- owner => decode("utf8", $owner, Encode::FB_DEFAULT),
+ owner => utf8_decode($owner),
};
push @list, $pr
}
@@ -748,7 +761,7 @@ sub git_get_project_owner {
$pr = unescape($pr);
$ow = unescape($ow);
if ($pr eq $project) {
- $owner = decode("utf8", $ow, Encode::FB_DEFAULT);
+ $owner = utf8_decode($ow);
last;
}
}
@@ -771,7 +784,7 @@ sub git_get_references {
open $fd, "$projectroot/$project/info/refs"
or return;
} else {
- open $fd, "-|", git_cmd(), "ls-remote", "."
+ $fd = git_pipe("ls-remote", ".")
or return;
}
@@ -792,7 +805,7 @@ sub git_get_references {
sub git_get_rev_name_tags {
my $hash = shift || return undef;
- open my $fd, "-|", git_cmd(), "name-rev", "--tags", $hash
+ my $fd = git_pipe("name-rev", "--tags", $hash)
or return;
my $name_rev = <$fd>;
close $fd;
@@ -840,7 +853,7 @@ sub parse_tag {
my %tag;
my @comment;
- open my $fd, "-|", git_cmd(), "cat-file", "tag", $tag_id or return;
+ my $fd = git_pipe("cat-file", "tag", $tag_id) or return;
$tag{'id'} = $tag_id;
while (my $line = <$fd>) {
chomp $line;
@@ -881,7 +894,7 @@ sub parse_commit {
@commit_lines = @$commit_text;
} else {
$/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", "--max-count=1", $commit_id
+ my $fd = git_pipe("rev-list", "--header", "--parents", "--max-count=1", $commit_id)
or return;
@commit_lines = split '\n', <$fd>;
close $fd or return;
@@ -1098,7 +1111,7 @@ sub get_file_owner {
}
my $owner = $gcos;
$owner =~ s/[,;].*$//;
- return decode("utf8", $owner, Encode::FB_DEFAULT);
+ return utf8_decode($owner);
}
## ......................................................................
@@ -2206,8 +2219,8 @@ sub git_summary {
}
print "</table>\n";
- open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
- git_get_head_hash($project)
+ my $fd = git_pipe("rev-list", "--max-count=17",
+ git_get_head_hash($project))
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -2286,7 +2299,7 @@ sub git_blame2 {
if ($ftype !~ "blob") {
die_error("400 Bad Request", "Object is not a blob");
}
- open ($fd, "-|", git_cmd(), "blame", '-l', $file_name, $hash_base)
+ $fd = git_pipe("blame", '-l', $file_name, $hash_base)
or die_error(undef, "Open git-blame failed");
git_header_html();
my $formats_nav =
@@ -2352,7 +2365,7 @@ sub git_blame {
$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
or die_error(undef, "Error lookup file");
}
- open ($fd, "-|", git_cmd(), "annotate", '-l', '-t', '-r', $file_name, $hash_base)
+ $fd = git_pipe("annotate", '-l', '-t', '-r', $file_name, $hash_base)
or die_error(undef, "Open git-annotate failed");
git_header_html();
my $formats_nav =
@@ -2473,7 +2486,7 @@ sub git_blob_plain {
}
}
my $type = shift;
- open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
+ my $fd = git_pipe("cat-file", "blob", $hash)
or die_error(undef, "Couldn't cat $file_name, $hash");
$type ||= blob_mimetype($fd, $file_name);
@@ -2491,9 +2504,9 @@ sub git_blob_plain {
-expires=>$expires,
-content_disposition => "inline; filename=\"$save_as\"");
undef $/;
- binmode STDOUT, ':raw';
+ binmode STDOUT, ':raw' if $have_encode;
print <$fd>;
- binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ binmode STDOUT, ':utf8' if $have_encode; # as set at the beginning of gitweb.cgi
$/ = "\n";
close $fd;
}
@@ -2515,7 +2528,7 @@ sub git_blob {
}
}
my ($have_blame) = gitweb_check_feature('blame');
- open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
+ my $fd =git_pipe("cat-file", "blob", $hash)
or die_error(undef, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
if ($mimetype !~ m/^text\//) {
@@ -2580,7 +2593,7 @@ sub git_tree {
}
}
$/ = "\0";
- open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
+ my $fd = git_pipe("ls-tree", '-z', $hash)
or die_error(undef, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading tree failed");
@@ -2648,9 +2661,9 @@ sub git_snapshot {
my $git_command = git_cmd_str();
open my $fd, "-|", "$git_command tar-tree $hash \'$project\' | $command" or
die_error(undef, "Execute git-tar-tree failed.");
- binmode STDOUT, ':raw';
+ binmode STDOUT, ':raw' if $have_encode;
print <$fd>;
- binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ binmode STDOUT, ':utf8' if $have_encode; # as set at the beginning of gitweb.cgi
close $fd;
}
@@ -2666,7 +2679,7 @@ sub git_log {
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
- open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+ my $fd = git_pipe("rev-list", $limit, $hash)
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -2721,7 +2734,7 @@ sub git_commit {
if (!defined $parent) {
$parent = "--root";
}
- open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $parent, $hash
+ my $fd = git_pipe("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");
@@ -2828,8 +2841,8 @@ sub git_blobdiff {
if (defined $hash_base && defined $hash_parent_base) {
if (defined $file_name) {
# read raw output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base,
- "--", $file_name
+ $fd = git_pipe("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>;
close $fd
@@ -2842,7 +2855,7 @@ sub git_blobdiff {
# try to find filename from $hash
# read filtered raw output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base
+ $fd = git_pipe("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'
@@ -2876,9 +2889,9 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
'-p', $hash_parent_base, $hash_base,
- "--", $file_name
+ "--", $file_name)
or die_error(undef, "Open git-diff-tree failed");
}
@@ -2912,7 +2925,7 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts, $hash_parent, $hash
+ $fd = git_pipe("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")
@@ -2997,8 +3010,8 @@ sub git_commitdiff {
my $fd;
my @difftree;
if ($format eq 'html') {
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- "--patch-with-raw", "--full-index", $hash_parent, $hash
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ "--patch-with-raw", "--full-index", $hash_parent, $hash)
or die_error(undef, "Open git-diff-tree failed");
while (chomp(my $line = <$fd>)) {
@@ -3008,8 +3021,8 @@ sub git_commitdiff {
}
} elsif ($format eq 'plain') {
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- '-p', $hash_parent, $hash
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ '-p', $hash_parent, $hash)
or die_error(undef, "Open git-diff-tree failed");
} else {
@@ -3108,8 +3121,7 @@ sub git_history {
}
git_print_page_path($file_name, $ftype, $hash_base);
- open my $fd, "-|",
- git_cmd(), "rev-list", "--full-history", $hash_base, "--", $file_name;
+ my $fd = git_pipe("rev-list", "--full-history", $hash_base, "--", $file_name);
git_history_body($fd, $refs, $hash_base, $ftype);
@@ -3150,7 +3162,7 @@ sub git_search {
my $alternate = 0;
if ($commit_search) {
$/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", $hash or next;
+ my $fd = git_pipe("rev-list", "--header", "--parents", $hash or next);
while (my $commit_text = <$fd>) {
if (!grep m/$searchtext/i, $commit_text) {
next;
@@ -3271,7 +3283,7 @@ sub git_shortlog {
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
- open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+ my $fd = git_pipe("rev-list", $limit, $hash)
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -3299,7 +3311,7 @@ ## feeds (RSS, OPML)
sub git_rss {
# http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
- open my $fd, "-|", git_cmd(), "rev-list", "--max-count=150", git_get_head_hash($project)
+ my $fd = git_pipe("rev-list", "--max-count=150", git_get_head_hash($project))
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading git-rev-list failed");
@@ -3322,8 +3334,8 @@ XML
last;
}
my %cd = parse_date($co{'committer_epoch'});
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- $co{'parent'}, $co{'id'}
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ $co{'parent'}, $co{'id'})
or next;
my @difftree = map { chomp; $_ } <$fd>;
close $fd
@@ -3341,7 +3353,7 @@ XML
"<![CDATA[\n";
my $comment = $co{'comment'};
foreach my $line (@$comment) {
- $line = decode("utf8", $line, Encode::FB_DEFAULT);
+ $line = utf8_decode($line);
print "$line<br/>\n";
}
print "<br/>\n";
@@ -3350,7 +3362,7 @@ XML
next;
}
my $file = validate_input(unquote($7));
- $file = decode("utf8", $file, Encode::FB_DEFAULT);
+ $file = utf8_decode($file);
print "$file<br/>\n";
}
print "]]>\n" .
--
0.99.8c.g64e8-dirty
^ permalink raw reply related
* [PATCH] gitweb: support perl 5.6
From: Sven Verdoolaege @ 2006-09-09 14:42 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Specifically, perl 5.6 doesn't have the Encode module and
doesn't support the open "-|" list form.
Signed-off-by: Sven Verdoolage <skimo@liacs.nl>
---
This is take two, hopefully addressing Jakub's remark.
gitweb/gitweb.perl | 94 +++++++++++++++++++++++++++++-----------------------
1 files changed, 53 insertions(+), 41 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d89f709..9992046 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -12,11 +12,12 @@ use warnings;
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser);
-use Encode;
+eval { require Encode; import Encode; };
+our $have_encode = !$@;
use Fcntl ':mode';
use File::Find qw();
use File::Basename qw(basename);
-binmode STDOUT, ':utf8';
+binmode STDOUT, ':utf8' if $have_encode;
our $cgi = new CGI;
our $version = "++GIT_VERSION++";
@@ -347,10 +348,16 @@ sub esc_param {
return $str;
}
+sub utf8_decode {
+ my $str = shift;
+ $str = decode("utf8", $str, eval "Encode::FB_DEFAULT") if $have_encode;
+ return $str;
+}
+
# replace invalid utf8 character with SUBSTITUTION sequence
sub esc_html {
my $str = shift;
- $str = decode("utf8", $str, Encode::FB_DEFAULT);
+ $str = utf8_decode($str);
$str = escapeHTML($str);
$str =~ s/\014/^L/g; # escape FORM FEED (FF) character (e.g. in COPYING file)
return $str;
@@ -582,6 +589,12 @@ sub git_cmd {
return $GIT, '--git-dir='.$git_dir;
}
+# returns pipe reading from git command with given arguments
+sub git_pipe {
+ open my $fd, "-|", join(' ', git_cmd(), @_) or return undef;
+ return $fd;
+}
+
# returns path to the core git executable and the --git-dir parameter as string
sub git_cmd_str {
return join(' ', git_cmd());
@@ -593,7 +606,7 @@ sub git_get_head_hash {
my $o_git_dir = $git_dir;
my $retval = undef;
$git_dir = "$projectroot/$project";
- if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
+ if (my $fd = git_pipe("rev-parse", "--verify", "HEAD")) {
my $head = <$fd>;
close $fd;
if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
@@ -610,7 +623,7 @@ # get type of given object
sub git_get_type {
my $hash = shift;
- open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
+ my $fd = git_pipe("cat-file", '-t', $hash) or return;
my $type = <$fd>;
close $fd or return;
chomp $type;
@@ -640,7 +653,7 @@ sub git_get_hash_by_path {
my $tree = $base;
- open my $fd, "-|", git_cmd(), "ls-tree", $base, "--", $path
+ my $fd = git_pipe("ls-tree", $base, "--", $path)
or die_error(undef, "Open git-ls-tree failed");
my $line = <$fd>;
close $fd or return undef;
@@ -719,7 +732,7 @@ sub git_get_projects_list {
if (-e "$projectroot/$path/HEAD") {
my $pr = {
path => $path,
- owner => decode("utf8", $owner, Encode::FB_DEFAULT),
+ owner => utf8_decode($owner),
};
push @list, $pr
}
@@ -748,7 +761,7 @@ sub git_get_project_owner {
$pr = unescape($pr);
$ow = unescape($ow);
if ($pr eq $project) {
- $owner = decode("utf8", $ow, Encode::FB_DEFAULT);
+ $owner = utf8_decode($ow);
last;
}
}
@@ -771,7 +784,7 @@ sub git_get_references {
open $fd, "$projectroot/$project/info/refs"
or return;
} else {
- open $fd, "-|", git_cmd(), "ls-remote", "."
+ $fd = git_pipe("ls-remote", ".")
or return;
}
@@ -792,7 +805,7 @@ sub git_get_references {
sub git_get_rev_name_tags {
my $hash = shift || return undef;
- open my $fd, "-|", git_cmd(), "name-rev", "--tags", $hash
+ my $fd = git_pipe("name-rev", "--tags", $hash)
or return;
my $name_rev = <$fd>;
close $fd;
@@ -840,7 +853,7 @@ sub parse_tag {
my %tag;
my @comment;
- open my $fd, "-|", git_cmd(), "cat-file", "tag", $tag_id or return;
+ my $fd = git_pipe("cat-file", "tag", $tag_id) or return;
$tag{'id'} = $tag_id;
while (my $line = <$fd>) {
chomp $line;
@@ -881,7 +894,7 @@ sub parse_commit {
@commit_lines = @$commit_text;
} else {
$/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", "--max-count=1", $commit_id
+ my $fd = git_pipe("rev-list", "--header", "--parents", "--max-count=1", $commit_id)
or return;
@commit_lines = split '\n', <$fd>;
close $fd or return;
@@ -1098,7 +1111,7 @@ sub get_file_owner {
}
my $owner = $gcos;
$owner =~ s/[,;].*$//;
- return decode("utf8", $owner, Encode::FB_DEFAULT);
+ return utf8_decode($owner);
}
## ......................................................................
@@ -2206,8 +2219,8 @@ sub git_summary {
}
print "</table>\n";
- open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
- git_get_head_hash($project)
+ my $fd = git_pipe("rev-list", "--max-count=17",
+ git_get_head_hash($project))
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -2286,7 +2299,7 @@ sub git_blame2 {
if ($ftype !~ "blob") {
die_error("400 Bad Request", "Object is not a blob");
}
- open ($fd, "-|", git_cmd(), "blame", '-l', $file_name, $hash_base)
+ $fd = git_pipe("blame", '-l', $file_name, $hash_base)
or die_error(undef, "Open git-blame failed");
git_header_html();
my $formats_nav =
@@ -2352,7 +2365,7 @@ sub git_blame {
$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
or die_error(undef, "Error lookup file");
}
- open ($fd, "-|", git_cmd(), "annotate", '-l', '-t', '-r', $file_name, $hash_base)
+ $fd = git_pipe("annotate", '-l', '-t', '-r', $file_name, $hash_base)
or die_error(undef, "Open git-annotate failed");
git_header_html();
my $formats_nav =
@@ -2473,7 +2486,7 @@ sub git_blob_plain {
}
}
my $type = shift;
- open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
+ my $fd = git_pipe("cat-file", "blob", $hash)
or die_error(undef, "Couldn't cat $file_name, $hash");
$type ||= blob_mimetype($fd, $file_name);
@@ -2515,7 +2528,7 @@ sub git_blob {
}
}
my ($have_blame) = gitweb_check_feature('blame');
- open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
+ my $fd =git_pipe("cat-file", "blob", $hash)
or die_error(undef, "Couldn't cat $file_name, $hash");
my $mimetype = blob_mimetype($fd, $file_name);
if ($mimetype !~ m/^text\//) {
@@ -2580,7 +2593,7 @@ sub git_tree {
}
}
$/ = "\0";
- open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
+ my $fd = git_pipe("ls-tree", '-z', $hash)
or die_error(undef, "Open git-ls-tree failed");
my @entries = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading tree failed");
@@ -2666,7 +2679,7 @@ sub git_log {
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
- open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+ my $fd = git_pipe("rev-list", $limit, $hash)
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -2721,7 +2734,7 @@ sub git_commit {
if (!defined $parent) {
$parent = "--root";
}
- open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $parent, $hash
+ my $fd = git_pipe("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");
@@ -2828,8 +2841,8 @@ sub git_blobdiff {
if (defined $hash_base && defined $hash_parent_base) {
if (defined $file_name) {
# read raw output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base,
- "--", $file_name
+ $fd = git_pipe("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>;
close $fd
@@ -2842,7 +2855,7 @@ sub git_blobdiff {
# try to find filename from $hash
# read filtered raw output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts, $hash_parent_base, $hash_base
+ $fd = git_pipe("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'
@@ -2876,9 +2889,9 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
'-p', $hash_parent_base, $hash_base,
- "--", $file_name
+ "--", $file_name)
or die_error(undef, "Open git-diff-tree failed");
}
@@ -2912,7 +2925,7 @@ sub git_blobdiff {
}
# open patch output
- open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts, $hash_parent, $hash
+ $fd = git_pipe("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")
@@ -2997,8 +3010,8 @@ sub git_commitdiff {
my $fd;
my @difftree;
if ($format eq 'html') {
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- "--patch-with-raw", "--full-index", $hash_parent, $hash
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ "--patch-with-raw", "--full-index", $hash_parent, $hash)
or die_error(undef, "Open git-diff-tree failed");
while (chomp(my $line = <$fd>)) {
@@ -3008,8 +3021,8 @@ sub git_commitdiff {
}
} elsif ($format eq 'plain') {
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- '-p', $hash_parent, $hash
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ '-p', $hash_parent, $hash)
or die_error(undef, "Open git-diff-tree failed");
} else {
@@ -3108,8 +3121,7 @@ sub git_history {
}
git_print_page_path($file_name, $ftype, $hash_base);
- open my $fd, "-|",
- git_cmd(), "rev-list", "--full-history", $hash_base, "--", $file_name;
+ my $fd = git_pipe("rev-list", "--full-history", $hash_base, "--", $file_name);
git_history_body($fd, $refs, $hash_base, $ftype);
@@ -3150,7 +3162,7 @@ sub git_search {
my $alternate = 0;
if ($commit_search) {
$/ = "\0";
- open my $fd, "-|", git_cmd(), "rev-list", "--header", "--parents", $hash or next;
+ my $fd = git_pipe("rev-list", "--header", "--parents", $hash or next);
while (my $commit_text = <$fd>) {
if (!grep m/$searchtext/i, $commit_text) {
next;
@@ -3271,7 +3283,7 @@ sub git_shortlog {
my $refs = git_get_references();
my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
- open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash
+ my $fd = git_pipe("rev-list", $limit, $hash)
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd;
@@ -3299,7 +3311,7 @@ ## feeds (RSS, OPML)
sub git_rss {
# http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
- open my $fd, "-|", git_cmd(), "rev-list", "--max-count=150", git_get_head_hash($project)
+ my $fd = git_pipe("rev-list", "--max-count=150", git_get_head_hash($project))
or die_error(undef, "Open git-rev-list failed");
my @revlist = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading git-rev-list failed");
@@ -3322,8 +3334,8 @@ XML
last;
}
my %cd = parse_date($co{'committer_epoch'});
- open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- $co{'parent'}, $co{'id'}
+ $fd = git_pipe("diff-tree", '-r', @diff_opts,
+ $co{'parent'}, $co{'id'})
or next;
my @difftree = map { chomp; $_ } <$fd>;
close $fd
@@ -3341,7 +3353,7 @@ XML
"<![CDATA[\n";
my $comment = $co{'comment'};
foreach my $line (@$comment) {
- $line = decode("utf8", $line, Encode::FB_DEFAULT);
+ $line = utf8_decode($line);
print "$line<br/>\n";
}
print "<br/>\n";
@@ -3350,7 +3362,7 @@ XML
next;
}
my $file = validate_input(unquote($7));
- $file = decode("utf8", $file, Encode::FB_DEFAULT);
+ $file = utf8_decode($file);
print "$file<br/>\n";
}
print "]]>\n" .
--
0.99.8c.g64e8-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