git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>
To: "Devin Doucette" <devin@doucette.cc>
Cc: "Jakub Narebski" <jnareb@gmail.com>,
	git@vger.kernel.org, "Petr Baudis" <pasky@suse.cz>
Subject: Re: [PATCH] gitweb: Handle actions with no project in evaluate_path_info
Date: Fri, 2 Jan 2009 13:09:29 +0100	[thread overview]
Message-ID: <cb7bb73a0901020409u304e6344x13b6090732c2d5cd@mail.gmail.com> (raw)
In-Reply-To: <a899d7ef0901011821x166f173dx47bdb41326391696@mail.gmail.com>

On Fri, Jan 2, 2009 at 3:21 AM, Devin Doucette <devin@doucette.cc> wrote:
> On Thu, Jan 1, 2009 at 5:46 PM, Giuseppe Bilotta
> <giuseppe.bilotta@gmail.com> wrote:
>> Actually, the early bailout was sort of intentional. The problem is
>> the ambiguity: does git.example.com/opml refer to the opml project, or
>> does it refer to the opml action?
>
> Good point. Though my patch does not break any existing functionality,
> it does not fix the case where a project matches the action.
>
>> HOWEVER, href() *does* create the opml action as git.example.com/opml,
>> so gitweb is currently broken in the sense that ti doesn't correctly
>> parse its own pathinfo output. So the question is: shall we go with
>> this patch, preventing pathinfo from working for projects named like a
>> no-project gitweb action, or should we fix href() to not generate
>> pathinfo unless project is defined?
>
> A variation of the latter approach is to modify href() to use pathinfo
> if there is a project or there is no project matching the name of the
> action. The only downside to this approach is that a URI that refers to
> an action when first generated could resolve to a project in the future,
> if a project of the same name were added.

And don't forget the case of a new action being introduced which is
named like an existing project.

I suspect we'd be much better off by not using pathinfo at all for
global actions. Other possible solutions would be to have something
like an ADDITIONAL prefix to the path (like project/projectname vs
action/actionname, so we would have git.example.com/action/opml or
git.example.com/project/example-project) but this is WAY too verbose.

At most, we could add a suffix to global actions to reduce the chance
of name conflict, such as using

git.example.com/opml.xml

or

git.example.com/project_index.txt

but I don't like these solution either: I'd rather have these be
provided as filenames in the response header.

Incoming patch to not make use of path_info without project.


-- 
Giuseppe "Oblomov" Bilotta

      reply	other threads:[~2009-01-02 12:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-28  7:26 [PATCH] gitweb: Handle actions with no project in evaluate_path_info Devin Doucette
2009-01-01 23:58 ` Jakub Narebski
2009-01-02  0:46   ` Giuseppe Bilotta
2009-01-02  2:21     ` Devin Doucette
2009-01-02 12:09       ` Giuseppe Bilotta [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cb7bb73a0901020409u304e6344x13b6090732c2d5cd@mail.gmail.com \
    --to=giuseppe.bilotta@gmail.com \
    --cc=devin@doucette.cc \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=pasky@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).