On Mon, Jul 22, 2013 at 01:24:06PM -0700, Kyle J. McKay wrote:
> >I am not yet convinced that the precedence rule specified in this
> >what we want (I do not have an example why it is *not* what we want,
> >either). Another definition could be "if user@ is present in the
> >request, give lower precedence to config entries for the site
> >without user@ than entries with user@", and I do not have a strong
> >opinion myself which one between the two is better (and there may be
> >third and other possible rule).
> >
> >Comments?
>
> Consider this site:
> [...]
Thanks for explaining, and sorry I missed out on the last few rounds of
review.
I think your scheme (normalization plus special handling of the username
field) addresses my biggest concern, which is matching in the face of
optional usernames. The only two things that make me wary are:
1. The explanation and special-casing of username is a little
complicated to explain.
2. The behavior for resolving the value when faced with multiple
possibilities is completely unlike the rest of the config system
(both dropping last-one-wins, and unlike the URL matching for
credentials).
I think we can decide that (2) is worth it if your semantics are more
flexible in practice. It would be nice to see real-world feedback on how
people use it before setting the behavior in stone, but there's sort of
a chicken and egg problem there.
For (1), I wonder if the explanation would be simpler if the precedences
of each sub-part were simply laid out. That is, would it be correct to
say something like:
For a config key to match a URL, each element of the config key (if
present) is compared to that of the URL, in the following order:
1. Protocol (e.g., `https` in `https://example.com/`). This field
must match exactly between the config key and the URL.
2. Host/domain name (e.g., `example.com` in `https://example.com/`).
This field must match exactly between the config key and the URL.
3. Path (e.g., `repo.git` in `https://example.com/repo.git`). This
field is prefix-matched by slash-delimited path elements, so that
config key `foo/` matches URL `foo/bar`. Longer matches take
precedence (so `foo/bar`, if it exists, is a better match than
just `foo/`).
4. Username (e.g., `user` in `https://user@xxxxxxxxxxx/repo.git`).
The list above is ordered by decreasing precedence; a URL that matches
a config key's path is preferred to one that matches its username.
I don't know if that is more or less clear of an explanation. It makes
more sense to me, but that is probably because I wrote it. I'm also not
100% sure it describes your implementation, but I think it is equivalent
to the prefix matching with normalization.
I have a few other comments on specific patches; I'll send them
separately.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html