[BUG] 'add -u' doesn't work from untracked subdir

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[BUG] 'add -u' doesn't work from untracked subdir

SZEDER Gábor

Hi,


As the subject says, 'git add -u' does not work from an untracked
subdir, because it doesn't add modified files to the index.  The
following script reproduces the issue:

mkdir repo
cd repo
git init
echo 1 >foo
git add foo
git commit -m first
echo 2 >foo
mkdir untracked_subdir
cd untracked_subdir
git add -u
git diff

It worked in the initial 'git add -u' implementation (dfdac5d, git-add
-u: match the index with working tree, 2007-04-20), but 2ed2c222
(git-add -u paths... now works from subdirectory, 2007-08-16) broke it
later, and is broken ever since.


Regards,
Gábor

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jeff King
On Wed, Sep 02, 2009 at 10:03:05AM +0200, SZEDER Gábor wrote:

> As the subject says, 'git add -u' does not work from an untracked
> subdir, because it doesn't add modified files to the index.  The
> following script reproduces the issue:
>
> mkdir repo
> cd repo
> git init
> echo 1 >foo
> git add foo
> git commit -m first
> echo 2 >foo
> mkdir untracked_subdir
> cd untracked_subdir
> git add -u
> git diff
>
> It worked in the initial 'git add -u' implementation (dfdac5d, git-add
> -u: match the index with working tree, 2007-04-20), but 2ed2c222
> (git-add -u paths... now works from subdirectory, 2007-08-16) broke it
> later, and is broken ever since.

It is not just untracked subdirs. Try:

  mkdir repo && cd repo && git init
  echo 1 >foo
  mkdir subdir
  echo 1 >subdir/bar
  git add . && git commit -m first
  echo 2 >foo
  echo 2 >subdir/bar
  cd subdir
  git add -u
  git diff ;# still shows foo/1 in index
  git diff --cached ;# shows subdir/bar was updated

While I have sometimes found the behavior a bit annoying[1], I always
assumed that was the intended behavior.

And indeed, in modern builtin-add.c, we find this:

        if ((addremove || take_worktree_changes) && !argc) {
                static const char *here[2] = { ".", NULL };
                argc = 1;
                argv = here;
        }

which seems pretty explicit.

-Peff

[1] I would prefer "git add -u ." to add only the current directory, and
"git add -u" to touch everything. But then, I am one of the people who
turn off status.relativepaths, so I think I may be in the minority in
always wanting to think of the project as a whole.
--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Clemens Buchacher
On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:

> [1] I would prefer "git add -u ." to add only the current directory, and
> "git add -u" to touch everything.

FWIW, I feel the same way. And there is no easy way to do that now. (cd `git
rev-parse --show-cdup`; git add -u) ?

> But then, I am one of the people who
> turn off status.relativepaths, so I think I may be in the minority in
> always wanting to think of the project as a whole.

That mindset is one of git's greatest strengths IMO.

Clemens
--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

SZEDER Gábor
In reply to this post by Jeff King

[Oops, I 've just noticed that my reply to Jeff didn't made it to the
git list, because I hit 'reply' instead of 'reply to all'...]



Hi Jeff,


thanks for your quick reply.

On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:

> On Wed, Sep 02, 2009 at 10:03:05AM +0200, SZEDER Gábor wrote:
>
> > As the subject says, 'git add -u' does not work from an untracked
> > subdir, because it doesn't add modified files to the index.  The
> > following script reproduces the issue:
> >
> > mkdir repo
> > cd repo
> > git init
> > echo 1 >foo
> > git add foo
> > git commit -m first
> > echo 2 >foo
> > mkdir untracked_subdir
> > cd untracked_subdir
> > git add -u
> > git diff
> >
> > It worked in the initial 'git add -u' implementation (dfdac5d,
> > git-add
> > -u: match the index with working tree, 2007-04-20), but 2ed2c222
> > (git-add -u paths... now works from subdirectory, 2007-08-16)
> > broke it
> > later, and is broken ever since.
>
> It is not just untracked subdirs. Try:
>
>   mkdir repo && cd repo && git init
>   echo 1 >foo
>   mkdir subdir
>   echo 1 >subdir/bar
>   git add . && git commit -m first
>   echo 2 >foo
>   echo 2 >subdir/bar
>   cd subdir
>   git add -u
>   git diff ;# still shows foo/1 in index
>   git diff --cached ;# shows subdir/bar was updated
>
> While I have sometimes found the behavior a bit annoying[1], I
> always
> assumed that was the intended behavior.
>
> And indeed, in modern builtin-add.c, we find this:
>
>         if ((addremove || take_worktree_changes) && !argc) {
>                 static const char *here[2] = { ".", NULL };
>                 argc = 1;
>                 argv = here;
>         }
>
> which seems pretty explicit.

