Git development
 help / color / mirror / Atom feed
* Re: git svn with non-standard svn layout
From: Eric Wong @ 2009-10-14  6:33 UTC (permalink / raw)
  To: Fabian Molder; +Cc: git
In-Reply-To: <loom.20091011T205226-197@post.gmane.org>

Fabian Molder <fm122@arcor.de> wrote:
> Eric Wong <normalperson <at> yhbt.net> writes:
> > Hi Fabian,
> > 
> > Since you don't want to track the entire repo and these seem like
> > unrelated (history-wise) trees, you probably want the simplest cases:
> > 
> >   git svn clone svn://example.com/path/to/xapplication2
> >   git svn clone svn://example.com/path/to/aa/bb/cc/xapplication1
> > 
> > These commands are like doing the following with plain old svn:
> > 
> >   svn co svn://example.com/path/to/xapplication2
> >   svn co svn://example.com/path/to/aa/bb/cc/xapplication1
> > 
> > > I tried to use "git config svn-remote.svn.branches" to do this,
> > >  please see in function "do_git_svn" in bash-script - but no success
> > 
> > svn-remote.svn.branches and tags are really only for repos with
> > standard layouts.
> > 
> 
> Hello Eric,
>   hmm, understand,
>   but this just does an checkout to the working dir
> 
>   the reason for using git is:
>     - work offline, with (at least read) access to all the svn branches
>     - have some more (privat, not commit back to svn) branches for experiments
>     - all the nice git stuff ..

Sorry, not sure if I understood you the first time.  The pastie was
hard to read on my screen (big fonts, low res).

I think what you need is to match the number of globs (*) on
both the remote and local sides:

  REMOTE=branches/*/*/aa/bb/cc/zapplication1
  LOCAL=refs/remotes/svn/aa/bb/cc/zapplication1/*/*
  git config svn-remote.svn.branches "${REMOTE}:${LOCAL}"

  #---> matches all in svn repo and also creates subdirs
#
#  git config svn-remote.svn.branches \
#  "branches/r*/development/aa/bb/cc/zapplication1:refs/remotes/svn/branches/*"
#
  #--> Missing trailing '/' on left side of: 'branches/r*/development/aa/bb/cc/zapplication1' (branches/r)

The glob code is still a bit wonky, but it needs to be "/*/" (that is
"*" must have a "/" or nothing, but not "/r*/"

But it looks like the example with 2 globs works.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Bert Wesarg @ 2009-10-14  6:49 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <1255495525-11254-1-git-send-email-sam.vilain@catalyst.net.nz>

On Wed, Oct 14, 2009 at 06:45, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 06e0f31..8dc3fbf 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -143,21 +143,31 @@ pre-rebase
>  This hook is called by 'git-rebase' and can be used to prevent a branch
>  from getting rebased.
>
> +pre-checkout
> +-----------
>
> -post-checkout
> -~~~~~~~~~~~~~
> -
Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the
>  worktree.  The hook is given three parameters: the ref of the previous HEAD,
>  the ref of the new HEAD (which may or may not have changed), and a flag
>  indicating whether the checkout was a branch checkout (changing branches,
>  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> -This hook cannot affect the outcome of 'git-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.
>
> +This hook can be used to perform any clean-up deemed necessary before
> +checking out the new branch/files.
> +
> +post-checkout
> +-----------
Ditto.

> +
> +This hook is invoked when a 'git-checkout' is run after having updated the
> +worktree.  It takes the same arguments as the 'pre-checkout' hook.
> +This hook cannot affect the outcome of 'git-checkout'.
> +
>  This hook can be used to perform repository validity checks, auto-display
>  differences from the previous HEAD if different, or set working dir metadata
>  properties.

Bert

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Sam Vilain @ 2009-10-14  6:50 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git, elliot
In-Reply-To: <36ca99e90910132349o25322021l266124bd8b0d30b3@mail.gmail.com>

Bert Wesarg wrote:
>> +pre-checkout
>> +-----------
>>
>> -post-checkout
>> -~~~~~~~~~~~~~
>> -
> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

Possibly the headings changed from the version I originally patched to
the current 'master'.  More likely, I just made a simple mistake :-}
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Bert Wesarg @ 2009-10-14  7:04 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <4AD574CA.8010900@catalyst.net.nz>

On Wed, Oct 14, 2009 at 08:50, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> Bert Wesarg wrote:
>>> +pre-checkout
>>> +-----------
>>>
>>> -post-checkout
>>> -~~~~~~~~~~~~~
>>> -
>> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?
>
> Possibly the headings changed from the version I originally patched to
> the current 'master'.
That was me one month ago ;-)

^ permalink raw reply

* Re: Bad URL passed to RA lay
From: Eric Wong @ 2009-10-14  7:07 UTC (permalink / raw)
  To: m.skoric; +Cc: git
In-Reply-To: <1952825702@web.de>

