Git development
 help / color / mirror / Atom feed
* 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 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Paolo Bonzini @ 2010-02-06  9:40 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
	Stephan Beyer, Daniel Barkalow, Stephen Boyd
In-Reply-To: <20100205231112.3689.67634.chriscool@tuxfamily.org>

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).

Paolo

^ permalink raw reply

* Re: [PATCH] grep: simple test for operation in a bare repository
From: René Scharfe @ 2010-02-06  9:35 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano
In-Reply-To: <fcaeb9bf1002041840l4d8e8ac9k3ad5e9e8761aa1b@mail.gmail.com>

Am 05.02.2010 03:40, schrieb Nguyen Thai Ngoc Duy:
> setup_pager() calls git_config(), which indirectly calls get_git_dir()
> and sets git_dir in stone. Changing GIT_DIR environment variable alone
> won't work, as you have seen.
> 
> When RUN_SETUP is set, setup_git_directory() would be called before
> setup_pager() can kick in, so everything is properly set.
> 
>> There are five more sites in git.c, path.c and setup.c where $GIT_DIR
>> is set directly with setenv().  I wonder if they should better call
>> set_git_dir() instead, too.
> 
> Yes, they should.

This patch converts the setenv() calls in path.c and setup.c.  After
the call, git grep with a pager works again in bare repos.

It leaves the setenv(GIT_DIR_ENVIRONMENT, ...) calls in git.c alone, as
they respond to command line switches that emulate the effect of setting
the environment variable directly.

The remaining site in environment.c is in set_git_dir() and is left
alone, too, of course.  Finally, builtin-init-db.c is left changed
because the repo is still being carefully constructed when the
environment variable is set.

This fixes git shortlog when run inside a git directory, which had been
broken by abe549e1.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Since it's doesn't fix a regression (abe549e1 was committed in March
2008), this patch doesn't have to go in at this point in the release
cycle.  And perhaps it's even superseded by the more general fix Duy
is working on?

 path.c  |    2 +-
 setup.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/path.c b/path.c
index 79aa104..0005df3 100644
--- a/path.c
+++ b/path.c
@@ -336,7 +336,7 @@ char *enter_repo(char *path, int strict)
  	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
 	    validate_headref("HEAD") == 0) {
-		setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+		set_git_dir(".");
 		check_repository_format();
 		return path;
 	}
diff --git a/setup.c b/setup.c
index 710e2f3..b38cbee 100644
--- a/setup.c
+++ b/setup.c
@@ -404,9 +404,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				inside_work_tree = 0;
 			if (offset != len) {
 				cwd[offset] = '\0';
-				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
+				set_git_dir(cwd);
 			} else
-				setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+				set_git_dir(".");
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}

^ permalink raw reply related

* Re: [PATCH 2/4] gitweb: show notes in shortlog view
From: Giuseppe Bilotta @ 2010-02-06  9:24 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002060118.21137.jnareb@gmail.com>

2010/2/6 Jakub Narebski <jnareb@gmail.com>:
> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
>
>> Subject: [PATCH 2/4] gitweb: show notes in shortlog view
>
> Is it RFC?

See reply to comments on 1/4 8-/

> Why it is only for 'shortlog' view, and not also for 'history' which is
> also shortlog-like view?  Or is there reason why it is not present for
> 'history' view?

I always forget about history view, probably because I never use it.

>> The presence of the note is shown by a small icon, hovering on which
>> reveals the actual note content.
>
> Signoff?

A-hem. (whistles innocently)

>> +
>> +span.notes span.note-container:before {
>> +     content: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');
>> +}
>
> Not all web browsers support ':before' pseudo-element, and 'content'
> (pseudo-)property.

I know it's neither good form nor good webdesigner attitude, but I
stopped caring about IE a long time ago. I understand however that
some ancient versions of Mozilla browsers might have the same issue
too.

> Not all web browsers support 'data:' URI schema in CSS; also such image
> cannot be cached (on the other hand it doesn't require extra TCP
> connection on first access, and CSS file is cached anyway).
>
> On the other hand adding extra images to gitweb would probably require
> additional (yet another) build time parameter to tell where static
> images are (besides logo and favicon).
>
> So perhaps it is good solution, at least for a first attempt.

A possible alternative could maybe do without images and just use
borders and backgrounds of an 8x8 fixed-size element. Wouldn't look as
nice, probably, but should render decently in everything that supports
CSS1.