Since then I looked at the man page (I should have done that right
away ;), and it says under the description of -u that "If no paths are
specified, all tracked files in the current directory and its
subdirectories are updated."  So this is indeed the intended
behaviour, but I was just not aware of it.  Oh well, sorry for the
noise.

> [1] I would prefer "git add -u ." to add only the current directory,
> and
> "git add -u" to touch everything. But then, I am one of the people
> who
> turn off status.relativepaths, so I think I may be in the minority
> in
> always wanting to think of the project as a whole.

I don't really know which would I prefer.

I was updating some Javadoc documentation in Eclipse, and checking the
generated docs in terminal, deep down in an untracked subdir, and
performed some 'add -u ; commit --amend' from there (and was rather
surprised after the fifth amend to see all the changes still in the
worktree).  Doing perform the desired add -u from there I should have
run 'git add -u ../../../../../..', what doesn't seem very convenient.
But since this was the first time I've done that since 2007-08-16, I
guess it's not a very common use case.


Gábor

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jeff King
In reply to this post by Clemens Buchacher
On Fri, Sep 04, 2009 at 09:02:16AM +0200, Clemens Buchacher wrote:

> On Wed, Sep 02, 2009 at 04:19:17AM -0400, Jeff King wrote:
>
> > [1] I would prefer "git add -u ." to add only the current directory, and
> > "git add -u" to touch everything.
>
> FWIW, I feel the same way. And there is no easy way to do that now. (cd `git
> rev-parse --show-cdup`; git add -u) ?

I suspect it is too late to change it due to compatibility issues. OTOH,
I think the intent of v1.7.0 is to allow a few small breakages like
these. You could always write an RFC patch and generate some discussion;
I'm not 100% sure that there are enough people that agree with us to
change the default.

I guess we could add a "git add --absolute" option, though it is
slightly annoying, because I generally do not realize that I needed to
use such an option until several minutes after running "git add".

I would be fine with a "be absolute, not relative" config option such as
what we have for status (in fact, a global "be absolute, not relative"
option to cover all commands might be handy). The only obstacle is that
I think "git add" is often used as plumbing in scripts (arguably, they
should be using update-index, but "git add ." is just so convenient).

-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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Junio C Hamano
Jeff King <[hidden email]> writes:

> I suspect it is too late to change it due to compatibility issues. OTOH,
> I think the intent of v1.7.0 is to allow a few small breakages like
> these. You could always write an RFC patch and generate some discussion;
> I'm not 100% sure that there are enough people that agree with us to
> change the default.

The intent of 1.7.0 is to fix usability glitches and warts that everybody
has long agreed are problematic.  People have *just started* discussing
about this---it is not remotely close to "everybody has long agreed are
problematic" criteria.  It is too late for 1.7.0.

I agree that there are parts of git that is very whole tree oriented, and
the later "usability" part that are cwd centric.  "add -u" and "grep" are
examples of the latter.

I personally find "add -u" that defaults to the current directory more
natural than always going to the root; same preference for "grep".
Besides, "add -u subdir" must add subdir relative to the cwd, without
going to the root.  Why should "add -u" sans argument behave drastically
differently?

Speaking of cwd-ness, I sometimes find ls-tree irritating, but I think
this is in "if we had known better we would have designed it differently,
but because we didn't, because many scripts already depend on the current
behaviour, and because we have an --full-name escape hatch, we are not
likely to change it, ever" category.

If "git add -u ../.." (I mean "the grand parent directory", not "an
unnamed subdirectory") did not work, it would be unexcusable and we would
want to devise an migration path, but otherwise I do not think it is such
a big deal.  I would say the commands that are used to incrementally build
towards the next commit should be friendly to the practice of limiting the
scope of the work by chdir, i.e. they should be cwd centric.  On the other
hand, the commands that are used to review the next commit as a whole,
e.g. diff and patch, should be whole-tree oriented.

Oh, "git grep -e foo ../..", however, does not seem to work.  That might be
something people may want to tackle.

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jeff King
On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:

> The intent of 1.7.0 is to fix usability glitches and warts that everybody
> has long agreed are problematic.  People have *just started* discussing
> about this---it is not remotely close to "everybody has long agreed are
> problematic" criteria.  It is too late for 1.7.0.

What about a config option that could change the behavior? True, the
time-frame for discussion is much shorter, but we are not proposing to
make a change that would affect users who do not agree to it. And I
think the point of giving a long time-frame for discussion is to let
people decide whether a change that users do not agree to may affect
them in a bad way.

The real danger here is that users of the config option may be breaking
an interface that is used by scripts. But I feel that 1.7.0 is probably
the best time in the forseeable future to do that, as script-writers
already must be wary of the version change.

> I personally find "add -u" that defaults to the current directory more
> natural than always going to the root; same preference for "grep".
> Besides, "add -u subdir" must add subdir relative to the cwd, without
> going to the root.  Why should "add -u" sans argument behave drastically
> differently?

I agree that there is a certain consistency to the current behavior. But
I also find it terribly annoying, because I _always_ want it to do the
other thing, and it silently accepts the command without even telling
me, leaving me to find out ten minutes later that what I thought was
added was not ("git add", by contrast, yells at you in the same
situation).

I also happen to prefer the other behavior because it is easy to switch
the two options: "git add -u" versus "git add -u .", whereas with
current behavior I am stuck calculating (and typing) the correct number
of "../" markers.

But I respect the fact that even if we had infinite time for discussion,
there would be people who prefer it the opposite way to me. So how about
that config option?

> Speaking of cwd-ness, I sometimes find ls-tree irritating, but I think
> this is in "if we had known better we would have designed it differently,
> but because we didn't, because many scripts already depend on the current
> behaviour, and because we have an --full-name escape hatch, we are not
> likely to change it, ever" category.

I assume you mean "ls-files".  I have every once in a while been annoyed
by that, but given how infrequently I run ls-files, it is not a big
deal. :)