m.skoric@web.de wrote:
> Hi List,
> 
> i have a problem with git-svn clone / fetch. I get following error
> while doing one of previous command -> "Bad URL passed to RA lay" This
> happens because a branch doesn't exists in svn anymore and git wants
> to retrieve data from it. Here is the complete error message
> 
> Initializing parent: Abo-Uebernahme (Bug #994)@341
> Found possible branch point: "quoted"..trunk => "quoted"...Abo-Uebernahme (Bug #994), 203
> Found branch parent: (Abo-Uebernahme (Bug #994)@341) bb831869748c98bf97d105c5894ae65331c95c08
> Bad URL passed to RA layer: Malformed URL for repository at /usr/bin/git-svn line 4311
> 
> git version 1.6.3.3
> 
> Aynone else has this Problem?

Hi,

Unlikely, not many people use URIs as weird as yours :)
The existing test case (t9118) we have was also inspired by you,
on the same branch, even.

> Can anyone help me?

What exactly is the "quoted" you refer to?  That's not an actual branch
name, is it?

Can you try it with v1.6.5?  You might need to edit your $GIT_CONFIG,
but commit 5268f9edc3c86b07a64fcc2679e5ffe39be28d97 was the last
fix for URI-escaping problems:

  Author: Eric Wong <normalperson@yhbt.net>
  Date:   Sun Aug 16 14:22:12 2009 -0700

    svn: assume URLs from the command-line are URI-encoded
    
    And then unescape them when writing to $GIT_CONFIG.
    
    SVN has different rules for repository URLs (usually the root)
    and for paths within that repository (below the HTTP layer).
    Thus, for the request URI path at the HTTP level, the URI needs
    to be encoded.  However, in the body of the HTTP request (the
    with underlying SVN XML protocol), those paths should not be
    URI-encoded[1].  For non-HTTP(S) requests, SVN appears to be
    more flexible and will except weird characters in the URL as
    well as URI-encoded ones.
    
    Since users are used to using URLs being entirely URI-encoded,
    git svn will now attempt to unescape the path portion of URLs
    while leaving the actual repository URL untouched.
    
    This change will be reflected in newly-created $GIT_CONFIG files
    only.  This allows users to switch between svn(+ssh)://, file://
    and http(s):// urls without changing the fetch/branches/tags
    config keys.  This won't affect existing imports at all (since
    things didn't work before this commit anyways), and will allow
    users to force escaping into repository paths that look like
    they're escaped (but are not).
    
    Thanks to Mike Smullin for the original bug report and Björn
    Steinbrink for summarizing it into testable cases for me.
    
    [1] Except when committing copies/renames, see
        commit 29633bb91c7bcff31ff3bb59378709e3e3ef627d


And if git v1.6.5 doesn't work, would you mind looking at (and possibly
modifying) the t9118 test in git to reproduce since it's already got your
oddly-named branch in it?   Thanks!

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] change throughput display units with fast links
From: Johannes Sixt @ 2009-10-14  7:07 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0910132256280.20122@xanadu.home>

Nicolas Pitre schrieb:
> +	if (rate > 1 << 10) {
> +		int x = rate + 5;  /* for rounding */
> +		snprintf(tp->display + sizeof(tp->display) - l, l,
> +			 " | %u.%2.2u MiB/s",

Shouldn't the fractional part be "%02.2u" (leading zeros instead of blanks)?

> +			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> +	} else if (rate)
>  		snprintf(tp->display + sizeof(tp->display) - l, l,
>  			 " | %u KiB/s", rate);
>  }

-- Hannes

^ permalink raw reply

* Re: [PATCH] gitweb: fix esc_param
From: Jakub Narebski @ 2009-10-14  8:23 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Stephen Boyd, Junio C Hamano
In-Reply-To: <1255463496-21617-1-git-send-email-giuseppe.bilotta@gmail.com>

On Tue, 13 Oct 2009, Giuseppe Bilotta wrote:

> The custom CGI escaping done in esc_param failed to escape UTF-8
> properly. Fix by using CGI::escape on each sequence of matched
> characters instead of sprintf()ing a custom escaping for each byte.

Hmmm... I wonder if this bug isn't caused by failing to mark some
input as utf8 using to_utf8() subroutine... or by using 
binmode $fd, ':utf8' on $fd from opening git-rev-list, after ensuring
that it outputs utf8 by --encoding=utf8 (or is it only git-log that
accepts that option?).

> 
> Additionally, the space -> + escape was being escaped due to greedy
> matching on the first substitution. Fix by adding space to the
> list of characters not handled on the first substitution.

Thanks.

> 
> Finally, remove an unnecessary escaping of the + sign.

Signoff?

> ---
>  gitweb/gitweb.perl |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> The issues with this routine were exposed by Stephen's
> "author as search link" patch. This should fix them.
> 
> Since the idea of esc_param is to replicate CGI::escape except for the /
> character (if I read the comment correclty), a possible alternative
> would be to just use CGI::escape on the whole string and then undo the
> escaping for the / character.

Well, that and widely used but non-standard (well, not using percent
encoding) escaping of space with '+'; CGI::escape(" ") is %20, not '+'.
Se we would have to turn '%2F' into '/', and '%20' into '+'.

> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6237865..6593e5c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1115,8 +1115,7 @@ sub to_utf8 {
>  # correct, but quoted slashes look too horrible in bookmarks
>  sub esc_param {
>  	my $str = shift;
> -	$str =~ s/([^A-Za-z0-9\-_.~()\/:@])/sprintf("%%%02X", ord($1))/eg;
> -	$str =~ s/\+/%2B/g;
> +	$str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
>  	$str =~ s/ /\+/g;
>  	return $str;
>  }
> -- 
> 1.6.3.rc1.192.gdbfcb
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] gitweb: fix esc_param
From: Jakub Narebski @ 2009-10-14  9:03 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Giuseppe Bilotta, git, Junio C Hamano
In-Reply-To: <4AD56FD9.70602@gmail.com>

On Wed, 14 Oct 2009, Stephen Boyd wrote:
> Giuseppe Bilotta wrote:
>> On Wed, Oct 14, 2009 at 3:13 AM, Stephen Boyd <bebarino@gmail.com> wrote:
>>
>>> This works great for my purposes. Thanks.
>>>
>> Can you also check if this fixes the branch name issue you mentioned
>> in the other branch? (And/or do you have a repository exposing the
>> problem if not?)
> 
> (We're jumping back and forth between threads haha)
> 
> Sorry, it doesn't. But this diff fixes the first part of the problem.
> There are still problems with the RSS/Atom feed names being mangled. The
> branch name I'm using is gitwéb, but I imagine any utf8 character will fail.

That it is (probably) not because of lack of esc_html, but because
of lack of to_utf8.  Grrr... we really should convert to utf8 (usually
just mark as utf8) on reading input, to avoid such kind of errors.

> 
> I see the title and the actual text being mangled without this patch.
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4b21ad2..910c370 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1500,9 +1500,10 @@ sub format_ref_marker {
>                                 -href => href(
>                                         action=>$dest_action,
>                                         hash=>$dest
> -                               )}, $name);
> +                               )}, esc_html($name));

Hmmm... to_utf8 would be enough here, but for the fact that 
git-check-ref-format doesn't prohibit '<', '>' in ref names, 
so it is possible that somebody somewhere is using such strange
ref names (e.g. branch named '<b>' is valid branch name).

>  
> -                       $markers .= " <span class=\"$class\" title=\"$ref\">" .
> +                       my $title_ref = esc_html($ref);
> +                       $markers .= " <span class=\"$class\" title=\"$title_ref\">" .

Here it would be really useful to have esc_attr (which would do to_utf8
plus escaping of '"' and '%', quote and escape characters), although
I guess that full HTML escaping done with esc_html wouldn't do anything
bad.

>                                 $link . "</span>";
>                 }
>         }

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] change throughput display units with fast links
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nicolas Pitre, git
In-Reply-To: <4AD578BD.7080201@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Nicolas Pitre schrieb:
>> +	if (rate > 1 << 10) {
>> +		int x = rate + 5;  /* for rounding */
>> +		snprintf(tp->display + sizeof(tp->display) - l, l,
>> +			 " | %u.%2.2u MiB/s",
>
> Shouldn't the fractional part be "%02.2u" (leading zeros instead of blanks)?
>
>> +			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
>> +	} else if (rate)
>>  		snprintf(tp->display + sizeof(tp->display) - l, l,
>>  			 " | %u KiB/s", rate);
>>  }
>
> -- Hannes

Judging from the surrounding existing code, I do not think so.

    $ cat <<\EOF >j.c
    #include <stdio.h>

    int main(int ac, char **av)
    {
            printf("%u.%2.2u\n", 4, 5);
            return 0;
    }
    EOF
    $ gcc -o j j.c && ./j
    4.05

^ permalink raw reply

* Re: Efficient cloning from svn (with multiple branches/tags   subdirs)
From: Bruno Harbulot @ 2009-10-14  9:07 UTC (permalink / raw)
  To: git
In-Reply-To: <20091014060307.GA17178@dcvr.yhbt.net>

Hi Eric,

Eric Wong wrote:
> Hi Bruno,
> 
> That looks like there's two levels of tags.  You should be able to do
> this with your version of git in $GIT_CONFIG:
> 
> 	[svn-remote "svn"]
> 		url = http://restlet.tigris.org/svn/restlet
> 		fetch = trunk:refs/remotes/svn/trunk
> 		branches = branches/*:refs/remotes/svn/*
> 		tags = tags/*/*:refs/remotes/svn/tags/*/*
> 		; note the */* to glob at multiple levels

Thank you, here is what I had (with the multiple -t/-b):