>> +# display notes next to a commit
>> +sub format_notes_html {
>> +     my %notes = %{$_[0]};
>
> Why not use 'my $notes = shift;', and later '%$notes'?

No particular reason. I didn't check for syntax preferences regarding
this in gitweb, or I would have noticed there was a preference for the
one you mention.

>> +     my $ret = "";
>
> Perhaps $return or $result would be a better name, to better distinguish
> it from visually similar $ref (see $ref vs $res);

Yep, good point.

>> +     while (my ($ref, $text) = each %notes) {
>> +             # remove 'refs/notes/' and an optional final s
>> +             $ref =~ s/^refs\/notes\///;
>
> You can use different delimiter than / to avoid 'leaning toothpick'
> syndrome, e.g.: $ref =~ s!^refs/notes/!!;

Indeed I should.

>> +             $ref =~ s/s$//;
>> +
>> +             # 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'>";
>> +             foreach my $line (split /\n/, $text) {
>> +                     $ret .= esc_html($line) . "<br/>";
>
> Probably would want
>
>                        $ret .= esc_html($line) . "<br/>\n";
>
> here.  Or do we want single string here?

It's within a span element so I was trying to stick to single line in
the HTML source.

> Also, do you want/need final <br>?  If not, perhaps
>
>                join("<br/>", map { esc_html($_) } split(/\n/, $text);
>
> would be a better solution (you can always add final "<br/>" later)?

I did notice that the final br didn't seem to affect the box height,
so I didn't bother looking at ways to do without it, but it's probably
nicer to not have it.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Giuseppe Bilotta @ 2010-02-06  9:02 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Johannes Schindelin, Johan Herland, Junio C Hamano
In-Reply-To: <201002060044.11225.jnareb@gmail.com>

2010/2/6 Jakub Narebski <jnareb@gmail.com>:
> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
>
> BTW. shouldn't this series be marked as RFC?
>

[snip]

>
> Signoff?

Y'know, one would figure that, this not being my first contribution
and what, I'd have learned to do this properly 8-/.

>> +     # Notes support. When this feature is enabled, the presence of notes
>> +     # for any commit is signaled, and the note content is made available
>> +     # in a way appropriate for the current view.
>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob
>> +     # specification to enable specific namespaces only.
>
> It is not obvious from this description that you can provide _list_ of
> notes namespaces (or list of shell-globs).

I'm starting to think it might make sense to not have a list here, but
rather a single value only. First of all, multiple refs can be
indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the
intended command-line syntax ref1:ref2:ref3, which would help
consistency. It also makes things easier for project overrides, as per
your subsequent comment:

>> +
>> +     # To enable system wide have in $GITWEB_CONFIG
>> +     # $feature{'notes'}{'default'} = ['*'];
>> +     # To have project specific config enable override in $GITWEB_CONFIG
>> +     # $feature{'notes'}{'override'} = 1;
>> +     # and in project config gitweb.notes = namespace;
>
> How you can provide list of notes here?  Is overriding limited to single
> name or shell-glob?
>
> See feature_snapshot for example implementation.

As mentioned above, I'd rather use the same syntax deployed on the
command line, either shell-like or PATH-like multiple paths.

> Second, perhaps it is time to refactor all those similar feature_xxx
> subroutines (just a possible suggestion)?

feature_notes looks remarkably like feature_avatar, indeed.

>> +# return all refs matching refs/notes/<globspecs> where the globspecs
>> +# are taken from the notes feature content.
>> +sub get_note_refs {
>> +     my @globs = gitweb_get_feature('notes');
>> +     my @note_refs = ();
>> +     foreach my $glob (@globs) {
>> +             if (open my $fd, '-|', git_cmd(), 'for-each-ref',
>> +                 '--format=%(refname)', "refs/notes/$glob") {
>
>                open my $fd, '-|', git_cmd(), 'for-each-ref',
>                        '--format=%(refname)', "refs/notes/$glob"
>                        or return;
>
> would reduce indent level a bit.

Good idea, thanks.

>> +                     while (<$fd>) {
>> +                             chomp;
>> +                             push @note_refs, $_ if $_;
>> +                     }
>
> Why not simply
>
>                chomp(@note_refs = <$fd>);

Because I didn't know chomp worked on lists. Thanks for the idea.

>> +             my %notes = () ;
>> +             foreach my $note_ref (@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;
>> +                                     }
>> +                             }
>> +                     }
>> +             }
>
> First, there are '--batch' and '--batch-check' options to git-cat-file.
> With these I think you can get all notes with just single git command,
> although using it is a bit complicated (requires open2 from IPC::Open2
> for bidi communication).

Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.

> Second, if not using 'git cat-file --batch', perhaps it would be easier
> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and
> parse its output to check for which commits/objects there are notes
> available, and only then call 'git show' (or reuse connection to
> 'git cat-file --batch').
>
> The second solution, with a bit more work, could work even in presence
> of fan-out schemes for notes, I think.

An interesting approach. Without fan-out, git ls-tree -r
refs/notes/whatever [hash ...] gives us the blobs we're interested in.
In case of fan-out schemes, the efficiency of this approach probably
depends on the kind of fan-out we have, and would require some
heavy-duty grepping. A git ls-notes plumbing with a similar syntax and
output would be a nice thing to have.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* [PATCH] bash: support the --autosquash option for rebase
From: Björn Gustavsson @ 2010-02-06  8:37 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
 contrib/completion/git-completion.bash |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index da46bf8..35acad0 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1386,6 +1386,7 @@ _git_rebase ()
 			--preserve-merges --stat --no-stat
 			--committer-date-is-author-date --ignore-date
 			--ignore-whitespace --whitespace=
+			--autosquash
 			"
 
 		return
-- 
1.7.0.rc1.10.gb8bb

^ permalink raw reply related

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Miles Bader @ 2010-02-06  8:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jan Engelhardt, git
In-Reply-To: <7v7hqrdkxb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> 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?

Well, it is a natural consequence of the way command-line parsing works
in typical GNU progs; "--" doesn't mean "files follow", it means "no
more options"....

-Miles

-- 
I'd rather be consing.

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: notes feature
From: Giuseppe Bilotta @ 2010-02-06  8:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johan Herland, Jakub Narebski, git, Johannes Schindelin
In-Reply-To: <7vsk9fwbzj.fsf@alter.siamese.dyndns.org>

On Fri, Feb 5, 2010 at 11:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Giuseppe was wondering about multi-line thing, so the illustration should
> be adjusted to match the "format-patch" example to show a multi-line note,
> I think.  Here is what I meant.
>
> Instead of showing:
>
>    $ git log --notes-ref=amlog -1 4d0cc22
>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
>    Author: Junio C Hamano <gitster@pobox.com>
>    Date:   Thu Feb 4 11:10:44 2010 -0800
>
>        fast-import: count --max-pack-size in bytes
>
>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
>        ...
>
>    Notes:
>        pulled on Fri Feb 5 07:36:12 2010 -0800
>        from git://git.bogomips.org/git-svn.git/
>    Notes-amlog:
>        <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>
>
> which is what 1.6.6 added, showing it like this:
>
>    $ git log --notes-ref=amlog -1 4d0cc22
>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070
>    Author: Junio C Hamano <gitster@pobox.com>
>    Date:   Thu Feb 4 11:10:44 2010 -0800
>    Notes: pulled on Fri Feb 5 07:36:12 2010 -0800
>     from git://git.bogomips.org/git-svn.git/
>    Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>
>
>        fast-import: count --max-pack-size in bytes
>
>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git
>        ...
>
> might be easier to see.  After all, notes are metainformation on commits,
> and people who are interested will look at the header, and those who are
> not will skim over the block of text at the beginning, knowing that is
> where all the metainformation is.
>
> But this is just "might", not "should---I strongly believe".


I'm not convinced. I believe format-patch output should be as close as
possible to RFC2822 compliance, given how it can be used as basis for
actual email.

According to the RFC, lines should be no more than 78 characters and
must be less than 998. The 'one space indent on newline', if I'm not
mistaken, is a way to indicate a _wrapped_ single line header, rather
than a multi-line one.

A scenario such as the following is thus possible: format-patch
creates a single-line header which is longer than 78 characters
(X-Git-Notes-somerefname: 70ish characters line). The patch gets sent
by email, and a properly configured send-email sends the X-Git-Notes*
headers through (BTW, this should probably be added to the
notes/send-email TODO). Some intermediate forwarder or user agent
decides to rewrap the longish line so that it isn't longer than 78
characters. The user saves the raw email and uses git am to apply it
-> oops, the single line note has become a two-liner.

If we use the convention (at least in format-patch, not necessarily in
log) of one header line per notes line we should be more on the safe
side. It would also allow us to keep the headers wrapped at < 78
characters which is nice for legibility on terminals.


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [gitolite] repo config for delegated projects
From: Sitaram Chamarty @ 2010-02-06  6:45 UTC (permalink / raw)
  To: martin f krafft; +Cc: git discussion list, Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <20100206042222.GA7825@lapse.rw.madduck.net>

On Sat, Feb 06, 2010 at 05:22:22PM +1300, martin f krafft wrote:
> also sprach Sitaram Chamarty <sitaramc@gmail.com> [2010.02.06.1350 +1300]:
> > OK I've run into a little decision-point here.
> > 
> > The problem above is of making sure that a delegated admin cannot
> > misuse the gitconfig mechanism to do stuff he's not allowed to do,
> > but it's actually worse than that :(
> 
> Let me thus challenge the whole delegation mechanism.
> 
> When I first encountered it, I thought it was a great idea, but it
> seems to promise more than it can do. I understand that the reasons
> for that are security-related, and I tip my hat to you for being so
> conscious about this — better have a secure system with limited
> functionality, than an insecure system that can do everything (why
> am I thinking of PHP apps right now???).
> 
> The wildrepos branch is a definite improvement to proper delegation.
> Without it, the main admin has to change the main configuration file
> every time that a delegated admin wants to add a new repo.
> 
> However, given the somewhat awkward configuration (you need to add
> delegated admins in multiple places), and the restrictions, I am
> starting to wonder what use-case delegations solve that couldn't be
> addressed easier with multiple accounts and gitolite instances.
> Thoughts?

In theory, none at all.  And if you're not in a corporate
environment, having separate accounts and instances is
probably easier, as I myself suggest to people sometimes
when they need *more* from delegation than what it has now.

What it gives you is *one* central place for all the code,
one set of users/ids and one entity to approve new ones, and
the means to delegate only the most volatile changes (like
at the branch-within-project level), and not the longer term
ones like actually adding a new user or a new project
(assuming you're not using wildrepos).

It turns out that this is closer to what corporate
environments want.  I use it, and a few groups in my $DAYJOB
also use it, afaik.

But I guess my original question was more for Teemu.  As it
stands, I need to redo the ability for even the "main" admin
to add gitconfigs... it allows him to get a shell too
easily.

> > Regardless of how I look at it, I can't think of a cure for this short
> > of either:
> >   - putting all the allowed gitconfigs in the RC file, and not in the
> > config (writing the RC file requires shell access, and we presume the
> > "root of trust" person has enough smarts to know what to allow and
> > what not to allow), and allowing repo admins to *refer* to them to use
> > whichever they want
> >   - someone coming up with a list of gitconfig's that are "safe", and
> > specific values for those that are unsafe (like saying "if you use
> > showrev, you can only use this command  as the value", and forcing
> > only those.
> 
> I think the second path is a red herring. However, I don't
> understand why we would need to go via the RC file instead of the
> main config. Only the main admin can modify that, or appoint others
> to modify it. Plus, it's managed in Git and thus has a history
> attached to it.

That's what I used to think, that it doesn't matter.

But there's a notion (and once I realised it I agreed with
it) that the ability to do "shell" things on the server is a
step higher than the ability to update gitolite's access
list.

Some people needed this, and I agreed.  Nothing prevents any
installation from giving anyone any rights, but I'd like to
prevent them acquiring it via gitolite, even if they can
write to the admin repo.

So right now (coming back to the ability to set gitconfig
from within gitolite) I'm thinking:

    $GL_GITCONFIG_KEYS = "core.foo core.baz";
        # actually list of regexes for valid keys

and advising people that these are the choices in terms of
allowing gitconfig from the admin repo (or delegation; no
difference):

  - (ultra paranoid mode): set the variable above to empty;
    no gitconfig allowed from gitolite.conf or delegated
    conf files
  - (just your normal everyday paranoia mode): set the
    variable to stuff that you know will not give shell
    access (example: the aforementioned hooks.mailinglist)
  - ("what, me worry?" mode): set that variable to ".*" and
    allow any damn gitconfig so they won't keep pestering
    you to add a config for them!

In the first 2 cases, someone with shell access must do all
required gitconfigs manually on the server when needed.

> Speaking of shell access, I notice gl-auth-command has the -s
> option. Is there a configuration variable that I overlooked which
> allows me to give shell login rights to specific users?

yes; it's in the RC file.  If you want to give anyone shell
access, add their name to a space-sep list in $SHELL_USERS
and push the config once.

The documentation for this is in a weird place (doc/6);
sorry about that!  Will move it to doc/3 soon.

-- 
Sitaram

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Jay Soffian @ 2010-02-06  5:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vbpg3dldq.fsf@alter.siamese.dyndns.org>

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. :-)

> Maybe, but it calls cmt_metadata() which is "grep ^git-svn-id:" of the
> commit object, so we know why it doesn't X-<.

Bah, that's too bad. :-(

j.

^ permalink raw reply

* [PATCH] t9501: Skip testing load if we can't detect it
From: Brian Gernhardt @ 2010-02-06  5:00 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

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>
---
 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

^ permalink raw reply related

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Junio C Hamano @ 2010-02-06  4:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Engelhardt, git
In-Reply-To: <20100206035143.GA31784@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The worst I could come up with is the
> double-double-dash case:
>
>   git grep -- pattern revision -- pathname
>
> It is perhaps not as pretty as
>
>   git grep -e pattern revision -- pathname
>
> but I don't think it is ambiguous.

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?

> So actually my patch above is breaking somebody who truly wanted to grep
> for "--" by doing
>
>   git grep --
>
> but that is sufficiently insane that I'm not too worried about it.

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.

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.

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Junio C Hamano @ 2010-02-06  4:43 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Junio C Hamano, Eric Wong, git
In-Reply-To: <76718491002052018s324747ecj31963b493868dbbd@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Since there is a difference of opinion here, how about this proposal:
>
> 1) We keep -x in cherry-pick
>
> 2) I convince you to add my -x patch to rebase.sh (hmm, how to do that?) :-)
>
> 3) We add a -X option to both cherry-pick and rebase.sh that records
> in the notes instead of in the log message.