> If "git add -u ../.." (I mean "the grand parent directory", not "an
> unnamed subdirectory") did not work, it would be unexcusable and we would
> want to devise an migration path, but otherwise I do not think it is such
> a big deal.  I would say the commands that are used to incrementally build

As I mentioned above, not only is that annoying to use, but the real
problem is that I _expect_ the other behavior and it silently does the
opposite of what I want. You can argue that my brain is defective (for
not remembering, I mean -- we _know_ it's defective in other ways), but
certainly a config option would be useful to me.

> Oh, "git grep -e foo ../..", however, does not seem to work.  That might be
> something people may want to tackle.

Thanks for mentioning "git grep"; I had forgotten that I have been
bitten by expecting full-tree behavior from that in the past, too. A
config option should cover that, too. ;)

-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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Junio C Hamano
In reply to this post by Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Jeff King <[hidden email]> writes:
>
>> I suspect it is too late to change it due to compatibility issues. OTOH,
>> I think the intent of v1.7.0 is to allow a few small breakages like
>> these. You could always write an RFC patch and generate some discussion;
>> I'm not 100% sure that there are enough people that agree with us to
>> change the default.
>
> The intent of 1.7.0 is to fix usability glitches and warts that everybody
> has long agreed are problematic.  People have *just started* discussing
> about this---it is not remotely close to "everybody has long agreed are
> problematic" criteria.  It is too late for 1.7.0.

I just wanted to see if I am being fair.  Here are the topics that we may
have in 1.7.0.

 * jc/1.7.0-push-safety

   Prevents gremlin updates from sideways to a repository with a work
   tree, that confused countless new people.  I've resisted this change
   for a long time on the backward compatiblity ground, but it is very
   fair to say that it was long agreed that the benefit from the change
   far outweigh the donesides of having to say "I do want the old
   behaviour" by setting an configuration variable or two.

 * jc/1.7.0-send-email-no-thread-default

   Defaults multi-message send-email to thread shallowly.  This change was
   requested by kernel folks for a long time ago, and discussion on-list
   resulted in a declaration that unless nobody objects 1.6.2 release
   notes will say the default will change in 1.6.3.  We did not hear any
   objection, but the switchover did not happen ;-).

 * jc/1.7.0-status

   Everybody hated that "status $args" being "commit --dry-run $args"
   since 1.4.0 days.  We will give "commit --dry-run $args" in 1.6.5.

 * jc/1.7.0-diff-whitespace-only-status

   We said "diff -w" only affects the appearance but not the exit code, so
   "diff -w --exit-code" never returned success if there were only
   whitespace changes.  It was noticed to be illogical since day one
   of the introduction of --exit-code, but we simply did not bother to fix
   the implementation of -b and -w, since the combination of these two
   options were thought to be unlikely, and we were simply lazy ;-)

I think the first three are clearly 1.7.0 candidates, judging from the
benefit/downside perspective and also from the escape-hatch perspective,
The last one is probably less so, but on the other hand it is of far
lessor impact that we could even roll it out as a bugfix on 'maint'.

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Junio C Hamano
In reply to this post by Jeff King
Jeff King <[hidden email]> writes:

> I assume you mean "ls-files".  I have every once in a while been annoyed
> by that, but given how infrequently I run ls-files, it is not a big
> deal. :)

I did mean ls-tree, but I misspelled the name of the escape hatch.

Try this:

    $ cd Documentation
    $ git ls-tree HEAD

If I were designing this as a proper plumbing command from scratch, I
wouldn't have given it a cwd behaviour.  ls-files is somewhat more
understandable, as it has other cruft relating the work tree, but ls-tree
is worse:

    $ cd Documentation
    $ git ls-tree origin/html

Whoa???  Yes, it tried to do what "git ls-tree origin/html:Documentation"
would have done if it were unaware of cwd.  It's just crazy.

>> If "git add -u ../.." (I mean "the grand parent directory", not "an
>> unnamed subdirectory") did not work, it would be unexcusable and we would
>> want to devise an migration path, but otherwise I do not think it is such
>> a big deal.  I would say the commands that are used to incrementally build
>
> As I mentioned above, not only is that annoying to use, but the real
> problem is that I _expect_ the other behavior and it silently does the
> opposite of what I want. You can argue that my brain is defective (for
> not remembering, I mean -- we _know_ it's defective in other ways), but
> certainly a config option would be useful to me.

At this moment (as my brain is not quite functioning), I can only say we
agreed to disagree what feels more natural here.

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jeff King
On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:

> Jeff King <[hidden email]> writes:
>
> > I assume you mean "ls-files".  I have every once in a while been annoyed
> > by that, but given how infrequently I run ls-files, it is not a big
> > deal. :)
>
> I did mean ls-tree, but I misspelled the name of the escape hatch.

Oh, I never noticed that behavior before. For "ls-files", I think it is
at least a little sane, but it makes no sense whatsoever for ls-tree.

> At this moment (as my brain is not quite functioning), I can only say we
> agreed to disagree what feels more natural here.

I agree that we agreed to disagree. But there is still one open
question: would you take a patch for a "full-tree" config variable that
would impact add (and probably grep) for 1.7.0? You can sleep on it if
you want. ;)

-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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jeff King
In reply to this post by Jeff King
On Sat, Sep 05, 2009 at 03:20:17AM -0400, Jeff King wrote:

> As I mentioned above, not only is that annoying to use, but the real
> problem is that I _expect_ the other behavior and it silently does the
> opposite of what I want. You can argue that my brain is defective (for
> not remembering, I mean -- we _know_ it's defective in other ways), but
> certainly a config option would be useful to me.

Bah. Even after this long thread, I _still_ forgot. I just now typed
"git add -u" from t/ and got annoyed that my changes in the root weren't
added.

-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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Junio C Hamano
In reply to this post by Jeff King
Jeff King <[hidden email]> writes:

> I agree that we agreed to disagree. But there is still one open
> question: would you take a patch for a "full-tree" config variable that
> would impact add (and probably grep) for 1.7.0?

Unless there is a simple and sane way for script writers (and by "script",
I do not mean "Porcelain that is supposed to be written using only the
plumbing", but things like scripts you would give to "git bisect run",
which would freely use Porcelains like "git reset" etc.) to defeat the
configuration without much pain, I am fairly negative on adding any
configuration variable of that nature.

We could probably declare "In 1.X.0 everything will be relative to the
root and you have to give an explicit '.' if you mean the cwd".

Three questions:

 #1 What are the commands that will be affected, other than "add -u" and
    "grep"?  Are there others?

 #2 Do all the commands in the answer to #1 currently behave exactly the
    same when run without any path parameter and when run with a single
    '.'?

 #3 Do all the commands that are already relative to the root currently
    limit their action to the cwd when run with a single '.'?

If the number of commands in the answer to #1 is not too excessive, it is
a plus, but even if it is more than just several, we will be getting
consistency and sanity if #2 and #3 hold.  However, if there are even a
single violator in #2 and #3, we would need to fix them first before we
can proceed.  And the transition clock starts ticking after everything is
fixed (if such a fix is indeed needed).  As usual, I'd prefer to keep the
clock running for at least 6 months, preferrably longer, and during that
time, we may need the usual "You invoked me without any paths, but this
command will start behaving differently in 1.X.0, you have been warned."

A command line option to explicitly ask full-tree can be added anytime
without waiting for 1.7.0.  I do not think it will be ready for 1.6.5 but
we can always have 1.6.6 if needed.
--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Clemens Buchacher
In reply to this post by Junio C Hamano
On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:

> I personally find "add -u" that defaults to the current directory more
> natural than always going to the root; same preference for "grep".
> Besides, "add -u subdir" must add subdir relative to the cwd, without
> going to the root.  Why should "add -u" sans argument behave drastically
> differently?

Sorry for stating the obvious here, but the following commands affect the
entire repository, even though they limit themselves to the current
directory, if passed a '.'.

        git commit
        git log
        git diff
        git checkout
        git reset

Due to the frequent use of these commands, I believe many users (myself
included) expect "git add" and "git grep" to do the same. AFAICT the
following commands are the only non-plumbing ones that behave differently:

        git add -u
        git add -A
        git grep

So I argue that _that_ is the real inconsistency.

> If "git add -u ../.." (I mean "the grand parent directory", not "an
> unnamed subdirectory") did not work, it would be unexcusable and we would
> want to devise an migration path, but otherwise I do not think it is such
> a big deal.

> I would say the commands that are used to incrementally build
> towards the next commit should be friendly to the practice of limiting the
> scope of the work by chdir.

"git add -u ." is friendly enough. Just like "git commit ." versus "git
commit -a", which is exactly the same concept and should therefore have the
same behavior.

You are assuming that people are in a subdirectory because they want to
limit the scope. But I am usually in a subdirectory for totally
versioning-unrelated reasons. Like running tests in git.git:t/ . I
mistakenly use "git add -u" in there all the time, because I think I don't
have to worry about which directory I'm in. Except in this instance I do.

In any case, I think it is better to have consistent behavior than to try
and read users' minds with defaults.

Clemens
--
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
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] grep: accept relative paths outside current working directory

Clemens Buchacher
In reply to this post by Junio C Hamano
On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:
> >> If "git add -u ../.." (I mean "the grand parent directory", not "an
> >> unnamed subdirectory") did not work

I just noticed that this doesn't work with grep. In git.git:

$ cd t
$ git grep addremove -- ../
fatal: git grep: cannot generate relative filenames containing '..'

So here's a fix for that. And a configurable solution for add and grep's
scope as a follow-up. I did not look at any other commands yet.

Clemens

--o<--
Previously, "git grep" would bark at relative paths pointing outside
the current working directory (or subdirectories thereof). We already
have quote_path_relative(), which can handle such cases just fine. So
use that instead.

Signed-off-by: Clemens Buchacher <[hidden email]>
---
 builtin-grep.c |   44 ++++++++++++++------------------------------
 grep.h         |    1 +
 2 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index ad0e0a5..f6af3d4 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -13,6 +13,7 @@
 #include "parse-options.h"
 #include "userdiff.h"
 #include "grep.h"
+#include "quote.h"
 
 #ifndef NO_EXTERNAL_GREP
 #ifdef __unix__
@@ -152,40 +153,27 @@ static int pathspec_matches(const char **paths, const char *name, int max_depth)
  return 0;
 }
 
-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char *name, int tree_name_len)
+static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char *path, int tree_name_len)
 {
  unsigned long size;
  char *data;
  enum object_type type;
- char *to_free = NULL;
  int hit;
+ struct strbuf pathbuf = STRBUF_INIT;
 
  data = read_sha1_file(sha1, &type, &size);
  if (!data) {
- error("'%s': unable to read %s", name, sha1_to_hex(sha1));
+ error("'%s': unable to read %s", path, sha1_to_hex(sha1));
  return 0;
  }
  if (opt->relative && opt->prefix_length) {
- static char name_buf[PATH_MAX];
- char *cp;
- int name_len = strlen(name) - opt->prefix_length + 1;
-
- if (!tree_name_len)
- name += opt->prefix_length;
- else {
- if (ARRAY_SIZE(name_buf) <= name_len)
- cp = to_free = xmalloc(name_len);
- else
- cp = name_buf;
- memcpy(cp, name, tree_name_len);
- strcpy(cp + tree_name_len,
-       name + tree_name_len + opt->prefix_length);
- name = cp;
- }
+ quote_path_relative(path + tree_name_len, -1, &pathbuf, opt->prefix);
+ strbuf_insert(&pathbuf, 0, path, tree_name_len);
+ path = pathbuf.buf;
  }
- hit = grep_buffer(opt, name, data, size);
+ hit = grep_buffer(opt, path, data, size);
+ strbuf_release(&pathbuf);
  free(data);
- free(to_free);
  return hit;
 }
 
@@ -195,6 +183,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
  int i;
  char *data;
  size_t sz;
+ struct strbuf buf = STRBUF_INIT;
 
  if (lstat(filename, &st) < 0) {
  err_ret:
@@ -219,8 +208,9 @@ static int grep_file(struct grep_opt *opt, const char *filename)
  }
  close(i);
  if (opt->relative && opt->prefix_length)
- filename += opt->prefix_length;
+ filename = quote_path_relative(filename, -1, &buf, opt->prefix);
  i = grep_buffer(opt, filename, data, sz);
+ strbuf_release(&buf);
  free(data);
  return i;
 }
@@ -798,6 +788,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
  };
 
  memset(&opt, 0, sizeof(opt));
+ opt.prefix = prefix;
  opt.prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
  opt.relative = 1;
  opt.pathname = 1;
@@ -868,15 +859,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
  verify_filename(prefix, argv[j]);
  }
 
