* Re: [PATCH] rehabilitate 'git index-pack' inside the object store
From: Johannes Schindelin @ 2008-10-21 17:02 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <fcaeb9bf0810210757w1c14e0a3x1eb61a589a089f10@mail.gmail.com>
Hi,
On Tue, 21 Oct 2008, Nguyen Thai Ngoc Duy wrote:
> Maybe we should let setup_*_gently() do read-only stuff only and let its
> consumers to handle cwd.
I think that makes sense. This would allow setup*gently() to be much
cleaner, and only setup_git_directory() itself would do the
chdir(worktree).
However, let's think this really through this time, so that that darned
worktree stuff is fixed for good.
So I propose this change in semantics:
- setup_git_directory_gently(): rename to discover_git_directory(),
and avoid any chdir() at all.
- setup_git_directory(): keep the semantics that it chdir()s to the
worktree, or to the git directory for bare repositories.
Using _gently() even for RUN_SETUP builtins should solve the long standing
pager problem, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-fetch should not strip off ".git" extension
From: Junio C Hamano @ 2008-10-21 16:56 UTC (permalink / raw)
To: SLONIK.AZ; +Cc: Junio C Hamano, Andreas Ericsson, git
In-Reply-To: <ee2a733e0810210323j249c3460x881af6d6aefc647c@mail.gmail.com>
"Leo Razoumov" <slonik.az@gmail.com> writes:
> Even though the old behavior is "long established", it introduces
> unnecessary ambiguity. If I have two repos
> ...
Of course. Now you know why people don't name such a pair of repositories
like that ;-).
^ permalink raw reply
* Re: [PATCH] rehabilitate 'git index-pack' inside the object store
From: Jeff King @ 2008-10-21 17:43 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Nguyen Thai Ngoc Duy, Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0810211856090.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Tue, Oct 21, 2008 at 07:02:48PM +0200, Johannes Schindelin wrote:
> So I propose this change in semantics:
>
> - setup_git_directory_gently(): rename to discover_git_directory(),
> and avoid any chdir() at all.
> - setup_git_directory(): keep the semantics that it chdir()s to the
> worktree, or to the git directory for bare repositories.
>
> Using _gently() even for RUN_SETUP builtins should solve the long standing
> pager problem, too.
I'm not sure there aren't hidden problems lurking in that strategy
(every time I look at this area of code, something unexpected prevents
what I think should Just Work from Just Working), but I think that is a
promising direction to go for clearing up some of the long-standing
issues.
I think you will need to do something for a few commands which use
_gently() but then, if we _are_ in a git repo, assume we are chdir'd
properly.
We may also be able to just call setup_git_directory_gently() as the
first thing in the wrapper, which should help us more consistently find
config before the 'exec' stage (see 4e10738a for a discussion of some of
the issues).
-Peff
^ permalink raw reply
* Re: [EGIT PATCH] git property page for project properties.
From: Robin Rosenberg @ 2008-10-21 17:50 UTC (permalink / raw)
To: Tomi Pakarinen; +Cc: spearce, git
In-Reply-To: <1224529604-42397-1-git-send-email-tomi.pakarinen@iki.fi>
måndagen den 20 oktober 2008 21.06.44 skrev Tomi Pakarinen:
> Show git property page in project's properties, if project
> has git repository provider.
[...]
> + switch (repository.getRepositoryState()) {
> + case BISECTING:
> + state.setText("Bisecting");
> + break;
> + case MERGING:
> + state.setText("Merging");
> + break;
> + case REBASING:
> + state.setText("Rebasing");
> + break;
> + case REBASING_INTERACTIVE:
> + state.setText("Rebasing interactive");
> + break;
> + case REBASING_MERGE:
> + state.setText("Rebasing merge");
> + break;
> + case SAFE:
> + state.setText("Safe");
> + break;
> + }
Why not this:
state.setText(repository.getRepositoryState().getDescription());
Description is ment for presentation purposes. The enum for code.
-- robin
^ permalink raw reply
* new plan for cleaning up the worktree mess, was Re: [PATCH] rehabilitate 'git index-pack' inside the object store
From: Johannes Schindelin @ 2008-10-21 17:59 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <20081021174303.GA25827@coredump.intra.peff.net>
Hi,
On Tue, 21 Oct 2008, Jeff King wrote:
> On Tue, Oct 21, 2008 at 07:02:48PM +0200, Johannes Schindelin wrote:
>
> > So I propose this change in semantics:
> >
> > - setup_git_directory_gently(): rename to discover_git_directory(),
> > and avoid any chdir() at all.
> > - setup_git_directory(): keep the semantics that it chdir()s to the
> > worktree, or to the git directory for bare repositories.
> >
> > Using _gently() even for RUN_SETUP builtins should solve the long
> > standing pager problem, too.
>
> I'm not sure there aren't hidden problems lurking in that strategy
> (every time I look at this area of code, something unexpected prevents
> what I think should Just Work from Just Working), but I think that is a
> promising direction to go for clearing up some of the long-standing
> issues.
Same here. I grew a pretty strong opinion about the whole worktree thing,
but maybe that is only because it was done trying to change as little as
possible.
> I think you will need to do something for a few commands which use
> _gently() but then, if we _are_ in a git repo, assume we are chdir'd
> properly.
Right.
> We may also be able to just call setup_git_directory_gently() as the
> first thing in the wrapper, which should help us more consistently find
> config before the 'exec' stage (see 4e10738a for a discussion of some of
> the issues).
Yep, that's what I referred to as "pager" problems. Thanks for the
pointer! For other people's convenience I quote it here :-)
-- snip --
Allow per-command pager config
There is great debate over whether some commands should set
up a pager automatically. This patch allows individuals to
set their own pager preferences for each command, overriding
the default. For example, to disable the pager for git
status:
git config pager.status false
If "--pager" or "--no-pager" is specified on the command
line, it takes precedence over the config option.
There are two caveats:
- you can turn on the pager for plumbing commands.
Combined with "core.pager = always", this will probably
break a lot of things. Don't do it.
- This only works for builtin commands. The reason is
somewhat complex:
Calling git_config before we do setup_git_directory
has bad side effects, because it wants to know where
the git_dir is to find ".git/config". Unfortunately,
we cannot call setup_git_directory indiscriminately,
because some builtins (like "init") break if we do.
For builtins, this is OK, since we can just wait until
after we call setup_git_directory. But for aliases, we
don't know until we expand (recursively) which command
we're doing. This should not be a huge problem for
aliases, which can simply use "--pager" or "--no-pager"
in the alias as appropriate.
For external commands, however, we don't know we even
have an external command until we exec it, and by then
it is too late to check the config.
An alternative approach would be to have a config mode
where we don't bother looking at .git/config, but only
at the user and system config files. This would make the
behavior consistent across builtins, aliases, and
external commands, at the cost of not allowing per-repo
pager config for at all.
-- snap --
Ciao,
Dscho
^ permalink raw reply
* Re: [EGIT PATCH] git property page for project properties.
From: Tomi Pakarinen @ 2008-10-21 18:09 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: spearce, git
In-Reply-To: <200810211950.50540.robin.rosenberg.lists@dewire.com>
On Tue, Oct 21, 2008 at 8:50 PM, Robin Rosenberg
<robin.rosenberg.lists@dewire.com> wrote:
> måndagen den 20 oktober 2008 21.06.44 skrev Tomi Pakarinen:
>> Show git property page in project's properties, if project
>> has git repository provider.
> [...]
>> + switch (repository.getRepositoryState()) {
>> + case BISECTING:
>> + state.setText("Bisecting");
>> + break;
>> + case MERGING:
>> + state.setText("Merging");
>> + break;
>> + case REBASING:
>> + state.setText("Rebasing");
>> + break;
>> + case REBASING_INTERACTIVE:
>> + state.setText("Rebasing interactive");
>> + break;
>> + case REBASING_MERGE:
>> + state.setText("Rebasing merge");
>> + break;
>> + case SAFE:
>> + state.setText("Safe");
>> + break;
>> + }
> Why not this:
>
> state.setText(repository.getRepositoryState().getDescription());
>
> Description is ment for presentation purposes. The enum for code.
>
> -- robin
>
That is ok. I just didn't notice that method before. But, what if
someone wants localize this plugin, he'll propably have to explode
this back to switch statement.
Tomi.
^ permalink raw reply
* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: Daniel Barkalow @ 2008-10-21 18:16 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, René Scharfe, Git Mailing List,
Davide Libenzi
In-Reply-To: <48FD781C.2000103@viscovery.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1363 bytes --]
On Tue, 21 Oct 2008, Johannes Sixt wrote:
> Junio C Hamano schrieb:
> > René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> >
> >> I think it makes sense to make 1, or even 3, the default for this
> >> option for all commands that create patches intended for human
> >> consumption. The patch keeps the default at 0, though.
> >
> > I think defaulting to 1 would make sense, or alternatively, just
> > hardcoding that behaviour without any new option. That would give you
> > more information with the same number of patch lines, iow, upside without
> > any downside.
>
> Are you sure about the "without any downside" part? The extra context line
> inhibits that the patch applies cleanly to a version of the file that has
> that very line modified (including a different number of lines).
We could start allowing "fuzz" by default in the case of a patch with more
context than we'd expect to see. That is, git-apply would ignore context
lines more than 3 lines away from any changed lines, sharing the
assumption of our patch-generation side that lines that far away don't
matter in general. (Now, if people were in the habit of including as
context additional lines in fragile locations, this wouldn't be a good
assumption, but I doubt anybody would be able to identify such lines to
include them).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATHv2 6/8] gitweb: retrieve snapshot format from PATH_INFO
From: Giuseppe Bilotta @ 2008-10-21 18:36 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <200810211844.35714.jnareb@gmail.com>
On Tue, Oct 21, 2008 at 6:44 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> I like the idea behind this patch, to enable to use path_info for as
> much gitweb parameters as possible. After this patch series the only
> parameters which wouldn't be possible to represent in path_info would
> be:
> * @extra_options ('opt') multi-valued parameter, used to pass
> thinks like '--no-merges', which cannot be fit in the "simplified"
> list-like (as opposed to hash-like query string) path_info URL.
> * $searchtype ('st') and $searchtext ('s') etc. parameters, which
> are generated by HTML form, and are naturally generated in query
> string format.
> * $page ('pg') parameter, which could theoretically be added as last
> part of path_info URL, for example $project/next/2/... if not for
> pesky $project/history/next:/Documentation/2/ where you cannot be
> sure that having /<number>/ at the end is rare.
> * $order ('o') parameter, which would be hard to fit in path_info,
> with its limitation of parameters being specified by position.
> Or even next to impossible.
> * 'by_tag'...
>
> But I'd rather have this patch series to be in separate thread...
Yes, a posteriori I think it's better too. I'll resend the 5 path_info
patches with the minor stylistic corrections you suggested, and send
these 3 separately.
> On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
>
>> We parse requests for $project/snapshot/$head.$sfx as equivalent to
>> $project/snapshot/$head?sf=$sfx, where $sfx is any of the known
>> (although not necessarily supported) snapshot formats (or its default
>> suffix).
>>
>> The filename for the resulting package preserves the requested
>> extensions (so asking for a .tgz gives a .tgz, and asking for a .tar.gz
>> gives a .tar.gz), although for obvious reasons it doesn't preserve the
>> basename (git/snapshot/next.tgz returns a file names git-next.tgz).
>
> That is a bit of difference from sf=<format> in CGI query string, where
> <format> is always a name of a format (for example 'tgz' or 'tbz2'),
> and actual suffix is defined in %known_snapshot_formats (for example
> '.tar.gz' and '.tar.bz2' respectively). Now you can specify snapshot
> format either either by its name, for example 'tgz' (which is simple
> lookup in hash) which result in proposed filename with '.tgz' suffix,
> or you can specify suffix, for example 'tar.gz' (which requires
> searching through all hash) which result in proposed filename with
> '.tar.gz' suffix.
>
> This is a bit of inconsistency; to be consistent with how we handle
> 'sf' CGI parameter we would translate 'tgz' $sfx into 'tar.gz' in
> snapshot filename. This would also cover currently purely theoretical
> case when different snapshot formats (for example 'tgz' and 'tgz9')
> would use the same snapshot suffix (extension), but differ for example
> in parameters passed to compressor (for example '-9' or '--best' in
> the 'tgz9' case).
>
> On the other hand one would expect that when URL which looks like
> URL to snapshot ends with '.$sfx', then filename for snapshot would
> also end with '.$sfx'.
>
> This certainly requires some further thoughts.
What I decided was to set gitweb to always produce links with the
suffix (.e.g .tar.gz), but I saw no particular reason not to accept
the shorter version which is (1) commonly used as a suffix as well and
(2) happens to be the actual format key used by gitweb.
A different, possibly cleaner approach, but a more extensive change,
would be to have each format describe a list of suffixes, defaulting
to the first one on creation by identifying all of them. This is more
invasive because all of the uses of {'suffix'} have to be replaced
with {'suffix'}[0], or something like that (maybe we could add a
separate key 'other_suffixes' instead?)
>> This introduces a potential case for ambiguity if a project has a head
>> that ends with a snapshot-like suffix (.zip, .tgz, .tar.gz, etc) and the
>> sf CGI parameter is not present; however, gitweb only produces URLs with
>> the sf parameter, so this is only a potential issue for hand-coded URLs
>> for extremely unusual project.
>
> I think you wanted to say here "_currently_ produces URLs with the 'sf'
> parameter" as the next patch in series changes this.
Ah yes, good point.
>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
>> ---
>>
>> I had second thoughts on this. Now we always look for the snapshot extension if
>> the sf CGI parameter is missing, even if the project has a head that matches
>> the full pseudo-refname $head.$sfx.
>>
>> The reason for this is that (1) there is no ambiguity for gitweb-generated
>> URLs (2) the only URLs that could fail are hand-made URLs for extremely
>> unusual projects and (3) it allows us to set gitweb up to generate
>> (unambiguous) URLs without the sf CGI parameter.
>
> This is also simpler and cheaper solution.
That, too 8-)
>> This also means that I can add 3 patches to the series, instead of just one:
>> * patch #6 that parses the new format
>> * patch #7 that generates the new URLs
>> * patch #8 for some code refactoring
>
> Now, I haven't yet read the last patch in series, so I don't know if
> it is independent refactoring, making sense even before patches named
> #6 and #7 here, or is it connected with searching for snapshot format
> by suffix it uses. If the former, it should be done upfront, as it
> shouldn't need discussion, and being easier to be accepted into git.git.
> If the latter, then it should probably be folded (squashed) into #6,
> first patch in the series.
In fact, patch #8 can be written independently of the other too, and
would provide a significant speed benefit for generation of pages with
lots of 'snapshot' links: what it does is just to make the 'supported
formats' array global, preparing it only once instead of re-preparing
it every time a snapshot link is created.
>> gitweb/gitweb.perl | 34 ++++++++++++++++++++++++++++++++++
>> 1 files changed, 34 insertions(+), 0 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 99c8c20..e9e9e60 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -609,6 +609,40 @@ sub evaluate_path_info {
>> $input_params{'hash_parent'} ||= $parentrefname;
>> }
>> }
>> +
>> + # for the snapshot action, we allow URLs in the form
>> + # $project/snapshot/$hash.ext
>> + # where .ext determines the snapshot and gets removed from the
>> + # passed $refname to provide the $hash.
>> + #
>> + # To be able to tell that $refname includes the format extension, we
>> + # require the following two conditions to be satisfied:
>> + # - the hash input parameter MUST have been set from the $refname part
>> + # of the URL (i.e. they must be equal)
>
> This means no "$project/.tgz?h=next", isn't it?
Right.
>> + # - the snapshot format MUST NOT have been defined already
>
> I would add "which means that 'sf' parameter is not set in URL", or
> something like that as the last line of above comment.
Good idea. I'll make it an 'e.g.' to keep the comment valid for future
additional parameter evaluation such as command-line input.
> I like that the code is so well commented, by the way.
Thanks.
>> + if ($input_params{'action'} eq 'snapshot' && defined $refname &&
>> + $refname eq $input_params{'hash'} &&
>
> Minor nit.
>
> I would use here (the question of style / better readability):
>
> + if ($input_params{'action'} eq 'snapshot' &&
> + defined $refname && $refname eq $input_params{'hash'} &&
>
> to have both conditions about $refname in the same line.
Yes, it'd look much better.
>> + !defined $input_params{'snapshot_format'}) {
>> + # We loop over the known snapshot formats, checking for
>> + # extensions. Allowed extensions are both the defined suffix
>> + # (which includes the initial dot already) and the snapshot
>> + # format key itself, with a prepended dot
>> + while (my ($fmt, %opt) = each %known_snapshot_formats) {
>> + my $hash = $refname;
>> + my $sfx;
>> + $hash =~ s/(\Q$opt{'suffix'}\E|\Q.$fmt\E)$//;
>> + next unless $sfx = $1;
>> + # a valid suffix was found, so set the snapshot format
>> + # and reset the hash parameter
>> + $input_params{'snapshot_format'} = $fmt;
>> + $input_params{'hash'} = $hash;
>> + # we also set the format suffix to the one requested
>> + # in the URL: this way a request for e.g. .tgz returns
>> + # a .tgz instead of a .tar.gz
>> + $known_snapshot_formats{$fmt}{'suffix'} = $sfx;
>> + last;
>> + }
>
> I'm not sure if it worth (see comment at the beginning of this mail)
> adding this code, or just allow $sfx to be snapshot _name_ (key in
> %known_snapshot_formats hash).
>
> Otherwise it would be as simple as checking if $known_snapshot_formats{$sfx}
> exists (assuming that snapshot format names does not contain '.').
>
> If we decide to go more complicated route, then refactoring it in such
> a way that suffixes are also keys to %known_snapshot_formats would be
> preferred... err, sorry, not so simple. But refactoring this check
> into separate subroutine (as I think last patch in series does) would
> be good idea.
See comments above.
> Also, I'd rather you checked if the $refname part contains '.' for it
> to even consider that it can be suffix.
Ah, good idea.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: gitk: Turn short SHA1 names into links too
From: Junio C Hamano @ 2008-10-21 19:09 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <18685.4652.287143.717452@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> Linus Torvalds writes:
>
>> And the thing I wanted to work was to have the abbreviated SHA1's that
>> have started to get more common in the kernel commit logs work as links in
>> gitk too, just the way a full 40-character SHA1 link works.
>
> I just pushed out a commit to gitk that makes this work, and fixes the
> other bugs you mentioned.
FWIW, pulled and pushed out. Thanks.
^ permalink raw reply
* Re: [PATHv2 6/8] gitweb: retrieve snapshot format from PATH_INFO
From: Junio C Hamano @ 2008-10-21 19:09 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Jakub Narebski, git, Petr Baudis, Junio C Hamano
In-Reply-To: <cb7bb73a0810211136n452ac8bdp7814ff09749b3142@mail.gmail.com>
"Giuseppe Bilotta" <giuseppe.bilotta@gmail.com> writes:
>> But I'd rather have this patch series to be in separate thread...
>
> Yes, a posteriori I think it's better too. I'll resend the 5 path_info
> patches with the minor stylistic corrections you suggested, and send
> these 3 separately.
I've only been watching from the sidelines, but the discussion seemed
sensible. With the resend with Acks I think they are already 'next'
material.
Thanks, both of you.
^ permalink raw reply
* Re: [EGIT PATCH] git property page for project properties.
From: Robin Rosenberg @ 2008-10-21 19:13 UTC (permalink / raw)
To: tomi.pakarinen; +Cc: spearce, git
In-Reply-To: <f299b4f30810211109q7f2919f2r1d5cd8faf0048154@mail.gmail.com>
tisdagen den 21 oktober 2008 20.09.13 skrev Tomi Pakarinen:
> That is ok. I just didn't notice that method before. But, what if
> someone wants localize this plugin, he'll propably have to explode
> this back to switch statement.
No, we'd localize getDescription().
-- robin
^ permalink raw reply
* git 1.6.0.2 make test fails at t1301 under mac os x 10.4.
From: Fergus McMenemie @ 2008-10-21 18:32 UTC (permalink / raw)
To: git
Hi,
Hoping for a quick pointer. I compiled git 1.6.0.2 on three
macs. Works fine on two fails on one. It only fails at test
t1301-shared-repo.sh all other tests pass.
Imac 2.16GHz Intel Core 2 Duo running MacOS X 10.4.11 - OK
MacMini 1.5Ghz PowerPC G4 running MacOS X 10.4.11 - Duff
Powerbook 1Ghz PowerPC G4 running MacOX X 10.5.5 - OK
All disks are formatted "Journaled HFS+". I have the same
version of gcc and the development environment on both the
tiger machines. Here is a trace from the machine and test
that fails:-
>fergus:bash -x t1301-shared-repo.sh -d -v -i
>+ test_description=Test shared repository initialization
>+ . ./test-lib.sh
>++ ORIGINAL_TERM=xterm-color
>++ LANG=C
>++ LC_ALL=C
>++ PAGER=cat
>++ TZ=UTC
>++ TERM=dumb
>++ export LANG LC_ALL PAGER TERM TZ
>++ EDITOR=:
>++ VISUAL=:
>++ unset GIT_EDITOR
>++ unset AUTHOR_DATE
>++ unset AUTHOR_EMAIL
>++ unset AUTHOR_NAME
>++ unset COMMIT_AUTHOR_EMAIL
>++ unset COMMIT_AUTHOR_NAME
>++ unset EMAIL
>++ unset GIT_ALTERNATE_OBJECT_DIRECTORIES
>++ unset GIT_AUTHOR_DATE
>++ GIT_AUTHOR_EMAIL=author@example.com
>++ GIT_AUTHOR_NAME=A U Thor
>++ unset GIT_COMMITTER_DATE
>++ GIT_COMMITTER_EMAIL=committer@example.com
>++ GIT_COMMITTER_NAME=C O Mitter
>++ unset GIT_DIFF_OPTS
>++ unset GIT_DIR
>++ unset GIT_WORK_TREE
>++ unset GIT_EXTERNAL_DIFF
>++ unset GIT_INDEX_FILE
>++ unset GIT_OBJECT_DIRECTORY
>++ unset GIT_CEILING_DIRECTORIES
>++ unset SHA1_FILE_DIRECTORIES
>++ unset SHA1_FILE_DIRECTORY
>++ GIT_MERGE_VERBOSITY=5
>++ export GIT_MERGE_VERBOSITY
>++ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
>++ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
>++ export EDITOR VISUAL
>++ GIT_TEST_CMP=diff -u
>++ unset CDPATH
>+++ echo
>+++ tr '[A-Z]' '[a-z]'
>++ '[' xxterm-color '!=' xdumb ']'
>++ TERM=xterm-color
>++ export TERM
>++ '[' -t 1 ']'
>++ tput bold
>++ tput setaf 1
>++ tput sgr0
>++ color=t
>++ test 3 -ne 0
>++ debug=t
>++ shift
>++ test 2 -ne 0
>++ verbose=t
>++ shift
>++ test 1 -ne 0
>++ immediate=t
>++ shift
>++ test 0 -ne 0
>++ test -n t
>++ test 'Test shared repository initialization' '!=' ''
>++ test '' = t
>++ exec
>++ test t = t
>++ exec
>++ test_failure=0
>++ test_count=0
>++ test_fixed=0
>++ test_broken=0
>++ test_success=0
>++ trap die exit
>+++ pwd
>++ TEST_DIRECTORY=/usr/local/packages/git-1.6.0.2/t
>++ PATH=/usr/local/packages/git-1.6.0.2/t/..:/usr/local/bin:/usr/local/pgsql/bin:/bin:/sbin:/usr/bin:/usr/local/bin:/usr/sbin:/Volumes/TechmorePB/fergus/Current\ Contract/cwid-fixup/rollitts
>+++ pwd
>++ GIT_EXEC_PATH=/usr/local/packages/git-1.6.0.2/t/..
>+++ pwd
>++ GIT_TEMPLATE_DIR=/usr/local/packages/git-1.6.0.2/t/../templates/blt
>++ unset GIT_CONFIG
>++ unset GIT_CONFIG_LOCAL
>++ GIT_CONFIG_NOSYSTEM=1
>++ GIT_CONFIG_NOGLOBAL=1
>++ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
>+++ pwd
>+++ pwd
>++ GITPERLLIB=/usr/local/packages/git-1.6.0.2/t/../perl/blib/lib:/usr/local/packages/git-1.6.0.2/t/../perl/blib/arch/auto/Git
>++ export GITPERLLIB
>++ test -d ../templates/blt
>++ test -x ../test-chmtime
>++ . ../GIT-BUILD-OPTIONS
>+++ SHELL_PATH=/bin/sh
>+++ TAR=tar
>++ test=trash directory
>++ rm -fr 'trash directory'
>++ test_create_repo 'trash directory'
>++ test 1 = 1
>+++ pwd
>++ owd=/usr/local/packages/git-1.6.0.2/t
>++ repo=trash directory
>++ mkdir 'trash directory'
>++ cd 'trash directory'
>++ /usr/local/packages/git-1.6.0.2/t/../git init --template=/usr/local/packages/git-1.6.0.2/t/../templates/blt/
>Initialized empty Git repository in /usr/local/packages/git-1.6.0.2/t/trash directory/.git/
>++ mv .git/hooks .git/hooks-disabled
>++ cd /usr/local/packages/git-1.6.0.2/t
>++ cd -P 'trash directory'
>+++ expr ./t1301-shared-repo.sh : '.*/\(t[0-9]*\)-[^/]*$'
>++ this_test=t1301
>+ test_expect_success 'shared = 0400 (faulty permission u-w)' '
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>+ test 2 = 2
>+ test_skip 'shared = 0400 (faulty permission u-w)' '
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>++ expr ./t1301-shared-repo.sh : '.*/\(t[0-9]*\)-[^/]*$'
>+ this_test=t1301
>++ expr 0 + 1
>+ this_test=t1301.1
>+ to_skip=
>+ false
>+ say 'expecting success:
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>+ say_color info 'expecting success:
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>+ TERM=xterm-color
>+ export TERM
>+ tput setaf 3
>+ shift
>+ echo '* expecting success:
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>* expecting success:
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>
>+ tput sgr0
>+ test_run_ '
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>+ eval '
> mkdir sub && (
> cd sub && git init --shared=0400
> )
> ret="$?"
> rm -rf sub
> test $ret != "0"
>'
>++ mkdir sub
>++ cd sub
>++ git init --shared=0400
>fatal: Problem with core.sharedRepository filemode value (0400).
>The owner of files must always have read and write permissions.
>++ ret=128
>++ rm -rf sub
>++ test 128 '!=' 0
>+ eval_ret=0
>+ return 0
>+ '[' 0 = 0 -a 0 = 0 ']'
>+ test_ok_ 'shared = 0400 (faulty permission u-w)'
>++ expr 0 + 1
>+ test_count=1
>++ expr 0 + 1
>+ test_success=1
>+ say_color '' ' ok 1: shared = 0400 (faulty permission u-w)'
>+ TERM=xterm-color
>+ export TERM
>+ test -n ''
>+ shift
>+ echo '* ok 1: shared = 0400 (faulty permission u-w)'
>* ok 1: shared = 0400 (faulty permission u-w)
>+ tput sgr0
>+ echo ''
>
>+ test_expect_success 'shared=1 does not clear bits preset by umask 002' '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>+ test 2 = 2
>+ test_skip 'shared=1 does not clear bits preset by umask 002' '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>++ expr ./t1301-shared-repo.sh : '.*/\(t[0-9]*\)-[^/]*$'
>+ this_test=t1301
>++ expr 1 + 1
>+ this_test=t1301.2
>+ to_skip=
>+ false
>+ say 'expecting success:
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>+ say_color info 'expecting success:
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>+ TERM=xterm-color
>+ export TERM
>+ tput setaf 3
>+ shift
>+ echo '* expecting success:
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>* expecting success:
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
>
>+ tput sgr0
>+ test_run_ '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>+ eval '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>++ mkdir sub
>++ cd sub
>++ umask 002
>++ git init --shared=1
>fatal: Could not make /usr/local/packages/git-1.6.0.2/t/trash directory/sub/.git/refs writable by group
>
>++ echo Oops, .git/HEAD is not 0664 but
>Oops, .git/HEAD is not 0664 but
>++ false
>+ eval_ret=1
>+ return 0
>+ '[' 0 = 0 -a 1 = 0 ']'
>+ test_failure_ 'shared=1 does not clear bits preset by umask 002' '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>++ expr 1 + 1
>+ test_count=2
>++ expr 0 + 1
>+ test_failure=1
>+ say_color error 'FAIL 2: shared=1 does not clear bits preset by umask 002'
>+ TERM=xterm-color
>+ export TERM
>+ tput bold
>+ tput setaf 1
>+ shift
>+ echo '* FAIL 2: shared=1 does not clear bits preset by umask 002'
>* FAIL 2: shared=1 does not clear bits preset by umask 002
>+ tput sgr0
>+ shift
>+ echo '
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
> '
>+ sed -e 's/^/ /'
>
> mkdir sub && (
> cd sub &&
> umask $u &&
> git init --shared=1 &&
> test 1 = "$(git config core.sharedrepository)"
> ) &&
> actual=$(ls -l sub/.git/HEAD)
> case "$actual" in
> -rw-rw-r--*)
> : happy
> ;;
> *)
> echo Oops, .git/HEAD is not 0664 but $actual
> false
> ;;
> esac
>
>+ test t = ''
>+ trap - exit
>+ exit 1
>fergus:
Doing this manually I get:-
fergus:mkdir sub
fergus:cd sub
fergus:../../git init --shared=0400
fatal: Problem with core.sharedRepository filemode value (0400).
The owner of files must always have read and write permissions.
fergus:echo $?
128
fergus:ls -al
total 0
drwxr-xr-x 3 fergus wheel 102 Oct 21 18:24 .
drwxr-xr-x 333 fergus wheel 11322 Oct 21 18:23 ..
drwxr-xr-x 2 fergus wheel 68 Oct 21 18:24 sub
fergus:ls -al sub
total 0
drwxr-xr-x 2 fergus wheel 68 Oct 21 18:24 .
drwxr-xr-x 3 fergus wheel 102 Oct 21 18:24 ..
Any ideas where is going on? I have the same version of gcc
and the development environment on both the tiger machines.
--
===============================================================
Fergus McMenemie Email:fergus@twig.me.uk
Techmore Ltd Phone:(UK) 07721 376021
Unix/Mac/Intranets Analyst Programmer
===============================================================
^ permalink raw reply
* [PATCHv7 0/5] gitweb: PATH_INFO enhancement
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
Sevent attempt for my gitweb PATH_INFO patchset, whose purpose is to
reduce the use of CGI parameters by embedding as many parameters as
possible in the URL path itself, provided the pathinfo feature is
enabled.
The new typical gitweb URL is therefore in the form
$project/$action/$parent:$file..$hash:$file
(with useless parts stripped). Backwards compatibility for old-style
$project/$hash[:$file] URLs is kept, as long as $hash is not a refname
whose name happens to match a git action.
The main implementation is provided by paired patches (#1#2, #4#5)
that implement parsing and generation of the new style URLs.
Patch #3 is a minor improvement to the URL syntax that allows web
sites to be properly browsable in raw mode.
The patchset depends on my previous input parameter handling patch
currently waiting in 'next'. This resend fixes some mainly aesthetical
issues spotted by Jakub
Giuseppe Bilotta (5):
gitweb: parse project/action/hash_base:filename PATH_INFO
gitweb: generate project/action/hash URLs
gitweb: use_pathinfo filenames start with /
gitweb: parse parent..current syntax from PATH_INFO
gitweb: generate parent..current URLs
gitweb/gitweb.perl | 133 +++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 122 insertions(+), 11 deletions(-)
^ permalink raw reply
* [PATCHv7 2/5] gitweb: generate project/action/hash URLs
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1224617694-29277-2-git-send-email-giuseppe.bilotta@gmail.com>
When generating path info URLs, reduce the number of CGI parameters by
embedding action and hash_parent:filename or hash in the path.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 33 ++++++++++++++++++++++++++++++---
1 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1f847bb..27587aa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -731,14 +731,41 @@ sub href (%) {
my ($use_pathinfo) = gitweb_check_feature('pathinfo');
if ($use_pathinfo) {
- # use PATH_INFO for project name
+ # try to put as many parameters as possible in PATH_INFO:
+ # - project name
+ # - action
+ # - hash or hash_base:filename
+
+ # When the script is the root DirectoryIndex for the domain,
+ # $href here would be something like http://gitweb.example.com/
+ # Thus, we strip any trailing / from $href, to spare us double
+ # slashes in the final URL
+ $href =~ s,/$,,;
+
+ # Then add the project name, if present
$href .= "/".esc_url($params{'project'}) if defined $params{'project'};
delete $params{'project'};
- # Summary just uses the project path URL
- if (defined $params{'action'} && $params{'action'} eq 'summary') {
+ # Summary just uses the project path URL, any other action is
+ # added to the URL
+ if (defined $params{'action'}) {
+ $href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
delete $params{'action'};
}
+
+ # Finally, we put either hash_base:file_name or hash
+ if (defined $params{'hash_base'}) {
+ $href .= "/".esc_url($params{'hash_base'});
+ if (defined $params{'file_name'}) {
+ $href .= ":".esc_url($params{'file_name'});
+ delete $params{'file_name'};
+ }
+ delete $params{'hash'};
+ delete $params{'hash_base'};
+ } elsif (defined $params{'hash'}) {
+ $href .= "/".esc_url($params{'hash'});
+ delete $params{'hash'};
+ }
}
# now encode the parameters explicitly
--
1.5.6.5
^ permalink raw reply related
* [PATCHv7 1/5] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1224617694-29277-1-git-send-email-giuseppe.bilotta@gmail.com>
This patch enables gitweb to parse URLs with more information embedded
in PATH_INFO, reducing the need for CGI parameters. The typical gitweb
path is now $project/$action/$hash_base:$file_name or
$project/$action/$hash
This is mostly backwards compatible with the old-style gitweb paths,
$project/$branch[:$filename], except when it was used to access a branch
whose name matches a gitweb action.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 46 +++++++++++++++++++++++++++++++++++++++-------
1 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 98f1bfa..1f847bb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -534,23 +534,55 @@ sub evaluate_path_info {
return if $input_params{'action'};
$path_info =~ s,^\Q$project\E/*,,;
+ # next, check if we have an action
+ my $action = $path_info;
+ $action =~ s,/.*$,,;
+ if (exists $actions{$action}) {
+ $path_info =~ s,^$action/*,,;
+ $input_params{'action'} = $action;
+ }
+
+ # list of actions that want hash_base instead of hash, but can have no
+ # pathname (f) parameter
+ my @wants_base = (
+ 'tree',
+ 'history',
+ );
+
my ($refname, $pathname) = split(/:/, $path_info, 2);
if (defined $pathname) {
- # we got "project.git/branch:filename" or "project.git/branch:dir/"
- # we could use git_get_type(branch:pathname), but it needs $git_dir
+ # we got "branch:filename" or "branch:dir/"
+ # we could use git_get_type(branch:pathname), but:
+ # - it needs $git_dir
+ # - it does a git() call
+ # - the convention of terminating directories with a slash
+ # makes it superfluous
+ # - embedding the action in the PATH_INFO would make it even
+ # more superfluous
$pathname =~ s,^/+,,;
if (!$pathname || substr($pathname, -1) eq "/") {
- $input_params{'action'} = "tree";
+ $input_params{'action'} ||= "tree";
$pathname =~ s,/$,,;
} else {
- $input_params{'action'} = "blob_plain";
+ $input_params{'action'} ||= "blob_plain";
}
$input_params{'hash_base'} ||= $refname;
$input_params{'file_name'} ||= $pathname;
} elsif (defined $refname) {
- # we got "project.git/branch"
- $input_params{'action'} = "shortlog";
- $input_params{'hash'} ||= $refname;
+ # we got "branch". In this case we have to choose if we have to
+ # set hash or hash_base.
+ #
+ # Most of the actions without a pathname only want hash to be
+ # set, except for the ones specified in @wants_base that want
+ # hash_base instead. It should also be noted that hand-crafted
+ # links having 'history' as an action and no pathname or hash
+ # set will fail, but that happens regardless of PATH_INFO.
+ $input_params{'action'} ||= "shortlog";
+ if (grep { $_ eq $input_params{'action'} } @wants_base) {
+ $input_params{'hash_base'} ||= $refname;
+ } else {
+ $input_params{'hash'} ||= $refname;
+ }
}
}
evaluate_path_info();
--
1.5.6.5
^ permalink raw reply related
* [PATCHv7 3/5] gitweb: use_pathinfo filenames start with /
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1224617694-29277-3-git-send-email-giuseppe.bilotta@gmail.com>
Generate PATH_INFO URLs in the form project/action/hash_base:/filename
rather than project/action/hash_base:filename (the latter form is still
accepted in input).
This minimal change allows relative navigation to work properly when
viewing HTML files in raw ('blob_plain') mode.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 27587aa..9da547d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -734,7 +734,7 @@ sub href (%) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
# - action
- # - hash or hash_base:filename
+ # - hash or hash_base:/filename
# When the script is the root DirectoryIndex for the domain,
# $href here would be something like http://gitweb.example.com/
@@ -753,11 +753,11 @@ sub href (%) {
delete $params{'action'};
}
- # Finally, we put either hash_base:file_name or hash
+ # Finally, we put either hash_base:/file_name or hash
if (defined $params{'hash_base'}) {
$href .= "/".esc_url($params{'hash_base'});
if (defined $params{'file_name'}) {
- $href .= ":".esc_url($params{'file_name'});
+ $href .= ":/".esc_url($params{'file_name'});
delete $params{'file_name'};
}
delete $params{'hash'};
--
1.5.6.5
^ permalink raw reply related
* [PATCHv7 4/5] gitweb: parse parent..current syntax from PATH_INFO
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1224617694-29277-4-git-send-email-giuseppe.bilotta@gmail.com>
This patch makes it possible to use an URL such as
project/action/somebranch..otherbranch:/filename to get a diff between
different version of a file. Paths like
project/action/somebranch:/somefile..otherbranch:/otherfile are parsed
as well.
All '*diff' actions and in general actions that use $hash_parent[_base]
and $file_parent (e.g. 'shortlog') can now get all of their parameters
from PATH_INFO
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 36 ++++++++++++++++++++++++++++++++++--
1 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9da547d..59449de 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -549,7 +549,12 @@ sub evaluate_path_info {
'history',
);
- my ($refname, $pathname) = split(/:/, $path_info, 2);
+ # we want to catch
+ # [$hash_parent_base[:$file_parent]..]$hash_parent[:$file_name]
+ my ($parentrefname, $parentpathname, $refname, $pathname) =
+ ($path_info =~ /^(?:(.+?)(?::(.+))?\.\.)?(.+?)(?::(.+))?$/);
+
+ # first, analyze the 'current' part
if (defined $pathname) {
# we got "branch:filename" or "branch:dir/"
# we could use git_get_type(branch:pathname), but:
@@ -564,7 +569,13 @@ sub evaluate_path_info {
$input_params{'action'} ||= "tree";
$pathname =~ s,/$,,;
} else {
- $input_params{'action'} ||= "blob_plain";
+ # the default action depends on whether we had parent info
+ # or not
+ if ($parentrefname) {
+ $input_params{'action'} ||= "blobdiff_plain";
+ } else {
+ $input_params{'action'} ||= "blob_plain";
+ }
}
$input_params{'hash_base'} ||= $refname;
$input_params{'file_name'} ||= $pathname;
@@ -584,6 +595,27 @@ sub evaluate_path_info {
$input_params{'hash'} ||= $refname;
}
}
+
+ # next, handle the 'parent' part, if present
+ if (defined $parentrefname) {
+ # a missing pathspec defaults to the 'current' filename, allowing e.g.
+ # someproject/blobdiff/oldrev..newrev:/filename
+ if ($parentpathname) {
+ $parentpathname =~ s,^/+,,;
+ $parentpathname =~ s,/$,,;
+ $input_params{'file_parent'} ||= $parentpathname;
+ } else {
+ $input_params{'file_parent'} ||= $input_params{'file_name'};
+ }
+ # we assume that hash_parent_base is wanted if a path was specified,
+ # or if the action wants hash_base instead of hash
+ if (defined $input_params{'file_parent'} ||
+ grep { $_ eq $input_params{'action'} } @wants_base) {
+ $input_params{'hash_parent_base'} ||= $parentrefname;
+ } else {
+ $input_params{'hash_parent'} ||= $parentrefname;
+ }
+ }
}
evaluate_path_info();
--
1.5.6.5
^ permalink raw reply related
* [PATCHv7 5/5] gitweb: generate parent..current URLs
From: Giuseppe Bilotta @ 2008-10-21 19:34 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1224617694-29277-5-git-send-email-giuseppe.bilotta@gmail.com>
If use_pathinfo is enabled, href now creates links that contain paths in
the form $project/$action/oldhash:/oldname..newhash:/newname for actions
that use hash_parent etc.
If any of the filename contains two consecutive dots, it's kept as a CGI
parameter since the resulting path would otherwise be ambiguous.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 28 ++++++++++++++++++++++++----
1 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 59449de..9d1af7e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -766,6 +766,7 @@ sub href (%) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
# - action
+ # - hash_parent or hash_parent_base:/file_parent
# - hash or hash_base:/filename
# When the script is the root DirectoryIndex for the domain,
@@ -785,17 +786,36 @@ sub href (%) {
delete $params{'action'};
}
- # Finally, we put either hash_base:/file_name or hash
+ # Next, we put hash_parent_base:/file_parent..hash_base:/file_name,
+ # stripping nonexistent or useless pieces
+ $href .= "/" if ($params{'hash_base'} || $params{'hash_parent_base'}
+ || $params{'hash_parent'} || $params{'hash'});
if (defined $params{'hash_base'}) {
- $href .= "/".esc_url($params{'hash_base'});
- if (defined $params{'file_name'}) {
+ if (defined $params{'hash_parent_base'}) {
+ $href .= esc_url($params{'hash_parent_base'});
+ # skip the file_parent if it's the same as the file_name
+ delete $params{'file_parent'} if $params{'file_parent'} eq $params{'file_name'};
+ if (defined $params{'file_parent'} && $params{'file_parent'} !~ /\.\./) {
+ $href .= ":/".esc_url($params{'file_parent'});
+ delete $params{'file_parent'};
+ }
+ $href .= "..";
+ delete $params{'hash_parent'};
+ delete $params{'hash_parent_base'};
+ } elsif (defined $params{'hash_parent'}) {
+ $href .= esc_url($params{'hash_parent'}). "..";
+ delete $params{'hash_parent'};
+ }
+
+ $href .= esc_url($params{'hash_base'});
+ if (defined $params{'file_name'} && $params{'file_name'} !~ /\.\./) {
$href .= ":/".esc_url($params{'file_name'});
delete $params{'file_name'};
}
delete $params{'hash'};
delete $params{'hash_base'};
} elsif (defined $params{'hash'}) {
- $href .= "/".esc_url($params{'hash'});
+ $href .= esc_url($params{'hash'});
delete $params{'hash'};
}
}
--
1.5.6.5
^ permalink raw reply related
* [JGIT PATCH] Detect repository states according to post Git 1.5
From: Robin Rosenberg @ 2008-10-21 20:13 UTC (permalink / raw)
To: tomi.pakarinen; +Cc: spearce, git
In-Reply-To: <f299b4f30810211109q7f2919f2r1d5cd8faf0048154@mail.gmail.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
Seems we need this patch too in order to detect repository states. The new
resolution is more granular that before, though I suggest that one should
be restrictive when interpreting the state and not assume that these states
are the only ones. The methods on RepositoryState are the /only/ valid
ways of deciding what to, or not to, do. This implies we need to extend this
class somewhat, but I'm no hurry yet so we can think about what methods
we need.
.../src/org/spearce/jgit/lib/Repository.java | 17 +++++++++++++
.../src/org/spearce/jgit/lib/RepositoryState.java | 25 +++++++++++++++++++-
2 files changed, 41 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index dfce1b8..26748e2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -1017,14 +1017,31 @@ public GitIndex getIndex() throws IOException {
* @return an important state
*/
public RepositoryState getRepositoryState() {
+ // Pre Git-1.6 logic
if (new File(getWorkDir(), ".dotest").exists())
return RepositoryState.REBASING;
if (new File(gitDir,".dotest-merge").exists())
return RepositoryState.REBASING_INTERACTIVE;
+
+ // From 1.6 onwards
+ if (new File(getDirectory(),"rebase-apply/rebasing").exists())
+ return RepositoryState.REBASING_REBASING;
+ if (new File(getDirectory(),"rebase-apply/applying").exists())
+ return RepositoryState.APPLY;
+ if (new File(getDirectory(),"rebase-apply").exists())
+ return RepositoryState.REBASING;
+
+ if (new File(getDirectory(),"rebase-merge/interactive").exists())
+ return RepositoryState.REBASING_INTERACTIVE;
+ if (new File(getDirectory(),"rebase-merge").exists())
+ return RepositoryState.REBASING_MERGE;
+
+ // Both versions
if (new File(gitDir,"MERGE_HEAD").exists())
return RepositoryState.MERGING;
if (new File(gitDir,"BISECT_LOG").exists())
return RepositoryState.BISECTING;
+
return RepositoryState.SAFE;
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryState.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryState.java
index c32c381..a916924 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryState.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryState.java
@@ -40,6 +40,9 @@
/**
* Important state of the repository that affects what can and cannot bed
* done. This is things like unhandled conflicted merges and unfinished rebase.
+ *
+ * The granularity and set of states are somewhat arbitrary. The methods
+ * on the state are the only supported means of deciding what to do.
*/
public enum RepositoryState {
/**
@@ -62,7 +65,7 @@
},
/**
- * An unfinished rebase. Must resolve, skip or abort before normal work can take place
+ * An unfinished rebase or am. Must resolve, skip or abort before normal work can take place
*/
REBASING {
public boolean canCheckout() { return false; }
@@ -72,6 +75,26 @@
},
/**
+ * An unfinished rebase. Must resolve, skip or abort before normal work can take place
+ */
+ REBASING_REBASING {
+ public boolean canCheckout() { return false; }
+ public boolean canResetHead() { return false; }
+ public boolean canCommit() { return true; }
+ public String getDescription() { return "Rebase"; }
+ },
+
+ /**
+ * An unfinished apply. Must resolve, skip or abort before normal work can take place
+ */
+ APPLY {
+ public boolean canCheckout() { return false; }
+ public boolean canResetHead() { return false; }
+ public boolean canCommit() { return true; }
+ public String getDescription() { return "Apply mailbox"; }
+ },
+
+ /**
* An unfinished rebase with merge. Must resolve, skip or abort before normal work can take place
*/
REBASING_MERGE {
--
1.6.0.2.308.gef4a
^ permalink raw reply related
* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: René Scharfe @ 2008-10-21 20:45 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List, Davide Libenzi
In-Reply-To: <48FD721D.9030105@viscovery.net>
Johannes Sixt schrieb:
> René Scharfe schrieb:
>> I have to admit my main motivation was that one line gap, where a chunk
>> header hid an interesting line of context. Showing it didn't change the
>> length of the patch, so I found this to be a sad wastage.
>
> "Wastage" is relative. For a given patch, the one line of context that was
> hidden by the hunk header would be welcome by a human reader, but it is
> not necessarily useful if the patch is to be applied, in particular, if it
> is applied to a version of the file that has *more* than one line between
> the hunk contexts. This is the reason that diff does not produce 7 lines
> of context between changes in -U3 mode ("you asked for 3 lines of context,
> you get 3 lines of context").
Yes, that's an interesting example of the possible "merge conflicts" I
mentioned in my original mail, and one I didn't think of. And since I
don't do any merges myself, I don't know how much of a problem this is.
As Daniel writes, one could teach git-apply to ignore extra context. It
should even be possible to infer the -U option used to create a patch
and to remove any extra lines (which might get complicated for patches
that change both the start and the end of a file, though).
Also, I'd like to know how many patches of a given repo would be have
created such a problem, but I can't think of a way to count them at the
moment. I need some sleep first.
> BTW, nomenclature seems to have settled at the word "hunk", not "chunk".
While http://en.wikipedia.org/wiki/Diff uses both and defines "chunk" as
"change hunk", the GNU patch(1) manpage uses "hunk" throughout. "Hunk"
it is, then. :)
René
^ permalink raw reply
* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: René Scharfe @ 2008-10-21 20:48 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, Git Mailing List, Davide Libenzi
In-Reply-To: <20081021112040.GB17363@coredump.intra.peff.net>
Jeff King schrieb:
> On Tue, Oct 21, 2008 at 12:12:17AM -0700, Junio C Hamano wrote:
>
>> Yeah. René wanted this for _human consumption_, not mechanical patch
>> application, so "hardcoding" literally there in the very low level of the
>> diff callchain is not quite right (it would affect format-patch which is
>> primarily for mechanical application).
>>
>> I guess you could make the hardcoded value 1 for everybody else and 0 for
>> format-patch.
>
> I see your reasoning, but at the same time, a large portion of patches I
> read are from format-patch (and René even said that he was trying to
> save the user from the "apply then diff just to look at the patch"
> annoyance). And I have personally, as a patch submitter, created some
> format-patch output sent to the git list with -U5 to combine hunks and
> make it more readable for reviewers.
>
> Not to mention that I sometimes apply or post the output of "git diff".
Well, yes, perhaps I was trying to get ahead of myself. I sure would
like to see everyone create patches with fused hunks (because they are
easier to read), but step 1 is to have the option to create such patches
at all. We should then try it out for some time or verify its
usefulness statistically and only then turn it on by default. Or
perhaps throw it away, depending on the results.
And I consider the output of format-patch and git-diff to be intended
primarily for human consumption.
> To me that it implies that either:
>
> - the increased chance of conflict is not a problem in practice, and we
> should have the option on by default everywhere
>
> - it is a problem, in which case we should ask the user to turn on the
> feature manually instead of second-guessing how they will use the
> resulting patch (which they might not even know, since they are
> making assumptions about how other people might use the patch, and
> they must decide for their situation between shipping something that
> is more readable but slightly more conflict prone, or as easy to
> apply as possible)
To decide which one it is, I'd like to see numbers: how many times would
a patch with fused hunks have led to a problem for e.g. the kernel repo?
What is the optimal default value (0, 1, even more)? Before even
thinking about how to get these stats, I'd better head for bed for
today, though..
René
^ permalink raw reply
* [PATCH v2] builtin-blame: Reencode commit messages according to git-log rules.
From: Alexander Gavrilov @ 2008-10-21 20:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Currently git-blame outputs text from the commit messages
(e.g. the author name and the summary string) as-is, without
even providing any information about the encoding used for
the data. It makes interpreting the data in multilingual
environment very difficult.
This commit changes the blame implementation to recode the
messages using the rules used by other commands like git-log.
Namely, the target encoding can be specified through the
i18n.commitEncoding or i18n.logOutputEncoding options, or
directly on the command line using the --encoding parameter.
Converting the encoding before output seems to be more
friendly to the porcelain tools than simply providing the
value of the encoding header, and does not require changing
the output format.
If anybody needs the old behavior, it is possible to
achieve it by specifying --encoding=none.
Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
I intentionally forced binary encoding for files
with the localized strings, to avoid any possible
corruption.
-- Alexander
Documentation/blame-options.txt | 7 +++
Documentation/i18n.txt | 6 +-
builtin-blame.c | 16 +++++--
commit.h | 2 +
pretty.c | 21 ++++++---
t/t8005-blame-i18n.sh | 92 +++++++++++++++++++++++++++++++++++++++
t/t8005/cp1251.txt | Bin 0 -> 68 bytes
t/t8005/sjis.txt | Bin 0 -> 100 bytes
t/t8005/utf8.txt | Bin 0 -> 100 bytes
9 files changed, 130 insertions(+), 14 deletions(-)
create mode 100755 t/t8005-blame-i18n.sh
create mode 100644 t/t8005/cp1251.txt
create mode 100644 t/t8005/sjis.txt
create mode 100644 t/t8005/utf8.txt
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 5428111..1ab1b96 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,13 @@ of lines before or after the line given by <start>.
Show the result incrementally in a format designed for
machine consumption.
+--encoding=<encoding>::
+ Specifies the encoding used to output author names
+ and commit summaries. Setting it to `none` makes blame
+ output unconverted data. For more information see the
+ discussion about encoding in the linkgit:git-log[1]
+ manual page.
+
--contents <file>::
When <rev> is not specified, the command annotates the
changes starting backwards from the working tree copy.
diff --git a/Documentation/i18n.txt b/Documentation/i18n.txt
index d2970f8..2cdacd9 100644
--- a/Documentation/i18n.txt
+++ b/Documentation/i18n.txt
@@ -37,9 +37,9 @@ of `i18n.commitencoding` in its `encoding` header. This is to
help other people who look at them later. Lack of this header
implies that the commit log message is encoded in UTF-8.
-. 'git-log', 'git-show' and friends looks at the `encoding`
- header of a commit object, and tries to re-code the log
- message into UTF-8 unless otherwise specified. You can
+. 'git-log', 'git-show', 'git-blame' and friends look at the
+ `encoding` header of a commit object, and try to re-code the
+ log message into UTF-8 unless otherwise specified. You can
specify the desired output encoding with
`i18n.logoutputencoding` in `.git/config` file, like this:
+
diff --git a/builtin-blame.c b/builtin-blame.c
index 48cc0c1..2457e71 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1431,7 +1431,7 @@ static void get_commit_info(struct commit *commit,
int detailed)
{
int len;
- char *tmp, *endp;
+ char *tmp, *endp, *reencoded, *message;
static char author_buf[1024];
static char committer_buf[1024];
static char summary_buf[1024];
@@ -1449,24 +1449,29 @@ static void get_commit_info(struct commit *commit,
die("Cannot read commit %s",
sha1_to_hex(commit->object.sha1));
}
+ reencoded = reencode_commit_message(commit, NULL);
+ message = reencoded ? reencoded : commit->buffer;
ret->author = author_buf;
- get_ac_line(commit->buffer, "\nauthor ",
+ get_ac_line(message, "\nauthor ",
sizeof(author_buf), author_buf, &ret->author_mail,
&ret->author_time, &ret->author_tz);
- if (!detailed)
+ if (!detailed) {
+ free(reencoded);
return;
+ }
ret->committer = committer_buf;
- get_ac_line(commit->buffer, "\ncommitter ",
+ get_ac_line(message, "\ncommitter ",
sizeof(committer_buf), committer_buf, &ret->committer_mail,
&ret->committer_time, &ret->committer_tz);
ret->summary = summary_buf;
- tmp = strstr(commit->buffer, "\n\n");
+ tmp = strstr(message, "\n\n");
if (!tmp) {
error_out:
sprintf(summary_buf, "(%s)", sha1_to_hex(commit->object.sha1));
+ free(reencoded);
return;
}
tmp += 2;
@@ -1478,6 +1483,7 @@ static void get_commit_info(struct commit *commit,
goto error_out;
memcpy(summary_buf, tmp, len);
summary_buf[len] = 0;
+ free(reencoded);
}
/*
diff --git a/commit.h b/commit.h
index 4c05864..3a7b06a 100644
--- a/commit.h
+++ b/commit.h
@@ -65,6 +65,8 @@ enum cmit_fmt {
extern int non_ascii(int);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
+extern char *reencode_commit_message(const struct commit *commit,
+ const char **encoding_p);
extern void get_commit_format(const char *arg, struct rev_info *);
extern void format_commit_message(const struct commit *commit,
const void *format, struct strbuf *sb,
diff --git a/pretty.c b/pretty.c
index 1e79943..f6ff312 100644
--- a/pretty.c
+++ b/pretty.c
@@ -783,6 +783,20 @@ void pp_remainder(enum cmit_fmt fmt,
}
}
+char *reencode_commit_message(const struct commit *commit, const char **encoding_p)
+{
+ const char *encoding;
+
+ encoding = (git_log_output_encoding
+ ? git_log_output_encoding
+ : git_commit_encoding);
+ if (!encoding)
+ encoding = "utf-8";
+ if (encoding_p)
+ *encoding_p = encoding;
+ return logmsg_reencode(commit, encoding);
+}
+
void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
struct strbuf *sb, int abbrev,
const char *subject, const char *after_subject,
@@ -799,12 +813,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
return;
}
- encoding = (git_log_output_encoding
- ? git_log_output_encoding
- : git_commit_encoding);
- if (!encoding)
- encoding = "utf-8";
- reencoded = logmsg_reencode(commit, encoding);
+ reencoded = reencode_commit_message(commit, &encoding);
if (reencoded) {
msg = reencoded;
}
diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
new file mode 100755
index 0000000..4470a92
--- /dev/null
+++ b/t/t8005-blame-i18n.sh
@@ -0,0 +1,92 @@
+#!/bin/sh
+
+test_description='git blame encoding conversion'
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/t8005/utf8.txt
+. "$TEST_DIRECTORY"/t8005/cp1251.txt
+. "$TEST_DIRECTORY"/t8005/sjis.txt
+
+test_expect_success 'setup the repository' '
+ # Create the file
+ echo "UTF-8 LINE" > file &&
+ git add file &&
+ git commit --author "$UTF8_NAME <utf8@localhost>" -m "$UTF8_MSG" &&
+
+ echo "CP1251 LINE" >> file &&
+ git add file &&
+ git config i18n.commitencoding cp1251 &&
+ git commit --author "$CP1251_NAME <cp1251@localhost>" -m "$CP1251_MSG" &&
+
+ echo "SJIS LINE" >> file &&
+ git add file &&
+ git config i18n.commitencoding shift-jis &&
+ git commit --author "$SJIS_NAME <sjis@localhost>" -m "$SJIS_MSG"
+'
+
+cat >expected <<EOF
+author $SJIS_NAME
+summary $SJIS_MSG
+author $SJIS_NAME
+summary $SJIS_MSG
+author $SJIS_NAME
+summary $SJIS_MSG
+EOF
+
+test_expect_success \
+ 'blame respects i18n.commitencoding' '
+ git blame --incremental file | \
+ grep "^\(author\|summary\) " > actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<EOF
+author $CP1251_NAME
+summary $CP1251_MSG
+author $CP1251_NAME
+summary $CP1251_MSG
+author $CP1251_NAME
+summary $CP1251_MSG
+EOF
+
+test_expect_success \
+ 'blame respects i18n.logoutputencoding' '
+ git config i18n.logoutputencoding cp1251 &&
+ git blame --incremental file | \
+ grep "^\(author\|summary\) " > actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<EOF
+author $UTF8_NAME
+summary $UTF8_MSG
+author $UTF8_NAME
+summary $UTF8_MSG
+author $UTF8_NAME
+summary $UTF8_MSG
+EOF
+
+test_expect_success \
+ 'blame respects --encoding=utf-8' '
+ git blame --incremental --encoding=utf-8 file | \
+ grep "^\(author\|summary\) " > actual &&
+ test_cmp actual expected
+'
+
+cat >expected <<EOF
+author $SJIS_NAME
+summary $SJIS_MSG
+author $CP1251_NAME
+summary $CP1251_MSG
+author $UTF8_NAME
+summary $UTF8_MSG
+EOF
+
+test_expect_success \
+ 'blame respects --encoding=none' '
+ git blame --incremental --encoding=none file | \
+ grep "^\(author\|summary\) " > actual &&
+ test_cmp actual expected
+'
+
+test_done
diff --git a/t/t8005/cp1251.txt b/t/t8005/cp1251.txt
new file mode 100644
index 0000000000000000000000000000000000000000..ce41e98b811b09c135115529b41e227ce5ec4ff6
GIT binary patch
literal 68
zcmZ<`Ff=kXjQ4Z&b+uJG@#w)@h4W88eR%ii#dn2^FP^*uvXr=-A!>Ys-EEaFJ^lFU
W-J^F;6+XUu_weV_w=bS5aRC6=1t)(1
literal 0
HcmV?d00001
diff --git a/t/t8005/sjis.txt b/t/t8005/sjis.txt
new file mode 100644
index 0000000000000000000000000000000000000000..2ccfbad207c6e96b1f4f528031d9e4938d364b92
GIT binary patch
literal 100
zcmWIc@(hmmbM$q!Rci5UDQYQbsZ(ePXen)JX=!R{018yLbSkt20jUxo7c8X26%5kk
k8|)6$6AV<^3{(tK+R##}0OT|PVPQ)*P@)c~tyGB%00x;U@Bjb+
literal 0
HcmV?d00001
diff --git a/t/t8005/utf8.txt b/t/t8005/utf8.txt
new file mode 100644
index 0000000000000000000000000000000000000000..f46cfc56d80797740c3ec15e166add052f905fcb
GIT binary patch
literal 100
zcmWFyakGf`bM$q!Rk|?a!lnxwF6>pfF#p2Vi%l0BF6;ve?6}yjaADzv9T&D-*as0(
t;tB<6@(p$e>RAL-+IX=EtaRUntqK<#fy{juHeT$!u=T=Tpth|_TmU(XJDmUk
literal 0
HcmV?d00001
--
1.6.0.20.g6148bc
^ permalink raw reply related
* [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs
From: Eric Wong @ 2008-10-21 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Björn Steinbrink, git, jsogo
In-Reply-To: <20081018224728.GD3107@atjola.homenet>
Thanks to Jose Carlos Garcia Sogo and Björn Steinbrink for the
bug report.
On 2008.10.18 23:39:19 +0200, Björn Steinbrink wrote:
> Hi,
>
> Jose Carlos Garcia Sogo reported on #git that a git-svn clone of this
> svn repo fails for him:
> https://sucs.org/~welshbyte/svn/backuptool/trunk
>
> I can reproduce that here with:
> git-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)
>
> The error message I get is:
> Apache got a malformed URI: Unusable URI: it does not refer to this
> repository at /usr/local/libexec/git-core/git-svn line 4057
>
> strace revealed that git-svn url-encodes ~ while svn does not do that.
>
> For svn we have:
> write(5, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
> <S:src-path>https://sucs.org/~welshbyte/svn/backuptool/trunk</S:src-path>...
>
> While git-svn shows:
> write(7, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
> <S:src-path>https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk</S:src-path>...
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-svn.perl | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index ef6d773..a97049a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -852,7 +852,7 @@ sub escape_uri_only {
my ($uri) = @_;
my @tmp;
foreach (split m{/}, $uri) {
- s/([^\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
+ s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
push @tmp, $_;
}
join('/', @tmp);
@@ -3537,7 +3537,7 @@ sub repo_path {
sub url_path {
my ($self, $path) = @_;
if ($self->{url} =~ m#^https?://#) {
- $path =~ s/([^a-zA-Z0-9_.-])/uc sprintf("%%%02x",ord($1))/eg;
+ $path =~ s/([^~a-zA-Z0-9_.-])/uc sprintf("%%%02x",ord($1))/eg;
}
$self->{url} . '/' . $self->repo_path($path);
}
@@ -3890,7 +3890,7 @@ sub escape_uri_only {
my ($uri) = @_;
my @tmp;
foreach (split m{/}, $uri) {
- s/([^\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
+ s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
push @tmp, $_;
}
join('/', @tmp);
--
Eric Wong
^ permalink raw reply related
* Re: git 1.6.0.2 make test fails at t1301 under mac os x 10.4.
From: Johannes Schindelin @ 2008-10-21 21:24 UTC (permalink / raw)
To: Fergus McMenemie; +Cc: git
In-Reply-To: <p06240800c523ac94cb4d@[192.168.47.9]>
Hi,
On Tue, 21 Oct 2008, Fergus McMenemie wrote:
> >++ mkdir sub
> >++ cd sub
> >++ umask 002
> >++ git init --shared=1
> >fatal: Could not make /usr/local/packages/git-1.6.0.2/t/trash
> >directory/sub/.git/refs writable by group
I guess this is the problem.
Could you inspect what owner/group that directory has?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs
From: Junio C Hamano @ 2008-10-21 21:53 UTC (permalink / raw)
To: Eric Wong; +Cc: Björn Steinbrink, git, jsogo
In-Reply-To: <20081021211131.GA21606@yp-box.dyndns.org>
Eric Wong <normalperson@yhbt.net> writes:
>> strace revealed that git-svn url-encodes ~ while svn does not do that.
>>
>> For svn we have:
>> write(5, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
>> <S:src-path>https://sucs.org/~welshbyte/svn/backuptool/trunk</S:src-path>...
>>
>> While git-svn shows:
>> write(7, "<S:update-report send-all=\"true\" xmlns:S=\"svn:\">
>> <S:src-path>https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk</S:src-path>...
This looks like an XML based request sequence to me (and svn is talking
WebDAV here, right?); it makes me wonder what exact quoting rules are used
there. I would expect $path in <S:src-path>$path</S:src-path> to quote
a letters in it e.g. '<' as "<" --- which is quite different from what
the s/// substitutions in the patch seem to be doing.
> diff --git a/git-svn.perl b/git-svn.perl
> index ef6d773..a97049a 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -852,7 +852,7 @@ sub escape_uri_only {
> - s/([^\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
> + s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
Admittedly I do not know git-svn (nor Perl svn bindings it uses), and I
suspect that some of the XML-level escaping is done in the libsvn side,
but it would be nice if somebody can at least verify that the code after
the patch works with repositories with funny characters in pathnames
(perhaps list all the printables including "<&>?*!@.+-%^"). Even nicer
would be a log message that says "the resulting code covers all cases
because it follows _that_ spec to escape _all_ problematic letters",
pointing at some in svn (or libsvn-perl) resource.
The patch may make a path with '~' work, but it (neither in the patch text
nor in the commit log message) does not have much to give readers enough
confidence that the code after the patch is the _final_ one, as opposed to
being just a band-aid for a single symptom that happened to have been
discovered this time.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox