* Re: upload-pack timing issue on windows?
From: Johannes Sixt @ 2010-02-06 10:06 UTC (permalink / raw)
To: kusmabite; +Cc: msysgit, Git Mailing List
In-Reply-To: <40aa078e1002051551o6d116a50uee3f6a32b16adb46@mail.gmail.com>
On Samstag, 6. Februar 2010, Erik Faye-Lund wrote:
> As some of you might know, I've been working on porting git-daemon to
> Windows for quite some time now. As it stands now, there's really only
> one known issue that is blocking on my end here:
>
> Something weird happens *sometimes* when upload-pack is exiting,
> leading to a client dying with a "fatal: read error: Invalid
> argument\nfatal: early EOF"-error. If I place a sleep(1) at some place
> after exiting the while(1)-loop in create_pack() in upload-pack.c, the
> symptom goes away. create_pack() contains some async-code, but this
> doesn't seem to be triggered in my minimal case at all. I've tried
> flushing stdout and stderr explicitly, no luck.
I've observed timing related issues in upload-pack as well, but only in the
case where the die() is called from the async thread. This is the reason why
t5530 does not pass.
But your case seems to be different - i.e. there is no die() involved. Sorry,
can't help more...
Perhaps use Procmon to analyse differences among the different successful and
failing cases.
Try hacking fetch-pack so that it does not announce side-band(-64k). Perhaps
it makes a difference.
-- Hannes
^ permalink raw reply
* Re: [PATCH] t9501: Skip testing load if we can't detect it
From: Jakub Narebski @ 2010-02-06 11:22 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List, Junio C Hamano
In-Reply-To: <1265432440-52146-1-git-send-email-brian@gernhardtsoftware.com>
Brian Gernhardt <brian@gernhardtsoftware.com> writes:
> Currently gitweb only knows how to check for load using /proc/loadavg,
> which isn't available on all systems. We shouldn't fail the test just
> because we don't know how to check the system load.
>
> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
NAK. It is not necessary, and it would be hindrance (one more place
to update) if we are to extend get_loadavg() in gitweb to work without
/proc/loadavg, e.g. via BSD::loadavg module.
Let me explain how it currently works without /proc/loadavg.
First, load average is by definition non-negative number (>= 0).
Second, the get_loadavg() subroutine is written to be resilent and
robust, and it returns 0 if it can't get load average from system
(which _currently_ means no well-formatted /proc/loadavg file)
Third, the test (as you can see below in context line in quoted diff
below) forces gitweb to go over maximum load by setting $maxload to 0.
This means that regardless of true load, and regardless whether gitweb
can detect system load (remember that if it cant get system load it
returns 0 instead) gitweb would be in "load too high" situation.
> ---
> t/t9501-gitweb-standalone-http-status.sh | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
> index 7590f10..992d729 100755
> --- a/t/t9501-gitweb-standalone-http-status.sh
> +++ b/t/t9501-gitweb-standalone-http-status.sh
> @@ -115,12 +115,19 @@ test_debug 'cat gitweb.output'
> # ----------------------------------------------------------------------
> # load checking
>
> +if test -e /proc/loadavg
> +then
> + test_set_prereq PROC_LOADAVG
> +else
> + say 'skipping load tests (no /proc/loadavg found)'
> +fi
> +
> # always hit the load limit
> cat >>gitweb_config.perl <<\EOF
> our $maxload = 0;
> EOF
>
> -test_expect_success 'load checking: load too high (default action)' '
> +test_expect_success PROC_LOADAVG 'load checking: load too high (default action)' '
> gitweb_run "p=.git" &&
> grep "Status: 503 Service Unavailable" gitweb.headers &&
> grep "503 - The load average on the server is too high" gitweb.body
> --
> 1.7.0.rc1.141.gd3fd2
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: notes TODOs (was: Re: [PATCH 1/4] gitweb: notes feature)
From: Jakub Narebski @ 2010-02-06 11:30 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Johan Herland, Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <cb7bb73a1002050444y55f57696gb1b3bd06ab9261ac@mail.gmail.com>
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> If I may be allowed to add a suggestion to put in the list, [...]
What I would like to see (besides --notes-ref or --notes option,
either as an option to git wrapper, or as an option to git-show,
git-format-patch, git-log and git-rev-list (I think that's all)),
is for git-notes to have a subcommand to move notes from one object
to the other:
'git notes' move [-f] <oldobject> <newobject>
Both source and target should be required, as I am not sure if more
common would be moving note from some object to HEAD, or from HEAD to
some object. The '-f'/'--force' option is in the case when note for
<newobject> exists (although it might be better to concatenate notes
in the future), and when <oldobject> and <newobject> are of different
types (unless we want som DWIM type coercion).
It could be used to make notes move to amended commit, for example:
$ git commit --amend
$ git notes move ORIG_HEAD HEAD # or HEAD@{1} HEAD
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Linux Kernel based project in git
From: Stephen Kelly @ 2010-02-06 11:43 UTC (permalink / raw)
To: git
In-Reply-To: <be6731d603701c3c5befc79613fd451b.squirrel@webmail.eisendle.net>
Christian Eisendle wrote:
>>> IMHO it would make sense to have 3 branches (BSP, USB, WiFi) each based
>>> on
>>> unmodified 2.6.22 Kernel. USB and WiFi branch is used for generating the
>>> patch and for applying possible fixes. BSP branch for actual BSP related
>>> feature development and fixes.
>>> The changes in these branches are merged into the master branch which is
>>> used for compiling/testing the whole BSP.
>>
>> Are you planning to submit these patches upstream at any point? If
>> not, it might be easiest to just jam them all together in one branch
>> and not look back. Since it seems like they probably affect quite
>> different parts of the code, you could always extract a clean set of
>> patches *later* and submit those patches upstream.
>
> For BSP I plan to upstream eventually.
>
> The basic idea was to divide the project in three different patches since
> USB and WiFi comes from a third party and is not released under GPL (well,
> different story...)
Do you distribute this kernel?
^ permalink raw reply
* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Jeff King @ 2010-02-06 11:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jan Engelhardt, git
In-Reply-To: <7v7hqrdkxb.fsf@alter.siamese.dyndns.org>
On Fri, Feb 05, 2010 at 08:53:36PM -0800, Junio C Hamano wrote:
> > git grep -- pattern revision -- pathname
> [...]
> I don't think if "ambiguous or not" is what we are after to begin with.
>
> I have known GNU extended grep implementations long enough but never saw
> that "--" used to quote a pattern. Is it worth supporting to begin with?
I think so. It was the first thing the original poster in this thread
tried. It is also something I have tried (and still do, then grumblingly
retype "-e pattern"). And it certainly makes sense from a user
perspective; it is the same end-of-options signal that most other
programs take.
So I think it is a convenient interface improvement, nothing more. If it
were somehow onerous to support, I would say that no, it is not worth
it. But it really is only a few lines of code, and I do not think the
behavior change is hurting any real-world cases (which is what I was
trying to show earlier).
I suspect you are not familiar with it because you are enough of an
old-timer to have worked with the many non-GNU greps that require "-e"
to specify a funny pattern and so got used to that habit.
> I would say "git grep -- pattern" is sufficiently insane enough that
> I'm not worried about it at all. Interpreting "git grep --" as a request
> to look for double-dash feels million times saner than that, actually.
I don't think "grep --" is sane at all, since it is broken under GNU
grep. And because "--" is a special token in option parsing, I would
expect it to need "git grep -e --".
> Unless somebody comes up with example of that pattern's wide use. Point
> me to some well known open source software's source trees that use "--"
> for such a purpose in one of its shell script or Makefile.
OK. Try:
http://www.google.com/codesearch?hl=en&sa=N&q=grep.*%5Cs--%5Cs++lang:shell&ct=rr&cs_r=lang:shell
Some are false positives, but it looks like libtool's generated
configure scripts use it (which is in literally hundreds of projects),
openssh's fixpaths script, ffmpeg's configure script, even a use in a
plan9 script.
And that's just the first page of results. So I think I am not the only
one.
-Peff
^ permalink raw reply
* Re: [msysGit] upload-pack timing issue on windows?
From: Erik Faye-Lund @ 2010-02-06 12:01 UTC (permalink / raw)
To: Johannes Sixt; +Cc: msysgit, Git Mailing List
In-Reply-To: <201002061106.04305.j6t@kdbg.org>
On Sat, Feb 6, 2010 at 11:06 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Samstag, 6. Februar 2010, Erik Faye-Lund wrote:
>> As some of you might know, I've been working on porting git-daemon to
>> Windows for quite some time now. As it stands now, there's really only
>> one known issue that is blocking on my end here:
>>
>> Something weird happens *sometimes* when upload-pack is exiting,
>> leading to a client dying with a "fatal: read error: Invalid
>> argument\nfatal: early EOF"-error. If I place a sleep(1) at some place
>> after exiting the while(1)-loop in create_pack() in upload-pack.c, the
>> symptom goes away. create_pack() contains some async-code, but this
>> doesn't seem to be triggered in my minimal case at all. I've tried
>> flushing stdout and stderr explicitly, no luck.
>
> I've observed timing related issues in upload-pack as well, but only in the
> case where the die() is called from the async thread. This is the reason why
> t5530 does not pass.
>
> But your case seems to be different - i.e. there is no die() involved. Sorry,
> can't help more...
>
Yeah, it's probably not the same case, but I certainly do find it
interesting that we seemingly have two separate timing-related around
here somewhere...
> Perhaps use Procmon to analyse differences among the different successful and
> failing cases.
>
I'm not entirely sure what to look for, but I do see that there's
difference. There's about 3.5k lines of logging from git.exe,
git-daemon.exe and git-upload-pack.exe for the failure case versus
2.5k for the successful case. And the last sequence of TCP Send in the
success case is a send of 8 bytes, followed by a send of 212 bytes,
followed again by a send of 1 byte. In the failure case, there's only
a send of 8 bytes in the end. This sequence is reported as sent by
git-daemon.exe. In fact, all TCP actions are reported from
git-daemon.exe, and apart from the last sequence the lengths are
reported as identical.
> Try hacking fetch-pack so that it does not announce side-band(-64k). Perhaps
> it makes a difference.
>
This didn't make any difference. I removed "side-band" and
"side-band-64k" from capabilities in send_ref() in upload-pack.c, as
well as the "if (server_supports("side-band<...>"-lines in
builtin-fetch-pack.c.
While I was at it, I also tried to disable all other capabilities; no luck.
However, I have tracked down a bit of what goes on in the client.
There's a call to read_in_full, called from pack-write.c, line 246
that fails in the failure-case, but not in the success-case. This is
where the client expects "pack\tSHA-1" or "keep\tSHA-1". There "fatal:
early EOF"-messages seems to originate from index-pack.c, line 197.
This is the first line of code in parse_pack_header(), it's also
AFAICT the first call-site for any read(0, <...>) (though fill()).
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: [PATCH 3/4] gitweb: show notes in log
From: Jakub Narebski @ 2010-02-06 12:57 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <1265300338-25021-4-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
> The notes are shown in full to the left of the log message.
Thats all good if you have wide (high resolution) screen, and your
project follows common commit message conventions of keeping lines in
commit message no longer than at most 80 characters, and you don't need
to use large size fonts.
What happens if screen size is too small to contain both commit message
and notes? Does it do the sensible thing of putting notes _below_
commit message in such situation? I do not know CSS+HTML enogh to
answer this question myself.
BTW. signoff?
P.S. We would probably want some support for notes also in feeds (Atom
and RSS feed), but this can be left for the future commit.
> @@ -1631,6 +1631,7 @@ sub format_subject_html {
> # display notes next to a commit
> sub format_notes_html {
> my %notes = %{$_[0]};
> + my $tag = $_[1] || 'span' ;
This could be
my $notes = shift;
my $tag = shift || 'span' ;
and then use %$notes.
> my $ret = "";
> while (my ($ref, $text) = each %notes) {
> # remove 'refs/notes/' and an optional final s
> @@ -1639,15 +1640,15 @@ sub format_notes_html {
>
> # double markup is needed to allow pure CSS cross-browser 'popup'
> # of the note
> - $ret .= "<span title='$ref' class='note-container $ref'>";
> - $ret .= "<span title='$ref' class='note $ref'>";
> + $ret .= "<$tag title='$ref' class='note-container $ref'>";
> + $ret .= "<$tag title='$ref' class='note $ref'>";
> foreach my $line (split /\n/, $text) {
> $ret .= esc_html($line) . "<br/>";
> }
> - $ret .= "</span></span>";
> + $ret .= "</$tag></$tag>";
> }
> if ($ret) {
> - return "<span class='notes'>$ret</span>";
> + return "<$tag class='notes'>$ret</$tag>";
> } else {
> return $ret;
> }
Nice trick, but is this distinction really necessary?
> @@ -4581,6 +4582,7 @@ sub git_log_body {
> next if !%co;
> my $commit = $co{'id'};
> my $ref = format_ref_marker($refs, $commit);
> + my $notes = format_notes_html($co{'notes'}, 'div');
> my %ad = parse_date($co{'author_epoch'});
> git_print_header_div('commit',
> "<span class=\"age\">$co{'age_string'}</span>" .
> @@ -4598,6 +4600,7 @@ sub git_log_body {
> git_print_authorship(\%co, -tag => 'span');
> print "<br/>\n</div>\n";
>
> + print "$notes\n";
> print "<div class=\"log_body\">\n";
> git_print_log($co{'comment'}, -final_empty_line=> 1);
> print "</div>\n";
With respect to the question about what happens if the screen is not
wide enough, shouldn't notes be put in HTML source below body (commit
message)?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 3/4] gitweb: show notes in log
From: Giuseppe Bilotta @ 2010-02-06 13:14 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002061357.59245.jnareb@gmail.com>
2010/2/6 Jakub Narebski <jnareb@gmail.com>:
> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
>
>> The notes are shown in full to the left of the log message.
>
> Thats all good if you have wide (high resolution) screen, and your
> project follows common commit message conventions of keeping lines in
> commit message no longer than at most 80 characters, and you don't need
> to use large size fonts.
>
> What happens if screen size is too small to contain both commit message
> and notes? Does it do the sensible thing of putting notes _below_
> commit message in such situation? I do not know CSS+HTML enogh to
> answer this question myself.
The CSS forces the width of the notes div at 150px, which is the
amount left to the left of the commit message. This means that notes
will line-wrap, but they will not shift the text.
> BTW. signoff?
As usual, I forgot.
> P.S. We would probably want some support for notes also in feeds (Atom
> and RSS feed), but this can be left for the future commit.
I honestly have absolutely no idea how to do that.
>> @@ -1631,6 +1631,7 @@ sub format_subject_html {
>> # display notes next to a commit
>> sub format_notes_html {
>> my %notes = %{$_[0]};
>> + my $tag = $_[1] || 'span' ;
>
> This could be
>
> my $notes = shift;
> my $tag = shift || 'span' ;
>
> and then use %$notes.
Would be much bettere, yes.
>> my $ret = "";
>> while (my ($ref, $text) = each %notes) {
>> # remove 'refs/notes/' and an optional final s
>> @@ -1639,15 +1640,15 @@ sub format_notes_html {
>>
>> # double markup is needed to allow pure CSS cross-browser 'popup'
>> # of the note
>> - $ret .= "<span title='$ref' class='note-container $ref'>";
>> - $ret .= "<span title='$ref' class='note $ref'>";
>> + $ret .= "<$tag title='$ref' class='note-container $ref'>";
>> + $ret .= "<$tag title='$ref' class='note $ref'>";
>> foreach my $line (split /\n/, $text) {
>> $ret .= esc_html($line) . "<br/>";
>> }
>> - $ret .= "</span></span>";
>> + $ret .= "</$tag></$tag>";
>> }
>> if ($ret) {
>> - return "<span class='notes'>$ret</span>";
>> + return "<$tag class='notes'>$ret</$tag>";
>> } else {
>> return $ret;
>> }
>
> Nice trick, but is this distinction really necessary?
I think so. The distinction is useful both from the structural point
of view (block elements with block elements, inline elements with
inline elements) and for CSS selection (the block case has totally
different styling than the inline case).
>> + print "$notes\n";
>> print "<div class=\"log_body\">\n";
>> git_print_log($co{'comment'}, -final_empty_line=> 1);
>> print "</div>\n";
>
> With respect to the question about what happens if the screen is not
> wide enough, shouldn't notes be put in HTML source below body (commit
> message)?
As I mentioned, notes width is fixed at the amount of the whitespace
to the left of the log, so this should not be an issue. Additionally,
putting notes below makes it much harder to let them float to the left
of the log body.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Jakub Narebski @ 2010-02-06 13:16 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <1265300338-25021-5-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:
> The notes are shown side-by-side along the bottom of the commit
> message.
The same question apply as for previous commit.
What happens if screen size is too small to contain both commit message
and notes? Does it do the sensible thing of putting notes _below_
commit message in such situation? I do not know CSS+HTML enogh to
answer this question myself.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0d0877e..0d03026 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2837,12 +2837,31 @@ sub parse_commit {
> %co = parse_commit_text(<$fd>, 1);
> close $fd;
>
> + my %notes = ();
> + foreach my $note_ref (get_note_refs()) {
> + my $obj = "$note_ref:$co{'id'}";
> + if (open my $fd, '-|', git_cmd(), 'rev-parse',
> + '--verify', '-q', $obj) {
> + my $exists = <$fd>;
> + close $fd;
> + if (defined $exists) {
> + if (open $fd, '-|', git_cmd(), 'show', $obj) {
> + $notes{$note_ref} = scalar <$fd>;
> + close $fd;
> + }
> + }
> + }
> + }
> + $co{'notes'} = \%notes;
> +
> return %co;
> }
Duplicated code. Please put this code in a separate subroutine, to be
called in those two places.
> # return all refs matching refs/notes/<globspecs> where the globspecs
> # are taken from the notes feature content.
> sub get_note_refs {
> + local $/ = "";
> +
Why it is needed here? Why you want to use empty lines as terminator
(which means reading whole paragraphs), while treating two or more
consecutive empty lines as a single empty line (according to
perlvar(1))?
If you want to slurp whole file, this should be
local $/;
or more explicit
local $/ = undef;
> my @globs = gitweb_get_feature('notes');
> my @note_refs = ();
> foreach my $glob (@globs) {
> @@ -5875,6 +5894,7 @@ sub git_commit {
>
> print "<div class=\"page_body\">\n";
> git_print_log($co{'comment'});
> + print format_notes_html($co{'notes'}, 'div');
> print "</div>\n";
>
> git_difftree_body(\@difftree, $hash, @$parents);
> @@ -6230,6 +6250,7 @@ sub git_commitdiff {
> git_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);
> print "</div>\n"; # class="log"
> }
> + print format_notes_html($co{'notes'}, 'div');
>
> } elsif ($format eq 'plain') {
> my $refs = git_get_references("tags");
This of course assumes that we want notes treated exactly (or almost
exactly) the same way for 'log', 'commit' and 'commitdiff' views.
Perhaps it is a good assumption (at least for first step)...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: tytso @ 2010-02-06 13:32 UTC (permalink / raw)
To: Jay Soffian; +Cc: Junio C Hamano, Eric Wong, git
In-Reply-To: <76718491002052119j5d6bae35s146bed8da03e3983@mail.gmail.com>
On Sat, Feb 06, 2010 at 12:19:02AM -0500, Jay Soffian wrote:
> On Fri, Feb 5, 2010 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > How could that be a counterproposal to a deprecation of -x and adding
> > hooks as a replacement, as a solution to more general issues?
>
> Sorry, it's been a long week and I wasn't thinking clearly.
>
> Yes, adding post-cherry/rebase hook(s) would keep all parties happy. :-)
I really don't think it's a contamination of the log when it's used to
record a cherry pick of patch from the dev stream to an older
maintainance branch, but I'll grudgingly accept doing via a hook; but
can there be an easy way to control whether or not the hook is
actually executed?
*Normally* I don't want cherry-pick -x, but some of the time I do want
it, and manually enabling and disabling the hook by having to do some
kind of "chmod -x .git/hooks/cherry-pick" command. I suppose I could
create an alias for cherry-pick-record which does a "chmod a+x `git
rev-parse --git-dir`/hooks/cherry-pick; git cherry-pick $*; chmod a-x
`git rev-parse --git-dir`/hooks/cherry-pick", but that seems really
hacky. :-(
So it would be nice if there was some kind of command-line option to
cherry-pick which could be passed to the hook script.
- Ted
^ permalink raw reply
* A generalization of git notes from blobs to trees - git metadata?
From: Jon Seymour @ 2010-02-06 13:32 UTC (permalink / raw)
To: Git Mailing List
git notes is a nice innovation - well done to all those involved.
Has consideration ever been given to generalizing the concept to allow
note (or more correctly - metadata) trees with arbitrary sha1s?
For example, suppose you had reason to cache the distribution that
resulted from the build of a particular commit, then it'd be nice to
be able to do this using a notes like mechanism.
git metadata import foo-1.1.0 dist ~/foo/dist
would create a git tree from the contents of ~/foo/dist and then bind
it to meta item called dist associated with the sha1 corresponding to
foo-1.1.0
To retrieve the contents of the previous build, you'd do something like
get metadata export foo-1.1.0 dist /tmp/foo-1.1.0
This would find the metadata tree associated with foo-1.1.0, extract
the dist subtree from that tree and write it to disk at /tmp/foo-1.1.0
I've used build outputs as an example here, but really it needn't be
limited to that. I can see this facility would be useful for any kind
of annotation or derived result that is more complex than a single
text blob. Metadata trees in combination with a name spacing
technique, could be used to store arbitrary metadata created by an
arbitrary set of tools to arbitrary SHA1 objects.
jon.
^ permalink raw reply
* Re: [PATCH] t9501: Skip testing load if we can't detect it
From: Brian Gernhardt @ 2010-02-06 13:46 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Git List, Junio C Hamano
In-Reply-To: <m3eikya9tn.fsf@localhost.localdomain>
On Feb 6, 2010, at 6:22 AM, Jakub Narebski wrote:
> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>
>> Currently gitweb only knows how to check for load using /proc/loadavg,
>> which isn't available on all systems. We shouldn't fail the test just
>> because we don't know how to check the system load.
>>
>> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
>
> NAK. It is not necessary, and it would be hindrance (one more place
> to update) if we are to extend get_loadavg() in gitweb to work without
> /proc/loadavg, e.g. via BSD::loadavg module.
Without this patch, the test fails on my OS X machine (which doesn't have /proc). So _something_ is necessary. Skipping the test because we can't use the feature on the host machine seemed more in line with what the other tests do with things like symlinks and file modes. And if get_loadavg() is updated to use BSD::loadavg, the test should still be skipped if the module isn't installed.
Furthermore, tests should always be updated when a feature is changed.
> Let me explain how it currently works without /proc/loadavg.
I did check the code.
> Third, the test (as you can see below in context line in quoted diff
> below) forces gitweb to go over maximum load by setting $maxload to 0.
> This means that regardless of true load, and regardless whether gitweb
> can detect system load (remember that if it cant get system load it
> returns 0 instead) gitweb would be in "load too high" situation.
I did check the code. Skipping the test seemed more in line with other tests.
sub get_loadavg {
if( -e '/proc/loadavg' ){}
return 0;
}
if (defined $maxload && get_loadavg() > $maxload) {
}
Setting $maxload to 0 does _not_ trigger failure because zero is not greater than 0. Setting $maxload to -1 might work though. I'll try it and test it in a little bit. While I disagree that it's a good way to handle the situation, I will see if it works.
~~ Brian
^ permalink raw reply
* Re: git gc / git repack not removing unused objects?
From: Jon Nelson @ 2010-02-06 13:53 UTC (permalink / raw)
Cc: git
In-Reply-To: <alpine.LFD.2.00.1002051634380.1681@xanadu.home>
On Fri, Feb 5, 2010 at 3:45 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Fri, 5 Feb 2010, Jon Nelson wrote:
>
>> On Fri, Feb 5, 2010 at 2:51 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
>> > On Fri, 5 Feb 2010, Jon Nelson wrote:
>> >
>> >> [Using git 1.6.4.2]
>> >>
>> >> In one repo I have (136G objects directory, fully packed) I'm having
>> >> some trouble.
>> >> I've run git-gc --prune=now, git repack -Adf, and so on a half-dozen
>> >> times and each time I do so it gets bigger, not smaller.
>> >
>> > Please tell us more.
>>
>> I'll tell you whatever I can -- as soon as I know what it is you want.
>
> I want to know what you did and what results you obtained to lead you to
> believe that 'git-gc --prune=now' makes your repository bigger.
Last night, the repo size was 153G after removing some commits and
objects by way of git filter-branch.
I'm using "du -sh" in the .git directory to determine the disk usage.
Before: 136G
git repack -dAl
After: 153G
Then, just to make sure of some things, I changed nothing and simply
re-ran "git repack -dAl".
After: 167G
Config:
[core]
repositoryformatversion = 0
filemode = true
bare = true
logallrefupdates = true
compression = 0
loosecompression = 0
packedGitLimit = 1500M
#packedGitWindowSize = 32m
[pack]
packsizelimit = 256m
windowMemory = 128m
deltaCacheSize = 1m
[gc]
threads = 1
packrefs = true
[repack]
usedeltabaseoffset = true
Incorporating the system and global config (git config -l):
pack.packsizelimit=2M
pack.threads=1
gc.auto=0
gc.autopacklimit=0
core.repositoryformatversion=0
core.filemode=true
core.bare=true
core.logallrefupdates=true
core.compression=0
core.loosecompression=0
core.packedgitlimit=1500M
pack.packsizelimit=256m
pack.windowmemory=128m
pack.deltacachesize=1m
gc.threads=1
gc.packrefs=true
repack.usedeltabaseoffset=true
--
Jon
^ permalink raw reply
* Re: [PATCH] t9501: Skip testing load if we can't detect it
From: Jakub Narebski @ 2010-02-06 14:05 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List, Junio C Hamano
In-Reply-To: <0CD6B283-3181-4FAB-A6B2-13AFF9E5071C@gernhardtsoftware.com>
Brian Gernhardt wrote:
> On Feb 6, 2010, at 6:22 AM, Jakub Narebski wrote:
>> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>>
>>> Currently gitweb only knows how to check for load using /proc/loadavg,
>>> which isn't available on all systems. We shouldn't fail the test just
>>> because we don't know how to check the system load.
>>>
>>> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
>>
>> NAK. It is not necessary, and it would be hindrance (one more place
>> to update) if we are to extend get_loadavg() in gitweb to work without
>> /proc/loadavg, e.g. via BSD::loadavg module.
>
> Without this patch, the test fails on my OS X machine (which doesn't
> have /proc). So _something_ is necessary.
Right.
> Skipping the test because we can't use the feature on the host machine
> seemed more in line with what the other tests do with things like
> symlinks and file modes.
This is more test that gitweb returns correct HTTP response in 'load
too high' situation, than testing of it finding average system load.
> And if get_loadavg() is updated to use BSD::loadavg, the test should
> still be skipped if the module isn't installed.
This test should succeed both when get_loadavg() returns true system
load and when it can't read load average and it returns 0.
>> Third, the test (as you can see below in context line in quoted diff
>> below) forces gitweb to go over maximum load by setting $maxload to 0.
>> This means that regardless of true load, and regardless whether gitweb
>> can detect system load (remember that if it cant get system load it
>> returns 0 instead) gitweb would be in "load too high" situation.
>
> I did check the code. Skipping the test seemed more in line with other tests.
>
> sub get_loadavg {
> if( -e '/proc/loadavg' ){}
> return 0;
> }
>
> if (defined $maxload && get_loadavg() > $maxload) {
> }
>
> Setting $maxload to 0 does _not_ trigger failure because zero is not
> greater than 0. Setting $maxload to -1 might work though. I'll try
> it and test it in a little bit. While I disagree that it's a good
> way to handle the situation, I will see if it works.
Oops.
Either gitweb should be modified to read
if (defined $maxload && get_loadavg() >= $maxload) {
or the test should use
$maxload = -1;
to artificially force 'load too high' situation.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Giuseppe Bilotta @ 2010-02-06 14:15 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002061416.29481.jnareb@gmail.com>
2010/2/6 Jakub Narebski <jnareb@gmail.com>:
> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:
>
>> The notes are shown side-by-side along the bottom of the commit
>> message.
>
> The same question apply as for previous commit.
>
> What happens if screen size is too small to contain both commit message
> and notes? Does it do the sensible thing of putting notes _below_
> commit message in such situation? I do not know CSS+HTML enogh to
> answer this question myself.
In this view the notes are printed side-by-side to each other, but at
the end of the commit message, so there's no interference at all.
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 0d0877e..0d03026 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -2837,12 +2837,31 @@ sub parse_commit {
>> %co = parse_commit_text(<$fd>, 1);
>> close $fd;
>>
>> + my %notes = ();
>> + foreach my $note_ref (get_note_refs()) {
>> + my $obj = "$note_ref:$co{'id'}";
>> + if (open my $fd, '-|', git_cmd(), 'rev-parse',
>> + '--verify', '-q', $obj) {
>> + my $exists = <$fd>;
>> + close $fd;
>> + if (defined $exists) {
>> + if (open $fd, '-|', git_cmd(), 'show', $obj) {
>> + $notes{$note_ref} = scalar <$fd>;
>> + close $fd;
>> + }
>> + }
>> + }
>> + }
>> + $co{'notes'} = \%notes;
>> +
>> return %co;
>> }
>
> Duplicated code. Please put this code in a separate subroutine, to be
> called in those two places.
Yup, definitely a good idea.
>> # return all refs matching refs/notes/<globspecs> where the globspecs
>> # are taken from the notes feature content.
>> sub get_note_refs {
>> + local $/ = "";
>> +
>
> Why it is needed here? Why you want to use empty lines as terminator
> (which means reading whole paragraphs), while treating two or more
> consecutive empty lines as a single empty line (according to
> perlvar(1))?
>
> If you want to slurp whole file, this should be
>
> local $/;
>
> or more explicit
>
> local $/ = undef;
Ah, sorry, for some reason I thought "" was the default.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Jakub Narebski @ 2010-02-06 14:34 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <cb7bb73a1002060615o261d974cnf347ba5af5ae324b@mail.gmail.com>
Giuseppe Bilotta wrote:
> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:
[...]
>>> # return all refs matching refs/notes/<globspecs> where the globspecs
>>> # are taken from the notes feature content.
>>> sub get_note_refs {
>>> + local $/ = "";
>>> +
>>
>> Why it is needed here? Why you want to use empty lines as terminator
>> (which means reading whole paragraphs), while treating two or more
>> consecutive empty lines as a single empty line (according to
>> perlvar(1))?
>>
>> If you want to slurp whole file, this should be
>>
>> local $/;
>>
>> or more explicit
>>
>> local $/ = undef;
>
> Ah, sorry, for some reason I thought "" was the default.
If you wanted to use default value, why set it at all?
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] t9501: Correctly force over max load everywhere
From: Brian Gernhardt @ 2010-02-06 14:50 UTC (permalink / raw)
To: Jakub Narebski, Junio C Hamano; +Cc: Git List
In-Reply-To: <201002061505.13886.jnareb@gmail.com>
The code to check for load returns 0 if it doesn't know how to find
load. It also checks to see if the current load is higher than the
max load. So to force the script to quit early by setting the maxload
variable negative which should work for systems where we can detect
load (which should be a positive number) and systems where we can't
(where detected load is 0)
Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
Tested and works.
t/t9501-gitweb-standalone-http-status.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 7590f10..d196cc5 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -117,7 +117,7 @@ test_debug 'cat gitweb.output'
# always hit the load limit
cat >>gitweb_config.perl <<\EOF
-our $maxload = 0;
+our $maxload = -1;
EOF
test_expect_success 'load checking: load too high (default action)' '
--
1.7.0.rc1.205.g52ece
^ permalink raw reply related
* Re: [PATCH 0/9] add --ff option to cherry-pick
From: Christian Couder @ 2010-02-06 15:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <7vtytvnt5h.fsf@alter.siamese.dyndns.org>
On samedi 06 février 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Changes since the previous series are the following:
>
> Could you please briefly describe what good does "adding -ff option to
> cherry-pick" do in the first place?
Ok I will do that.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Christian Couder @ 2010-02-06 15:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Stephen Boyd
In-Reply-To: <4B6D38FB.9000307@gnu.org>
On samedi 06 février 2010, Paolo Bonzini wrote:
> On 02/06/2010 12:11 AM, Christian Couder wrote:
> > As "git merge" fast forwards if possible, it seems sensible to
> > have such a feature for "git cherry-pick" too, especially as it
> > could be used in git-rebase--interactive.sh.
> >
> > Maybe this option could be made the default in the long run, with
> > another --no-ff option to disable this default behavior, but that
> > could make some scripts backward incompatible and/or that would
> > require testing if some GIT_AUTHOR_* environment variables are
> > set. So we don't do that for now.
>
> I would still like to have a no-op --no-ff so that scripts that do rely
> on that can be future proofed (or also, scripts that do "git cherry-pick
> $blah -e COMMIT" could use --no-ff to avoid errors in case $blah
> contains --ff).
Ok, I will add a --no-ff option but I think it should be incompatible
with --ff rather than overide it.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH 9/9] merge: use new "reset" function instead of running "git read-tree"
From: Christian Couder @ 2010-02-06 15:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <7vbpg3nsbd.fsf@alter.siamese.dyndns.org>
On samedi 06 février 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > This simplifies "git merge" code and make it more efficient in some
> > cases.
>
> I vaguely recall somebody (perhaps it was you) tried to do something like
> this before to drive unpack_trees() inside the main process, broke the
> program rather badly, and then we ended up keeping read-tree invocation
> external to the process. Am I misremembering things?
I don't think it was me and I don't recall that, but I don't follow all the
threads on the mailing list.
I will search the archive, but any pointer would be very appreciated.
Thanks anyway,
Christian.
> If not, could you describe how is this round different from the old one?
^ permalink raw reply
* Re: [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Paolo Bonzini @ 2010-02-06 15:41 UTC (permalink / raw)
To: git
In-Reply-To: <201002061629.05640.chriscool@tuxfamily.org>
On 02/06/2010 04:29 PM, Christian Couder wrote:
> On samedi 06 février 2010, Paolo Bonzini wrote:
>> On 02/06/2010 12:11 AM, Christian Couder wrote:
>>> As "git merge" fast forwards if possible, it seems sensible to
>>> have such a feature for "git cherry-pick" too, especially as it
>>> could be used in git-rebase--interactive.sh.
>>>
>>> Maybe this option could be made the default in the long run, with
>>> another --no-ff option to disable this default behavior, but that
>>> could make some scripts backward incompatible and/or that would
>>> require testing if some GIT_AUTHOR_* environment variables are
>>> set. So we don't do that for now.
>>
>> I would still like to have a no-op --no-ff so that scripts that do rely
>> on that can be future proofed (or also, scripts that do "git cherry-pick
>> $blah -e COMMIT" could use --no-ff to avoid errors in case $blah
>> contains --ff).
>
> Ok, I will add a --no-ff option but I think it should be incompatible
> with --ff rather than overide it.
Whatever it does it should be the same as git merge.
Paolo
^ permalink raw reply
* Re: [PATCH 4/4] gitweb: show notes in commit(diff) view
From: Giuseppe Bilotta @ 2010-02-06 16:13 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002061534.02972.jnareb@gmail.com>
On Sat, Feb 6, 2010 at 3:34 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Giuseppe Bilotta wrote:
>> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:
>
> [...]
>>>> # return all refs matching refs/notes/<globspecs> where the globspecs
>>>> # are taken from the notes feature content.
>>>> sub get_note_refs {
>>>> + local $/ = "";
>>>> +
>>>
>>> Why it is needed here? Why you want to use empty lines as terminator
>>> (which means reading whole paragraphs), while treating two or more
>>> consecutive empty lines as a single empty line (according to
>>> perlvar(1))?
>>>
>>> If you want to slurp whole file, this should be
>>>
>>> local $/;
>>>
>>> or more explicit
>>>
>>> local $/ = undef;
>>
>> Ah, sorry, for some reason I thought "" was the default.
>
> If you wanted to use default value, why set it at all?
Ach, sorry, forgot to reply to the first part of the question. It's
used in a context where $/ is locally set to \0, so it needs to be
reset.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* using gitconfig to get shell access
From: Sitaram Chamarty @ 2010-02-06 17:33 UTC (permalink / raw)
To: git
Hello all,
If I have a bare repo that is accessed only remotely, but I
allow arbitrary 'git config' keys to be set on it by the
remote user (somehow), can any of them be used to make git
run arbitrary commands on the server?
For example, 'hooks.showrev', is a "shell command used to
format each revision in the email".
Of course that's only in contrib, but are there any like
that in the main git config settings? I took a look through
man git-config and offhand I couldn't find any, but if
someone knows more definitively I'd appreciate hearing about
it.
Thanks,
Sitaram
^ permalink raw reply
* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Junio C Hamano @ 2010-02-06 17:39 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jan Engelhardt, git
In-Reply-To: <20100206115817.GA11605@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Feb 05, 2010 at 08:53:36PM -0800, Junio C Hamano wrote:
>
>> > git grep -- pattern revision -- pathname
>> [...]
>> I don't think if "ambiguous or not" is what we are after to begin with.
>>
>> I have known GNU extended grep implementations long enough but never saw
>> that "--" used to quote a pattern. Is it worth supporting to begin with?
>
> I think so. It was the first thing the original poster in this thread
> tried. It is also something I have tried (and still do, then grumblingly
> retype "-e pattern"). And it certainly makes sense from a user
> perspective; it is the same end-of-options signal that most other
> programs take.
Ok, then let's take that (perhaps before 1.7.0 perhaps after).
^ permalink raw reply
* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Junio C Hamano @ 2010-02-06 17:47 UTC (permalink / raw)
To: tytso; +Cc: Jay Soffian, Eric Wong, git
In-Reply-To: <20100206133227.GE321@thunk.org>
tytso@mit.edu writes:
> I really don't think it's a contamination of the log when it's used to
> record a cherry pick of patch from the dev stream to an older
> maintainance branch,...
Ok, then I'll happily scrap my earlier comment and accept -x to rebase,
too.
Thanks for a feedback and injection of some sanity.
^ 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