[svn-remote "svn"]
         url = http://restlet.tigris.org/svn/restlet
         branches = branches/1.0/*:refs/remotes/svn/*
         branches = branches/1.1/*:refs/remotes/svn/*
         tags = tags/1.0/*:refs/remotes/svn/tags/*
         tags = tags/1.1/*:refs/remotes/svn/tags/*
         tags = tags/1.2/*:refs/remotes/svn/tags/*
         tags = tags/2.0/*:refs/remotes/svn/tags/*

I think the notation you suggest "*/*" is indeed better, since I don't 
have to specify each tag sub-directory. However, they change so rarely 
that it was only a minor issue.


>> What surprises me is that it looks like it's looping over and over,  
>> since sometimes it starts back from SVN revision 1 when it's trying to  
>> import a new tag.
> 
> Yeah, that's an unfortunate thing about the flexibility of Subversion,
> basically anything can be a "tag" or a directory and it's extremely
> hard for git svn to support any uncommon cases for tags/branches
> out-of-the box, so the manual config editing is needed.

I must admit I don't fully understand how git-svn does the import, but 
even with this manual configuration, it still tries to pull (almost) 
every revision from revision 1 for each tag, a bit as if there was:
   for each tag:
      for revision in 1 to tag.latest revision:
         pull the revision

(This isn't even for each tag, but for each modification of each tag, 
since tags aren't really tags in SVN).

What I'd like to be able to do (mainly for efficiency and more 
importantly not to hammer tigris.org) is to pull each revision at most 
once (even if it's for the directory at the top of trunk, branches and 
tags).

Best wishes,

Bruno.

^ permalink raw reply

* Re: [PATCH] gitweb: linkify author/committer names with search
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, Junio C Hamano, Giuseppe Bilotta, Jakub Narebski
In-Reply-To: <1255486344-11891-1-git-send-email-bebarino@gmail.com>

Stephen Boyd <bebarino@gmail.com> writes:

> It's nice to search for an author by merely clicking on their name in
> gitweb. This is usually faster than selecting the name, copying the
> selection, pasting it into the search box, selecting between
> author/committer and then hitting enter.
>
> Linkify the avatar icon in log/shortlog view because the icon is directly
> adjacent to the name and thus more related. The same is not true
> when in commit/tag view where the icon is farther away.
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
>
> This is on top of Giuseppe's fix esc_param patch.
>
>  gitweb/gitweb.css  |    1 +
>  gitweb/gitweb.perl |   21 ++++++++++++++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 8f68fe3..e2cd9d7 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -30,6 +30,7 @@ img.logo {
>  
>  img.avatar {
>  	vertical-align: middle;
> +	border-width: 0px;
>  }

Even though it _might_ be a good thing to do, this looks an unrelated
change,

>  div.page_header {
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 4b21ad2..bdf1037 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1602,8 +1602,12 @@ sub format_author_html {
>  	my $co = shift;
>  	my $author = chop_and_escape_str($co->{'author_name'}, @_);
>  	return "<$tag class=\"author\">" .
> -	       git_get_avatar($co->{'author_email'}, -pad_after => 1) .
> -	       $author . "</$tag>";
> +	       $cgi->a({-href => href(action=>"search", hash=>$hash,
> +			searchtext=>$co->{'author_name'},
> +			searchtype=>"author"), class=>"list"},
> +			git_get_avatar($co->{'author_email'}, -pad_after => 1) .
> +			$author) .
> +	       "</$tag>";
>  }
>  
>  # format git diff header line, i.e. "diff --(git|combined|cc) ..."
> @@ -3372,10 +3376,13 @@ sub git_print_authorship {
>  	my $co = shift;
>  	my %opts = @_;
>  	my $tag = $opts{-tag} || 'div';
> +	my $author = $co->{'author_name'};
>  
>  	my %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});
>  	print "<$tag class=\"author_date\">" .
> -	      esc_html($co->{'author_name'}) .
> +	      $cgi->a({-href => href(action=>"search", searchtext=>$author,
> +		      searchtype=>"author"), class=>"list"},
> +		      esc_html($author)) .
>  	      " [$ad{'rfc2822'}";
>  	print_local_time(%ad) if ($opts{-localtime});
>  	print "]" . git_get_avatar($co->{'author_email'}, -pad_before => 1)
> @@ -3394,8 +3401,12 @@ sub git_print_authorship_rows {
>  	@people = ('author', 'committer') unless @people;
>  	foreach my $who (@people) {
>  		my %wd = parse_date($co->{"${who}_epoch"}, $co->{"${who}_tz"});
> -		print "<tr><td>$who</td><td>" . esc_html($co->{$who}) . "</td>" .
> -		      "<td rowspan=\"2\">" .
> +		print "<tr><td>$who</td><td>" .
> +		      $cgi->a({-href => href(action=>"search",
> +			       searchtext=>$co->{"${who}_name"},
> +			       searchtype=>$who), class=>"list"},
> +			       esc_html($co->{$who})) .
> +		      "</td><td rowspan=\"2\">" .
>  		      git_get_avatar($co->{"${who}_email"}, -size => 'double') .
>  		      "</td></tr>\n" .
>  		      "<tr>" .
> -- 
> 1.6.5.1.g75846.dirty

^ permalink raw reply

* Re: [RFC PATCH 2/5] Introduce new pretty formats %g and %G for reflog information
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jef Driesen, Jeff King, Nanako Shiraishi, git
In-Reply-To: <e0321a8af8d702d24ace33510daff22d02f4e116.1255380039.git.trast@student.ethz.ch>

Squash this patch on top, as

    cc1: warnings being treated as errors
    reflog-walk.c: In function 'get_reflog_message':
    reflog-walk.c:278: error: ISO C90 forbids mixed declarations and code

---

 reflog-walk.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index 929f025..5cbcb1a 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -270,12 +270,13 @@ void get_reflog_message(struct strbuf *sb,
 {
 	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
 	struct reflog_info *info;
+	size_t len;
 
 	if (!commit_reflog)
 		return;
 
 	info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
-	size_t len = strlen(info->message);
+	len = strlen(info->message);
 	if (len > 0)
 		len--;
 	strbuf_add(sb, info->message, len);

^ permalink raw reply related

* Re: [PATCH v3] bisect reset: Allow resetting to any commit, not just a branch
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: Christian Couder, git
In-Reply-To: <alpine.DEB.2.00.0910131700320.5105@dr-wily.mit.edu>

Thanks; will replace your v2 that was queued in 'pu'.

^ permalink raw reply

* Re: [PATCH] clone: Supply the right commit hash to post-checkout when -b is used
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Björn Steinbrink, Junio C Hamano, git, ranguvar
In-Reply-To: <20091014000619.GA20496@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> Reported-by: Devin Cofer <ranguvar@archlinux.us>
>> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
>
> Acked-by: Jeff King <peff@peff.net>
>
> Thanks.

Thanks; will queue on 'maint' for 1.6.5.1.

^ permalink raw reply

* Re: [PATCH] gitweb: fix esc_param
From: Junio C Hamano @ 2009-10-14  9:13 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Stephen Boyd, Junio C Hamano
In-Reply-To: <1255463496-21617-1-git-send-email-giuseppe.bilotta@gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> ...
> Finally, remove an unnecessary escaping of the + sign.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

Thanks.

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-14  9:33 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Jeff King, Daniel Barkalow, Johannes Sixt,
	Euguess, Mikael Magnusson, Matthieu Moy, Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910140108110.4985@pacific.mpi-cbg.de>

Johannes Schindelin wrote:
> 
> Let me tell you this from my experience: the least likely answer is "the 
> messages are too scary".  Invariably, the answer I get is "it is totally 
> unintuitive".  Often followed by "I tell Git to do something 
> straight-forward, and it refuses to do it."

<aside>

Actually I had a rather insightful discussion yesterday (spawned by
this exact thread) with someone here at the institute.  He said
something to the effect that git's problem is mostly that it is unlike
everything else.  You cannot explain git in simple metaphors like
files, copies and such.  Any attempt to do so will just fall short
really soon.

[On the other hand, some users appear unwilling to learn something new
because they "just want to version control this" or "just need to make
a commit to this project".]

</aside>

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-14  9:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Euguess, Mikael Magnusson, Matthieu Moy,
	Jeff King, Jay Soffian, git
In-Reply-To: <200910131051.47117.trast@student.ethz.ch>

Thomas Rast wrote:
> Junio C Hamano wrote:
> > 
> > #1. These used to detach, but will create a local branch
> > 
> >  $ git checkout origin/next        ;# as if with -t
> >  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
> 
> Agreed, though I'm still in favour of a cleaner syntax for explicit
> detaching.  (Cleaner in the sense that ^0 is documented as having a
> completely different purpose and only works by accident.)

Not sure if it's too late in the thread, but after sleeping over it
and re-reading (and the other developments in the thread) I'm not
happy with my earlier opinion any more.  I think the DWIM part of it
is a bad idea because of this:

> >  $ git checkout origin/master      ;# detach, or refuse???
> 
> This seems to be the trickiest of them.  Maybe check out 'master', to
> make the process repeatable.  Imagine, in your setting,
> 
>   git checkout origin/next           ;# creates 'next' as with -t
>   git checkout -                     ;# back
>   git checkout origin/next           ;# should go to 'next' again
> 
> Then again, that would trade the confusion of detaching for the
> confusion of not checking out the exact commit that the user
> specified.  Worse, 'next' could conceivably be tracking (as per
> branch.next.merge) some entirely different branch, making the "Your
> branch is behind..." message misleading.

So I think we're now mixing up two different goals in this thread:
a) Stopping the users from hurting themselves by inadvertent detaching
b) Helping the users by DWIMming local branches for them

I'm all for (a), but (b) is much harder.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [RFC PATCH 2/5] Introduce new pretty formats %g and %G for reflog information
From: Thomas Rast @ 2009-10-14  9:58 UTC (permalink / raw)
  To: Jeff King; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <20091014045918.GA31810@coredump.intra.peff.net>

Jeff King wrote:
> On Mon, Oct 12, 2009 at 11:06:04PM +0200, Thomas Rast wrote:
> 
> > Unfortunately, we also need to pass down the reflog_walk_info from
> > show_log(), so this commit touches a lot of (unrelated) callers to
> > pretty_print_commit() and format_commit_message() to accomodate the
> > extra argument.
> 
> A while back I wanted to add a feature to pretty-printing, and I ran
> into the same situation (though my feature never made it to the list).
> We really end up passing around the same arguments over and over.  Maybe
> it makes sense instead of adding another argument to refactor into a
> "pretty_print_context" struct that contains all of the arguments and
> current state.
> 
> It would be an even more invasive patch, but I think it would make
> things more readable, and make future changes much easier to see.

Ok, I'll try for the next round.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Johannes Schindelin @ 2009-10-14 10:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Daniel Barkalow, Junio C Hamano, git
In-Reply-To: <20091014050851.GE31810@coredump.intra.peff.net>

Hi,

On Wed, 14 Oct 2009, Jeff King wrote:

> On Wed, Oct 14, 2009 at 12:44:34AM -0400, Daniel Barkalow wrote:
> 
> > +char *get_detached_head_string(void)
> > +{
> > +	char *filename = git_path("DETACH_NAME");
> > +	struct stat st;
> > +	if (stat(filename, &st) || !S_ISREG(st.st_mode))
> > +		return NULL;
> > +	struct strbuf buf = STRBUF_INIT;
> > +	strbuf_read_file(&buf, filename, st.st_size);
> > +	strbuf_trim(&buf);
> > +	return strbuf_detach(&buf, 0);
> > +}
> 
> Would it hurt to tuck this information into HEAD itself, as we already
> put arbitrary text into FETCH_HEAD?

AFAIR we still remember HEAD to be a symlink.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 5/6] This assertion is valid for both loose and packed objects
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <1255516669-26745-4-git-send-email-herve@itaapy.com>

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/odb.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/odb.c b/src/odb.c
index c71948b..a612299 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -1149,11 +1149,10 @@ void git_odb_close(git_odb *db)
 	free(db);
 }
 
-int git_odb_read(
-	git_obj *out,
-	git_odb *db,
-	const git_oid *id)
+int git_odb_read(git_obj *out, git_odb *db, const git_oid *id)
 {
+	assert(out && db && id);
+
 attempt:
 	if (!git_odb__read_packed(out, db, id))
 		return GIT_SUCCESS;
@@ -1171,8 +1170,6 @@ int git_odb__read_loose(git_obj *out, git_odb *db, const git_oid *id)
 	char file[GIT_PATH_MAX];
 	gitfo_buf obj = GITFO_BUF_INIT;
 
-	assert(out && db && id);
-
 	out->data = NULL;
 	out->len  = 0;
 	out->type = GIT_OBJ_BAD;
-- 
1.6.5

^ permalink raw reply related

* [PATCH 2/6] Read the base offset or name of delta objects
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <1255516669-26745-1-git-send-email-herve@itaapy.com>

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/cc-compat.h |    3 +++
 src/odb.c       |   28 ++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/cc-compat.h b/src/cc-compat.h
index 8997caa..8dd6774 100644
--- a/src/cc-compat.h
+++ b/src/cc-compat.h
@@ -30,6 +30,9 @@
 # define GIT_TYPEOF(x)
 #endif
 
+#define bitsizeof(x)  (CHAR_BIT * sizeof(x))
+#define MSB(x, bits) ((x) & GIT_TYPEOF(x)(~0ULL << (bitsizeof(x) - (bits))))
+
 /*
  * Does our compiler/platform support the C99 <inttypes.h> and
  * <stdint.h> header files. (C99 requires that <inttypes.h>
diff --git a/src/odb.c b/src/odb.c
index 2319998..2b4b016 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -97,8 +97,10 @@ struct git_odb {
 };
 
 typedef struct {  /* object header data */
-	git_otype type;  /* object type */
-	size_t    size;  /* object size */
+	git_otype type;         /* object type */
+	size_t    size;         /* object size */
+	off_t     base_offset;  /* delta base offset (GIT_OBJ_OFS_DELTA) */
+	git_oid   base_name;    /* delta base name (GIT_OBJ_REF_DELTA) */
 } obj_hdr;
 
 static struct {
@@ -238,6 +240,7 @@ static size_t get_binary_object_header(obj_hdr *hdr, gitfo_buf *obj)
 	unsigned char c;
 	unsigned char *data = obj->data;
 	size_t shift, size, used = 0;
+	off_t base_offset;
 
 	if (obj->len == 0)
 		return 0;
@@ -258,6 +261,27 @@ static size_t get_binary_object_header(obj_hdr *hdr, gitfo_buf *obj)
 	}
 	hdr->size = size;
 
+	hdr->base_offset = 0;
+	hdr->base_name.id[0] = '\0';
+
+	if (hdr->type == GIT_OBJ_OFS_DELTA) {
+		c = data[used++];
+		base_offset = c & 127;
+		while (c & 128) {
+			base_offset++;
+			if (!base_offset || MSB(base_offset, 7))
+				return 0;  /* overflow */
+			c = data[used++];
+			base_offset = (base_offset << 7) + (c & 127);
+		}
+		assert(base_offset > 0);
+		hdr->base_offset = base_offset;
+	}
+	else if (hdr->type == GIT_OBJ_REF_DELTA) {
+		git_oid_mkraw(&hdr->base_name, data + used);
+		used += 20;
+	}
+
 	return used;
 }
 
-- 
1.6.5

^ permalink raw reply related

* [PATCH 1/6] Open the pack file and keep a map on it.
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git

On the same model than the idx file.

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/odb.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/odb.h |    1 +
 2 files changed, 65 insertions(+), 1 deletions(-)

diff --git a/src/odb.c b/src/odb.c
index 6d646a4..2319998 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -64,6 +64,10 @@ struct git_pack {
 
 	/** Name of the pack file(s), without extension ("pack-abc"). */
 	char pack_name[GIT_PACK_NAME_MAX];
+
+	/** The .pack file, mapped into memory. */
+	git_file pack_fd;
+	git_map pack_map;
 };
 typedef struct git_pack git_pack;
 
@@ -809,6 +813,59 @@ unlock_fail:
 	return GIT_ERROR;
 }
 
