* [PATCH] run-command.c: remove run_command_v_opt_cd()
From: Nanako Shiraishi @ 2008-10-02 10:14 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
This function is not used anywhere.
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
run-command.c | 8 --------
run-command.h | 1 -
2 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/run-command.c b/run-command.c
index caab374..c90cdc5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -273,14 +273,6 @@ int run_command_v_opt(const char **argv, int opt)
return run_command(&cmd);
}
-int run_command_v_opt_cd(const char **argv, int opt, const char *dir)
-{
- struct child_process cmd;
- prepare_run_command_v_opt(&cmd, argv, opt);
- cmd.dir = dir;
- return run_command(&cmd);
-}
-
int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env)
{
struct child_process cmd;
diff --git a/run-command.h b/run-command.h
index 4f2b7d7..a8b0c20 100644
--- a/run-command.h
+++ b/run-command.h
@@ -53,7 +53,6 @@ int run_command(struct child_process *);
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
#define RUN_COMMAND_STDOUT_TO_STDERR 4
int run_command_v_opt(const char **argv, int opt);
-int run_command_v_opt_cd(const char **argv, int opt, const char *dir);
/*
* env (the environment) is to be formatted like environ: "VAR=VALUE".
--
1.6.0.2
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* [PATCH] dir.c: make dir_add_name() and dir_add_ignored() static
From: Nanako Shiraishi @ 2008-10-02 10:14 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
These functions are not used by any other file.
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
dir.c | 4 ++--
dir.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index acf1001..f79ec61 100644
--- a/dir.c
+++ b/dir.c
@@ -382,7 +382,7 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)
return ent;
}
-struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
+static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)
{
if (cache_name_exists(pathname, len, ignore_case))
return NULL;
@@ -391,7 +391,7 @@ struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int
return dir->entries[dir->nr++] = dir_entry_new(pathname, len);
}
-struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
+static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
{
if (cache_name_pos(pathname, len) >= 0)
return NULL;
diff --git a/dir.h b/dir.h
index 2df15de..c98ad98 100644
--- a/dir.h
+++ b/dir.h
@@ -73,7 +73,6 @@ extern void add_excludes_from_file(struct dir_struct *, const char *fname);
extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *which);
extern int file_exists(const char *);
-extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len);
extern char *get_relative_cwd(char *buffer, int size, const char *dir);
extern int is_inside_dir(const char *dir);
--
1.6.0.2
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* Re: [PATCH] Solaris: Use OLD_ICONV to avoid compile warnings
From: David Soria Parra @ 2008-10-02 10:09 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20081002010816.GA27415@coredump.intra.peff.net>
Jeff King schrieb:
> Do you also unset NEEDS_LIBICONV (and which version of Solaris are you
> running)? Our Makefile sets it to "yes" for Solaris 8, but my build box
> requires that it be unset. I'm not sure if my setup is somehow
> deficient, or if it is only other versions that need it.
I'm using OpenSolaris which is identified as SunOS 5.11. Therefore the
Makefile doesn't set NEEDS_LIBICONV. I'm not sure if it's needed on
Solaris 8 or not, but it's not needed on OpenSolaris.
David
^ permalink raw reply
* Re: Git commit hash clash prevention
From: Johannes Schindelin @ 2008-10-02 10:07 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list
In-Reply-To: <20081002085358.GA5342@lapse.rw.madduck.net>
Hi,
On Thu, 2 Oct 2008, martin f krafft wrote:
> the other day during a workshop on Git, one of the attendants asked
> about the scenario when two developers, Jane and David, both working on
> the same project, both create a commit and the two just so happen to
> have the same SHA-1. I realise that the likelihood of this happening is
> about as high as the chance of <insert witty joke here>, but it *is*
> possible, isn't it? Even though this is thus somewhat academic, I am
> still very curious about it.
It _is_ academic. Did you already discuss the chance that your wife gives
birth to a mouse? I haven't done the maths yet, but I am pretty certain
that this would be more likely than an unintended SHA-1 collision.
> What happens when David now pulls from Jane? How does Git deal with
> this?
Basically, the commit that David has will not be overwritten. So every
commit referring to Jane's commit would point to David's in his
repository.
But the more likely case (well, as likely goes) would be that either
Jane's or David's object is actually a blob. And Git would complain about
a type mismatch then.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-02 9:43 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <200810021059.19708.jnareb@gmail.com>
On Thu, Oct 2, 2008 at 10:59 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Giuseppe Bilotta wrote:
>>
>> +# dispatch
>> +my %actions = (
>> + "blame" => \&git_blame,
> [...]
>> +);
>
> I'm not sure if the '# dispatch' comment is correct here now that
> %actions hash is moved away from actual dispatch (selecting action
> to run)
Bingo.
>> @@ -519,9 +550,19 @@ sub evaluate_path_info {
>> # do not change any parameters if an action is given using the query string
>> return if $action;
>> $path_info =~ s,^\Q$project\E/*,,;
>> +
>> + # next comes the action
>> + $action = $path_info;
>> + $action =~ s,/.*$,,;
>
> I would use here pattern matching, but your code is also good and
> doesn't need changing; just for completeness below there is alternate
> solution:
>
> + $path_info =~ m,^(.*?)/,;
> + $action = $1;
Yeah, I just followed the existing code style.
>> @@ -534,8 +575,9 @@ sub evaluate_path_info {
>> $file_name ||= validate_pathname($pathname);
>> } elsif (defined $refname) {
>> # we got "project.git/branch"
>
> You meant here
>
> # we got "project.git/branch" or "project.git/action/branch"
Yes I do.
>> - $action ||= "shortlog";
>> - $hash ||= validate_refname($refname);
>> + $action ||= "shortlog";
>> + $hash ||= validate_refname($refname);
>> + $hash_base ||= validate_refname($refname);
>> }
>> }
>
> This hunk is IMHO incorrect. First, $refname is _either_ $hash, or
> $hash_base; it cannot be both. Second, in most cases (like the case
> of 'shortlog' action, either explicit or implicit) it is simply $hash;
> I think it can be $hash_base when $file_name is not set only in
> singular exception case of 'tree' view for the top tree (lack of
> filename is not an error, but is equivalent to $file_name='/').
OTOH, while setting both $hash and $hash_base has worked fine for me
so far (because the right one is automatically used and apparently
setting the other doesn't hurt), choosing which one to set is a much
hairier case. Do you have suggestions for a better way to always make
it work?
>> @@ -544,37 +586,6 @@ evaluate_path_info();
>> our $git_dir;
>> $git_dir = "$projectroot/$project" if $project;
>>
>> -# dispatch
>> -my %actions = (
> [...]
>> -);
>> -
>> if (!defined $action) {
>> if (defined $hash) {
>> $action = git_get_type($hash);
>
> I _think_ the '# dispatch' comment should be left here, and not moved
> with the %actions hash.
I agree.
>> @@ -631,8 +642,15 @@ sub href (%) {
>> if ($params{-replay}) {
>> while (my ($name, $symbol) = each %mapping) {
>> if (!exists $params{$name}) {
>> - # to allow for multivalued params we use arrayref form
>> - $params{$name} = [ $cgi->param($symbol) ];
>> + # the parameter we want to recycle may be either part of the
>> + # list of CGI parameter, or recovered from PATH_INFO
>> + if ($cgi->param($symbol)) {
>> + # to allow for multivalued params we use arrayref form
>> + $params{$name} = [ $cgi->param($symbol) ];
>> + } else {
>> + no strict 'refs';
>> + $params{$name} = $$name if $$name;
>
> I would _perhaps_ add here comment that multivalued parameters can come
> only from CGI query string, so there is no need for something like:
>
> + $params{$name} = (ref($$name) ? @$name : $$name) if $$name;
>
>> + }
>> }
>> }
>> }
>
> This fragment is a bit of ugly code, hopefully corrected in later patch.
> I think it would be better to have 'refactor parsing/validation of input
> parameters' to be very fist patch in series; I am not sure but I suspect
> that is a kind of bugfix for current "$project/$hash" ('shortlog' view)
> and "$project/$hash_base:$file_name" ('blob_plain' and 'tree' view)
> path_info.
But implementing the path_info parsing first makes the input param
refactoring SO much nicer that I would rather put a comment here
saying "this code sucks: we should rather collect all input
parameters" and then clean it up on the subsequent patch.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: Git commit hash clash prevention
From: Thomas Rast @ 2008-10-02 9:18 UTC (permalink / raw)
To: martin f krafft; +Cc: git discussion list
In-Reply-To: <20081002085358.GA5342@lapse.rw.madduck.net>
[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]
martin f krafft wrote:
> the other day during a workshop on Git, one of the attendants asked
> about the scenario when two developers, Jane and David, both working
> on the same project, both create a commit and the two just so happen
> to have the same SHA-1. I realise that the likelihood of this
> happening is about as high as the chance of <insert witty joke
> here>, but it *is* possible, isn't it? Even though this is thus
> somewhat academic, I am still very curious about it.
>
> What happens when David now pulls from Jane? How does Git deal with
> this?
There are two cases:
* The commits are exactly identical. This won't happen in your
scenario, but is still theoretically possible if you commit the same
tree with the same author info, timestamps, etc. on two different
machines. Then there is no problem, because they really are the
same.
* They're not identical, but there is a hash collision. Git will
become very confused because it only ever saves one of them. (I
suppose it'd "only" corrupt the DAG if the two are commits, but in
the general case a commit could collide with a tree etc.)
However, the expected number of objects needed to get a collision is
on the order of 2**80 (http://en.wikipedia.org/wiki/Birthday_attack),
and since there are (very roughly) 2**25 seconds in a year and 2**34
years in the age of the universe, that still leaves you with 2**21
ages of the universe to go.
(I hope I did the counting right...)
> I imagine it'll be able to distinguish the two commits based on
> metadata, but won't the DAG get corrupted?
No, it does not distinguish between objects in any way but the SHA1.
- Thomas
--
Thomas Rast
trast@student.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH v3] tests: add a testcase for "git submodule sync"
From: David Aguilar @ 2008-10-02 9:11 UTC (permalink / raw)
To: spearce; +Cc: git, mlevedahl, gitster, David Aguilar
This testcase ensures that upstream changes to submodule properties
can be updated using the sync subcommand. This particular test
changes the submodule URL upstream and uses the sync command to update
an existing checkout.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
sorry for the noise, :-/ please disregard the last two emails
t/t7403-submodule-sync.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
new file mode 100755
index 0000000..d9e04d4
--- /dev/null
+++ b/t/t7403-submodule-sync.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 David Aguilar
+#
+
+test_description='git submodule sync
+
+These tests exercise the "git submodule sync" subcommand.
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ git clone . super &&
+ git clone super submodule &&
+ (cd super &&
+ git submodule add ../submodule submodule &&
+ test_tick &&
+ git commit -m "submodule"
+ ) &&
+ git clone super super-clone &&
+ (cd super-clone && git submodule update --init)
+'
+
+test_expect_success 'change submodule' '
+ (cd submodule &&
+ echo second line >> file &&
+ test_tick &&
+ git commit -a -m "change submodule"
+ )
+'
+
+test_expect_success 'change submodule url' '
+ (cd super &&
+ cd submodule &&
+ git checkout master &&
+ git pull
+ ) &&
+ mv submodule moved-submodule &&
+ (cd super &&
+ git config -f .gitmodules submodule.submodule.url ../moved-submodule
+ test_tick &&
+ git commit -a -m moved-submodule
+ )
+'
+
+test_expect_success '"git submodule sync" should update submodule URLs' '
+ (cd super-clone &&
+ git pull &&
+ git submodule sync
+ ) &&
+ test -d "$(git config -f super-clone/submodule/.git/config \
+ remote.origin.url)" &&
+ (cd super-clone/submodule &&
+ git checkout master &&
+ git pull
+ )
+'
+
+test_done
--
1.6.0.2.428.g5e22e
^ permalink raw reply related
* [PATCH v2] tests: add a testcase for "git submodule sync"
From: David Aguilar @ 2008-10-02 8:55 UTC (permalink / raw)
To: spearce; +Cc: git, mlevedahl, gitster, David Aguilar
This testcase ensures that upstream changes to submodule properties
can be updated using the sync subcommand. This particular test
changes the submodule URL upstream and uses the sync command to update
an existing checkout.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
t/t7403-submodule-sync.sh | 71 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
new file mode 100755
index 0000000..c034c9e
--- /dev/null
+++ b/t/t7403-submodule-sync.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 David Aguilar
+#
+
+test_description='git submodule sync
+
+These tests exercise the "git submodule sync" subcommand.
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ mkdir upstream &&
+ (cd upstream &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone upstream super &&
+ git clone super submodule &&
+ (cd super &&
+ git submodule add ../submodule submodule &&
+ git commit -m "submodule"
+ ) &&
+ git clone super super-clone &&
+ (cd super-clone && git submodule update --init)
+'
+
+test_expect_success 'change submodule' '
+ (cd submodule &&
+ echo second line >> file &&
+ test_tick &&
+ git commit -a -m "change submodule"
+ )
+'
+
+test_expect_success 'change submodule url' '
+ (cd super &&
+ cd submodule &&
+ git checkout master &&
+ test_tick &&
+ git pull
+ ) &&
+ mv submodule moved-submodule &&
+ (cd super &&
+ git config -f .gitmodules submodule.submodule.url ../moved-submodule
+ test_tick &&
+ git commit -a -m moved-submodule
+ )
+'
+
+test_expect_success '"git submodule sync" should update submodule URLs' '
+ (cd super-clone &&
+ git pull &&
+ test_tick &&
+ git submodule sync
+ ) &&
+ test_tick &&
+ test -d "$(git config -f super-clone/submodule/.git/config \
+ remote.origin.url)" &&
+ (cd super-clone/submodule &&
+ git checkout master &&
+ test_tick &&
+ git pull
+ )
+'
+
+test_done
--
1.6.0.2.428.g5e22e
^ permalink raw reply related
* Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Jakub Narebski @ 2008-10-02 8:59 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1222906234-8182-2-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta wrote:
> 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,
> except when $project/$branch was used to access a branch whose name
> matches a gitweb action.
Nice summary.
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> gitweb/gitweb.perl | 90 +++++++++++++++++++++++++++++++---------------------
> 1 files changed, 54 insertions(+), 36 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e7e4d6b..f088681 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -495,6 +495,37 @@ if (defined $searchtext) {
> $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
> }
>
> +# dispatch
> +my %actions = (
> + "blame" => \&git_blame,
[...]
> +);
I'm not sure if the '# dispatch' comment is correct here now that
%actions hash is moved away from actual dispatch (selecting action
to run)
> @@ -519,9 +550,19 @@ sub evaluate_path_info {
> # do not change any parameters if an action is given using the query string
> return if $action;
> $path_info =~ s,^\Q$project\E/*,,;
> +
> + # next comes the action
> + $action = $path_info;
> + $action =~ s,/.*$,,;
I would use here pattern matching, but your code is also good and
doesn't need changing; just for completeness below there is alternate
solution:
+ $path_info =~ m,^(.*?)/,;
+ $action = $1;
> + if (exists $actions{$action}) {
> + $path_info =~ s,^$action/*,,;
> + } else {
> + $action = undef;
> + }
> +
[...]
> @@ -534,8 +575,9 @@ sub evaluate_path_info {
> $file_name ||= validate_pathname($pathname);
> } elsif (defined $refname) {
> # we got "project.git/branch"
You meant here
# we got "project.git/branch" or "project.git/action/branch"
> - $action ||= "shortlog";
> - $hash ||= validate_refname($refname);
> + $action ||= "shortlog";
> + $hash ||= validate_refname($refname);
> + $hash_base ||= validate_refname($refname);
> }
> }
This hunk is IMHO incorrect. First, $refname is _either_ $hash, or
$hash_base; it cannot be both. Second, in most cases (like the case
of 'shortlog' action, either explicit or implicit) it is simply $hash;
I think it can be $hash_base when $file_name is not set only in
singular exception case of 'tree' view for the top tree (lack of
filename is not an error, but is equivalent to $file_name='/').
> @@ -544,37 +586,6 @@ evaluate_path_info();
> our $git_dir;
> $git_dir = "$projectroot/$project" if $project;
>
> -# dispatch
> -my %actions = (
[...]
> -);
> -
> if (!defined $action) {
> if (defined $hash) {
> $action = git_get_type($hash);
I _think_ the '# dispatch' comment should be left here, and not moved
with the %actions hash.
> @@ -631,8 +642,15 @@ sub href (%) {
> if ($params{-replay}) {
> while (my ($name, $symbol) = each %mapping) {
> if (!exists $params{$name}) {
> - # to allow for multivalued params we use arrayref form
> - $params{$name} = [ $cgi->param($symbol) ];
> + # the parameter we want to recycle may be either part of the
> + # list of CGI parameter, or recovered from PATH_INFO
> + if ($cgi->param($symbol)) {
> + # to allow for multivalued params we use arrayref form
> + $params{$name} = [ $cgi->param($symbol) ];
> + } else {
> + no strict 'refs';
> + $params{$name} = $$name if $$name;
I would _perhaps_ add here comment that multivalued parameters can come
only from CGI query string, so there is no need for something like:
+ $params{$name} = (ref($$name) ? @$name : $$name) if $$name;
> + }
> }
> }
> }
This fragment is a bit of ugly code, hopefully corrected in later patch.
I think it would be better to have 'refactor parsing/validation of input
parameters' to be very fist patch in series; I am not sure but I suspect
that is a kind of bugfix for current "$project/$hash" ('shortlog' view)
and "$project/$hash_base:$file_name" ('blob_plain' and 'tree' view)
path_info.
P.S. It is a bit of pity that Mechanize test from Lea Wiemann caching
gitweb code is not in the 'master' or at least 'pu'. Using big, single,
monolithic patch instead of patch series of small, easy reviewable
commits strikes again... ;-(
--
Jakub Narebski
Poland
^ permalink raw reply
* Git commit hash clash prevention
From: martin f krafft @ 2008-10-02 8:53 UTC (permalink / raw)
To: git discussion list
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
Hi folks,
the other day during a workshop on Git, one of the attendants asked
about the scenario when two developers, Jane and David, both working
on the same project, both create a commit and the two just so happen
to have the same SHA-1. I realise that the likelihood of this
happening is about as high as the chance of <insert witty joke
here>, but it *is* possible, isn't it? Even though this is thus
somewhat academic, I am still very curious about it.
What happens when David now pulls from Jane? How does Git deal with
this?
I imagine it'll be able to distinguish the two commits based on
metadata, but won't the DAG get corrupted?
Cheers,
--
martin | http://madduck.net/ | http://two.sentenc.es/
"and no one sings me lullabies,
and no one makes me close my eyes,
and so i throw the windows wide,
and call to you across the sky"
-- pink floyd, 1971
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* git svn bug?
From: Ark Xu @ 2008-10-02 8:50 UTC (permalink / raw)
To: git
Hi guys,
I like git very much so I also use git svn for subversion repo.
Now I got a tough problem which stopping me for any svn related
operations. There is a file log4j.xml that is somehow strange because
when I fetch it, i got the following message:
tms-service/src/test/resources/log4j.xml has mode 120000 but is not a
link at /opt/local/bin/git-svn line 3230.
M tms-service/src/test/resources/log4j.xml
And then, all the operation will lead to :
Checksum mismatch: branches/5.3/tms-service/src/test/resources/
log4j.xml 51031cbb0e1a2e878e4a3836cf0baa9b80a037a5
expected: fab6daef9fc355b9342e26047f5d0141
got: 0e7e9081d608c80d2de0340ba5cd1600
Could anybody help?
regards,
Ark
^ permalink raw reply
* Re: [PATCHv4] gitweb: PATH_INFO support improvements
From: Giuseppe Bilotta @ 2008-10-02 8:49 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <200810021019.27383.jnareb@gmail.com>
On Thu, Oct 2, 2008 at 10:19 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Giuseppe Bilotta wrote:
>
>> Fourth version of 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.
>
> A nit: when sending longer patch series you should use numbered
> format in the form of [PATCH m/n] or [PATCH m/n vX] prefix.
W00t, I still manage to get this wrong. Kudos to me 8-/
I wonder why these options are not the default when there is more than
one patch, btw?
(And yes, I tried looking into the builtin-log.c code but making it
automatic is somewhat less trivial than I can dedicate time to.)
>> 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 URLs is kept, as long as $hash is not a refname whose
>> name happens to match a git action.
>
> Minor nit: there was also old-style $project/$hash_base:$file_name
> path_info format.
Right, forgot about that.
>> The main implementation is provided by paired patches (#1#3, #5#6)
>> that implement parsing and generation of the new style URLs.
>>
>> Patch #2 deals with a refactoring of the input parameters parsing and
>> validation, so that the rest of gitweb can be agnostic wrt to the
>> parameters' origin (CGI vs PATH_INFO vs possible other future inputs
>> such as CLI).
>>
>> Patch #4 is a minor improvement to the URL syntax that allows web
>> documents to be properly browsable in raw mode.
>
> Very nice summary of patchset and patch coverage in this cover letter.
Thanks. At least I'm learning from my past errors. I'll manage to send
the perfect patchset sooner or later ;)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] git commit: Repaint the output format bikeshed (again)
From: Wincent Colaiuta @ 2008-10-02 8:36 UTC (permalink / raw)
To: Jeff King; +Cc: Andreas Ericsson, Shawn Pearce, Git Mailing List
In-Reply-To: <20081001220604.GB18058@coredump.intra.peff.net>
El 2/10/2008, a las 0:06, Jeff King escribió:
> better. But in the interests of just agreeing on something, I am
> willing
> to accept this. FWIW, the git-reset command doesn't use any delimiter
> for the message:
>
> <branch> is now at <hash> <subject>
>
> So perhaps they should be the same. I don't think it overly matters.
If you're wanting to trim horizontal fat then the "is" isn't really
required.
<branch> now at <hash> <subject>
Reads just as well.
Cheers,
Wincent
^ permalink raw reply
* [PATCH] tests: add a testcase for "git submodule sync"
From: David Aguilar @ 2008-10-02 8:29 UTC (permalink / raw)
To: spearce; +Cc: git, mlevedahl, gitster, David Aguilar
This testcase ensures that upstream changes to submodule properties
can be updated using the sync subcommand. This particular test
changes the submodule URL upstream and uses the sync command to update
an existing checkout.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
t/t7403-submodule-sync.sh | 71 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
new file mode 100755
index 0000000..c034c9e
--- /dev/null
+++ b/t/t7403-submodule-sync.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 David Aguilar
+#
+
+test_description='git submodule sync
+
+These tests exercise the "git submodule sync" subcommand.
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ mkdir upstream &&
+ (cd upstream &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone upstream super &&
+ git clone super submodule &&
+ (cd super &&
+ git submodule add ../submodule submodule &&
+ git commit -m "submodule"
+ ) &&
+ git clone super super-clone &&
+ (cd super-clone && git submodule update --init)
+'
+
+test_expect_success 'change submodule' '
+ (cd submodule &&
+ echo second line >> file &&
+ test_tick &&
+ git commit -a -m "change submodule"
+ )
+'
+
+test_expect_success 'change submodule url' '
+ (cd super &&
+ cd submodule &&
+ git checkout master &&
+ test_tick &&
+ git pull
+ ) &&
+ mv submodule moved-submodule &&
+ (cd super &&
+ git config -f .gitmodules submodule.submodule.url ../moved-submodule
+ test_tick &&
+ git commit -a -m moved-submodule
+ )
+'
+
+test_expect_success '"git submodule sync" should update submodule URLs' '
+ (cd super-clone &&
+ git pull &&
+ test_tick &&
+ git submodule sync
+ ) &&
+ test_tick &&
+ test -d "$(git config -f super-clone/submodule/.git/config \
+ remote.origin.url)" &&
+ (cd super-clone/submodule &&
+ git checkout master &&
+ test_tick &&
+ git pull
+ )
+'
+
+test_done
--
1.6.0.2.428.g5e22e
^ permalink raw reply related
* Re: [PATCHv4] gitweb: PATH_INFO support improvements
From: Jakub Narebski @ 2008-10-02 8:19 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano, Shawn O. Pearce
In-Reply-To: <1222906234-8182-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta wrote:
> Fourth version of 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.
A nit: when sending longer patch series you should use numbered
format in the form of [PATCH m/n] or [PATCH m/n vX] prefix.
>
> 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 URLs is kept, as long as $hash is not a refname whose
> name happens to match a git action.
Minor nit: there was also old-style $project/$hash_base:$file_name
path_info format.
>
> The main implementation is provided by paired patches (#1#3, #5#6)
> that implement parsing and generation of the new style URLs.
>
> Patch #2 deals with a refactoring of the input parameters parsing and
> validation, so that the rest of gitweb can be agnostic wrt to the
> parameters' origin (CGI vs PATH_INFO vs possible other future inputs
> such as CLI).
>
> Patch #4 is a minor improvement to the URL syntax that allows web
> documents to be properly browsable in raw mode.
Very nice summary of patchset and patch coverage in this cover letter.
>
> Giuseppe Bilotta (6):
> gitweb: parse project/action/hash_base:filename PATH_INFO
> gitweb: refactor input parameters parse/validation
> gitweb: generate project/action/hash URLs
> gitweb: use_pathinfo filenames start with /
> gitweb: parse parent..current syntax from pathinfo
> gitweb: generate parent..current URLs
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] git commit: Repaint the output format bikeshed (again)
From: Andreas Ericsson @ 2008-10-02 5:40 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn Pearce, Git Mailing List
In-Reply-To: <20081001223125.GA25267@coredump.intra.peff.net>
Jeff King wrote:
> On Wed, Oct 01, 2008 at 06:06:04PM -0400, Jeff King wrote:
>
>> I think I still like your other proposal:
>>
>> [branch] created b930c4a: "i386: Snib the sprock"
>
> And here is the patch, since it was sitting uncommitted in my working
> tree. Feel free to ignore.
>
> BTW, we should apply _something_ since what is currently in next has a
> bug: it lacks a space between "DETACHED commit" and the hash:
>
> Created DETACHED commit4fde0d0 (subject line)
>
> -- >8 --
> reformat informational commit message
>
> When committing, we print a message like:
>
> Created [DETACHED commit] <hash> (<subject>) on <branch>
>
> The most useful bit of information there (besides the
> detached status, if it is present) is which branch you made
> the commit on. However, it is sometimes hard to see because
> the subject dominates the line.
>
> Instead, let's put the most useful information (detached
> status and commit branch) on the far left, with the subject
> (which is least likely to be interesting) on the far right.
>
> We'll use brackets to offset the branch name so the line is
> not mistaken for an error line of the form "program: some
> sort of error". E.g.,:
>
> [jk/bikeshed] created bd8098f: "reformat informational commit message"
> ---
No sign-off.
> builtin-commit.c | 37 ++++++++++---------------------------
> 1 files changed, 10 insertions(+), 27 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index e4e1448..7a66e5a 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -878,35 +878,13 @@ int cmd_status(int argc, const char **argv, const char *prefix)
> return commitable ? 0 : 1;
> }
>
> -static char *get_commit_format_string(void)
> -{
> - unsigned char sha[20];
> - const char *head = resolve_ref("HEAD", sha, 0, NULL);
> - struct strbuf buf = STRBUF_INIT;
> -
> - /* use shouty-caps if we're on detached HEAD */
> - strbuf_addf(&buf, "format:%s", strcmp("HEAD", head) ? "" : "DETACHED commit");
> - strbuf_addstr(&buf, "%h (%s)");
> -
> - if (!prefixcmp(head, "refs/heads/")) {
> - const char *cp;
> - strbuf_addstr(&buf, " on ");
> - for (cp = head + 11; *cp; cp++) {
> - if (*cp == '%')
> - strbuf_addstr(&buf, "%x25");
> - else
> - strbuf_addch(&buf, *cp);
> - }
> - }
> -
> - return strbuf_detach(&buf, NULL);
> -}
> -
> static void print_summary(const char *prefix, const unsigned char *sha1)
> {
> struct rev_info rev;
> struct commit *commit;
> - char *format = get_commit_format_string();
> + static const char *format = "format:%h: \"%s\"";
> + unsigned char junk_sha1[20];
> + const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
>
> commit = lookup_commit(sha1);
> if (!commit)
> @@ -931,7 +909,13 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
> rev.diffopt.break_opt = 0;
> diff_setup_done(&rev.diffopt);
>
> - printf("Created %s", initial_commit ? "root-commit " : "");
> + printf("[%s%s]: created ",
> + !prefixcmp(head, "refs/heads/") ?
> + head + 11 :
> + !strcmp(head, "HEAD") ?
> + "detached HEAD" :
> + head,
> + initial_commit ? " (root-commit)" : "");
>
Personally, I'm not overly fond of things like
something ? yay : nay_but_try ? worked_now : still_no_go
since I find them hard to read without thinking a lot.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Help with a tcl/tk gui thing..
From: Mikael Magnusson @ 2008-10-02 5:18 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0810011605001.3341@nehalem.linux-foundation.org>
2008/10/2 Linus Torvalds <torvalds@linux-foundation.org>:
>
>
> On Wed, 1 Oct 2008, Mikael Magnusson wrote:
>>
>> git clone git://mika.l3ib.org/tracker.git
>>
>> I wrote it in pygtk since I know zero to no tcl/tk, hope that's okay.
>> It has a label with the time remaining (simply read from the daemon file),
>> and shows the text in red if less than 10% is remaining. You'll need to
>> change the ./ in cb_function to /var/log/tracker since I forgot to change
>> that and I'm lazy :).
>
> Well, it doesn't do anything at all for me except change the cursor to a
> cross, and if I'm a n00b with tcl/tk, I'm even more of one with pygtk.
>
> I merged the two other suggestions, though. And am open to seeing that
> pygtk thing too as an alternative, but only if it actually works for me ;)
Heh, sorry, I'm an idiot. I forgot to put '#!/usr/bin/python' on the first
line. I was running 'python tracker-ui.py' then did chmod +x just before
committing :). So the cross is coming from running 'import time'.
--
Mikael Magnusson
^ permalink raw reply
* Re: [PATCH] Solaris: Use OLD_ICONV to avoid compile warnings
From: Jeff King @ 2008-10-02 1:08 UTC (permalink / raw)
To: David Soria Parra; +Cc: git, David Soria Parra
In-Reply-To: <1222906127-16900-1-git-send-email-sn_@gmx.net>
On Thu, Oct 02, 2008 at 02:08:47AM +0200, David Soria Parra wrote:
> Solaris systems use the old styled iconv(3) call and therefore
> the OLD_ICONV variable should be set. Otherwise we get annoying compile
> warnings.
Acked-by: Jeff King <peff@peff.net>
I set OLD_ICONV on my Solaris build.
Do you also unset NEEDS_LIBICONV (and which version of Solaris are you
running)? Our Makefile sets it to "yes" for Solaris 8, but my build box
requires that it be unset. I'm not sure if my setup is somehow
deficient, or if it is only other versions that need it.
-Peff
^ permalink raw reply
* [PATCHv4] gitweb: generate parent..current URLs
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-6-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 | 30 +++++++++++++++++++++++++-----
1 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 89e360f..d863ef7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -712,7 +712,8 @@ sub href (%) {
# try to put as many parameters as possible in PATH_INFO:
# - project name
# - action
- # - hash or hash_base:/filename
+ # - hash_parent or hash_parent_base:/file_parent
+ # - hash or hash_base:/file_name
# Strip any trailing / from $href, or we might get double
# slashes when the script is the DirectoryIndex
@@ -730,17 +731,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
* [PATCHv4] gitweb: parse parent..current syntax from pathinfo
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-5-git-send-email-giuseppe.bilotta@gmail.com>
This makes it possible to use an URL such as
$project/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.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 26 ++++++++++++++++++++++++--
1 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3e5b2b7..89e360f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -534,7 +534,9 @@ if ($path_info && !defined $action) {
# we can now parse ref and pathnames in PATH_INFO
if ($path_info) {
- my ($refname, $pathname) = split(/:/, $path_info, 2);
+ $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/;
+ my ($parentrefname, $parentpathname, $refname, $pathname) = (
+ $2, $4, $5, $7);
if (defined $pathname) {
# we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
# we could use git_get_type(branch:pathname), but it needs $git_dir
@@ -543,7 +545,11 @@ if ($path_info) {
$input_params{'action'} ||= "tree";
$pathname =~ s,/$,,;
} else {
- $input_params{'action'} ||= "blob_plain";
+ if ($parentrefname) {
+ $input_params{'action'} ||= "blobdiff_plain";
+ } else {
+ $input_params{'action'} ||= "blob_plain";
+ }
}
$input_params{'hash_base'} ||= $refname;
$input_params{'file_name'} ||= $pathname;
@@ -553,6 +559,22 @@ if ($path_info) {
$input_params{'hash'} ||= $refname;
$input_params{'hash_base'} ||= $refname;
}
+ # the parent part might be missing the pathname, in which case we use the $file_name, if present
+ if (defined $parentrefname) {
+ $input_params{'hash_parent_base'} ||= $parentrefname;
+ if ($parentpathname) {
+ $parentpathname =~ s,^/+,,;
+ $parentpathname =~ s,/$,,;
+ $input_params{'file_parent'} ||= $parentpathname;
+ } else {
+ $input_params{'file_parent'} ||= $input_params{'file_name'};
+ }
+ if (defined $input_params{'file_parent'}) {
+ $input_params{'hash_parent'} ||= git_get_hash_by_path($input_params{'hash_parent_base'}, $input_params{'file_parent'});
+ } else {
+ $input_params{'hash_parent'} ||= $parentrefname;
+ }
+ }
}
# and now the rest of the validation
--
1.5.6.5
^ permalink raw reply related
* [PATCHv4] gitweb: use_pathinfo filenames start with /
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-4-git-send-email-giuseppe.bilotta@gmail.com>
When using path info, make filenames start with a / (right after the :
that separates them from the hash base). This minimal change allows
relative navigation to work properly when viewing HTML files.
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 2c380ac..3e5b2b7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -690,7 +690,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
# Strip any trailing / from $href, or we might get double
# slashes when the script is the DirectoryIndex
@@ -708,11 +708,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
* [PATCHv4] gitweb: generate project/action/hash URLs
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-3-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 | 32 +++++++++++++++++++++++++++++---
1 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ec4326f..2c380ac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -687,14 +687,40 @@ 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
+
+ # Strip any trailing / from $href, or we might get double
+ # slashes when the script is the DirectoryIndex
+ #
+ $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
* [PATCHv4] gitweb: refactor input parameters parse/validation
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-2-git-send-email-giuseppe.bilotta@gmail.com>
Since input parameters can now be obtained both from CGI parameters and
PATH_INFO, we would like most of the code to be agnostic about the way
parameters were retrieved.
We thus collect all the parameters into the new %input_params hash,
removing the need for ad-hoc kludgy code in href(). As much of the
parameters validation code is now shared between CGI and PATH_INFO,
although this requires PATH_INFO parsing to be interleaved into the main
code instead of being factored out into its own routine.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 336 ++++++++++++++++++++++++++++------------------------
1 files changed, 183 insertions(+), 153 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f088681..ec4326f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -28,8 +28,10 @@ our $my_url = $cgi->url();
our $my_uri = $cgi->url(-absolute => 1);
# if we're called with PATH_INFO, we have to strip that
-# from the URL to find our real URL
-if (my $path_info = $ENV{"PATH_INFO"}) {
+# from the URL to find our real URL. PATH_INFO is kept
+# as it's used later on for parameter extraction
+my $path_info = $ENV{"PATH_INFO"};
+if ($path_info) {
$my_url =~ s,\Q$path_info\E$,,;
$my_uri =~ s,\Q$path_info\E$,,;
}
@@ -390,15 +392,111 @@ $projects_list ||= $projectroot;
# ======================================================================
# input validation and dispatch
-our $action = $cgi->param('a');
-if (defined $action) {
- if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
- die_error(400, "Invalid action parameter");
+
+# input parameters can be collected from a variety of sources (presently, CGI
+# and PATH_INFO), so we define an %input_params hash that collects them all
+# together during validation: this allows subsequent uses (e.g. href()) to be
+# agnostic of the parameter origin
+
+my %input_params = ();
+
+# input parameters are stored with the long parameter name as key, so we need
+# the name -> CGI key mapping here. This will also be used in the href
+# subroutine to convert parameters to their CGI equivalent.
+#
+# XXX: Warning: If you touch this, check the search form for updating,
+# too.
+
+my @cgi_param_mapping = (
+ project => "p",
+ action => "a",
+ file_name => "f",
+ file_parent => "fp",
+ hash => "h",
+ hash_parent => "hp",
+ hash_base => "hb",
+ hash_parent_base => "hpb",
+ page => "pg",
+ order => "o",
+ searchtext => "s",
+ searchtype => "st",
+ snapshot_format => "sf",
+ extra_options => "opt",
+ search_use_regexp => "sr",
+);
+my %cgi_param_mapping = @cgi_param_mapping;
+
+# we will also need to know the possible actions, for validation
+my %actions = (
+ "blame" => \&git_blame,
+ "blobdiff" => \&git_blobdiff,
+ "blobdiff_plain" => \&git_blobdiff_plain,
+ "blob" => \&git_blob,
+ "blob_plain" => \&git_blob_plain,
+ "commitdiff" => \&git_commitdiff,
+ "commitdiff_plain" => \&git_commitdiff_plain,
+ "commit" => \&git_commit,
+ "forks" => \&git_forks,
+ "heads" => \&git_heads,
+ "history" => \&git_history,
+ "log" => \&git_log,
+ "rss" => \&git_rss,
+ "atom" => \&git_atom,
+ "search" => \&git_search,
+ "search_help" => \&git_search_help,
+ "shortlog" => \&git_shortlog,
+ "summary" => \&git_summary,
+ "tag" => \&git_tag,
+ "tags" => \&git_tags,
+ "tree" => \&git_tree,
+ "snapshot" => \&git_snapshot,
+ "object" => \&git_object,
+ # those below don't need $project
+ "opml" => \&git_opml,
+ "project_list" => \&git_project_list,
+ "project_index" => \&git_project_index,
+);
+
+# fill %input_params with the CGI parameters. All values except for 'opt'
+# should be single values, but opt can be an array. We should probably
+# build an array of parameters that can be multi-valued, but since for the time
+# being it's only this one, we just single it out
+while (my ($name, $symbol) = each %cgi_param_mapping) {
+ if ($symbol eq 'opt') {
+ $input_params{$name} = [$cgi->param($symbol)];
+ } else {
+ $input_params{$name} = $cgi->param($symbol);
}
}
-# parameters which are pathnames
-our $project = $cgi->param('p');
+# next, we want to parse PATH_INFO (which was already stored in $path_info at
+# the beginning). This is a little hairy because PATH_INFO parsing needs some
+# form of parameter validation, so we interleave parsing and validation.
+#
+# The accepted PATH_INFO syntax is $project/$action/$hash or
+# $project/$action/$hash_base:$file_name, where $action may be missing (mostly for
+# backwards compatibility), so we need to parse and validate the parameters in
+# this same order.
+
+# clear $path_info of any leading /
+$path_info =~ s,^/+,,;
+
+our $project = $input_params{'project'};
+if ($path_info && !defined $project) {
+ # if $project was not defined by CGI, we try to extract it from
+ # $path_info
+ $project = $path_info;
+ $project =~ s,/+$,,;
+ while ($project && !check_head_link("$projectroot/$project")) {
+ $project =~ s,/*[^/]*$,,;
+ }
+ $input_params{'project'} = $project;
+} else {
+ # otherwise, we suppress $path_info parsing altogether
+ $path_info = undef;
+}
+
+# project name validation
if (defined $project) {
if (!validate_pathname($project) ||
!(-d "$projectroot/$project") ||
@@ -408,16 +506,66 @@ if (defined $project) {
undef $project;
die_error(404, "No such project");
}
+
+ # we purge the $project name from the $path_info, preparing it for
+ # subsequent parameters extraction
+ $path_info =~ s,^\Q$project\E/*,, if defined $path_info;
+} else {
+ # we also suppress $path_info parsing if no project was defined
+ $path_info = undef;
+}
+
+# action parsing/validation
+our $action = $input_params{'action'};
+if ($path_info && !defined $action) {
+ # next comes the action
+ $action = $path_info;
+ $action =~ s,/.*$,,;
+ if (exists $actions{$action}) {
+ # remove the action from $path_info, and sync %input_params
+ $path_info =~ s,^$action/*,,;
+ $input_params{'action'} = $action;
+ } else {
+ $action = undef;
+ }
+} elsif (defined $action && !exists $actions{$action}) {
+ die_error(400, "Invalid action parameter");
+}
+
+# we can now parse ref and pathnames in PATH_INFO
+if ($path_info) {
+ my ($refname, $pathname) = split(/:/, $path_info, 2);
+ if (defined $pathname) {
+ # we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
+ # we could use git_get_type(branch:pathname), but it needs $git_dir
+ $pathname =~ s,^/+,,;
+ if (!$pathname || substr($pathname, -1) eq "/") {
+ $input_params{'action'} ||= "tree";
+ $pathname =~ s,/$,,;
+ } else {
+ $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;
+ $input_params{'hash_base'} ||= $refname;
+ }
}
-our $file_name = $cgi->param('f');
+# and now the rest of the validation
+
+# parameters which are pathnames
+our $file_name = $input_params{'file_name'};
if (defined $file_name) {
if (!validate_pathname($file_name)) {
die_error(400, "Invalid file parameter");
}
}
-our $file_parent = $cgi->param('fp');
+our $file_parent = $input_params{'file_parent'};
if (defined $file_parent) {
if (!validate_pathname($file_parent)) {
die_error(400, "Invalid file parent parameter");
@@ -425,21 +573,21 @@ if (defined $file_parent) {
}
# parameters which are refnames
-our $hash = $cgi->param('h');
+our $hash = $input_params{'hash'};
if (defined $hash) {
if (!validate_refname($hash)) {
die_error(400, "Invalid hash parameter");
}
}
-our $hash_parent = $cgi->param('hp');
+our $hash_parent = $input_params{'hash_parent'};
if (defined $hash_parent) {
if (!validate_refname($hash_parent)) {
die_error(400, "Invalid hash parent parameter");
}
}
-our $hash_base = $cgi->param('hb');
+our $hash_base = $input_params{'hash_base'};
if (defined $hash_base) {
if (!validate_refname($hash_base)) {
die_error(400, "Invalid hash base parameter");
@@ -450,19 +598,19 @@ my %allowed_options = (
"--no-merges" => [ qw(rss atom log shortlog history) ],
);
-our @extra_options = $cgi->param('opt');
-if (defined @extra_options) {
- foreach my $opt (@extra_options) {
- if (not exists $allowed_options{$opt}) {
- die_error(400, "Invalid option parameter");
- }
- if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
- die_error(400, "Invalid option parameter for this action");
- }
+our @extra_options = @{$input_params{'extra_options'}};
+# @extra_options is always defined, since it can only be (currently) set from
+# CGI, and $cgi->param() returns the empty array in array context
+foreach my $opt (@extra_options) {
+ if (not exists $allowed_options{$opt}) {
+ die_error(400, "Invalid option parameter");
+ }
+ if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
+ die_error(400, "Invalid option parameter for this action");
}
}
-our $hash_parent_base = $cgi->param('hpb');
+our $hash_parent_base = $input_params{'hash_parent_base'};
if (defined $hash_parent_base) {
if (!validate_refname($hash_parent_base)) {
die_error(400, "Invalid hash parent base parameter");
@@ -470,23 +618,23 @@ if (defined $hash_parent_base) {
}
# other parameters
-our $page = $cgi->param('pg');
+our $page = $input_params{'page'};
if (defined $page) {
if ($page =~ m/[^0-9]/) {
die_error(400, "Invalid page parameter");
}
}
-our $searchtype = $cgi->param('st');
+our $searchtype = $input_params{'searchtype'};
if (defined $searchtype) {
if ($searchtype =~ m/[^a-z]/) {
die_error(400, "Invalid searchtype parameter");
}
}
-our $search_use_regexp = $cgi->param('sr');
+our $search_use_regexp = $input_params{'search_use_regexp'};
-our $searchtext = $cgi->param('s');
+our $searchtext = $input_params{'searchtext'};
our $search_regexp;
if (defined $searchtext) {
if (length($searchtext) < 2) {
@@ -495,93 +643,6 @@ if (defined $searchtext) {
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
-# dispatch
-my %actions = (
- "blame" => \&git_blame,
- "blobdiff" => \&git_blobdiff,
- "blobdiff_plain" => \&git_blobdiff_plain,
- "blob" => \&git_blob,
- "blob_plain" => \&git_blob_plain,
- "commitdiff" => \&git_commitdiff,
- "commitdiff_plain" => \&git_commitdiff_plain,
- "commit" => \&git_commit,
- "forks" => \&git_forks,
- "heads" => \&git_heads,
- "history" => \&git_history,
- "log" => \&git_log,
- "rss" => \&git_rss,
- "atom" => \&git_atom,
- "search" => \&git_search,
- "search_help" => \&git_search_help,
- "shortlog" => \&git_shortlog,
- "summary" => \&git_summary,
- "tag" => \&git_tag,
- "tags" => \&git_tags,
- "tree" => \&git_tree,
- "snapshot" => \&git_snapshot,
- "object" => \&git_object,
- # those below don't need $project
- "opml" => \&git_opml,
- "project_list" => \&git_project_list,
- "project_index" => \&git_project_index,
-);
-
-# now read PATH_INFO and use it as alternative to parameters
-sub evaluate_path_info {
- return if defined $project;
- my $path_info = $ENV{"PATH_INFO"};
- return if !$path_info;
- $path_info =~ s,^/+,,;
- return if !$path_info;
- # find which part of PATH_INFO is project
- $project = $path_info;
- $project =~ s,/+$,,;
- while ($project && !check_head_link("$projectroot/$project")) {
- $project =~ s,/*[^/]*$,,;
- }
- # validate project
- $project = validate_pathname($project);
- if (!$project ||
- ($export_ok && !-e "$projectroot/$project/$export_ok") ||
- ($strict_export && !project_in_list($project))) {
- undef $project;
- return;
- }
- # do not change any parameters if an action is given using the query string
- return if $action;
- $path_info =~ s,^\Q$project\E/*,,;
-
- # next comes the action
- $action = $path_info;
- $action =~ s,/.*$,,;
- if (exists $actions{$action}) {
- $path_info =~ s,^$action/*,,;
- } else {
- $action = undef;
- }
-
- my ($refname, $pathname) = split(/:/, $path_info, 2);
- if (defined $pathname) {
- # we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
- # we could use git_get_type(branch:pathname), but it needs $git_dir
- $pathname =~ s,^/+,,;
- if (!$pathname || substr($pathname, -1) eq "/") {
- $action ||= "tree";
- $pathname =~ s,/$,,;
- } else {
- $action ||= "blob_plain";
- }
- $hash_base ||= validate_refname($refname);
- $file_name ||= validate_pathname($pathname);
- } elsif (defined $refname) {
- # we got "project.git/branch"
- $action ||= "shortlog";
- $hash ||= validate_refname($refname);
- $hash_base ||= validate_refname($refname);
- }
-}
-evaluate_path_info();
-
# path to the current git repository
our $git_dir;
$git_dir = "$projectroot/$project" if $project;
@@ -615,43 +676,12 @@ sub href (%) {
# default is to use -absolute url() i.e. $my_uri
my $href = $params{-full} ? $my_url : $my_uri;
- # XXX: Warning: If you touch this, check the search form for updating,
- # too.
-
- my @mapping = (
- project => "p",
- action => "a",
- file_name => "f",
- file_parent => "fp",
- hash => "h",
- hash_parent => "hp",
- hash_base => "hb",
- hash_parent_base => "hpb",
- page => "pg",
- order => "o",
- searchtext => "s",
- searchtype => "st",
- snapshot_format => "sf",
- extra_options => "opt",
- search_use_regexp => "sr",
- );
- my %mapping = @mapping;
-
$params{'project'} = $project unless exists $params{'project'};
if ($params{-replay}) {
- while (my ($name, $symbol) = each %mapping) {
- if (!exists $params{$name}) {
- # the parameter we want to recycle may be either part of the
- # list of CGI parameter, or recovered from PATH_INFO
- if ($cgi->param($symbol)) {
- # to allow for multivalued params we use arrayref form
- $params{$name} = [ $cgi->param($symbol) ];
- } else {
- no strict 'refs';
- $params{$name} = $$name if $$name;
- }
- }
+ while (my ($name, $val) = each %input_params) {
+ $params{$name} = $input_params{$name}
+ unless (exists $params{$name});
}
}
@@ -669,8 +699,8 @@ sub href (%) {
# now encode the parameters explicitly
my @result = ();
- for (my $i = 0; $i < @mapping; $i += 2) {
- my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
+ for (my $i = 0; $i < @cgi_param_mapping; $i += 2) {
+ my ($name, $symbol) = ($cgi_param_mapping[$i], $cgi_param_mapping[$i+1]);
if (defined $params{$name}) {
if (ref($params{$name}) eq "ARRAY") {
foreach my $par (@{$params{$name}}) {
@@ -4006,7 +4036,7 @@ sub git_search_grep_body {
## actions
sub git_project_list {
- my $order = $cgi->param('o');
+ my $order = $input_params{'order'};
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
die_error(400, "Unknown order parameter");
}
@@ -4029,7 +4059,7 @@ sub git_project_list {
}
sub git_forks {
- my $order = $cgi->param('o');
+ my $order = $input_params{'order'};
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
die_error(400, "Unknown order parameter");
}
@@ -4562,7 +4592,7 @@ sub git_snapshot {
my @supported_fmts = gitweb_check_feature('snapshot');
@supported_fmts = filter_snapshot_fmts(@supported_fmts);
- my $format = $cgi->param('sf');
+ my $format = $input_params{'snapshot_format'};
if (!@supported_fmts) {
die_error(403, "Snapshots not allowed");
}
--
1.5.6.5
^ permalink raw reply related
* [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
In-Reply-To: <1222906234-8182-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,
except when $project/$branch was used to access a branch whose name
matches a gitweb action.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 90 +++++++++++++++++++++++++++++++---------------------
1 files changed, 54 insertions(+), 36 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e7e4d6b..f088681 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -495,6 +495,37 @@ if (defined $searchtext) {
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
+# dispatch
+my %actions = (
+ "blame" => \&git_blame,
+ "blobdiff" => \&git_blobdiff,
+ "blobdiff_plain" => \&git_blobdiff_plain,
+ "blob" => \&git_blob,
+ "blob_plain" => \&git_blob_plain,
+ "commitdiff" => \&git_commitdiff,
+ "commitdiff_plain" => \&git_commitdiff_plain,
+ "commit" => \&git_commit,
+ "forks" => \&git_forks,
+ "heads" => \&git_heads,
+ "history" => \&git_history,
+ "log" => \&git_log,
+ "rss" => \&git_rss,
+ "atom" => \&git_atom,
+ "search" => \&git_search,
+ "search_help" => \&git_search_help,
+ "shortlog" => \&git_shortlog,
+ "summary" => \&git_summary,
+ "tag" => \&git_tag,
+ "tags" => \&git_tags,
+ "tree" => \&git_tree,
+ "snapshot" => \&git_snapshot,
+ "object" => \&git_object,
+ # those below don't need $project
+ "opml" => \&git_opml,
+ "project_list" => \&git_project_list,
+ "project_index" => \&git_project_index,
+);
+
# now read PATH_INFO and use it as alternative to parameters
sub evaluate_path_info {
return if defined $project;
@@ -519,9 +550,19 @@ sub evaluate_path_info {
# do not change any parameters if an action is given using the query string
return if $action;
$path_info =~ s,^\Q$project\E/*,,;
+
+ # next comes the action
+ $action = $path_info;
+ $action =~ s,/.*$,,;
+ if (exists $actions{$action}) {
+ $path_info =~ s,^$action/*,,;
+ } else {
+ $action = undef;
+ }
+
my ($refname, $pathname) = split(/:/, $path_info, 2);
if (defined $pathname) {
- # we got "project.git/branch:filename" or "project.git/branch:dir/"
+ # we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
# we could use git_get_type(branch:pathname), but it needs $git_dir
$pathname =~ s,^/+,,;
if (!$pathname || substr($pathname, -1) eq "/") {
@@ -534,8 +575,9 @@ sub evaluate_path_info {
$file_name ||= validate_pathname($pathname);
} elsif (defined $refname) {
# we got "project.git/branch"
- $action ||= "shortlog";
- $hash ||= validate_refname($refname);
+ $action ||= "shortlog";
+ $hash ||= validate_refname($refname);
+ $hash_base ||= validate_refname($refname);
}
}
evaluate_path_info();
@@ -544,37 +586,6 @@ evaluate_path_info();
our $git_dir;
$git_dir = "$projectroot/$project" if $project;
-# dispatch
-my %actions = (
- "blame" => \&git_blame,
- "blobdiff" => \&git_blobdiff,
- "blobdiff_plain" => \&git_blobdiff_plain,
- "blob" => \&git_blob,
- "blob_plain" => \&git_blob_plain,
- "commitdiff" => \&git_commitdiff,
- "commitdiff_plain" => \&git_commitdiff_plain,
- "commit" => \&git_commit,
- "forks" => \&git_forks,
- "heads" => \&git_heads,
- "history" => \&git_history,
- "log" => \&git_log,
- "rss" => \&git_rss,
- "atom" => \&git_atom,
- "search" => \&git_search,
- "search_help" => \&git_search_help,
- "shortlog" => \&git_shortlog,
- "summary" => \&git_summary,
- "tag" => \&git_tag,
- "tags" => \&git_tags,
- "tree" => \&git_tree,
- "snapshot" => \&git_snapshot,
- "object" => \&git_object,
- # those below don't need $project
- "opml" => \&git_opml,
- "project_list" => \&git_project_list,
- "project_index" => \&git_project_index,
-);
-
if (!defined $action) {
if (defined $hash) {
$action = git_get_type($hash);
@@ -631,8 +642,15 @@ sub href (%) {
if ($params{-replay}) {
while (my ($name, $symbol) = each %mapping) {
if (!exists $params{$name}) {
- # to allow for multivalued params we use arrayref form
- $params{$name} = [ $cgi->param($symbol) ];
+ # the parameter we want to recycle may be either part of the
+ # list of CGI parameter, or recovered from PATH_INFO
+ if ($cgi->param($symbol)) {
+ # to allow for multivalued params we use arrayref form
+ $params{$name} = [ $cgi->param($symbol) ];
+ } else {
+ no strict 'refs';
+ $params{$name} = $$name if $$name;
+ }
}
}
}
--
1.5.6.5
^ permalink raw reply related
* [PATCHv4] gitweb: PATH_INFO support improvements
From: Giuseppe Bilotta @ 2008-10-02 0:10 UTC (permalink / raw)
To: git
Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Shawn O. Pearce,
Giuseppe Bilotta
Fourth version of 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 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#3, #5#6)
that implement parsing and generation of the new style URLs.
Patch #2 deals with a refactoring of the input parameters parsing and
validation, so that the rest of gitweb can be agnostic wrt to the
parameters' origin (CGI vs PATH_INFO vs possible other future inputs
such as CLI).
Patch #4 is a minor improvement to the URL syntax that allows web
documents to be properly browsable in raw mode.
Giuseppe Bilotta (6):
gitweb: parse project/action/hash_base:filename PATH_INFO
gitweb: refactor input parameters parse/validation
gitweb: generate project/action/hash URLs
gitweb: use_pathinfo filenames start with /
gitweb: parse parent..current syntax from pathinfo
gitweb: generate parent..current URLs
gitweb/gitweb.perl | 392 ++++++++++++++++++++++++++++++++++------------------
1 files changed, 254 insertions(+), 138 deletions(-)
^ 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