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

[Karl Chen]

I keep my rc files, including .gitconfig and my default gitignore
list under version control and like to have the same contents
everywhere.  Unfortunately my home directory is at different
locations on different systems.

I'd like to be able to put something like this in my ~/.gitconfig:

[core]
        excludesfile = ~/.gitignore

or
        excludesfile = $HOME/.gitignore

Another idea is to have a non-absolute path be interpreted
relative to the location of .gitconfig, i.e. $HOME, instead of the
current directory.  $GIT_DIR/info/excludes is already for
repository-specific excludes so no functionality would be lost.


Below is a sample patch that works for me.  We could also use
getpwuid(getuid()) instead of getenv("HOME") to be consistent with
user_path() but this is simpler and arguably more likely what the
user wants when it matters.


From 6eb18f8ade791521bdad955e1da2b40399a426f0 Mon Sep 17 00:00:00 2001
From: Karl Chen <[hidden email]>
Date: Thu, 21 Aug 2008 21:00:26 -0700
Subject: [PATCH] Support "core.excludesfile = ~/.gitignore"

The config variable core.excludesfile is parsed to substitute leading "~/"
with getenv("HOME").

Signed-off-by: Karl Chen <[hidden email]>

---
 config.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 53f04a0..41061d2 100644
--- a/config.c
+++ b/config.c
@@ -334,6 +334,18 @@ int git_config_string(const char **dest, const char *var, const char *value)
  return 0;
 }
 
+static char const *git_config_subst_userdir(char const *value) {
+ if (value[0] == '~' && value[1] == '/') {
+ const char *home = getenv("HOME");
+ char *userdir_excludes_file = malloc(strlen(home) + strlen(value)-1 + 1);
+ strcpy(userdir_excludes_file, home);
+ strcat(userdir_excludes_file, value+1);
+ return userdir_excludes_file;
+ } else {
+ return xstrdup(value);
+ }
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
  /* This needs a better name */
@@ -456,8 +468,12 @@ static int git_default_core_config(const char *var, const char *value)
  if (!strcmp(var, "core.editor"))
  return git_config_string(&editor_program, var, value);
 
- if (!strcmp(var, "core.excludesfile"))
- return git_config_string(&excludes_file, var, value);
+ if (!strcmp(var, "core.excludesfile")) {
+ if (!value)
+ return config_error_nonbool(var);
+ excludes_file = git_config_subst_userdir(value);
+ return 0;
+ }
 
  if (!strcmp(var, "core.whitespace")) {
  if (!value)
--
1.5.6.2

--
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

[Eric Raible]

Karl Chen <quarl <at> cs.berkeley.edu> writes:

> +static char const *git_config_subst_userdir(char const *value) {
> + if (value[0] == '~' && value[1] == '/') {

Might you want to check that strlen(value) is at least 2?

- Eric

--
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

[Bert Wesarg]

On Fri, Aug 22, 2008 at 18:58, Eric Raible <[hidden email]> wrote:
> Karl Chen <quarl <at> cs.berkeley.edu> writes:
>
>> +static char const *git_config_subst_userdir(char const *value) {
>> +     if (value[0] == '~' && value[1] == '/') {
>
> Might you want to check that strlen(value) is at least 2?
No.

    swtich (strlen(value)) {
    case 0:
        /* value[0] == '\0' => value[0] != '~'
           value[1] will never be dereferenced, because of lazy && */
    case 1:
       /* value[0] != '\0'
          if (value[0] == '~')
              value[1] == '\0' => value[1] != '/' */
    default:
       /* ... */
    }

So no invalid memory dereferences.

Regards
Bert
>
> - Eric
>
> --
> 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
>
--
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:

> Another idea is to have a non-absolute path be interpreted
> relative to the location of .gitconfig.

If we were to support relative paths, I think it would be useful and
consistent if a relative path found in ".git/config" is relative to the
work tree root, in "config" in a bare repository relative to the bare
repository, and in "$HOME/.gitconfig" relative to $HOME.  I am not sure
what a relative path in "/etc/gitconfig" should be relative to, though.

However, this has a technical difficulty.  When configuration values are
read, the code that knows what the value means does not in general know
which configuration file is being read from.

> Below is a sample patch that works for me.  We could also use
> getpwuid(getuid()) instead of getenv("HOME") to be consistent with
> user_path() but this is simpler and arguably more likely what the
> user wants when it matters.

It is quite likely that somebody would want you to interpret "~name/" if
you advertize that you support "~/", so you would need to call getpwuid()
eventually if you go down this path.  I wonder how this would affect
Windows folks.

What are the paths valued configuration variables other than excludesfile
that we would want to support?  There was a topic to allow mail-aliases
lookup for parameters given to the "--author" option today, and send-email
takes aliasfile configuration.  Because the latter is a script, we would
need a "--path" option to "git config" (the idea is similar to existing
"--bool" option) so that calling scripts can ask the same "magic"
performed to configuration variables' values before being reported.
--
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-22 14:10 PDT, Junio C Hamano writes:

    Junio> Karl Chen <[hidden email]> writes:
    >> Another idea is to have a non-absolute path be interpreted
    >> relative to the location of .gitconfig.

    Junio> If we were to support relative paths, I think it would
    Junio> be useful and consistent if a relative path found in
    Junio> ".git/config" is relative to the work tree root, in
    Junio> "config" in a bare repository relative to the bare
    Junio> repository, and in "$HOME/.gitconfig" relative to
    Junio> $HOME.

Makes sense to support it everywhere.  For .git/config, isn't it
more consistent for it to be relative to .git?

    Junio> I am not sure what a relative path in "/etc/gitconfig"
    Junio> should be relative to, though.

Why not just relative to the location of that file?  Normally
/etc, but if some distro customizes the location of /etc/gitconfig
(/etc/git/config), or on non-Linux/posix systems it's somewhere
else, or git is installed in /usr/local or /opt or $HOME, then
it's still relative to the location of system gitconfig.

    Junio> However, this has a technical difficulty.  When
    Junio> configuration values are read, the code that knows what
    Junio> the value means does not in general know which
    Junio> configuration file is being read from.

Sounds like a refactoring issue.

    Junio> It is quite likely that somebody would want you to
    Junio> interpret "~name/" if you advertize that you support
    Junio> "~/", so you would need to call getpwuid() eventually
    Junio> if you go down this path.  I wonder how this would
    Junio> affect Windows folks.

I would be happy either way.  Though since git uses getenv("HOME")
to find ~/.gitconfig, I can see arguments for looking for the
ignore file there also, in case it's different.

    Junio> we would need a "--path" option to "git config" (the
    Junio> idea is similar to existing "--bool" option) so that
    Junio> calling scripts can ask the same "magic" performed to
    Junio> configuration variables' values before being reported.

Sounds fine.

So, being new to git development, am I correctly assessing your
response as "with refinement this can be included in git"?

Relative paths and ~ (and $HOME) are all mutually compatible so
they could all be implemented.  If $HOME were supported directly
(either just "$HOME" or parsing all $ENVVARS) then it'd be easier
to decide to use getpwuid for ~.  Personally I'd use: 1) relative
path, 2) $HOME (as "~" or "$HOME"), 3) getpwuid (as "~")

Karl
--
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-22 14:10 PDT, Junio C Hamano writes:
>
>     Junio> If we were to support relative paths, I think it would
>     Junio> be useful and consistent if a relative path found in
>     Junio> ".git/config" is relative to the work tree root, in
>     Junio> "config" in a bare repository relative to the bare
>     Junio> repository, and in "$HOME/.gitconfig" relative to
>     Junio> $HOME.
>
> Makes sense to support it everywhere.  For .git/config, isn't it
> more consistent for it to be relative to .git?

Consistency and usefulness are different things.  Suppose you want as the
upstream of your project maintain and distribute a mail-alias list in-tree
(say, the file is at the root level, CONTRIBUTORS), and you suggest
contributors to use it when using "commit --author".

Which one do you want to write in your README:

        [user]
        nicknamelistfile = ../CONTRIBUTORS

or

        [user]
        nicknamelistfile = CONTRIBUTORS

You have to say the former if it is relative to .git/config.

> So, being new to git development, am I correctly assessing your
> response as "with refinement this can be included in git"?

I do not have fundamental objection to what you are trying to achieve
(i.e. being able to say "relative to $HOME").  I personally think the
approach you took in your patch (i.e. only support "~/" and use $HOME,
without any other fancy stuff) is a sensible first cut for that issue.

I just pointed out possible design issues about the future direction after
that first cut.  When I make comments on design-level issues, I rarely
read the patch itself very carefully, so it is a different issue if your
particular implementation in the patch is the best implementation of that
first cut approach.

--
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 Sun, Aug 24, 2008 at 11:11:14AM -0700, Junio C Hamano wrote:

> Consistency and usefulness are different things.  Suppose you want as the
> upstream of your project maintain and distribute a mail-alias list in-tree
> (say, the file is at the root level, CONTRIBUTORS), and you suggest
> contributors to use it when using "commit --author".
>
> Which one do you want to write in your README:
>
> [user]
>         nicknamelistfile = ../CONTRIBUTORS
>
> or
>
> [user]
>         nicknamelistfile = CONTRIBUTORS
>
> You have to say the former if it is relative to .git/config.

Couldn't the exact opposite argument be made for "suppose you want to
put the mail-alias file in a repo-specific directory that was not
tracked?" I.e., you are trading off "CONTRIBUTORS" against
".git/CONTRIBUTORS". So which one inconveniences the smallest number of
people is really a question of what people want to do with such pointers
(and since we don't support any yet, we don't really know...).

But more worrisome to me is that the working directory and git directory
do not necessarily follow a "../" and ".git/" relationship. How would
you resolve "../foo" with:

  GIT_DIR=/path/to/other/place git ...

or

  git --work-tree=/path/to/other/place

?

If you want to be able to point to either, I suspect we are better off
simply introducing some basic substitutions like $GIT_DIR and
$GIT_WORK_TREE. Maybe even just allow environment variable expansion,
and then promise to set those variables, which takes care of $HOME
automagically.

-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

[Junio C Hamano]

Jeff King <[hidden email]> writes:

> On Sun, Aug 24, 2008 at 11:11:14AM -0700, Junio C Hamano wrote:
>
>> Consistency and usefulness are different things.  Suppose you want as the
>> upstream of your project maintain and distribute a mail-alias list in-tree
>> (say, the file is at the root level, CONTRIBUTORS), and you suggest
>> contributors to use it when using "commit --author".
>>
>> Which one do you want to write in your README:
>>
>> [user]
>>         nicknamelistfile = ../CONTRIBUTORS
>>
>> or
>>
>> [user]
>>         nicknamelistfile = CONTRIBUTORS
>>
>> You have to say the former if it is relative to .git/config.
>
> Couldn't the exact opposite argument be made for "suppose you want to
> put the mail-alias file in a repo-specific directory that was not
> tracked?" I.e., you are trading off "CONTRIBUTORS" against
> ".git/CONTRIBUTORS".

No, I couldn't ;-)

Why would you write what you wrote in README?

Anything you store in .git is not propagated, so the instruction would not
likely to be "store it in .git/CONTRIBUTORS and point at it".  There is no
merit in forcing users to standardize on "in .git".  The instruction would
be to "store it anywhere you want, and point at it".

The example I gave is very different.  It points at an in-tree thing, and
anybody who has worktree checked out will have it _at the location I as
the README writer expect it to be_.  That is the difference that makes the
exact opposite argument much weaker.

That's why I suggested "relative to work tree if in .git/config, or gitdir
if in config of a bare repository", although honestly speaking I do not
have very strong preference either way.

> If you want to be able to point to either, I suspect we are better off
> simply introducing some basic substitutions like $GIT_DIR and
> $GIT_WORK_TREE. Maybe even just allow environment variable expansion,
> and then promise to set those variables, which takes care of $HOME
> automagically.

Because we haven't deprecated core.worktree (or $GIT_WORK_TREE) yet, your
suggestion has an obvious chicken-and-egg problem, even though otherwise I
think it makes perfect sense and very much like it.

Perhaps we should rid of the worktree that is separate and floats
unrelated to where $GIT_DIR is.
--
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 Sun, Aug 24, 2008 at 03:59:49PM -0700, Junio C Hamano wrote:

> > Couldn't the exact opposite argument be made for "suppose you want to
> > put the mail-alias file in a repo-specific directory that was not
> > tracked?" I.e., you are trading off "CONTRIBUTORS" against
> > ".git/CONTRIBUTORS".
>
> No, I couldn't ;-)
>
> Why would you write what you wrote in README?
>
> Anything you store in .git is not propagated, so the instruction would not
> likely to be "store it in .git/CONTRIBUTORS and point at it".  There is no
> merit in forcing users to standardize on "in .git".  The instruction would
> be to "store it anywhere you want, and point at it".

Ah, right.

I still think there is a little bit of convenience when you are doing
something totally personal (i.e., not putting it in a README, but rather
just wanting to store the referenced file in .git for the sake of
simplicity). But in that case, it is only slightly less convenient to
just point to the full path. So your example trumps this, since you have
no sane way of knowing the full path in README instructions.

> Because we haven't deprecated core.worktree (or $GIT_WORK_TREE) yet, your
> suggestion has an obvious chicken-and-egg problem, even though otherwise I
> think it makes perfect sense and very much like it.

You might be able to get around that by lazily filling in the variable.
IOW, expand it at the point-of-use rather than while reading the config.
However, the point of use might easily have something to do with reading
config, so that re-creates the cycle.

In general, I think we treat config as order-independent. We could make
the use of such variables order-dependent (i.e., if you haven't set
core.worktree, then we give you the value without having set it, and we
recalculate later. Confusing results, but at least a simple rule to
understand).

I think there actually are a few other order-dependent things in the
config, like the order of multi-value keys like push and fetch refspecs.

Would you want this expansion only for specially marked variables, or
for all variables? I like the concept of general templates for config
values, but it will backwards compatibility, especially for alias.*.

> Perhaps we should rid of the worktree that is separate and floats
> unrelated to where $GIT_DIR is.

I assumed people were actually using it, which is why it was
implemented.

-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

[Junio C Hamano]

Jeff King <[hidden email]> writes:

>> Perhaps we should rid of the worktree that is separate and floats
>> unrelated to where $GIT_DIR is.
>
> I assumed people were actually using it, which is why it was
> implemented.

Judging from the occasional "I tried core.worktree but it does not work in
this and that situations" I see here and on #git, my impression is that
new people try it, saying "git is cool -- unlike cvs that sprinkles those
ugly CVS directories all over the place, it only contaminates my work tree
with a single directory '.git' and nothing else.  Ah, wait --- what's this
core.worktree thing?  Can I get rid of that last one as well?  That sounds
even cooler".

IOW, I do not think it is really _needed_ per-se as a feature, but it was
done because it was thought to be doable, which unfortunately turned out
to involve hair-pulling complexity that the two attempts that led to the
current code still haven't resolved.

I really wish we do not have to worry about that anymore.
--
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 Sun, Aug 24, 2008 at 04:40:21PM -0700, Junio C Hamano wrote:

> Judging from the occasional "I tried core.worktree but it does not work in
> this and that situations" I see here and on #git, my impression is that
> new people try it, saying "git is cool -- unlike cvs that sprinkles those
> ugly CVS directories all over the place, it only contaminates my work tree
> with a single directory '.git' and nothing else.  Ah, wait --- what's this
> core.worktree thing?  Can I get rid of that last one as well?  That sounds
> even cooler".
>
> IOW, I do not think it is really _needed_ per-se as a feature, but it was
> done because it was thought to be doable, which unfortunately turned out
> to involve hair-pulling complexity that the two attempts that led to the
> current code still haven't resolved.
>
> I really wish we do not have to worry about that anymore.

Well, as a non-user of this feature, I certainly have no argument
against taking it out. Maybe the subject line will pull some other
people into the discussion.

-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

[Junio C Hamano]

Jeff King <[hidden email]> writes:

> On Sun, Aug 24, 2008 at 04:40:21PM -0700, Junio C Hamano wrote:
>
>> Judging from the occasional "I tried core.worktree but it does not work in
>> this and that situations" I see here and on #git, my impression is that
>> new people try it, saying "git is cool -- unlike cvs that sprinkles those
>> ugly CVS directories all over the place, it only contaminates my work tree
>> with a single directory '.git' and nothing else.  Ah, wait --- what's this
>> core.worktree thing?  Can I get rid of that last one as well?  That sounds
>> even cooler".
>>
>> IOW, I do not think it is really _needed_ per-se as a feature, but it was
>> done because it was thought to be doable, which unfortunately turned out
>> to involve hair-pulling complexity that the two attempts that led to the
>> current code still haven't resolved.
>>
>> I really wish we do not have to worry about that anymore.
>
> Well, as a non-user of this feature, I certainly have no argument
> against taking it out. Maybe the subject line will pull some other
> people into the discussion.

Heh, if we are to do the attention-getter, let's do so more strongly ;-)
--
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

[Miklos Vajna]

On Sun, Aug 24, 2008 at 05:30:54PM -0700, Junio C Hamano <[hidden email]> wrote:
> Heh, if we are to do the attention-getter, let's do so more strongly ;-)

Does this include removing of --work-tree as well?

The git backend of Pootle (http://translate.sourceforge.net/wiki/) uses
it.

Also, here is a question:

$ git --git-dir git/.git --work-tree git diff --stat|tail -n 1
 1443 files changed, 0 insertions(+), 299668 deletions(-)

So, it's like it thinks every file is removed.

But then:

$ cd git
$ git diff --stat|wc -l
0

is this a bug, or a user error?

Thanks.

[Junio C Hamano]

Miklos Vajna <[hidden email]> writes:

> On Sun, Aug 24, 2008 at 05:30:54PM -0700, Junio C Hamano <[hidden email]> wrote:
>> Heh, if we are to do the attention-getter, let's do so more strongly ;-)
>
> Does this include removing of --work-tree as well?
>
> The git backend of Pootle (http://translate.sourceforge.net/wiki/) uses
> it.

Interesting.  Does it use it because it can (meaning, --work-tree is
supposed to work), or because --work-tree is the cleanest way to do what
it wants to do (if the feature worked properly, that is, which is not the
case)?

> Also, here is a question:
>
> $ git --git-dir git/.git --work-tree git diff --stat|tail -n 1
>  1443 files changed, 0 insertions(+), 299668 deletions(-)
>
> So, it's like it thinks every file is removed.
>
> But then:
>
> $ cd git
> $ git diff --stat|wc -l
> 0
>
> is this a bug, or a user error?

I  think it is among the many other things that falls into "the two
attempts still haven't resolved" category.
--
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

[Miklos Vajna]

On Sun, Aug 24, 2008 at 08:05:03PM -0700, Junio C Hamano <[hidden email]> wrote:
> > Does this include removing of --work-tree as well?
> >
> > The git backend of Pootle (http://translate.sourceforge.net/wiki/) uses
> > it.
>
> Interesting.  Does it use it because it can (meaning, --work-tree is
> supposed to work), or because --work-tree is the cleanest way to do what
> it wants to do (if the feature worked properly, that is, which is not the
> case)?

It's like:

The current working directory is like
/usr/lib/python2.5/site-packages/Pootle. The git repository is under
/some/other/path/outside/usr.

Then Pootle has two possibilities:

1) save the current directory, change to /some/other, execute git, and
change the directory back

2) use git --work-tree / --git-dir

I guess the second form is more elegant. Of course if it is decided that
this option will be removed then the old form can be still used, but I
think that would be a step back.

> > Also, here is a question:
> >
> > $ git --git-dir git/.git --work-tree git diff --stat|tail -n 1
> >  1443 files changed, 0 insertions(+), 299668 deletions(-)
> >
> > So, it's like it thinks every file is removed.
> >
> > But then:
> >
> > $ cd git
> > $ git diff --stat|wc -l
> > 0
> >
> > is this a bug, or a user error?
>
> I  think it is among the many other things that falls into "the two
> attempts still haven't resolved" category.

I'm unfamiliar with this part of the codebase, so in case somebody other
could look at it, that would be great, but I'm happy with write a
testcase for it. (Or in case nobody cares, I can try to fix it, but that
may take a bit more time.)

[Nguyen Thai Ngoc Duy]

On 8/25/08, Miklos Vajna <[hidden email]> wrote:

> On Sun, Aug 24, 2008 at 08:05:03PM -0700, Junio C Hamano <[hidden email]> wrote:
>  > > Does this include removing of --work-tree as well?
>  > >
>  > > The git backend of Pootle (http://translate.sourceforge.net/wiki/) uses
>  > > it.
>  >
>  > Interesting.  Does it use it because it can (meaning, --work-tree is
>  > supposed to work), or because --work-tree is the cleanest way to do what
>  > it wants to do (if the feature worked properly, that is, which is not the
>  > case)?
>
>
> It's like:
>
>  The current working directory is like
>  /usr/lib/python2.5/site-packages/Pootle. The git repository is under
>  /some/other/path/outside/usr.
>
>  Then Pootle has two possibilities:
>
>  1) save the current directory, change to /some/other, execute git, and
>  change the directory back
>
>  2) use git --work-tree / --git-dir
>
>  I guess the second form is more elegant. Of course if it is decided that
>  this option will be removed then the old form can be still used, but I
>  think that would be a step back.
>
>
>  > > Also, here is a question:
>  > >
>  > > $ git --git-dir git/.git --work-tree git diff --stat|tail -n 1
>  > >  1443 files changed, 0 insertions(+), 299668 deletions(-)
>  > >
>  > > So, it's like it thinks every file is removed.
>  > >
>  > > But then:
>  > >
>  > > $ cd git
>  > > $ git diff --stat|wc -l
>  > > 0
>  > >
>  > > is this a bug, or a user error?
>  >
>  > I  think it is among the many other things that falls into "the two
>  > attempts still haven't resolved" category.
>
>
> I'm unfamiliar with this part of the codebase, so in case somebody other
>  could look at it, that would be great, but I'm happy with write a
>  testcase for it. (Or in case nobody cares, I can try to fix it, but that
>  may take a bit more time.)

Because "git diff" did not call setup_work_tree(). The same happens
for "git diff-index" that someone reported recently. IIRC "git
diff-files" has the same problem.
--
Duy
--
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

[Miklos Vajna]

This makes it possible to use git diff when we are outside the repo but
--work-tree and --git-dir is used.

Signed-off-by: Miklos Vajna <[hidden email]>
---

On Mon, Aug 25, 2008 at 08:52:11PM +0700, Nguyen Thai Ngoc Duy <[hidden email]> wrote:
> Because "git diff" did not call setup_work_tree(). The same happens
> for "git diff-index" that someone reported recently. IIRC "git
> diff-files" has the same problem.

Thanks, that was the problem.

 builtin-diff-files.c |    1 +
 builtin-diff-index.c |    1 +
 builtin-diff.c       |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 9bf10bb..4802e00 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -19,6 +19,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
  int result;
  unsigned options = 0;
 
+ setup_work_tree();
  init_revisions(&rev, prefix);
  git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
  rev.abbrev = 0;
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 17d851b..b8e0656 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -16,6 +16,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
  int i;
  int result;
 
+ setup_work_tree();
  init_revisions(&rev, prefix);
  git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
  rev.abbrev = 0;
diff --git a/builtin-diff.c b/builtin-diff.c
index 7ffea97..86f9255 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -244,6 +244,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
  int nongit;
  int result = 0;
 
+ setup_work_tree();
  /*
  * We could get N tree-ish in the rev.pending_objects list.
  * Also there could be M blobs there, and P pathspecs.
--
1.6.0.rc3.17.gc14c8.dirty

--
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

[Nguyen Thai Ngoc Duy]

On 8/25/08, Miklos Vajna <[hidden email]> wrote:

>  diff --git a/builtin-diff-index.c b/builtin-diff-index.c
>  index 17d851b..b8e0656 100644
>  --- a/builtin-diff-index.c
>  +++ b/builtin-diff-index.c
>  @@ -16,6 +16,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
>         int i;
>         int result;
>
>  +       setup_work_tree();
>         init_revisions(&rev, prefix);
>         git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
>         rev.abbrev = 0;

I think this is only needed when cached == 0

>  diff --git a/builtin-diff.c b/builtin-diff.c
>  index 7ffea97..86f9255 100644
>  --- a/builtin-diff.c
>  +++ b/builtin-diff.c
>  @@ -244,6 +244,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>         int nongit;
>         int result = 0;
>
>  +       setup_work_tree();
>         /*
>          * We could get N tree-ish in the rev.pending_objects list.
>          * Also there could be M blobs there, and P pathspecs.
>

No. git-diff has too many modes, some does not need worktree. This
forces worktree on all modes.
--
Duy
--
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

[Miklos Vajna]

On Mon, Aug 25, 2008 at 09:46:37PM +0700, Nguyen Thai Ngoc Duy <[hidden email]> wrote:

> On 8/25/08, Miklos Vajna <[hidden email]> wrote:
> >  diff --git a/builtin-diff-index.c b/builtin-diff-index.c
> >  index 17d851b..b8e0656 100644
> >  --- a/builtin-diff-index.c
> >  +++ b/builtin-diff-index.c
> >  @@ -16,6 +16,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
> >         int i;
> >         int result;
> >
> >  +       setup_work_tree();
> >         init_revisions(&rev, prefix);
> >         git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
> >         rev.abbrev = 0;
>
> I think this is only needed when cached == 0
>
> >  diff --git a/builtin-diff.c b/builtin-diff.c
> >  index 7ffea97..86f9255 100644
> >  --- a/builtin-diff.c
> >  +++ b/builtin-diff.c
> >  @@ -244,6 +244,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> >         int nongit;
> >         int result = 0;
> >
> >  +       setup_work_tree();
> >         /*
> >          * We could get N tree-ish in the rev.pending_objects list.
> >          * Also there could be M blobs there, and P pathspecs.
> >
>
> No. git-diff has too many modes, some does not need worktree. This
> forces worktree on all modes.

Ah, yes. I just wanted to say that I forgot do a 'make test' and
actually this breaks at least t0020-crlf.sh. I'll post a fixed patch in
a bit.

Sorry.

[Miklos Vajna]

This makes it possible to use git diff when we are outside the repo but
--work-tree and --git-dir is used.

Signed-off-by: Miklos Vajna <[hidden email]>
---
 builtin-diff-files.c |    1 +
 builtin-diff-index.c |    2 ++
 builtin-diff.c       |    1 +
 3 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 9bf10bb..4802e00 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -19,6 +19,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
  int result;
  unsigned options = 0;
 
+ setup_work_tree();
  init_revisions(&rev, prefix);
  git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
  rev.abbrev = 0;
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 17d851b..5510291 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -29,6 +29,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
  else
  usage(diff_cache_usage);
  }
+ if (!cached)
+ setup_work_tree();
  if (!rev.diffopt.output_format)
  rev.diffopt.output_format = DIFF_FORMAT_RAW;
 
diff --git a/builtin-diff.c b/builtin-diff.c
index 7ffea97..57da6ed 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -279,6 +279,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
  diff_no_index(&rev, argc, argv, nongit, prefix);
 
  /* Otherwise, we are doing the usual "git" diff */
+ setup_work_tree();
  rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
 
  if (nongit)
--
1.6.0.rc3.17.gc14c8.dirty

--
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