+static int pack_openpack_map(git_pack *p)
+{
+	char pb[GIT_PATH_MAX];
+	off_t len;
+
+	if (git__fmt(pb, sizeof(pb), "%s/pack/%s.pack",
+			p->db->objects_dir,
+			p->pack_name) < 0)
+		return GIT_ERROR;
+
+	if ((p->pack_fd = gitfo_open(pb, O_RDONLY)) < 0)
+		return GIT_ERROR;
+
+	if ((len = gitfo_size(p->pack_fd)) < 0
+			|| !git__is_sizet(len)
+			|| gitfo_map_ro(&p->pack_map, p->pack_fd, 0, (size_t)len)) {
+		gitfo_close(p->pack_fd);
+		return GIT_ERROR;
+	}
+
+	return GIT_SUCCESS;
+}
+
+static int pack_openpack(git_pack *p)
+{
+	gitlck_lock(&p->lock);
+	if (p->invalid)
+		goto unlock_fail;
+	if (p->pack_fd < 0) {
+		uint32_t *data;
+
+		if (pack_openpack_map(p))
+			goto invalid_fail;
+		data = p->pack_map.data;
+
+		if (decode32(&data[0]) != PACK_HDR)
+			goto unmap_fail;
+	}
+	gitlck_unlock(&p->lock);
+	return GIT_SUCCESS;
+
+unmap_fail:
+	gitfo_free_map(&p->pack_map);
+
+invalid_fail:
+	p->invalid = 1;
+	p->pack_fd = -1;
+
+unlock_fail:
+	gitlck_unlock(&p->lock);
+	return GIT_ERROR;
+}
+
 static void pack_decidx(git_pack *p)
 {
 	gitlck_lock(&p->lock);
@@ -830,6 +887,11 @@ static void pack_dec(git_pack *p)
 			gitfo_close(p->idx_fd);
 			free(p->im_fanout);
 		}
+		if (p->pack_fd >= 0) {
+			gitfo_free_map(&p->pack_map);
+			gitfo_close(p->pack_fd);
+			p->pack_fd = -1;
+		}
 
 		gitlck_free(&p->lock);
 		free(p);
@@ -861,6 +923,7 @@ static git_pack *alloc_pack(const char *pack_name)
 	gitlck_init(&p->lock);
 	strcpy(p->pack_name, pack_name);
 	p->refcnt = 1;
+	p->pack_fd = -1;
 	return p;
 }
 
@@ -895,7 +958,7 @@ static int scan_one_pack(void *state, char *name)
 
 	r->next = *ret;
 	*ret = r;
-	return 0;
+	return GIT_SUCCESS;
 }
 
 static git_packlist* scan_packs(git_odb *db)
diff --git a/src/odb.h b/src/odb.h
index 2f205b2..121583f 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -15,5 +15,6 @@
  * cannot be true for an idx v1 file.
  */
 #define PACK_TOC 0xff744f63 /* -1tOc */
+#define PACK_HDR 0x5041434b /* PACK */
 
 #endif
-- 
1.6.5

^ permalink raw reply related

* [PATCH 4/6] Inflate an object from a pack file
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <1255516669-26745-3-git-send-email-herve@itaapy.com>

Support delta objects too.

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/odb.c |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 68 insertions(+), 1 deletions(-)

diff --git a/src/odb.c b/src/odb.c
index 349747b..c71948b 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -28,6 +28,7 @@
 #include "git/zlib.h"
 #include "fileops.h"
 #include "hash.h"