How could that be a counterproposal to a deprecation of -x and adding
hooks as a replacement, as a solution to more general issues?

>> I ended up using this hacky script for that.  It finds the svn uuid from
>> the metadata file, and then finds ".rev_map.$uuid" files from all over the
>> place to see if any of them contains a record that points at the git
>> commit I am interested in.
>>
>> I really wish "git svn" has a built-in way to do something like that;
>> perhaps I didn't look hard enough.
>
> Is git svn find-rev not what you want? (Caveat, it doesn't seem to
> work for me, but it's claimed description seems to be what you're
> asking for.)

Maybe, but it calls cmt_metadata() which is "grep ^git-svn-id:" of the
commit object, so we know why it doesn't X-<.

^ permalink raw reply

* Re: [gitolite] repo config for delegated projects
From: martin f krafft @ 2010-02-06  4:22 UTC (permalink / raw)
  To: git discussion list; +Cc: Sitaram Chamarty, Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <2e24e5b91002051650k3c7cf14ev8752d36b5616e9a4@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

also sprach Sitaram Chamarty <sitaramc@gmail.com> [2010.02.06.1350 +1300]:
> OK I've run into a little decision-point here.
> 
> The problem above is of making sure that a delegated admin cannot
> misuse the gitconfig mechanism to do stuff he's not allowed to do,
> but it's actually worse than that :(

