* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Nguyen Thai Ngoc Duy @ 2010-12-09 1:28 UTC (permalink / raw)
To: Jakub Narebski
Cc: Jonathan Nieder, git, Junio C Hamano, Kevin Ballard, Yann Dirson,
Jeff King
In-Reply-To: <201012082051.09730.jnareb@gmail.com>
On Thu, Dec 9, 2010 at 2:51 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Dnia środa 8. grudnia 2010 19:06, Jonathan Nieder napisał:
>> Nguyễn Thái Ngọc Duy wrote:
>>
>> > Let's start off from where the previous discussion [1] stopped. People
>> > seem to agree ref^{/regex} is a good choice. But we have not come to
>> > conclusion how to specify the count yet. Possible suggestions are
>> >
>> > - ref^{/foo}2
>> > - ref^{2/foo}
>> > - ref^{:2/foo}
>> > - ref^{2nd/foo}
>>
>> How about
>>
>> ref^{/foo}^^{/foo}
>>
>> ?
>
> I'll assume that there is invisible ";)" emoticon here.
>
>
> First, it would be ref^{/foo}^@^{/foo}, otherwise you would follow only
> first parent.
>
> Second, consider ref^{:nth(10)/foo} in your workaround...
Maybe we should generalize this to apply to all operators. Currently
foo~3 is expanded to foo^^^. How about ~~X (or xN) denote repeat the
last operator N times? For example, HEAD^2x3 => HEAD^2^2^2,
HEAD^{/foo}x3 => HEAD^{/foo}^{/foo}^{/foo}.
--
Duy
^ permalink raw reply
* Re: [RFC] Implementing gitweb output caching - issues to solve
From: J.H. @ 2010-12-09 1:31 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, John 'Warthog9' Hawley, Junio C Hamano, Petr Baudis,
admin
In-Reply-To: <201011041721.53371.jnareb@gmail.com>
> 1. Error handling
[...]
> Note that in my opinion cache should not be initialized if caching is
> disabled.
Well it wasn't getting initialized in my code if caching was turned off.
It might have been buffering, but it wasn't mucking with the directory
creation and such...
> The modern CHI caching interface provides a way to change how runtime
> errors are handled via 'on_get_error' and 'on_set_error' parameters to
> constructor. The default for CHI is to log error, or ignore if no
> logger is set. You can provide a coderef (callback).
>
>
> For errors during getting data from cache (case B), we can ignore error
> which means generating data as if cache was disabled/entry did not
> exist in cache, or erroring out and showing error to user. Such errors
> should be rare, so erroring out might be appropriate reaction. Note
> that we have special case of zero sized cache entry file, in which case
> we treat it as non-existent entry, already.
>
>
> For errors during setting (saving) data to cache (case C), we can
> ignore error and not re-generate cache (not even calling callback if
> error is detected early), or error out. Such errors can happen for
> example if filesystem gets full. It might be better to ignore such
> errore, at least in the case of ENOSPC / ENOMEM.
The potential, at least for the deployed sites who are already using the
gitweb caching, for disaster if caching isn't running is pretty high. I
would agree that erroring out with a 500 that explains the 'why' behind
the failure is the only real option right now.
The callback mechanism in CHI is nice, but I think for the time being is
a little overkill for what we've got right now. I also don't see that
many, typical, sysadmins adding in that many error handling hooks and
such into gitweb and/or their infrastructure. I could be wrong, I just
don't see that as being a common occurrence.
> 2. Progress info and not caching error pages
>
> J.H. find out or invented very nice hack to generate progress info
> page without Ajax (and therefore without need for JavaScript), to make
> user aware that data is being regenerated, and that he/she should be
> patient instead of refreshing or complaining.
Not so much found or invented, it's something that's been around since I
started doing any web development back in 1995. It's a bit old-school,
I'll admit, but it gets the job done still.
[...]
> JH> There are still a few known "issues" with respect to this:
> JH> - Code needs to be added to be "browser" aware so
> JH> that clients like wget that are trying to get a
> JH> binary blob don't obtain a "Generating..." page
I've solved this in v8, there is now an explicit blacklist for clients
that we assume are only there for whatever is getting generated. Right
now the list is only wget and curl as those are the only two I can think
of immediately that wouldn't be able to parse the meta-equiv refresh.
There is already a special case for feeds to not get this, so those
shouldn't be affected as it is already.
Downside is it's a blacklist, and inherits all the problems of a
blacklist. Though there really isn't a better way to handle that, given
there's no way to really query a browser and ask what it's capabilities
really are.
> there is additional problem, at least in my rewrite. (I didn't follow
> the code flow in J.H. v7 patch series to check if it also affected).
>
>
> Let's assume that 'progress_info' feature is turned on, and that cache
> is generated in background. Let's assume that error pages (die_error)
> are not cached.
>
> Now client tries to access page which would/will result in an error.
> With caching it acquires exclusive (writers) lock, check that there
> are no stale data (error pages are never cached), checks that it is
> provided with 'generating_info' subroutine, so it forks a process to
> generate data in background.
>
> Background process detaches, tries to generate data to cache,
> die_error is called. die_error turns off capturing, and prints to
> STDOUT. At the end of die_error there is jump to outer scope to
> DONE_GITWEB; therefore $lock_fh goes out of scope and lockfile is
> closed, and lock released. Background process finishes.
>
> Parent process runs 'generating_info' subroutine. Now if it waits a
> bit like in my rewrite before employing trick mentioned above, and
> does not print anything if lockfile is released before startup delay,
> _and_ die_error finishes within this delay, then everything is all
> right: the error message is sent to client.
>
> If die_error doesn't finish before git_generating_data_html prints
> meta refresh, or there is no startup delay, then error pages would get
> infinite redirects (remember: there is never anything in cache for
> error pages). This is a bad thing.
>
>
> One possible solution for this problem (beside employing startup
> delay) is to have tee output, i.e. print it as it is being captured.
> Streamed (partial) response would serve as progress indicator for
> process (re)generating cache; only parallel processes waiting for
> cache would show 'generating_info'.
>
> I think that in current implementation of capturing it would be as
> simple as not closing STDOUT, but I'd have to check that.
Actually in reading through this I thought of a better way to get the
error message cleanly passed from the backend process to the frontend
waiting processes.
It's implemented in v8, but it basically boils down to, strangely
enough, caching the error. (I've got a hammer, it clearly solves all
problems!). It's not a full cache (like what I'm doing with the rest of
the cache) but it basically generates a file that can be used to short
circuit things a bit.
> 3. Using generic cache engine and memory consumption
>
> Most Perl caching interfaces support only $cache->set($key, $data),
> where $data is a Perl variable, and $data = $cache->get($key), or
> their equivalents. Even for file-based cache drivers you save from
> memory and read into memory.
>
> The only exception I know of is Cache interface with Cache::File
> driver, that provides $cache->handle($key [, $mode]), where optional
> $mode argument can be any of mode strings that can be used in 'open'
> function, or one of fopen(3) modes. (Of course for memory-based or
> network-based (like memcached) caches it might not make sense to
> provide such interface).
>
> The most often recommended capture module, Capture::Tiny, allows only
> capturing into scalar (into memory)
>
> $stdout = capture \&code;
>
> or (using its prototype)
>
> $stdout = capture { <code> };
>
> Well, it is *::Tiny, so it supports minimal API. From the list of
> different capture modules, that allow capturing of Perl code output,
> with different interfaces (API), in "See also" section of
> Capture::Tiny documentation[3], only IO::CaptureOutput allow capturing
> into specified file:
>
> capture \&code, \$stdout, undef, $outfile;
>
> [3] http://p3rl.org/Capture::Tiny
>
>
> J.H.'s gitweb output caching v7 captures output directly ito cache
> files. The problem with doing it in my rewrite is to allow capturing
> directly into cache entry file without losing ability to select
> different caching engine, which might be not file-based (like
> e.g. memcached-based).
One of the big things that my caching engine now stops is the stampeding
herd problem, basically by locking everyone out of the cache for that
entry till it's prepared. Assuming that can be preserved with any
arbitrary caching engine, then capturing directly to the file isn't that
big of a deal.
If that can be preserved in your full re-write, then it doesn't really
matter how or when the data makes it into the caching system be it
memory, disk, /dev/random or collapsing quantum wave form
I'm going to try and get v8 out tomorrow morning, and I'm going to let
this stew on kernel.org overnight. It's looking like 18 patches (which
includes Jakub's v7.2, since I was building straight on top of that).
In poking around, I'll admit, I keep finding things I'd love to fix /
cleanup throughout the entirety of the code base. I'm kinda hoping to
finally get caching dealt with and merged so I can start scratching
those other itches. Mind you this has been 4+ years in the making
already...
- John 'Warthog9' Hawley
P.S. Most of this e-mail was written a couple of weeks ago, I found it
in my drafts folder and wanted to get it out along with a note that I've
got v8 percolating. -JH
^ permalink raw reply
* Re: git describe weird behaviour
From: Miklos Vajna @ 2010-12-09 1:33 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20101110140334.GJ22105@genesis.frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 428 bytes --]
On Wed, Nov 10, 2010 at 03:03:34PM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
> > The patch below implements that in a very rough-and-dirty way. It does
> > find the 1.4 tag in your repository that you expect. However:
>
> Yes, works here as well:
>
> $ ~/git/git/git describe
> 1.4pre1-210-g48b67cd
Any update on this? I still have this patch in my tree to get correct
git describe output. :)
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Nguyen Thai Ngoc Duy @ 2010-12-09 1:42 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano, Kevin Ballard, Yann Dirson, Jeff King
In-Reply-To: <201012090144.19858.jnareb@gmail.com>
On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>
>> > I wonder if it would be possible to make :/<regex> (which looks a bit
>> > like searching the index) to be an alias to --all^{/<regex>}...
>>
>> It looks a bit strange to my eyes to merge normal option name with
>> revision syntax. But I think it's possible. Do we allow branch/tag
>> name with leading '-'?
>
> Well, with below proposal it would simply be
>
> --all ^{/<regexp>}
This hardly works with range and may conflict with "--all" being
already used by some commands.
I think we can move '/' out of {}, the space between '/' and '{' can
be used for optional parameters: ^/{foo}.
--
Duy
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Kevin Ballard @ 2010-12-09 1:46 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jakub Narebski, git, Junio C Hamano, Yann Dirson, Jeff King
In-Reply-To: <AANLkTin1SjEzBnLd-HK9fANShLezAKHAOai9MyF-cuoT@mail.gmail.com>
On Dec 8, 2010, at 5:42 PM, Nguyen Thai Ngoc Duy wrote:
> On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>>
>>>> I wonder if it would be possible to make :/<regex> (which looks a bit
>>>> like searching the index) to be an alias to --all^{/<regex>}...
>>>
>>> It looks a bit strange to my eyes to merge normal option name with
>>> revision syntax. But I think it's possible. Do we allow branch/tag
>>> name with leading '-'?
>>
>> Well, with below proposal it would simply be
>>
>> --all ^{/<regexp>}
>
> This hardly works with range and may conflict with "--all" being
> already used by some commands.
>
> I think we can move '/' out of {}, the space between '/' and '{' can
> be used for optional parameters: ^/{foo}
I thought ^{} was going to be an arbitrary grouping operator, capable of
embedding any other modifier, but primarily only useful for regex. This
change explicitly makes it an alternative regex syntax.
-Kevin Ballard
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Jakub Narebski @ 2010-12-09 1:54 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, git, Junio C Hamano, Kevin Ballard, Yann Dirson,
Jeff King
In-Reply-To: <AANLkTimU6Bhx-2XsZ45_7BmT9fo9MpK8TJWB3zZ=j-i7@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> On Thu, Dec 9, 2010 at 2:51 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Dnia środa 8. grudnia 2010 19:06, Jonathan Nieder napisał:
>>> Nguyễn Thái Ngọc Duy wrote:
>>>
>>>> Let's start off from where the previous discussion [1] stopped. People
>>>> seem to agree ref^{/regex} is a good choice. But we have not come to
>>>> conclusion how to specify the count yet. Possible suggestions are
>>>>
>>>> - ref^{/foo}2
>>>> - ref^{2/foo}
>>>> - ref^{:2/foo}
>>>> - ref^{2nd/foo}
>>>
>>> How about
>>>
>>> ref^{/foo}^^{/foo}
>>>
>>> ?
>>
>> I'll assume that there is invisible ";)" emoticon here.
>>
>>
>> First, it would be ref^{/foo}^@^{/foo}, otherwise you would follow only
>> first parent.
>>
>> Second, consider ref^{:nth(10)/foo} in your workaround...
>
> Maybe we should generalize this to apply to all operators. Currently
> foo~3 is expanded to foo^^^. How about ~~X (or xN) denote repeat the
> last operator N times? For example, HEAD^2x3 => HEAD^2^2^2,
> HEAD^{/foo}x3 => HEAD^{/foo}^{/foo}^{/foo}.
Unless you allow grouping, it wouldn't help in the case of ^{/foo},
because ^{/foo} is idempotent. HEAD^{/foo} finds first commit that
contains "foo", and HEAD^{/foo}^{/foo} finds first commit containing
"foo" starting from *and including* first commit from HEAD containing
"foo" - which is HEAD^{/foo}
HEAD^{/foo}^{/foo} === HEAD^{/foo}
You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Jonathan Nieder @ 2010-12-09 1:59 UTC (permalink / raw)
To: Jakub Narebski
Cc: Nguyen Thai Ngoc Duy, git, Junio C Hamano, Kevin Ballard,
Yann Dirson, Jeff King
In-Reply-To: <201012090254.24999.jnareb@gmail.com>
Jakub Narebski wrote:
> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
That said, does ^2x500 really do something meaningful that a person
would ever need? I like the
^{:nth(3)/foo}
syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
already-defined modifiers to implement when they prove useful, known
already to a certain subset of the audience and proven useful already
in a different context.
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Kevin Ballard @ 2010-12-09 2:02 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jakub Narebski, Nguyen Thai Ngoc Duy, git, Junio C Hamano,
Yann Dirson, Jeff King
In-Reply-To: <20101209015926.GA31119@burratino>
On Dec 8, 2010, at 5:59 PM, Jonathan Nieder wrote:
> Jakub Narebski wrote:
>
>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>
> That said, does ^2x500 really do something meaningful that a person
> would ever need? I like the
>
> ^{:nth(3)/foo}
>
> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
> already-defined modifiers to implement when they prove useful, known
> already to a certain subset of the audience and proven useful already
> in a different context.
I like the ^{:nth(3)/foo} syntax as well. Though I'm not familiar with Perl 6,
this does have the benefit of being fairly obvious to the reader as to what it
means.
-Kevin Ballard
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Nguyen Thai Ngoc Duy @ 2010-12-09 2:06 UTC (permalink / raw)
To: Kevin Ballard
Cc: Jonathan Nieder, Jakub Narebski, git, Junio C Hamano, Yann Dirson,
Jeff King
In-Reply-To: <192758EC-4276-445D-B1D5-284073D5AB32@sb.org>
On Thu, Dec 9, 2010 at 9:02 AM, Kevin Ballard <kevin@sb.org> wrote:
> On Dec 8, 2010, at 5:59 PM, Jonathan Nieder wrote:
>
>> Jakub Narebski wrote:
>>
>>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>>
>> That said, does ^2x500 really do something meaningful that a person
>> would ever need? I like the
>>
>> ^{:nth(3)/foo}
>>
>> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
>> already-defined modifiers to implement when they prove useful, known
>> already to a certain subset of the audience and proven useful already
>> in a different context.
>
> I like the ^{:nth(3)/foo} syntax as well. Though I'm not familiar with Perl 6,
> this does have the benefit of being fairly obvious to the reader as to what it
> means.
OK so :nth(3)/foo for all branches?
--
Duy
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Jonathan Nieder @ 2010-12-09 2:11 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Kevin Ballard, Jakub Narebski, git, Junio C Hamano, Yann Dirson,
Jeff King
In-Reply-To: <AANLkTikOehig7LjpUC=b6iSjGnuF=TsZmipWDfQB7AW6@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
> OK so :nth(3)/foo for all branches?
That steals namespace from "the path 'nth(3)/foo' in the index". But
is "the third instance of foo in all branches" something that needs to
be possible to say? Branches do not have a well defined order,
anyway. A command to list all commits with "foo" in the subject
like
git log --oneline --grep-subject=foo
sounds more useful (assuming --grep=foo yields too many false
positives).
^ permalink raw reply
* Re: git describe weird behaviour
From: Jeff King @ 2010-12-09 3:28 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Shawn O. Pearce, git
In-Reply-To: <20101209013323.GJ22105@genesis.frugalware.org>
On Thu, Dec 09, 2010 at 02:33:23AM +0100, Miklos Vajna wrote:
> On Wed, Nov 10, 2010 at 03:03:34PM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
> > > The patch below implements that in a very rough-and-dirty way. It does
> > > find the 1.4 tag in your repository that you expect. However:
> >
> > Yes, works here as well:
> >
> > $ ~/git/git/git describe
> > 1.4pre1-210-g48b67cd
>
> Any update on this? I still have this patch in my tree to get correct
> git describe output. :)
Sorry, no, I haven't had time to think about it, and I probably won't
for a few more weeks at least.
The patch I posted was giving some really weird results for git.git,
though, so it is definitely not OK for inclusion as-is.
-Peff
^ permalink raw reply
* [PATCH] git-send-email: Accept -n as a synonym for --dry-run
From: Alejandro R. Sedeño @ 2010-12-09 4:44 UTC (permalink / raw)
To: git
git-send-email is not currently using -n for anything else, and it
seems unlikely we will want to use it to mean anything else in the
future, so add it as an alias for convenience.
Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
---
git-send-email.perl | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 76565de..7e3df9a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -85,7 +85,7 @@ git send-email [options] <file | directory | rev-list options >
--confirm <str> * Confirm recipients before sending;
auto, cc, compose, always, or never.
--quiet * Output one line of info per email.
- --dry-run * Don't actually send the emails.
+ -n, --dry-run * Don't actually send the emails.
--[no-]validate * Perform patch sanity checks. Default on.
--[no-]format-patch * understand any non optional arguments as
`git format-patch` ones.
@@ -304,7 +304,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
"suppress-cc=s" => \@suppress_cc,
"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
"confirm=s" => \$confirm,
- "dry-run" => \$dry_run,
+ "dry-run|n" => \$dry_run,
"envelope-sender=s" => \$envelope_sender,
"thread!" => \$thread,
"validate!" => \$validate,
--
1.7.3.3
^ permalink raw reply related
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Junio C Hamano @ 2010-12-09 5:15 UTC (permalink / raw)
To: Jakub Narebski
Cc: Jonathan Nieder, Nguyễn Thái Ngọc Duy, git,
Kevin Ballard, Yann Dirson, Jeff King
In-Reply-To: <201012082051.09730.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Second, consider ref^{:nth(10)/foo} in your workaround...
Feels way over-engineered to me.
^ permalink raw reply
* Re: [RFC] Implementing gitweb output caching - issues to solve
From: Junio C Hamano @ 2010-12-09 5:22 UTC (permalink / raw)
To: J.H.
Cc: Jakub Narebski, git, John 'Warthog9' Hawley,
Junio C Hamano, Petr Baudis, admin
In-Reply-To: <4D00316F.9000305@eaglescrag.net>
"J.H." <warthog9@eaglescrag.net> writes:
> P.S. Most of this e-mail was written a couple of weeks ago, I found it
> in my drafts folder and wanted to get it out along with a note that I've
> got v8 percolating. -JH
Thanks. Is "Tomorrow morning" relative to "a couple of weeks ago"? ;-)
^ permalink raw reply
* Re: [RFC] Implementing gitweb output caching - issues to solve
From: J.H. @ 2010-12-09 5:28 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jakub Narebski, git, John 'Warthog9' Hawley, Petr Baudis,
admin
In-Reply-To: <7v62v3ikqj.fsf@alter.siamese.dyndns.org>
On 12/08/2010 09:22 PM, Junio C Hamano wrote:
> "J.H." <warthog9@eaglescrag.net> writes:
>
>> P.S. Most of this e-mail was written a couple of weeks ago, I found it
>> in my drafts folder and wanted to get it out along with a note that I've
>> got v8 percolating. -JH
>
> Thanks. Is "Tomorrow morning" relative to "a couple of weeks ago"? ;-)
No tomorrow morning is relative to today - so ~12hrs from now, ish,
maybe big ISH ;-)
I finished up the patch series today. The series itself is already
pushed out to my repo on kernel.org
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v8
just wanted to let that stew a bit before pushing it out to the mailing
list, since it is now an 18 part patch :-/
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [PATCH 2/3] diffstat: Use new diffstat config values
From: Junio C Hamano @ 2010-12-09 5:54 UTC (permalink / raw)
To: mmr15; +Cc: git, Matthew Ruffalo
In-Reply-To: <1291776263-16320-2-git-send-email-matthew.ruffalo@case.edu>
mmr15@case.edu writes:
> This required removing the diffstat options from 'struct diff_options'
> and adding these values as static ints in diff.c. This preserves the
> style of "config options are static ints, command-line options are
> diff_options members". stat_opt now directly sets the global options.
It is not that I do not trust/believe it, but I am very unhappy with the
above "This required".
The diff callchain was designed to be a highly reusable library and has
been kept callable multiple times with different settings in a single
program by passing different "struct diff_options". The above sounds like
a rather huge regression.
Aren't there any way to avoid this? Why do these two options need to be
any different from other variables (e.g. a_prefix, context) that can be
set from the config and can be overridden by the command line options
while having a built-in fallback default values?
In general, the callflow of each git subcommand looks like this:
(1) find $GIT_DIR;
(2) read $GIT_DIR/config and friends;
(3) parse command line options;
(4) decide what the user asked us to do and do it.
I would imagine that the following should do what you want:
* declare two static int variables, stat_name_width_default and
stat_width_default, that are initialized to 80/50 at compile time;
* add code to git_diff_ui_config() to update these two *_default
variables in step (2) above;
* add code to diff_setup() to initialize opt->stat_name_width and
opt->stat_width from these two *_default variables;
* add code to diff_opt_parse() to update opt->stat_name_width and
opt->stat_width from the command line parameters.
Then follow cmd_diff() in diff.c to make sure the above is sufficient.
Observe that:
- The first thing cmd_diff() does is to read the config;
- then init_revisions() will call diff_setup();
- then setup_revisions() will call into diff_opt_parse().
^ permalink raw reply
* Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators
From: Junio C Hamano @ 2010-12-09 6:22 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jakub Narebski, Nguyen Thai Ngoc Duy, git, Kevin Ballard,
Yann Dirson, Jeff King
In-Reply-To: <20101209015926.GA31119@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Jakub Narebski wrote:
>
>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>
> That said, does ^2x500 really do something meaningful that a person
> would ever need? I like the
>
> ^{:nth(3)/foo}
>
> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
> already-defined modifiers to implement when they prove useful,...
Can you explain what the colon in "$commit^{:nth(3)/foo}" is doing?
Are we declaring anything that begins with ':' is a magic inside ^{...}
construct?
I do not think nth($n) without specifying where to start (iow, what the
current ":/foo" implementation does but with "three levels deep") makes
any sense, but because the main point of your argument is that we can have
modifies other than nth($n) that may make sense in such a context, I would
want to make sure anything we come up with is extensible to that syntax.
On the "starting from any ref" front, I think "!" (as in ":/!some magic")
was the introducer we reserved for such a magic some time ago, so perhaps
on the "starting from this commit" side, "^{!magic/foo}" may be more
appropriate?
^ permalink raw reply
* [PATCH v3 1/4] describe: Use for_each_rawref
From: Anders Kaseorg @ 2010-12-09 6:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012081800540.23348@dr-wily.mit.edu>
Don’t waste time checking for dangling refs; they wouldn’t affect the
output of ‘git describe’ anyway. Although this doesn’t gain much
performance by itself, it does in conjunction with the next commits.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
builtin/describe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 43caff2..700f740 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -418,7 +418,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
return cmd_name_rev(i + argc, args, prefix);
}
- for_each_ref(get_name, NULL);
+ for_each_rawref(get_name, NULL);
if (!found_names && !always)
die("No names found, cannot describe anything.");
--
1.7.3.3
^ permalink raw reply related
* [PATCH v3 2/4] describe: Don’t use a flex array in struct commit_name
From: Anders Kaseorg @ 2010-12-09 6:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012090140390.23348@dr-wily.mit.edu>
Now add_to_known_names overwrites commit_names in place when multiple
tags point to the same commit. This will make it easier to store
commit_names in a hash table.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
builtin/describe.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 700f740..5b8461d 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -38,7 +38,7 @@ struct commit_name {
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
unsigned char sha1[20];
- char path[FLEX_ARRAY]; /* more */
+ const char *path;
};
static const char *prio_names[] = {
"head", "lightweight", "annotated",
@@ -85,15 +85,15 @@ static void add_to_known_names(const char *path,
struct commit_name *e = commit->util;
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
- size_t len = strlen(path)+1;
- free(e);
- e = xmalloc(sizeof(struct commit_name) + len);
+ if (!e) {
+ e = xmalloc(sizeof(struct commit_name));
+ commit->util = e;
+ }
e->tag = tag;
e->prio = prio;
e->name_checked = 0;
hashcpy(e->sha1, sha1);
- memcpy(e->path, path, len);
- commit->util = e;
+ e->path = path;
}
found_names = 1;
}
--
1.7.3.3
^ permalink raw reply related
* Re: [PATCH] git-send-email: Accept -n as a synonym for --dry-run
From: "Alejandro R. Sedeño" @ 2010-12-09 6:39 UTC (permalink / raw)
To: git
In-Reply-To: <1291869878-19645-1-git-send-email-asedeno@mit.edu>
I noticed I forgot to make the corresponding documentation change. I'll
include it in the next version of this patch, though I'm waiting to see
if there's any other feedback first.
-Alejandro
^ permalink raw reply
* [PATCH v3 3/4] describe: Store commit_names in a hash table by commit SHA1
From: Anders Kaseorg @ 2010-12-09 6:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012090140390.23348@dr-wily.mit.edu>
describe is currently forced to look up the commit at each tag in
order to store the struct commit_name pointers in struct commit.util.
For --exact-match queries, those lookups are wasteful. In preparation
for removing them, put the commit_names into a hash table, indexed by
commit SHA1, that can be used to quickly check for exact matches.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
Change from v2.1: Use memcpy to avoid unaligned accesses in hash_sha1().
builtin/describe.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 5b8461d..8149233 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -6,6 +6,7 @@
#include "exec_cmd.h"
#include "parse-options.h"
#include "diff.h"
+#include "hash.h"
#define SEEN (1u<<0)
#define MAX_TAGS (FLAG_BITS - 1)
@@ -22,7 +23,7 @@ static int tags; /* Allow lightweight tags */
static int longformat;
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
-static int found_names;
+static struct hash_table names;
static const char *pattern;
static int always;
static const char *dirty;
@@ -34,6 +35,8 @@ static const char *diff_index_args[] = {
struct commit_name {
+ struct commit_name *next;
+ unsigned char peeled[20];
struct tag *tag;
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
@@ -44,6 +47,21 @@ static const char *prio_names[] = {
"head", "lightweight", "annotated",
};
+static inline unsigned int hash_sha1(const unsigned char *sha1)
+{
+ unsigned int hash;
+ memcpy(&hash, sha1, sizeof(hash));
+ return hash;
+}
+
+static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+{
+ struct commit_name *n = lookup_hash(hash_sha1(peeled), &names);
+ while (n && !!hashcmp(peeled, n->peeled))
+ n = n->next;
+ return n;
+}
+
static int replace_name(struct commit_name *e,
int prio,
const unsigned char *sha1,
@@ -82,12 +100,22 @@ static void add_to_known_names(const char *path,
int prio,
const unsigned char *sha1)
{
- struct commit_name *e = commit->util;
+ const unsigned char *peeled = commit->object.sha1;
+ struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
if (!e) {
+ void **pos;
e = xmalloc(sizeof(struct commit_name));
commit->util = e;
+ hashcpy(e->peeled, peeled);
+ pos = insert_hash(hash_sha1(peeled), e, &names);
+ if (pos) {
+ e->next = *pos;
+ *pos = e;
+ } else {
+ e->next = NULL;
+ }
}
e->tag = tag;
e->prio = prio;
@@ -95,7 +123,6 @@ static void add_to_known_names(const char *path,
hashcpy(e->sha1, sha1);
e->path = path;
}
- found_names = 1;
}
static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
@@ -240,7 +267,7 @@ static void describe(const char *arg, int last_one)
if (!cmit)
die("%s is not a valid '%s' object", arg, commit_type);
- n = cmit->util;
+ n = find_commit_name(cmit->object.sha1);
if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
@@ -418,8 +445,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
return cmd_name_rev(i + argc, args, prefix);
}
+ init_hash(&names);
for_each_rawref(get_name, NULL);
- if (!found_names && !always)
+ if (!names.nr && !always)
die("No names found, cannot describe anything.");
if (argc == 0) {
--
1.7.3.3
^ permalink raw reply related
* [PATCH v3 4/4] describe: Delay looking up commits until searching for an inexact match
From: Anders Kaseorg @ 2010-12-09 6:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012090140390.23348@dr-wily.mit.edu>
Now that struct commit.util is not used until after we’ve checked that
the argument doesn’t exactly match a tag, we can wait until then to
look up the commits for each tag.
This avoids a lot of I/O on --exact-match queries in repositories with
many tags. For example, ‘git describe --exact-match HEAD’ becomes
about 12 times faster on a cold cache (3.2s instead of 39s) in a
linux-2.6 repository with 2000 packed tags. That’s a huge win for the
interactivity of the __git_ps1 shell prompt helper when on a detached
HEAD.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
Change from v2: Process the entire hash chain in set_util().
builtin/describe.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 8149233..a0f52c1 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -24,6 +24,7 @@ static int longformat;
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
static struct hash_table names;
+static int have_util;
static const char *pattern;
static int always;
static const char *dirty;
@@ -62,6 +63,17 @@ static inline struct commit_name *find_commit_name(const unsigned char *peeled)
return n;
}
+static int set_util(void *chain)
+{
+ struct commit_name *n;
+ for (n = chain; n; n = n->next) {
+ struct commit *c = lookup_commit_reference_gently(n->peeled, 1);
+ if (c)
+ c->util = n;
+ }
+ return 0;
+}
+
static int replace_name(struct commit_name *e,
int prio,
const unsigned char *sha1,
@@ -96,18 +108,16 @@ static int replace_name(struct commit_name *e,
}
static void add_to_known_names(const char *path,
- struct commit *commit,
+ const unsigned char *peeled,
int prio,
const unsigned char *sha1)
{
- const unsigned char *peeled = commit->object.sha1;
struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
if (!e) {
void **pos;
e = xmalloc(sizeof(struct commit_name));
- commit->util = e;
hashcpy(e->peeled, peeled);
pos = insert_hash(hash_sha1(peeled), e, &names);
if (pos) {
@@ -128,8 +138,6 @@ static void add_to_known_names(const char *path,
static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
{
int might_be_tag = !prefixcmp(path, "refs/tags/");
- struct commit *commit;
- struct object *object;
unsigned char peeled[20];
int is_tag, prio;
@@ -137,16 +145,10 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
return 0;
if (!peel_ref(path, peeled) && !is_null_sha1(peeled)) {
- commit = lookup_commit_reference_gently(peeled, 1);
- if (!commit)
- return 0;
- is_tag = !!hashcmp(sha1, commit->object.sha1);
+ is_tag = !!hashcmp(sha1, peeled);
} else {
- commit = lookup_commit_reference_gently(sha1, 1);
- object = parse_object(sha1);
- if (!commit || !object)
- return 0;
- is_tag = object->type == OBJ_TAG;
+ hashcpy(peeled, sha1);
+ is_tag = 0;
}
/* If --all, then any refs are used.
@@ -169,7 +171,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
if (!prio)
return 0;
}
- add_to_known_names(all ? path + 5 : path + 10, commit, prio, sha1);
+ add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
return 0;
}
@@ -286,6 +288,11 @@ static void describe(const char *arg, int last_one)
if (debug)
fprintf(stderr, "searching to describe %s\n", arg);
+ if (!have_util) {
+ for_each_hash(&names, set_util);
+ have_util = 1;
+ }
+
list = NULL;
cmit->object.flags = SEEN;
commit_list_insert(cmit, &list);
--
1.7.3.3
^ permalink raw reply related
* [PATCH] t9010 fails when no svn is available
From: Junio C Hamano @ 2010-12-09 6:53 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git
Running test t9010 without svn currently errors out for no good reason.
The test uses "svnadmin" without checking if svn is available. This was a
regression introduced by b0ad24b (t9010 (svn-fe): Eliminate dependency on
svn perl bindings, 2010-10-10) when it stopped including ./lib-git-svn.sh
that had the safety.
This should fix it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9010-svn-fe.sh | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index fd851a4..faf9092 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -4,6 +4,12 @@ test_description='check svn dumpfile importer'
. ./test-lib.sh
+if ! svnadmin -h >/dev/null 2>&1
+then
+ skip_all='skipping svn-fe tests, svn not available'
+ test_done
+fi
+
svnconf=$PWD/svnconf
export svnconf
^ permalink raw reply related
* blame formatting bug
From: Vitali @ 2010-12-09 8:11 UTC (permalink / raw)
To: git
Hi.
On the jquery repo the following command:
git blame df9c37ec852f8c873e226cd0ae190f969e0edc17 -- jquery/jquery.js >
blame
gives me the file with this:
c3c706d3 jquery/jquery.js (John Resig 2006-06-22 05:23:38 +0000 576) },
6ae392a4 jquery/jquery.js (John Resig 2006-06-22 20:14:41 +0000 577)
df9c37ec jquery/jquery.js (John Resig 2006-07-09 20:49:40 +0000 578)
extend: function(obj,prop) {
if ( !prop ) {
prop = obj;
obj = this;
}
for ( var i in prop )
obj[i] = prop[i];
return obj;
}
c3c706d3 jquery/jquery.js (John Resig 2006-06-22 05:23:38 +0000 579) };
^8a4a1ed core/core.js (John Resig 2006-03-22 03:33:07 +0000 580)
df9c37ec jquery/jquery.js (John Resig 2006-07-09 20:49:40 +0000 581)
jQuery.extend = jQuery.fn.extend;
Looks like bug.
^ permalink raw reply
* Re: [PATCH] t9010 fails when no svn is available
From: Ramkumar Ramachandra @ 2010-12-09 8:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmxofh1x4.fsf@alter.siamese.dyndns.org>
Junio C Hamano writes:
> Running test t9010 without svn currently errors out for no good reason.
>
> The test uses "svnadmin" without checking if svn is available. This was a
> regression introduced by b0ad24b (t9010 (svn-fe): Eliminate dependency on
> svn perl bindings, 2010-10-10) when it stopped including ./lib-git-svn.sh
> that had the safety.
>
> This should fix it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Right. Thanks for catching this.
-- Ram
^ permalink raw reply
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