+#include "delta-apply.h"
 #include "odb.h"
 
 #define GIT_PACK_NAME_MAX (5 + 40 + 1)
@@ -232,7 +233,7 @@ static int is_zlib_compressed_data(unsigned char *data)
 	unsigned int w;
 
 	w = ((unsigned int)(data[0]) << 8) + data[1];
-	return data[0] == 0x78 && !(w %31);
+	return data[0] == 0x78 && !(w % 31);
 }
 
 static size_t get_binary_object_header(obj_hdr *hdr, gitfo_buf *obj)
@@ -1192,6 +1193,72 @@ int git_odb__read_loose(git_obj *out, git_odb *db, const git_oid *id)
 	return GIT_SUCCESS;
 }
 
+static int inflate_pack_obj(git_obj *out, git_pack *p, off_t offset)
+{
+	obj_hdr hdr;
+	gitfo_buf buf;
+	size_t used;
+	void *data;
+	git_obj base;
+
+	/* Cast the map to a gitfo_buf */
+	buf.data = (unsigned char *)p->pack_map.data + offset;
+	buf.len = p->pack_map.len - offset;
+
+	/*
+	 * Read the object header, which is an (uncompressed)
+	 * binary encoding of the object type and size.
+	 */
+	if (!(used = get_binary_object_header(&hdr, &buf)))
+		return GIT_ERROR;
+
+	/*
+	 * Read the object data as a zlib compressed data
+	 */
+	buf.data += used;
+	buf.len -= used;
+	assert(is_zlib_compressed_data(buf.data));
+
+	if (!(data = git__malloc(hdr.size + 1)))
+		return GIT_ERROR;
+	if (inflate_buffer(buf.data, buf.len, data, hdr.size))
+		goto inflate_fail;
+
+	switch (hdr.type) {
+		case GIT_OBJ_COMMIT:
+		case GIT_OBJ_TREE:
+		case GIT_OBJ_BLOB:
+		case GIT_OBJ_TAG:
+			out->data = data;
+			out->len = hdr.size;
+			out->type = hdr.type;
+			return GIT_SUCCESS;
+		case GIT_OBJ_OFS_DELTA:
+			offset -= hdr.base_offset;
+			if (inflate_pack_obj(&base, p, offset))
+				goto inflate_fail;
+			if (git__delta_apply(out, base.data, base.len, data, hdr.size))
+				goto inflate_fail;
+			out->type = base.type;
+			return GIT_SUCCESS;
+		case GIT_OBJ_REF_DELTA:
+			if (p->idx_search(&offset, p, &hdr.base_name))
+				goto inflate_fail;
+			if (inflate_pack_obj(&base, p, offset))
+				goto inflate_fail;
+			if (git__delta_apply(out, base.data, base.len, data, hdr.size))
+				goto inflate_fail;
+			out->type = base.type;
+			return GIT_SUCCESS;
+		default:
+			goto inflate_fail;
+	}
+
+inflate_fail:
+	free(data);
+	return GIT_ERROR;
+}
+
 static int read_packed(git_obj *out, git_pack *p, const git_oid *id)
 {
 	off_t pos;
-- 
1.6.5

^ permalink raw reply related

* [PATCH 6/6] Read an object from a pack file
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <1255516669-26745-5-git-send-email-herve@itaapy.com>

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/odb.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/odb.c b/src/odb.c
index a612299..65a7993 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -1263,13 +1263,16 @@ static int read_packed(git_obj *out, git_pack *p, const git_oid *id)
 
 	if (pack_openidx(p))
 		return GIT_ERROR;
+	if (pack_openpack(p)) {
+		pack_decidx(p);
+		return GIT_ERROR;
+	}
 	res = p->idx_search(&pos, p, id);
 	pack_decidx(p);
+	assert(pos < p->pack_map.len);
 
-	if (!res) {
-		/* TODO unpack object at pos */
-		res = GIT_ERROR;
-	}
+	if (res == GIT_SUCCESS)
+		return inflate_pack_obj(out, p, pos);
 
 	return res;
 }
-- 
1.6.5

^ permalink raw reply related

* [PATCH 3/6] Allow zlib to read a pack buffer longer than the actual data
From: Hervé Cauwelier @ 2009-10-14 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <1255516669-26745-2-git-send-email-herve@itaapy.com>

As we don't know where the compressed data end, only the size of the
uncompressed data.

Signed-off-by: Hervé Cauwelier <herve@itaapy.com>
---
 src/odb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/odb.c b/src/odb.c
index 2b4b016..349747b 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -432,7 +432,7 @@ static int inflate_buffer(void *in, size_t inlen, void *out, size_t outlen)
 
 	inflateEnd(&zs);
 
-	if ((status != Z_STREAM_END) || (zs.avail_in != 0))
+	if (status != Z_STREAM_END)
 		return GIT_ERROR;
 
 	if (zs.total_out != outlen)
-- 
1.6.5

^ permalink raw reply related


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