Let me thus challenge the whole delegation mechanism.

When I first encountered it, I thought it was a great idea, but it
seems to promise more than it can do. I understand that the reasons
for that are security-related, and I tip my hat to you for being so
conscious about this — better have a secure system with limited
functionality, than an insecure system that can do everything (why
am I thinking of PHP apps right now???).

The wildrepos branch is a definite improvement to proper delegation.
Without it, the main admin has to change the main configuration file
every time that a delegated admin wants to add a new repo.

However, given the somewhat awkward configuration (you need to add
delegated admins in multiple places), and the restrictions, I am
starting to wonder what use-case delegations solve that couldn't be
addressed easier with multiple accounts and gitolite instances.
Thoughts?

> Regardless of how I look at it, I can't think of a cure for this short
> of either:
>   - putting all the allowed gitconfigs in the RC file, and not in the
> config (writing the RC file requires shell access, and we presume the
> "root of trust" person has enough smarts to know what to allow and
> what not to allow), and allowing repo admins to *refer* to them to use
> whichever they want
>   - someone coming up with a list of gitconfig's that are "safe", and
> specific values for those that are unsafe (like saying "if you use
> showrev, you can only use this command  as the value", and forcing
> only those.

I think the second path is a red herring. However, I don't
understand why we would need to go via the RC file instead of the
main config. Only the main admin can modify that, or appoint others
to modify it. Plus, it's managed in Git and thus has a history
attached to it.

Speaking of shell access, I notice gl-auth-command has the -s
option. Is there a configuration variable that I overlooked which
allows me to give shell login rights to specific users?

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
review of a chemistry paper:
  "paper should be greatly reduced or completely oxidized."
                                                    -- frank vastola
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Jay Soffian @ 2010-02-06  4:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vtytvf4vp.fsf@alter.siamese.dyndns.org>

On Fri, Feb 5, 2010 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Hmfph. So I know some folks don't like polluting log messages (e.g.,
>> the git-svn-id footer, or apparently, the cherry-pick -x message),...
>
> That reminds me of a slightly related topic.  I've been running git-svn to
> follow (but never build) a project without metadata, exactly because I do
> not want the log message contamination. I am having a hard time mapping
> the commit object name back to the upstream subversion serial number.

