Git development
 help / color / mirror / Atom feed
* Re: [PATCH] gitweb: merge boolean feature subroutines
From: Matt Kraai @ 2009-01-03 16:40 UTC (permalink / raw)
  To: demerphq; +Cc: git
In-Reply-To: <9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com>

On Sat, Jan 03, 2009 at 05:18:44PM +0100, demerphq wrote:
> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> [...]
> > -sub feature_blame {
> > -       my ($val) = git_get_project_config('blame', '--bool');
> > +sub feature_bool {
> > +       my $key = shift;
> > +       my ($val) = git_get_project_config($key, '--bool');
> >
> >        if ($val eq 'true') {
> >                return 1;
> 
> Maybe that should be:
> 
>            return ($val eq 'true');
> 
> as It is not a good idea to use 0 as a replacement for perls false, as
> the two have different behaviour.
> 
> $perl -wle'my $val=shift; my $x=$val eq "true"; print "<$_>" for $x, 0+$x' false
> <>
> <0>

I don't think Perl has *a* false value, but rather has multiple values
that are treated as false, such as undef, zero, and the empty string.
Personally, I find 0 clearer than the empty string, but that's
probably just my C bias sneaking in.

All of the boolean feature values use 0 or 1, so if this should be
changed, I think it should probably be done as a separate patch.

-- 
Matt                                                 http://ftbfs.org/

^ permalink raw reply

* Re: [PATCH] gitweb: merge boolean feature subroutines
From: demerphq @ 2009-01-03 16:51 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git
In-Reply-To: <20090103164024.GA4205@ftbfs.org>

2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> On Sat, Jan 03, 2009 at 05:18:44PM +0100, demerphq wrote:
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> [...]
>> > -sub feature_blame {
>> > -       my ($val) = git_get_project_config('blame', '--bool');
>> > +sub feature_bool {
>> > +       my $key = shift;
>> > +       my ($val) = git_get_project_config($key, '--bool');
>> >
>> >        if ($val eq 'true') {
>> >                return 1;
>>
>> Maybe that should be:
>>
>>            return ($val eq 'true');
>>
>> as It is not a good idea to use 0 as a replacement for perls false, as
>> the two have different behaviour.
>>
>> $perl -wle'my $val=shift; my $x=$val eq "true"; print "<$_>" for $x, 0+$x' false
>> <>
>> <0>
>
> I don't think Perl has *a* false value, but rather has multiple values
> that are treated as false, such as undef, zero, and the empty string.
> Personally, I find 0 clearer than the empty string, but that's
> probably just my C bias sneaking in.

Yes it definitely does have a false value, PL_sv_no, and a true value,
PL_sv_yes (although it is much less important).  It is these values
which are copied to signify true and false in the cases where the
internals need to, such as for boolean operators that must return
false, and things like negation and (in)equality checks.

It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
and the empty string in the PV (string) slot.

You can see one example of its behaviour in my previous mail, and can
see it further here:

$ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
SV = PVNV(0x952eb10) at 0x952b6f0
  REFCNT = 2147483647
  FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
  IV = 0
  NV = 0
  PV = 0x952eae8 ""\0
  CUR = 0
  LEN = 4

Compare that to:

perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
SV = IV(0x94d8398) at 0x94bd678
  REFCNT = 1
  FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
  IV = 0

> All of the boolean feature values use 0 or 1, so if this should be
> changed, I think it should probably be done as a separate patch.

As you think is best. It is after all a nit, and probably one that is
harmless. But I've been bitten by people not using the languages
native booleans before, and well, once bitten twice shy.

cheers,
Yves
-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

^ permalink raw reply

* Re: [PATCH] gitweb: merge boolean feature subroutines
From: Matt Kraai @ 2009-01-03 17:13 UTC (permalink / raw)
  To: demerphq; +Cc: git
In-Reply-To: <9b18b3110901030851t47c03d0ay75fc91b1ef2ed44b@mail.gmail.com>

On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:
> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> > I don't think Perl has *a* false value, but rather has multiple values
> > that are treated as false, such as undef, zero, and the empty string.
> > Personally, I find 0 clearer than the empty string, but that's
> > probably just my C bias sneaking in.
> 
> Yes it definitely does have a false value, PL_sv_no, and a true value,
> PL_sv_yes (although it is much less important).  It is these values
> which are copied to signify true and false in the cases where the
> internals need to, such as for boolean operators that must return
> false, and things like negation and (in)equality checks.
> 
> It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
> and the empty string in the PV (string) slot.
> 
> You can see one example of its behaviour in my previous mail, and can
> see it further here:
> 
> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
> SV = PVNV(0x952eb10) at 0x952b6f0
>   REFCNT = 2147483647
>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
>   IV = 0
>   NV = 0
>   PV = 0x952eae8 ""\0
>   CUR = 0
>   LEN = 4
> 
> Compare that to:
> 
> perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
> SV = IV(0x94d8398) at 0x94bd678
>   REFCNT = 1
>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
>   IV = 0

Wow, I had no idea about this.  Thanks for the information.

It seems like using these values would require obfuscating the code,
though.  :(  They only seem to be exposed directly via XS.

-- 
Matt                                                 http://ftbfs.org/

^ permalink raw reply

* Re: [PATCH] gitweb: merge boolean feature subroutines
From: demerphq @ 2009-01-03 17:41 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git
In-Reply-To: <20090103171333.GB4205@ftbfs.org>

2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> > I don't think Perl has *a* false value, but rather has multiple values
>> > that are treated as false, such as undef, zero, and the empty string.
>> > Personally, I find 0 clearer than the empty string, but that's
>> > probably just my C bias sneaking in.
>>
>> Yes it definitely does have a false value, PL_sv_no, and a true value,
>> PL_sv_yes (although it is much less important).  It is these values
>> which are copied to signify true and false in the cases where the
>> internals need to, such as for boolean operators that must return
>> false, and things like negation and (in)equality checks.
>>
>> It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
>> and the empty string in the PV (string) slot.
>>
>> You can see one example of its behaviour in my previous mail, and can
>> see it further here:
>>
>> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
>> SV = PVNV(0x952eb10) at 0x952b6f0
>>   REFCNT = 2147483647
>>   FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
>>   IV = 0
>>   NV = 0
>>   PV = 0x952eae8 ""\0
>>   CUR = 0
>>   LEN = 4
>>
>> Compare that to:
>>
>> perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
>> SV = IV(0x94d8398) at 0x94bd678
>>   REFCNT = 1
>>   FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
>>   IV = 0
>
> Wow, I had no idea about this.  Thanks for the information.
>
> It seems like using these values would require obfuscating the code,
> though.  :(  They only seem to be exposed directly via XS.

Depend how you look at it. You have access to them via the negation
and inequality operators. In a way you can think of the 'not' (or  !)
operator as being the "inverted boolean constructor". So for instance
the kind of surprising

  my $bool= !!$val;

can be used to get a copy of the appropriate PL_sv_yes/no. But the
need to do is rare. Luckily. :-)

But yeah, they are not exposed directly at the perl level. There is no
keyword to use to generate them like there is for undef/PL_sv_undef,
although I guess it would be fairly easy to expose them in a similar
fashion via Scalar::Util.

cheers,
Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

^ permalink raw reply

* gitweb config with some public, some basic-authenticated repos
From: Matt McCutchen @ 2009-01-03 18:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200901022033.18041.jnareb@gmail.com>

This thread's topic has moved from a proposed patch to how I should
configure my gitweb, so I'm updating the subject.  As a review: I have
several public repos and several basic-authentication realms, each of
which requires a single user and contains a single repo (some realms
might contain multiple repos in the future).  Each request has its
authorization checked by the Web server before it reaches gitweb, so my
main concern here is to avoid publicly disclosing the private repos'
paths, authors, and descriptions in the main project list.

On Fri, 2009-01-02 at 20:33 +0100, Jakub Narebski wrote: 
> On Wed, 2008-12-24, Matt McCutchen wrote:
> > On Sat, 2008-12-13 at 14:02 -0800, Jakub Narebski wrote:
> > >
> > > Cannot you do this with new $export_auth_hook gitweb configuration
> > > variable, added by Alexander Gavrilov in 
> > >    dd7f5f1 (gitweb: Add a per-repository authorization hook.)
> > > It is used in check_export_ok subroutine, and is is checked also when
> > > getting list of project from file
> > > 
> > > From gitweb/INSTALL
> [...]
> > >     For example, if you use mod_perl to run the script, and have dumb
> > >     http protocol authentication configured for your repositories, you
> > >     can use the following hook to allow access only if the user is
> > >     authorized to read the files:
> [...]
>  
> > $export_auth_hook would work, and it would have the nice (but not
> > essential) feature of including private projects in the list shown to
> > suitably authenticated users.  The only problem is that my Web host
> > doesn't support mod_perl.  Is there a practical way to accomplish the
> > same thing as the above example in a CGI script?  I would like to avoid
> > reimplementing Apache authentication-checking functionality if at all
> > possible.
> 
> I know it is written that the example code is for mod_perl, but I
> don't think it is mod_perl specific; have you checked if it works
> for you? I assume that you use Apache, and have Apache Perl bindings
> installed...

I'm quite sure that the code is mod_perl specific.  CGI scripts do get
some information from Apache via the environment, but interaction as
rich as executing Apache subrequests is only possible when the code is
running inside Apache via mod_perl.  In fact, the Apache2::SubRequest
and Apache2::RequestUtil modules are part of mod_perl.  To make sure I'm
not missing something, I tested the code on an Apache with mod_perl
enabled but gitweb executing as a CGI, and gitweb failed with the
following message:

        Can't locate object method "request" via package
        "Apache2::RequestUtil" at gitweb_config.perl line 60.

So this approach won't work for me.

But even ignoring this problem, I'm now thinking that trying to show
repos from *multiple authentication realms* in the main list according
to the user's credentials was a foolish idea.  I don't want to ask
anonymous visitors to my main list for multiple logins they probably
don't have, yet I think it would be poor practice from a predictability
standpoint for the list to behave differently if the user volunteers
login information that hasn't been requested.

Instead, I will use a separate project list file for public repositories
and for each realm, and no export_auth_hook.  This is simple and
requires no change to gitweb; my rewrite rule just has to tell my
gitweb_config via an environment variable which list to use.  Comments
on this solution?

(Note: I'm no longer advocating the hidden-repos feature at this time,
but I think I will still advocate the forks-and-strict-export bug fix
now that I have it written.)

-- 
Matt

^ permalink raw reply

* Re: git checkout does not warn about tags without corresponding commits
From: Daniel Barkalow @ 2009-01-03 19:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Henrik Austad, git
In-Reply-To: <7vr63k8vvf.fsf@gitster.siamese.dyndns.org>

On Sat, 3 Jan 2009, Junio C Hamano wrote:

> Henrik Austad <henrik@austad.us> writes:
> 
> > On Friday 02 January 2009 22:44:50 Junio C Hamano wrote:
> >> Henrik Austad <henrik@austad.us> writes:
> >> > I recently tried to do a checkout of (what I thought was the first) inux
> >> > kernel in the linux git repo.
> >> >
> >> > git checkout -b 2.6.11 v2.6.11
> >>
> >> This should have barfed, and indeed I think it is a regression around
> >> v1.5.5.  v1.5.4 and older git definitely fails to check out a tree object
> >> like that.
> >
> > You're right, I bisected it down to commit 
> > 782c2d65c24066a5d83453efb52763bc34c10f81
> 
> I am not surprised.
> 
> That one discarded an implementation of "git checkout" in Bourne shell,
> with a complete reimplementation in C.
> 
> I haven't looked at the code very closely, but I think this should fix
> it.  Thorough reviewing (not just running the test suite) is much
> appreciated.

That looks right to me.

Acked-by: Daniel Barkalow <barkalow@iabervon.org>

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Managing websites with git
From: Todd A. Jacobs @ 2009-01-03 21:29 UTC (permalink / raw)
  To: git
In-Reply-To: <fe5a74300811300830x850d81csc5cf1f9b367bac11@mail.gmail.com>

On Sun, Nov 30, 2008 at 05:30:30PM +0100, Felix Andersen wrote:

> I was thinking about any security issues with the .git dir being
> hosted. Or is that even the right way to do it?

With Apache, you can add the following to your httpd.conf file, or to an
.htaccess file within your document root:

    <DirectoryMatch "^\.git">
	Order allow,deny
	Deny from all
    </DirectoryMatch>
    <FilesMatch "^\.gitignore">
	Order allow,deny
	Deny from all
    </FilesMatch>

to prevent web access to the respository.

-- 
"Oh, look: rocks!"
	-- Doctor Who, "Destiny of the Daleks"

^ permalink raw reply

* Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X
From: Junio C Hamano @ 2009-01-03 22:01 UTC (permalink / raw)
  To: Marcel M. Cary; +Cc: git, jnareb, ae, j.sixt
In-Reply-To: <1230649824-1893-1-git-send-email-marcel@oak.homeunix.org>

"Marcel M. Cary" <marcel@oak.homeunix.org> writes:

> I sent the first rev of this patch to just Brian.  It didn't have
> either of the unit test changes.  He said it fixed all but t2300.3,
> where cd_to_toplevel doesn't actually "cd", so I made the same change
> to the unit test itself.  Can someone with OS X try running the test
> suite with v2 of this patch?  I don't have OS X readily available.

I think I saw a success report on the list.  Care to resend it with
Sign-off (by you) and

	Tested-by: tester <test@er.xz> (on PLATFORM)

lines as you see necessary for application?

Thanks.

^ permalink raw reply

* [EGIT PATCH] Allow non-ASCII ref names when writing packs
From: Robin Rosenberg @ 2009-01-03 22:04 UTC (permalink / raw)
  To: spearce; +Cc: git, Robin Rosenberg

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/transport/PacketLineOut.java  |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Disovered this bug when trying to clone a local repo with some "funny" 
ref names. C Git handles them so jgit should too.

-- robin

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
index aae4be5..cb6d89a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
@@ -54,7 +54,7 @@ PacketLineOut(final OutputStream i) {
 	}
 
 	void writeString(final String s) throws IOException {
-		writePacket(Constants.encodeASCII(s));
+		writePacket(Constants.encode(s));
 	}
 
 	void writePacket(final byte[] packet) throws IOException {
-- 
1.6.1.rc3.56.gd0306

^ permalink raw reply related

* Re: [EGIT PATCH] Allow non-ASCII ref names when writing packs
From: Shawn O. Pearce @ 2009-01-03 22:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1231020275-7637-1-git-send-email-robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  .../org/spearce/jgit/transport/PacketLineOut.java  |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Disovered this bug when trying to clone a local repo with some "funny" 
> ref names. C Git handles them so jgit should too.

Oy.  Good catch.
 
-- 
Shawn.

^ permalink raw reply

* Re: [JGIT PATCH] Improve the sideband progress scaper to be more accurate
From: Robin Rosenberg @ 2009-01-03 22:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081231190401.GI29071@spearce.org>

onsdag 31 december 2008 20:04:01 skrev Shawn O. Pearce:
> By matching only whole lines we should be able to improve the
> progress scaper so we avoid ugly output like we had been seeing:
> 
>   EGIT.contrib/jgit clone git://repo.or.cz/libgit2.git LIBGIT2
>   Initialized empty Git repository in /home/me/SW/LIBGIT2/.git
>   Counting objects:       547
>   Compressing objects:    100% (192/192)
>   ts:                     100% (192/192)
>   Compressing objects:    100% (192/192)
>   ng objects:             100% (192/192)
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
>  > Would it be hard to get the progress look better?
> 
>  Maybe this does the trick.  Its hard to reproduce so its hard to
>  come up with the condition that was giving us the problem before.
>  I suspect its because we were getting line fragments on the sideband
>  channel, but I'm not sure that was really the case.

Nasty. I couldn't reproduce it myself. I'll hold onto this one for a while and
see if I'll get the opportunity to test it live with this problem.

-- robin

^ permalink raw reply

* Re: [PATCH] cvsserver: change generation of CVS author names
From: Junio C Hamano @ 2009-01-03 22:14 UTC (permalink / raw)
  To: Fabian Emmes; +Cc: git, Lars Noschinski, Frank Lichtenheld, Martin Langhoff
In-Reply-To: <1230910814-32307-2-git-send-email-fabian.emmes@rwth-aachen.de>

Fabian Emmes <fabian.emmes@rwth-aachen.de> writes:

> CVS username is generated from local part email address.
> We take the whole local part but restrict the character set to the
> Portable Filename Character Set, which is used for Unix login names
> according to Single Unix Specification v3.
>
> Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>

Stating "we should have done this from day one" is one thing (even though
"because some standard says so" is not particularly a good justification
without "and matches the way people use CVS in the real world in practice"
appended to it).

"We should suddenly change the behaviour" is quite a different thing and
it depends on what follows that sentence if the change is justifiable.  We
do not want to hear "...; screw the existing repositories if they have
nonconforming names.".  It is Ok if it is "...; existing repositories will
be affected, but the damage is limited to very minor set of operations,
namely X, Y and Z".

In other words, is there any backward compatibility issue when a
repository that has served existing CVS users and checkouts with older
version switches to the patched one?  If there is one, is that grave
enough that we should care?

>  git-cvsserver.perl |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index cbcaeb4..fef7faf 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -2533,12 +2533,18 @@ sub open_blob_or_die
>      return $fh;
>  }
>  
> -# Generate a CVS author name from Git author information, by taking
> -# the first eight characters of the user part of the email address.
> +# Generate a CVS author name from Git author information, by taking the local
> +# part of the email address and replacing characters not in the Portable
> +# Filename Character Set (see IEEE Std 1003.1-2001, 3.276) by underscores. CVS
> +# Login names are Unix login names, which should be restricted to this
> +# character set.
>  sub cvs_author
>  {
>      my $author_line = shift;
> -    (my $author) = $author_line =~ /<([^>@]{1,8})/;
> +    (my $author) = $author_line =~ /<([^@>]*)/;
> +
> +    $author =~ s/[^-a-zA-Z0-9_.]/_/g;
> +    $author =~ s/^-/_/;
>  
>      $author;
>  }

^ permalink raw reply

* Wishing you the Best New Year!
From: Joseph @ 2009-01-03 23:00 UTC (permalink / raw)
  To: git

Joseph mailed a New Year greeting card.
Click the above link to view the page! If you can't click it, copy and paste it 
into your browser: http://bestyearcard.com/?ID=ba49b43e98ec608b3e653aa082186
Copyright (c) HallmarkCards All Rights Reserved

^ permalink raw reply

* Re: What about allowing multiple hooks?
From: Alexander Potashev @ 2009-01-03 23:32 UTC (permalink / raw)
  To: Marc Weber, git; +Cc: Rogan Dawes, martin f krafft
In-Reply-To: <20081121133828.GB5912@gmx.de>

On 14:38 Fri 21 Nov     , Marc Weber wrote:
> Use case:
> 
> I've been reading parts of the topGit code. And it does make for it to
> add its own checks. However having to change the existing scripts
> insterting a call to the tg hooks isn't the best way.
> Why? one is using #/bin/sh the next is using #/bin/ruby maybe..
> 
> So what about allowing (or even enforcing) ths directory layout?
> 
> .git/hooks/pre-commit/hook1.sh
> .git/hooks/pre-commit/hook2.sh
> .git/hooks/pre-commit/topGitcheck.sh
> 
> instead of
> .git/hooks/pre-commit # <- the one and only pre-commit hook
> 
> so that all can be run in squence?

If we have a single hook, git just runs a script. But multiple scripts
can be run in different orders. We can assume that git should run them
in lexicographical order, but sometimes it's not the best order can be
used.

However, prefixes can be used to force a particular lexicographical
order:
	.git/hooks/pre-commit/01-hook2.sh
	.git/hooks/pre-commit/02-topGitcheck.sh
	.git/hooks/pre-commit/03-hook1.sh

Is there a better way to choose the scripts order?

> 
> This way you can keep the original git sample files and update them
> while adding you very own checks more easily.
> 
> But maybe this isn't the best choice either and the way to go is
> 
> .git/hooks/list-of-hook-directories # eg containing ".git/hooks/samples\n.git/hooks/topgit" ?
> 
> .git/hooks/sample/<all the sample hook files>
> .git/hooks/topgit/pro-commit
> 
> ?
> 
> Then you can actually link in your own personal check script directories
> easily *and* you can add them to the repository eg by using
> comitted-repo-hooks instead of .git/hooks
> ?
> This way you could provide different hook directories for different
> platforms and all you have to do is enabling them by adding the path to
> .git/list-of-hook-directories ?
> 
> I guess the second approach of defining kind of overlays is better
> because it doesn't interfer with the existiing scheme?
> Maybe it should be implemented as git config option instead of a file
> containing the list of directories?
> 
> The hook direcotry list apporach is better because you've more control
> about order of execution..
> 
> Thoughts?
> 
> Marc Weber

^ permalink raw reply

* Re: [JGIT PATCH 13/13] Add basic git daemon support to publish receive-pack
From: Robin Rosenberg @ 2009-01-03 23:48 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1229992043-1053-14-git-send-email-spearce@spearce.org>

tisdag 23 december 2008 01:27:23 skrev Shawn O. Pearce:
> +	private static final Pattern SAFE_REPOSITORY_NAME = Pattern
> +			.compile("^[A-Za-z][A-Za-z0-9/_ -]+(\\.git)?$");

This restriction is too strict. Wouldn't any path not containing ".." be valid? In particular this did not work with my "EGIT.contrib" repo. I
have a lot of repos with names llike "name.purpose". Just adding
'.' to the character set isn't really enough.

-- robin

^ permalink raw reply

* JGit too strict?
From: Robin Rosenberg @ 2009-01-04  0:23 UTC (permalink / raw)
  To: Shawn O. Pearce, git

Is this repo corrupt or is jgit too strict? C Git doesn't complain.

-- robin

EGIT.contrib/jgit clone  git://github.com/pheew/dotgit DOTGIT
Initialized empty Git repository in /home/me/SW/DOTGIT/.git                    
Counting objects:       856
Compressing objects:    100% (352/352)
Receiving objects:      100% (856/856)
Resolving deltas:       100% (554/554)
From git://github.com/pheew/dotgit
 * [new branch]      alt-zlib   -> origin/alt-zlib
 * [new branch]      gh-pages   -> origin/gh-pages
 * [new branch]      master     -> origin/master
Exception in thread "main" java.lang.Error: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000
        at org.spearce.jgit.lib.TreeIterator.<init>(TreeIterator.java:118)
        at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:182)
        at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:169)
        at org.spearce.jgit.lib.TreeIterator.next(TreeIterator.java:125)
        at org.spearce.jgit.lib.IndexTreeWalker.walk(IndexTreeWalker.java:131)
        at org.spearce.jgit.lib.IndexTreeWalker.walk(IndexTreeWalker.java:107)
        at org.spearce.jgit.lib.WorkDirCheckout.prescanOneTree(WorkDirCheckout.java:208)
        at org.spearce.jgit.lib.WorkDirCheckout.checkout(WorkDirCheckout.java:125)
        at org.spearce.jgit.pgm.Clone.doCheckout(Clone.java:174)
        at org.spearce.jgit.pgm.Clone.run(Clone.java:111)
        at org.spearce.jgit.pgm.TextBuiltin.execute(TextBuiltin.java:131)
        at org.spearce.jgit.pgm.Main.execute(Main.java:159)
        at org.spearce.jgit.pgm.Main.main(Main.java:84)
Caused by: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000
        at org.spearce.jgit.lib.Tree.readTree(Tree.java:584)
        at org.spearce.jgit.lib.Tree.ensureLoaded(Tree.java:528)
        at org.spearce.jgit.lib.Tree.memberCount(Tree.java:394)
        at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:179)
        at org.spearce.jgit.lib.TreeIterator.<init>(TreeIterator.java:116)
        ... 12 more

^ permalink raw reply

* Re: JGit too strict?
From: Robin Rosenberg @ 2009-01-04  0:29 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <200901040123.30392.robin.rosenberg@dewire.com>

söndag 04 januari 2009 01:23:30 skrev Robin Rosenberg:
> Is this repo corrupt or is jgit too strict? C Git doesn't complain.

Forget it.. It's a gitlink, which jgit doesn't support yet.

-- robin

^ permalink raw reply

* Re: JGit too strict?
From: Junio C Hamano @ 2009-01-04  1:43 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Shawn O. Pearce, git
In-Reply-To: <200901040123.30392.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> writes:

> Is this repo corrupt or is jgit too strict? C Git doesn't complain.
>
> -- robin
>
> EGIT.contrib/jgit clone  git://github.com/pheew/dotgit DOTGIT
> Initialized empty Git repository in /home/me/SW/DOTGIT/.git                    

> Exception in thread "main" java.lang.Error: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000

Lack of support of submodules, I would guess.

^ permalink raw reply

* Re: git-branch --print-current
From: Karl Chen @ 2009-01-04  2:18 UTC (permalink / raw)
  To: David Aguilar; +Cc: Git mailing list
In-Reply-To: <402731c90901012026j470f35ffj1eaa189a837054f3@mail.gmail.com>

>>>>> On 2009-01-01 20:26 PST, David Aguilar writes:

    David> You might want to use 'git symbolic-ref' instead.

    David> $ git symbolic-ref HEAD | sed -e 's,refs/heads/,,'
    David> master

Thanks, that is better.

How about an option to git-symbolic-ref that gets rid of the
refs/heads/ ?

^ permalink raw reply

* Re: git-branch --print-current
From: Miklos Vajna @ 2009-01-04  3:38 UTC (permalink / raw)
  To: Karl Chen; +Cc: David Aguilar, Git mailing list
In-Reply-To: <quack.20090103T1818.lth7i5bg6f7@roar.cs.berkeley.edu>

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

On Sat, Jan 03, 2009 at 06:18:36PM -0800, Karl Chen <quarl@cs.berkeley.edu> wrote:
> How about an option to git-symbolic-ref that gets rid of the
> refs/heads/ ?

Make an alias?

git config alias.cb '!sh -c "git symbolic-ref HEAD|sed s,refs/heads/,,"'

$ git cb
master

(Where cb can stand for 'current branch', for example.)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: git-branch --print-current
From: Karl Chen @ 2009-01-04  4:26 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: David Aguilar, Git mailing list
In-Reply-To: <20090104033839.GD21154@genesis.frugalware.org>

>>>>> On 2009-01-03 19:38 PST, Miklos Vajna writes:

    Miklos> On Sat, Jan 03, 2009 at 06:18:36PM -0800, Karl Chen <quarl@cs.berkeley.edu> wrote:
    >> How about an option to git-symbolic-ref that gets rid of
    >> the refs/heads/ ?

    Miklos> Make an alias?

Thanks for the suggestion.  I don't have any problems making
aliases or using git-branch for interactive output; it's not an
issue of typing less.

I guess the broader point is that people use these "porcelain"
commands in scripts and parse their output even when they
shouldn't, and it's better to take action to prevent that.  This
reminds me of the issue of debugfs supposedly not being an ABI but
people rely on anyway since it's stable enough - people are
starting to rely on 'git branch' output just to print the current
branch name.  Better to create or at least publicly point out a
good alternative to nip this in the bud.

I suppose "user education" in the form of a big warning in the
git-branch man page would also help.  How do you even tell in the
man page whether a command is porcelain or not?  Still, I think
something like this is worth making slightly easier.  Another
minor argument for something like git branch --print-name is that
it's annoying to check the exit code inside a pipeline.

For example: Google for how to add the name of the git branch to
the bash prompt and you'll find countless examples of people using
git-branch.  And they're all different, so people aren't just
blindly copying one guy; here is a small sample:
     
    export PS1='...`git branch 2> /dev/null | grep -e ^* | sed
    -E  s/^\\\\\*\ \(.+\)$/\(\\\\\1\)\
     
    $(git branch &>/dev/null; if [ $? -eq 0 ]; then echo "
    ($(git branch | grep '^*' |sed s/\*\ //))"; fi)
     
    `ruby -e \"print (%x{git branch 2>
    /dev/null}.grep(/^\*/).first || '').gsub(/^\* (.+)$/, '(\1)
    ')\"\`
     
    parse_git_branch() {
      git branch 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/(\1)/'
    }
     
    `git branch 2>/dev/null|cut -f2 -d\* -s`
     
    git branch --no-color 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/(\1)/'
     
    git_branch=`git branch 2>/dev/null | grep -e '^*' | sed -E 's/^\* (.+)$/(\1) /'`
     
There were a few using git-symbolic-ref but most used git-branch.

^ permalink raw reply

* Re: git-branch --print-current
From: Junio C Hamano @ 2009-01-04  5:17 UTC (permalink / raw)
  To: Karl Chen; +Cc: Miklos Vajna, David Aguilar, Git mailing list
In-Reply-To: <quack.20090103T2026.lth3afzg0hx@roar.cs.berkeley.edu>

Karl Chen <quarl@cs.berkeley.edu> writes:

> For example: Google for how to add the name of the git branch to
> the bash prompt and you'll find countless examples of people using
> git-branch.  And they're all different, so people aren't just
> blindly copying one guy; here is a small sample:
> ...
> There were a few using git-symbolic-ref but most used git-branch.

That is a good point about user education, and is a demonstration why a
new option to cover a very narrow-special case to symbolic-ref will not
help the situation.  People will add their own embellishments around the
name of the branch anyway, and the most generic symbolic-ref output is
just as useful as a special case option to show without refs/heads/.

What you quoted are all inferior implementations of showing the name of
the current branch in the bash prompt.  The most correct way (in the sense
that it won't be broken in future git) is always found in the bash
completion script in contrib/completion/git-completion.bash and it reads:

    PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '

You can of course change this to suit your taste.  For example, here is a
variant I personally use:

    PS1=': \h \W$(__git_ps1 "/%s"); '

The point is that __git_ps1 shell function is defined to be used for this
exact purpose and is documented in the completion script.

Besides showing the current branch, it knows how to interpret the various
state clues git operations leave in the repository and the work tree, and
reminds them what you are in the middle of (e.g. applying patch series
using "git am", rebasing interactively, resolving conflicts after a merge
did not autoresolve, etc.), and also knows how to show the detached HEAD.

^ permalink raw reply

* Re: [PATCH] gitweb: merge boolean feature subroutines
From: Junio C Hamano @ 2009-01-04  5:30 UTC (permalink / raw)
  To: demerphq; +Cc: Matt Kraai, git
In-Reply-To: <9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com>

demerphq <demerphq@gmail.com> writes:

> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> [...]
>> -sub feature_blame {
>> -       my ($val) = git_get_project_config('blame', '--bool');
>> +sub feature_bool {
>> +       my $key = shift;
>> +       my ($val) = git_get_project_config($key, '--bool');
>>
>>        if ($val eq 'true') {
>>                return 1;
>
> Maybe that should be:
>
>            return ($val eq 'true');
>
> as It is not a good idea to use 0 as a replacement for perls false, as
> the two have different behaviour.

I'd rather want to keep our scripts free of deep Perl magic.  Even if
there are SVs that are interpreted as false other than 0, that does not
necessarily mean you have to fear that 0 can sometimes evaluate to true.

As long as you refrain from doing something crazy like "0 but true",
people who are not (and/or are not inclined to become) familiar with the
gory innards of Perl can rely on 0 being false and 1 being true when
calling feature_something subs, no?

^ permalink raw reply

* Re: [RFC/PATCH] git-web--browse: don't add start as candidate on Ubuntu
From: Christian Couder @ 2009-01-04  7:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Ramsay Jones, Junio C Hamano, GIT Mailing-list
In-Reply-To: <495D3964.6040006@ramsay1.demon.co.uk>

Le jeudi 1 janvier 2009, Ramsay Jones a écrit :
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi *,
>
> When upgrading to v1.6.1, I noticed that the html help had stopped
> working on Linux (Ububtu), viz:
>
>     $ git help -w tag
>     start: Need to be root
>
> So, after squinting at git-web--browse.sh, I tried a few things:
>
>     $ ls /bin/start
>     ls: /bin/start: No such file or directory
>     $ test -n /bin/start; echo $?
>     0
>     $ which start
>     /sbin/start
>     $ start fred
>     start: Need to be root
>     $ ls -l /sbin/start
>     lrwxrwxrwx 1 root root 7 2007-06-24 19:45 /sbin/start -> initctl*
>
> So, it would seem that /sbin/start is part of upstart, which would
> explain the "Need to be root" ;-)
>
>     $ test -x /bin/start; echo $?
>     1
>     $
>
> So, the patch below fixes the issue for me, but as I don't have MinGW
> installed, I can't test this fix works there.
>
> Does anybody else see this issue and can someone test the patch?

Petr, as you added support for using /bin/start on MinGW, could you test?

Thanks,
Christian.

^ permalink raw reply

* Re: [RFC/PATCH] git-web--browse: don't add start as candidate on Ubuntu
From: Junio C Hamano @ 2009-01-04  7:53 UTC (permalink / raw)
  To: Christian Couder; +Cc: Petr Baudis, Ramsay Jones, GIT Mailing-list
In-Reply-To: <200901040833.25849.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Le jeudi 1 janvier 2009, Ramsay Jones a écrit :
> ...
>> Does anybody else see this issue and can someone test the patch?
>
> Petr, as you added support for using /bin/start on MinGW, could you test?
>
> Thanks,
> Christian.

> diff --git a/git-web--browse.sh b/git-web--browse.sh
> index 78d236b..7ed0fad 100755
> --- a/git-web--browse.sh
> +++ b/git-web--browse.sh
> @@ -115,7 +115,7 @@ if test -z "$browser" ; then
>  	browser_candidates="open $browser_candidates"
>      fi
>      # /bin/start indicates MinGW
> -    if test -n /bin/start; then
> +    if test -x /bin/start; then
>  	browser_candidates="start $browser_candidates"
>      fi

In any case, the original test is simply bogus.  'test -n "$foo"' is to
see if $foo is an empty string, and giving a constant /bin/start always
yields true.

As an old timer, I tend to prefer "test -f" in this context, as anybody
sane can expect that the directory /bin will contain executables and
nothing else.  Another reason is purely stylistic.  Historically "-f" has
been much more portable than "-x" (which came from SVID), even though it
wouldn't make much difference in practice in the real world these days.

^ 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