- if (i < argc) {
+ if (i < argc)
  paths = get_pathspec(prefix, argv + i);
- if (opt.prefix_length && opt.relative) {
- /* Make sure we do not get outside of paths */
- for (i = 0; paths[i]; i++)
- if (strncmp(prefix, paths[i], opt.prefix_length))
- die("git grep: cannot generate relative filenames containing '..'");
- }
- }
  else if (prefix) {
  paths = xcalloc(2, sizeof(const char *));
  paths[0] = prefix;
diff --git a/grep.h b/grep.h
index 28e6b2a..f6eecc6 100644
--- a/grep.h
+++ b/grep.h
@@ -59,6 +59,7 @@ struct grep_opt {
  struct grep_pat *pattern_list;
  struct grep_pat **pattern_tail;
  struct grep_expr *pattern_expression;
+ const char *prefix;
  int prefix_length;
  regex_t regexp;
  int linenum;
--
1.6.4.2.264.g79b4f

--
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
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] add 'scope' config option

Clemens Buchacher
Documentation/config.txt says it all.

Signed-off-by: Clemens Buchacher <[hidden email]>
---
 Documentation/config.txt |    6 ++++++
 builtin-add.c            |    2 +-
 builtin-grep.c           |    2 +-
 cache.h                  |    1 +
 config.c                 |    8 ++++++++
 environment.c            |    1 +
 6 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5256c7f..f587cf1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -439,6 +439,12 @@ On some file system/operating system combinations, this is unreliable.
 Set this config setting to 'rename' there; However, This will remove the
 check that makes sure that existing object files will not get overwritten.
 
+core.scope::
+ By default, the commands 'git add -u', 'git add -A' and 'git grep'
+ are limited to files below the current working directory
+ (scope='workdir'). Set this variable to scope='global' to make these
+ commands act on the whole tree instead.
+
 add.ignore-errors::
  Tells 'git-add' to continue adding files when some files cannot be
  added due to indexing errors. Equivalent to the '--ignore-errors'
diff --git a/builtin-add.c b/builtin-add.c
index 006fd08..33ea3e4 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -285,7 +285,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
  if (addremove && take_worktree_changes)
  die("-A and -u are mutually incompatible");
- if ((addremove || take_worktree_changes) && !argc) {
+ if (!scope_global && (addremove || take_worktree_changes) && !argc) {
  static const char *here[2] = { ".", NULL };
  argc = 1;
  argv = here;
diff --git a/builtin-grep.c b/builtin-grep.c
index f6af3d4..447b195 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -861,7 +861,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 
  if (i < argc)
  paths = get_pathspec(prefix, argv + i);
- else if (prefix) {
+ else if (!scope_global && prefix) {
  paths = xcalloc(2, sizeof(const char *));
  paths[0] = prefix;
  paths[1] = NULL;
diff --git a/cache.h b/cache.h
index 5fad24c..85c5fee 100644
--- a/cache.h
+++ b/cache.h
@@ -523,6 +523,7 @@ extern int auto_crlf;
 extern int read_replace_refs;
 extern int fsync_object_files;
 extern int core_preload_index;
+extern int scope_global;
 
 enum safe_crlf {
  SAFE_CRLF_FALSE = 0,
diff --git a/config.c b/config.c
index e87edea..8dec019 100644
--- a/config.c
+++ b/config.c
@@ -503,6 +503,14 @@ static int git_default_core_config(const char *var, const char *value)
  return 0;
  }
 
+ if (!strcmp(var, "core.scope")) {
+ if (!strcasecmp(value, "global"))
+ scope_global = 1;
+ if (!strcasecmp(value, "worktree"))
+ scope_global = 0;
+ return 0;
+ }
+
  /* Add other config variables here and to Documentation/config.txt. */
  return 0;
 }
diff --git a/environment.c b/environment.c
index 5de6837..4d1c6e1 100644
--- a/environment.c
+++ b/environment.c
@@ -50,6 +50,7 @@ enum push_default_type push_default = PUSH_DEFAULT_MATCHING;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_replace_parents = 1;
+int scope_global = 0;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
--
1.6.4.2.264.g79b4f

--
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
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2 v2] add 'scope' config option

Clemens Buchacher
On Sat, Sep 05, 2009 at 02:33:04PM +0200, Clemens Buchacher wrote:
> - if ((addremove || take_worktree_changes) && !argc) {
> + if (!scope_global && (addremove || take_worktree_changes) && !argc) {
>   static const char *here[2] = { ".", NULL };
>   argc = 1;
>   argv = here;

Ok, scrape that. Seems validate_pathspec does that implicitly already. So
this code was redundant. The attached patch should now work (I tested it
this time...).

It also seems that "git add -A" doesn't work without a pathspec any more?
Was that intentional?

Clemens

--o<--

Signed-off-by: Clemens Buchacher <[hidden email]>
---
 Documentation/config.txt |    6 ++++++
 builtin-add.c            |   12 +++++-------
 builtin-grep.c           |    2 +-
 cache.h                  |    1 +
 config.c                 |    8 ++++++++
 environment.c            |    1 +
 6 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5256c7f..f587cf1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -439,6 +439,12 @@ On some file system/operating system combinations, this is unreliable.
 Set this config setting to 'rename' there; However, This will remove the
 check that makes sure that existing object files will not get overwritten.
 
+core.scope::
+ By default, the commands 'git add -u', 'git add -A' and 'git grep'
+ are limited to files below the current working directory
+ (scope='workdir'). Set this variable to scope='global' to make these
+ commands act on the whole tree instead.
+
 add.ignore-errors::
  Tells 'git-add' to continue adding files when some files cannot be
  added due to indexing errors. Equivalent to the '--ignore-errors'
diff --git a/builtin-add.c b/builtin-add.c
index 006fd08..737b155 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -285,14 +285,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
  if (addremove && take_worktree_changes)
  die("-A and -u are mutually incompatible");
- if ((addremove || take_worktree_changes) && !argc) {
- static const char *here[2] = { ".", NULL };
- argc = 1;
- argv = here;
- }
 
  add_new_files = !take_worktree_changes && !refresh_only;
- require_pathspec = !take_worktree_changes;
+ require_pathspec = !addremove && !take_worktree_changes;
 
  newfd = hold_locked_index(&lock_file, 1);
 
@@ -308,7 +303,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)
  fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
  return 0;
  }
- pathspec = validate_pathspec(argc, argv, prefix);
+ if (scope_global && !argc)
+ pathspec = NULL;
+ else
+ pathspec = validate_pathspec(argc, argv, prefix);
 
  if (read_cache() < 0)
  die("index file corrupt");
diff --git a/builtin-grep.c b/builtin-grep.c
index f6af3d4..447b195 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -861,7 +861,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 
  if (i < argc)
  paths = get_pathspec(prefix, argv + i);
- else if (prefix) {
+ else if (!scope_global && prefix) {
  paths = xcalloc(2, sizeof(const char *));
  paths[0] = prefix;
  paths[1] = NULL;
diff --git a/cache.h b/cache.h
index 5fad24c..85c5fee 100644
--- a/cache.h
+++ b/cache.h
@@ -523,6 +523,7 @@ extern int auto_crlf;
 extern int read_replace_refs;
 extern int fsync_object_files;
 extern int core_preload_index;
+extern int scope_global;
 
 enum safe_crlf {
  SAFE_CRLF_FALSE = 0,
diff --git a/config.c b/config.c
index e87edea..8dec019 100644
--- a/config.c
+++ b/config.c
@@ -503,6 +503,14 @@ static int git_default_core_config(const char *var, const char *value)
  return 0;
  }
 
+ if (!strcmp(var, "core.scope")) {
+ if (!strcasecmp(value, "global"))
+ scope_global = 1;
+ if (!strcasecmp(value, "worktree"))
+ scope_global = 0;
+ return 0;
+ }
+
  /* Add other config variables here and to Documentation/config.txt. */
  return 0;
 }
diff --git a/environment.c b/environment.c
index 5de6837..4d1c6e1 100644
--- a/environment.c
+++ b/environment.c
@@ -50,6 +50,7 @@ enum push_default_type push_default = PUSH_DEFAULT_MATCHING;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_replace_parents = 1;
+int scope_global = 0;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
--
1.6.4.2.266.gbaa17

--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Junio C Hamano
In reply to this post by Clemens Buchacher
Clemens Buchacher <[hidden email]> writes:

> "git add -u ." is friendly enough. Just like "git commit ." versus "git
> commit -a", which is exactly the same concept and should therefore have the
> same behavior.
>
> You are assuming that people are in a subdirectory because they want to
> limit the scope. But I am usually in a subdirectory for totally
> versioning-unrelated reasons.

Limit the scope of what you see in "ls" (no argument) output, shorten the
paths you must type to non-git commands.  They are the kind of "limit the
scope" I meant, and they are totally versioning-unrelated.  In other
words, cwd-centric default helps the users (especially the new ones) by
making git behave consistently with other commands.

So if anything, I personally think it would be much less surprising if all
git commands worked relative to the cwd (not whole tree root) when run
without path argument, at least from the newbie's point of view [*1*].

But notice that the above is qualified with "personally".  An alternative
would be to declare that in 1.8.0, all commands run without path argument
will work on the whole tree and you have to give an explicit '.' when you
want to limit their effect to the cwd.

This may be slightly less intuitive to newbies than the "relative to cwd",
but nevertheless that is the course I would suggest us taking, because of
the following observations:

 (1) if the commands work on the whole tree when run without paths, it is
     easy to limit to the cwd with "git frotz ." when you want to.

 (2) if the commands work on the cwd when run without paths, you have to
     always be aware how deep you are, and say "git frotz ../../.." when
     you want to extend their effects to the whole tree.

The latter is much more irritating.

Please also see:

    Message-ID: <[hidden email]> ($gmane/127795)

think about the three questions there, and help us move the discussion
forward.

The first part of the message has some comments related to your patch, by
the way.

[Footnote]

*1* Except for the ones that cannot make any sense to limit their
operation to a subdirectory you happen to be in (e.g. it would be insane
if "git am" run to accept somebody's patch ignored paths outside your
cwd).
--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Jakub Narębski
Junio C Hamano <[hidden email]> writes:

> Clemens Buchacher <[hidden email]> writes:
>
> > "git add -u ." is friendly enough. Just like "git commit ." versus "git
> > commit -a", which is exactly the same concept and should therefore have the
> > same behavior.
> >
> > You are assuming that people are in a subdirectory because they want to
> > limit the scope. But I am usually in a subdirectory for totally
> > versioning-unrelated reasons.
>
> Limit the scope of what you see in "ls" (no argument) output, shorten the
> paths you must type to non-git commands.  They are the kind of "limit the
> scope" I meant, and they are totally versioning-unrelated.  In other
> words, cwd-centric default helps the users (especially the new ones) by
> making git behave consistently with other commands.

Well, there is still complication that some commands are considered
whole-tree in absence of pathspec, like "git commit".

>
> So if anything, I personally think it would be much less surprising if all
> git commands worked relative to the cwd (not whole tree root) when run
> without path argument, at least from the newbie's point of view [*1*].

I think it would be very suprising if "git commit" in subdirectory was
limited to changes affecting given subdirectory...

>
> But notice that the above is qualified with "personally".  An alternative
> would be to declare that in 1.8.0, all commands run without path argument
> will work on the whole tree and you have to give an explicit '.' when you
> want to limit their effect to the cwd.
>
> This may be slightly less intuitive to newbies than the "relative to cwd",
> but nevertheless that is the course I would suggest us taking, because of
> the following observations:
>
>  (1) if the commands work on the whole tree when run without paths, it is
>      easy to limit to the cwd with "git frotz ." when you want to.
>
>  (2) if the commands work on the cwd when run without paths, you have to
>      always be aware how deep you are, and say "git frotz ../../.." when
>      you want to extend their effects to the whole tree.
>
> The latter is much more irritating.

Well, we can always invent some magic notation meaning either "up to
top directory", e.g. make

  $ git frotz ...

more or less equivalent to

  $ git frotz "$(git rev-parse --show-cdup)"

(The other solution of having "git frotz /" to refer to top directory
is slightly worse, because there are git commands that work without
git repository, and "/" is legitimate parameter, like e.g. for
"git diff --no-index").

--
Jakub Narebski
Poland
ShadeHawk on #git
--
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
Reply | Threaded
Open this post in threaded view
|

Re: [BUG] 'add -u' doesn't work from untracked subdir

Clemens Buchacher
In reply to this post by Junio C Hamano
On Sat, Sep 05, 2009 at 10:28:08AM -0700, Junio C Hamano wrote:
> But notice that the above is qualified with "personally".  An alternative
> would be to declare that in 1.8.0, all commands run without path argument
> will work on the whole tree and you have to give an explicit '.' when you
> want to limit their effect to the cwd.

That's fine by me.

[...]
>
> Please also see:
>
>     Message-ID: <[hidden email]> ($gmane/127795)
>
> think about the three questions there, and help us move the discussion
> forward.

The patch is my way of saying that I'm not just whining, but that I'm
willing to put some effort into getting this done. I am aware of the issues
you raised in the above message and I was hoping that my patch would help us
as a starting point to move the discussion forward.

> The first part of the message has some comments related to your patch, by
> the way.

I can only guess that you mean the "sane way for script writers to defeat
the configuration without much pain" part. But I'm not sure how that's a
problem. If you want the script to continue to work as before you either
configure "workdir scope", or you add a '.' to the affected commands.

Clemens
--
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
Reply | Threaded
Open this post in threaded view
|

Re: 'add -u' without path is relative to cwd

Junio C Hamano
Clemens Buchacher <[hidden email]> writes:

> I can only guess that you mean the "sane way for script writers to defeat
> the configuration without much pain" part. But I'm not sure how that's a
> problem. If you want the script to continue to work as before you either
> configure "workdir scope", or you add a '.' to the affected commands.

One who writes the script and lends it to you may be using different
`scope` from what the recipient uses, so that is not an escape hatch at
all.

One crudest form of workaround may be for your code to notice an
environment variable to override that `scope` configuration setting, so
that you can advise the script writers to set it in their script.  But
that is so ugly I'd rather not go there if we do not absolutely have to.

That is why in general we should be very careful and avoid any magic that
makes the same command behave completely differently depending on how a
repository is configured.

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