[PATCH] Support "core.excludesfile = ~/.gitignore"

[Karl Chen]

>>>>> On 2008-08-29 09:08 PDT, Junio C Hamano writes:

    Junio> I do not see any strong reason why the single caller of
    Junio> user_path() has to keep using the static allocation.
    Junio> Would it help to reduce the complexity of your
    Junio> expand_user_path() implementation, if we fixed the
    Junio> caller along the lines of this patch (untested, but
    Junio> just to illustrate the point)?

Yes, expand_user_path() would be much simpler, it would basically
be me original implementation except for returning NULL on error.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Junio C Hamano]

Karl Chen <[hidden email]> writes:

>>>>>> On 2008-08-29 09:08 PDT, Junio C Hamano writes:
>
>     Junio> I do not see any strong reason why the single caller of
>     Junio> user_path() has to keep using the static allocation.
>     Junio> Would it help to reduce the complexity of your
>     Junio> expand_user_path() implementation, if we fixed the
>     Junio> caller along the lines of this patch (untested, but
>     Junio> just to illustrate the point)?
>
> Yes, expand_user_path() would be much simpler, it would basically
> be me original implementation except for returning NULL on error.

Yeah, modulo those styles issues your v3 and v4 addressed, and use of
strbuf.

It might feel that we went full circles, wasting your time.  But it's not.
We found out that the final series would look like this:

 [1/3] Introduce expand_user_path();

 [2/3] Using #1, introduce git_config_pathname() and use it to parse your
       two variables;

 [3/3] Update the sole caller of user_path() to use expand_user_path().

Patch #1 and #2 can be squashed into one if you want.  Also you do not
have to do #3 yourself if you do not feel like it (but now we know how the
code would look like, why not?).

Thanks to these three initial rounds, we know whoever implements #1 knows
what kind of interface the (to-be-rewritten) user of user_path() would
want, so #3 will become much cleaner.  We made progress.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Karl Chen]

>>>>> On 2008-08-29 12:28 PDT, Junio C Hamano writes:

    Junio>  [3/3] Update the sole caller of user_path() to use
    Junio>  expand_user_path().

Actually I just looked closer at enter_repo() and it's not quite
as simple as your proposed patch, because enter_repo() wants to
concatenate suffixes like ".git".  So either the malloced string
would have to be copied to the static buffer again, or return a
strbuf, or take an argument for allocating extra chars.

Wow, I'd forgotten how much work it is to do string manipulation
in C.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Junio C Hamano]

Karl Chen <[hidden email]> writes:

>>>>>> On 2008-08-29 12:28 PDT, Junio C Hamano writes:
>
>     Junio>  [3/3] Update the sole caller of user_path() to use
>     Junio>  expand_user_path().
>
> Actually I just looked closer at enter_repo() and it's not quite
> as simple as your proposed patch, because enter_repo() wants to
> concatenate suffixes like ".git".

I thought the "just an illustration" patch at least took care of that
part.

The thing is that enter_repo() is not performance critical, it is where
server side programs validate the repository path received from the other
end of the network, and we can be stricter than necessary about path
lengths.  I do not particularly think it is necessary to convert it and
use strbuf to allow arbitrarily long paths --- rejecting requests to an
absurdly deep directory is not an end of the world there.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Jeff King]

On Fri, Aug 29, 2008 at 02:29:00AM -0700, Karl Chen wrote:

>  core.excludesfile::
>   In addition to '.gitignore' (per-directory) and
>   '.git/info/exclude', git looks into this file for patterns
> - of files which are not meant to be tracked.  See
> + of files which are not meant to be tracked.  "~" and "~user"
> + are expanded to the user's home directory.  See
>   linkgit:gitignore[5].

How about:

  A leading "~" or "~user" is expanded to the home directory of the
  current user or "user", as in the shell.

Specifically:

  1. We obviously handle only leading cases, so /path/to/~file~with~tildes
     is ok.

  2. It was a little unclear to me whether both "~" and "~user" expande
     to the same thing. I.e., can one use this for arbitrary "user" (and
     the answer of course is yes).

> +char *expand_user_path(char *buf, const char *path, int bufsz)
> +{
> + if (path == NULL)
> + return NULL;
> +
> + if (path[0] != '~')
> + return concatstr(buf, path, NULL, bufsz);
> +
> + const char *username = path + 1;
> + size_t username_len = strchrnul(username, '/') - username;
> + struct passwd *pw = getpw_str(username, username_len);

Declaration after statement (we try to remain portable to non-C99
systems).

Other than that, I think the patch is fine (though I am not opposed to
the improvements Junio has mentioned).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html