Since there is a difference of opinion here, how about this proposal:

1) We keep -x in cherry-pick

2) I convince you to add my -x patch to rebase.sh (hmm, how to do that?) :-)

3) We add a -X option to both cherry-pick and rebase.sh that records
in the notes instead of in the log message.

> I ended up using this hacky script for that.  It finds the svn uuid from
> the metadata file, and then finds ".rev_map.$uuid" files from all over the
> place to see if any of them contains a record that points at the git
> commit I am interested in.
>
> I really wish "git svn" has a built-in way to do something like that;
> perhaps I didn't look hard enough.

Is git svn find-rev not what you want? (Caveat, it doesn't seem to
work for me, but it's claimed description seems to be what you're
asking for.)

j.

^ permalink raw reply

* Re: git-grep: option parsing conflicts with prefix-dash searches
From: Jeff King @ 2010-02-06  3:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jan Engelhardt, git
In-Reply-To: <7vsk9fs1j9.fsf@alter.siamese.dyndns.org>

On Fri, Feb 05, 2010 at 03:31:06PM -0800, Junio C Hamano wrote:

> Jan Engelhardt <jengelh@medozas.de> writes:
> 
> > Just about now I wanted to grep for accesses of a particular struct 
> > member. Needless to say that it was not a very amusing experience.
> > I would expect that (1) probably fails:
> >
> > (1)	$ git grep '->cnt' net/ipv4/netfilter/
> > 	error: unknown switch `>'
> >
> > So far so good, seems reasonable and matches what I would expect from 
> > most other userspace tools. So let's add -- to terminate the option 
> > list:
> 
> Also you can say "grep -e '->cnt'".  Not just "git grep" but regular grep
> understands this, too.

GNU grep understands "grep -- '->cnt'", so I find myself typing it a
lot. Even though "--" is used for revision and pathname separation, I
don't think there is a conflict in also using it to separate options
from patterns for the case that there is no "-e" at all. In other words:

  git grep -- foo

is not ambiguous. That "--" could not possibly be separating revisions
from pathnames because we have not yet seen any pattern, which is bogus.
In a case like:

  git grep -e pattern -- foo

I think it is clear that the "--" is separating pathnames, because we
already have a pathname. This matches standard grep, which will treat
the first non-option as a pattern only if we have no "-e" pattern.

So I think we can do just do this:

diff --git a/builtin-grep.c b/builtin-grep.c
index 0ef849c..46ffc1d 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -889,6 +889,16 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		/* die the same way as if we did it at the beginning */
 		setup_git_directory();
 
+	/*
+	 * skip a -- separator; we know it cannot be
+	 * separating revisions from pathnames if
+	 * we haven't even had any patterns yet
+	 */
+	if (argc > 0 && !opt.pattern_list && !strcmp(argv[0], "--")) {
+		argv++;
+		argc--;
+	}
+
 	/* First unrecognized non-option token */
 	if (argc > 0 && !opt.pattern_list) {
 		append_grep_pattern(&opt, argv[0], "command line", 0,

and make everybody happy. But I admit I haven't thought about it more
than 5 minutes or so, so perhaps there is a case I am missing that will
be ambiguous or confusing. The worst I could come up with is the
double-double-dash case:

  git grep -- pattern revision -- pathname

It is perhaps not as pretty as

  git grep -e pattern revision -- pathname

but I don't think it is ambiguous.

> > (2)	$ git grep -- '->cnt' net/ipv4/netfilter/
> > 	fatal: bad flag '->cnt' used after filename
> >
> > *bzzt*.
> 
> This indeed is bzzt, especially if you had a file called "./->cnt" in the
> work tree.  That would mean that you cannot tell the command to look for a
> pattern in the work tree.
> 
> But because you are not giving anything before "--", that "git grep" is
> not looking for anything.  Indeed, (2) is a user error.  If you try this:

That is not quite true. The way "git grep" is implemented now, it is
actually grepping for "--". parse_options stops at the "--", leaving it
in argv, and then we assume whatever is left by parse_options is a
pattern (since we saw no "-e"). But of course it is looking in the
'->cnt' pathspec (or revision!), which is bogus (and gets you "bad flag used
after filename").

But as you noted with:

> > What works is (3).
> >
> > (3)	$ git grep -- -- '->cnt' net/ipv4/netfilter/

...disambiguating the pathspec with the extra "--" gets it past option
parsing and looking for "--".

So actually my patch above is breaking somebody who truly wanted to grep
for "--" by doing

  git grep --

but that is sufficiently insane that I'm not too worried about it.

-Peff

^ permalink raw reply related

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Junio C Hamano @ 2010-02-06  2:57 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Eric Wong, git
In-Reply-To: <76718491002051758s577d8b5eq2323cd66d479662@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

>> We might instead want to add a hook that is called from cherry-pick (and
>> rebase will get a similar one) immediately after the command creates a new
>> commit out of another commit, so that people can record the correspondence
>> in notes namespace if they choose to.
>
> Hmfph. So I know some folks don't like polluting log messages (e.g.,
> the git-svn-id footer, or apparently, the cherry-pick -x message),...

That reminds me of a slightly related topic.  I've been running git-svn to
follow (but never build) a project without metadata, exactly because I do
not want the log message contamination. I am having a hard time mapping
the commit object name back to the upstream subversion serial number.

I ended up using this hacky script for that.  It finds the svn uuid from
the metadata file, and then finds ".rev_map.$uuid" files from all over the
place to see if any of them contains a record that points at the git
commit I am interested in.

I really wish "git svn" has a built-in way to do something like that;
perhaps I didn't look hard enough.

-- >8 --

#!/usr/bin/perl -w

use strict;
use Carp qw/croak/;
use Fcntl qw/:DEFAULT :seek/;
use constant rev_map_fmt => 'NH40';
use File::Find;

my $uuid = `git config -f .git/svn/.metadata svn-remote.svn.uuid`;
chomp($uuid);

# remotes/trunk

sub find_version {
	my ($name, $sha1) = @_;
	my ($fh, $size);
	if (!sysopen($fh, "$name/.rev_map.$uuid", O_RDONLY)) {
		return undef;
	}
	if (!binmode $fh) {
		close($fh);
		return undef;
	}
	$size = (stat($fh))[7];
	if (($size % 24) != 0) {
		close($fh);
		return undef;
	}

	while (sysread($fh, my $buf, 24) == 24) {
		my ($rev, $commit) = unpack(rev_map_fmt, $buf);
		if ($sha1 eq $commit) {
			close($fh);
			return $rev;
		}
	}
	close($fh);
	return undef;
}

my $top = ".git/svn/refs";
my $commit = $ARGV[0] || "HEAD";
my $sha1 = `git rev-parse $commit`;
chomp($sha1);
my $found;

sub match_svn_revision {
	if (-f "$_/.rev_map.$uuid") {
		my $it = "$File::Find::dir/$_";
		$it =~ s|^\Q$top\E/||;
		my $v = find_version($_, $sha1);
		if ($v) {
			$found = [$it, $v];
		}
		$File::Find::prune = 1;
	}
	if ($found) {
		$File::Find::prune = 1;
	}
}

File::Find::find(\&match_svn_revision, $top);

if ($found) {
	my ($origin, $rev) = @$found;
	$origin =~ s|^remotes/||;
	$origin =~ s|/|-|g;
	if ($origin eq 'trunk') {
		$origin = '';
	} else {
		$origin = "-$origin";
	}
	print "$rev$origin\n";
}

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Jay Soffian @ 2010-02-06  2:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <76718491002051758s577d8b5eq2323cd66d479662@mail.gmail.com>

On Fri, Feb 5, 2010 at 8:58 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> Well now that's interesting. I am in the process of back porting a
> large amount of development, and it seems very useful to record this
> information in the log message. The subject messages are not always
> unique, so it's provides a very easy to find the original commit.

Sorry, that wasn't clear. I meant, it's a more convenient way to find
the original commit than looking through the original branch for a
commit with the same subject.

j.

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Jay Soffian @ 2010-02-06  1:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl3ngn7w.fsf@alter.siamese.dyndns.org>

On Fri, Feb 5, 2010 at 8:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In the longer term, we would rather deprecate -x from cherry-pick, so that
> we won't contaminate the commit log message.

Well now that's interesting. I am in the process of back porting a
large amount of development, and it seems very useful to record this
information in the log message. The subject messages are not always
unique, so it's provides a very easy to find the original commit.

I know you can use "git cherry" to look for identical commits, but I'm
actually adding the "(cherry picked from ...)" especially for when the
new commit is not identical, so that I can then easily emit
interdiffs. I want to make sure that the backported work is as true to
the original branch as possible, and it's much easier to compare
commit by commit than the end product.

> So in that sense, I would give a mildly negative response to this patch.
>
> We might instead want to add a hook that is called from cherry-pick (and
> rebase will get a similar one) immediately after the command creates a new
> commit out of another commit, so that people can record the correspondence
> in notes namespace if they choose to.

Hmfph. So I know some folks don't like polluting log messages (e.g.,
the git-svn-id footer, or apparently, the cherry-pick -x message), but
I actually prefer having this data embedded there than hidden in
notes.

j.

^ permalink raw reply

* Re: [RFC/PATCH] rebase: add -x option to record original commit name
From: Junio C Hamano @ 2010-02-06  1:35 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1265419166-21388-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> Thoughts?

In the longer term, we would rather deprecate -x from cherry-pick, so that
we won't contaminate the commit log message.  So in that sense, I would
give a mildly negative response to this patch.

We might instead want to add a hook that is called from cherry-pick (and
rebase will get a similar one) immediately after the command creates a new
commit out of another commit, so that people can record the correspondence
in notes namespace if they choose to.

^ permalink raw reply

* [RFC/PATCH] rebase: add -x option to record original commit name
From: Jay Soffian @ 2010-02-06  1:19 UTC (permalink / raw)
  To: git; +Cc: Jay Soffian

It is often more convenient to use rebase --onto than cherry-pick when
relocating a range of commits, as cherry-pick only supports a single commit at a
time.

This commit teaches rebase the "-x" to record the original commit name, in the
same way as cherry-pick's "-x" option.
---

Note that I explictly did not add support for anything but the simple
format-patch + am codepath. I think it's unlikely to want to record the
original commit name in cases where "-m/-p" are needed. 

However, this option might be useful when using "--interactive", but I wanted
to get feedback first as rebase--interactive is a little scarier to patch. :-)

Thoughts?

 Documentation/git-rebase.txt |    5 +++++
 git-am.sh                    |   14 ++++++++++++++
 git-rebase.sh                |    9 +++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..2fb0e96 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -315,6 +315,11 @@ which makes little sense.
 	so that the commit marked for squashing comes right after the
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
+
+-x::
+	Append to the original commit message a note that indicates which commit
+	this change originated from, similar to cherry-pick's -x option. This
+	option is not compatible with -m, -p or --interactive.
 +
 This option is only valid when '--interactive' option is used.
 
diff --git a/git-am.sh b/git-am.sh
index c8b9cbb..83805eb 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -31,6 +31,7 @@ abort           restore the original branch and abort the patching operation.
 committer-date-is-author-date    lie about committer date
 ignore-date     use current timestamp for author date
 rerere-autoupdate update the index with reused conflict resolution if possible
+add-commit-name* (internal use for git-rebase)
 rebasing*       (internal use for git-rebase)"
 
 . git-sh-setup
@@ -296,6 +297,7 @@ git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
 allow_rerere_autoupdate=
+add_commit_name=
 
 while test $# != 0
 do
@@ -347,6 +349,8 @@ do
 		allow_rerere_autoupdate="$1" ;;
 	-q|--quiet)
 		GIT_QUIET=t ;;
+	--add-commit-name)
+		add_commit_name=t ;;
 	--)
 		shift; break ;;
 	*)
@@ -454,6 +458,7 @@ else
 	echo "$keep" >"$dotest/keep"
 	echo "$scissors" >"$dotest/scissors"
 	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
+	echo "$add_commit_name" > "$dotest/add_commit_name"
 	echo "$GIT_QUIET" >"$dotest/quiet"
 	echo 1 >"$dotest/next"
 	if test -n "$rebasing"
@@ -507,6 +512,10 @@ then
 else
 	no_inbody_headers=
 fi
+if test "$(cat "$dotest/add_commit_name")" = t
+then
+	add_commit_name=t
+fi
 if test "$(cat "$dotest/quiet")" = t
 then
 	GIT_QUIET=t
@@ -630,6 +639,11 @@ do
 		then
 			echo "$ADD_SIGNOFF"
 		fi
+		if test "$add_commit_name" = t
+		then
+			test -z "$commit" && die "Internal error: expecting a commit name."
+			echo "(cherry picked from commit $commit)"
+		fi
 	    } >"$dotest/final-commit"
 	    ;;
 	*)
diff --git a/git-rebase.sh b/git-rebase.sh
index fb4fef7..511599e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -53,6 +53,7 @@ git_am_opt=
 rebase_root=
 force_rebase=
 allow_rerere_autoupdate=
+add_commit_name=
 
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
@@ -353,6 +354,10 @@ do
 	--rerere-autoupdate|--no-rerere-autoupdate)
 		allow_rerere_autoupdate="$1"
 		;;
+	-x)
+		git_am_opt="$git_am_opt --add-commit-name"
+		add_commit_name=t
+		;;
 	-*)
 		usage
 		;;
@@ -386,6 +391,10 @@ else
 		die "previous rebase directory $dotest still exists." \
 			'Try git rebase (--continue | --abort | --skip)'
 	fi
+	if test -n "$add_commit_name"
+	then
+		die "-x not compatible with --merge"
+	fi
 fi
 
 # The tree must be really really clean.
-- 
1.7.0.rc1.200.g9c1f9

^ permalink raw reply related

* Re: [PATCH] add new options to git format-patch: --cover-subject and --cover-blurb
From: Junio C Hamano @ 2010-02-06  1:10 UTC (permalink / raw)
  To: Larry D'Anna; +Cc: Wesley J. Landaker, git
In-Reply-To: <20100205225901.GA29821@cthulhu>

Larry D'Anna <larry@elder-gods.org> writes:

> 1) make your branch
>
> 2) git format-patch --cover-letter
>
> 3) edit the cover letter
>
> 3) review the series, and realize you need to fix something, fix it.

Hmph, this begs a natural question: why didn't you review and realize that
in step (1)?

> 4) git format-patch --cover-letter again
>
> 5) edit the cover letter, *again*.  hopefully you didn't overwrite the
> old one.

This step I can understand and am very sympathetic to the cause, even
though I may not be convinced that the patch under discussion is the best
solution to the issue.

What argument are you giving to the "-o" option?  If your series changed
(e.g. inserted or deleted a commit in the middle, retitled, etc.), and
your output is going to the same directory, you would end up with files
with duplicate serial numbers and you would need to purge the old one
before your next invocation of send-email.  For this reason, people
quickly learn to either give a different -o location (so that they can
compare two versions), or to purge the old contents before running
format-patch.  If the latter, it would be sufficient to save the old
0000-cover before removing them, and if the former, the old cover is
already there.  You can cut and paste from there while editing the new
one.

The thing I found suboptimal in your approach is that most often the cover
letter is written to explain what the overall goal of the series is and
how each patch relates to each other to achieve that goal.  In order to
effectively do so, the overview format-patch leaves in 0000-cover template
file helps a lot (actually that is half the reason why it shows the
overview---the other is for the recipients).

Your approach forces the user to write the blurb part in a separate file
on blank sheet of paper _before_ running format-patch, iow, without the
help of that series overview, if they want to take advantage of your "I
don't want to lose what I wrote already" feature.  To put it another way,
people who use --cover-blurb would write suboptimal (or maybe useless)
blurb text exactly because they don't look at the series overview while
they write it---the option encourages a bad cover letter to be sent to
reviewers.

I am hoping we can do better than that.

It might be sufficient for format-patch to notice a 0000-cover file that
is already there, read the subject and blurb part and carry that forward,
instead of unconditionally writing "*** SUBJECT HERE ***" and stuff.  That
way, the user does not have to prepare a separate file before running
format-patch.

By scanning from the bottom of the existing 0000-cover file, skipping
diffstat part (easy to spot with regexp) and then skip backwards a block
of text whose lines are one of:

 (1) two space indented---that's one-line-per-commit;

 (2) empty line---separator; or

 (3) unindented line that ends with '(' number ')' ':'---the author.

The remainder would be the BLURB.  And you know it is much easier to find
where the Subject: is ;-)

^ permalink raw reply

* Re: [gitolite] repo config for delegated projects
From: Sitaram Chamarty @ 2010-02-06  0:50 UTC (permalink / raw)
  To: martin f krafft; +Cc: Sitaram Chamarty, git discussion list, Teemu Matilainen
In-Reply-To: <20100204040812.GC13411@lapse.rw.madduck.net>

On Thu, Feb 4, 2010 at 9:38 AM, martin f krafft <madduck@madduck.net> wrote:
> also sprach Sitaram Chamarty <sitaram@atc.tcs.com> [2010.02.04.1418 +1300]:
>> how about
>>
>>     $DELEGATED_CONFIGS = "hooks.mailinglist,hooks.showrev";
>
> Excellent idea.

OK I've run into a little decision-point here.

The problem above is of making sure that a delegated admin cannot
misuse the gitconfig mechanism to do stuff he's not allowed to do, but
it's actually worse than that :(

First some background.  For a long time I treated the "main" admin
(anyone who has RW/RW+ rights to gitolite.conf) to be eqvt to having
shell access.  Then we started moving away from that, and that is good
because having shell access allows him to bypass the logging that
gitolite does, thus polluting the audit trail.  Preventing that makes
a lot of sense in a corporate environment, and lets you allow a lot
more people to manage the gitolite access list.

Now I just looked up hooks.showrev, and it's supposed to be any shell
command.  Clearly this means anyone who can set that gitconfig option
now has shell capability, and it's game over.

Regardless of how I look at it, I can't think of a cure for this short
of either:
  - putting all the allowed gitconfigs in the RC file, and not in the
config (writing the RC file requires shell access, and we presume the
"root of trust" person has enough smarts to know what to allow and
what not to allow), and allowing repo admins to *refer* to them to use
whichever they want
  - someone coming up with a list of gitconfig's that are "safe", and
specific values for those that are unsafe (like saying "if you use
showrev, you can only use this command  as the value", and forcing
only those.

I'm leaning toward the former; easier for me ;-)  Meanwhile, I'm
punting this to Teemu until the morning fog in my brain clears :)

^ permalink raw reply

* Re: [RFC PATCH 00/10] gitweb: Simple file based output caching
From: J.H. @ 2010-02-06  0:51 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <cover.1264198194.git.jnareb@gmail.com>

> Table of contents:
> ~~~~~~~~~~~~~~~~~~
>  [RFC PATCH 01/10] gitweb: Print to explicit filehandle (preparing
>                    for caching)

This looks fine, I did some quick testing to verify that this would work
- and it does.

The only caveat that needs to be aware is that if the layer is going to
output binary data it needs to flip the whole stream to :raw before
outputting (this is going to be more specific to the caching layer).

One advantage to having the file handles separate is that it's easier to
distinguish if the data is going to need to be binary data that will
need to be flipped properly.

Also means you could cache the binary data differently than textual data.

I.E. binary data gets saved to disk, but page data gets saved to memcached.

Just food for thought, I'm not sure which way makes more sense
personally, though I would have a tendency to err on the side of
flexibility and have both.

>  [RFC PATCH 02/10] gitweb: href(..., -path_info => 0|1)

note: delaying additional comment till I've finished reading through the
basics of the following patches.

>  [RFC PATCH 03/10] gitweb/cache.pm - Very simple file based caching

Ok this is quite the departure from what I had, I'm unsure that it's the
right way to go, but it obviously has merits (I.E. much simpler addition
of any Cache::Cache or CHI compatible caching layer)

This patch itself looks fine, and as it states it borrows heavily from
my basic implementation - just wraps it differently.  I might have some
thoughts on extending this a bit to be a bit more flushed out from a
basic standpoint.

Need to dig through it some more, but I'm generally ok with it.

>  [RFC PATCH 04/10] gitweb/cache.pm - Stat-based cache expiration

Looks fine to me, though the note about getting the errors should get
moved to previous patch, as it says.

Note: I'm going to stop here as the following are WIP and I want to play
around with this particular direction on my own a little more before
further comment.  There's some ideas running around I want to try and
get down in code first.  Me moving on and trying these other ideas is
not a reflection on the following patches, just some alternative
thinking before I discuss some other ideas on the following patches.

Also I've been sitting on this e-mail in this state for almost a week
while I've been playing with this and having to fight other fires and I
know that Jakub has been looking for commentary on this.

>  [RFC PATCH 05/10] gitweb: Use Cache::Cache compatibile (get, set)
>                    output caching (WIP)
>  [RFC PATCH 06/10] gitweb/cache.pm - Adaptive cache expiration time (WIP)
>  [RFC PATCH 07/10] gitweb: Use CHI compatibile (compute method) caching (WIP)
>  [RFC PATCH 08/10] gitweb/cache.pm - Use locking to avoid 'stampeding herd'
>                    problem (WIP)
>  [RFC PATCH 09/10] gitweb/cache.pm - Serve stale data when waiting for
>                    filling cache (WIP)
>  [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
>                    regenerating cache (WIP)

^ permalink raw reply

* Re: [PATCH 5/9] revert: add --ff option to allow fast forward when cherry-picking
From: Junio C Hamano @ 2010-02-06  0:21 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Paolo Bonzini, Stephen Boyd
In-Reply-To: <7vpr4jnsm9.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> 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.

One more thing, in the same part of the code:

>> +	if (!(flags & PICK_REVERSE) && ff_ok && commit->parents) {
>> +		unsigned char head_sha1[20];
>> +		if (get_sha1("HEAD", head_sha1))
>> +			die("You do not have a valid HEAD.");
>> +		if (!hashcmp(commit->parents->item->object.sha1, head_sha1)) {
>> +			char *hex = sha1_to_hex(commit->object.sha1);

Is there a need to check commit->parents->next?

Should this code work the same way if the commit being cherry-picked is a
merge?  Should "-m <parent-num>" option affect this codepath in any way?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox