patch, 4 partsRe-fix get_pathspec()

After 1.5.4, we improved get_pathspec() to allow absolute paths, because people wanted to cut&paste from file managers and such, so that you can feed absolute paths to "git add" and friends.

However, the paths still must be inside work tree, so there must be a way to ensure that. d089eba (setup: sanitize absolute and funny paths in get_pathspec()) botched the interface, by making the check the responsibility of the callers, while keeping the interface to the function intact, which meant that the callers needed to count returned pathspecs and compared it with the number of paths fed to the function.

This miniseries cleans up the interface by making get_pathspec() to die again.

Junio C Hamano (4):
  get_pathspec(): die when an out-of-tree path is given
  Revert part of 744dacd (builtin-mv: minimum fix to avoid losing files)
  Revert part of 1abf095 (git-add: adjust to the get_pathspec() changes)
  Revert part of d089eba (setup: sanitize absolute and funny paths in
    get_pathspec())

 builtin-add.c              |   12 ------------
 builtin-ls-files.c         |   11 +----------
 builtin-mv.c               |    6 +-----
 setup.c                    |    2 ++
 t/t3101-ls-tree-dirname.sh |    2 +-
 t/t7010-setup.sh           |    7 ++++---
 6 files changed, 9 insertions(+), 31 deletions(-)

An earlier commit d089ebaa (setup: sanitize absolute and funny paths) made get_pathspec() aware of absolute paths, but with a botched interface that forced the callers to count the resulting pathspecs in order to detect an error of giving a path that is outside the work tree.

This fixes it, by dying inside the function.

We had ls-tree test that relied on a misfeature in the original implementation of its pathspec handling. Leading slashes were silently removed from them. However we allow giving absolute pathnames (people want to cut and paste from elsewhere) that are inside work tree these days, so a pathspec that begin with slash _should_ be treated as a full path. The test is adjusted to match the updated rule for get_pathspec().

Earlier I mistook three tests given by Robin that they should succeed, but these are attempts to add path outside work tree, which should fail loudly. These tests also have been fixed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 setup.c                    |    2 ++
 t/t3101-ls-tree-dirname.sh |    2 +-
 t/t7010-setup.sh           |    7 ++++---
 3 files changed, 7 insertions(+), 4 deletions(-)

My gcc refuses to cooperate, giving me SEGV at line 666 or combine-diff, so I'll ask instead of testing.

Do absolute paths to diff still work without exiting?

-- robin

Robin Rosenberg <robin.rosenberg@dewire.com> writes:

They should, as long as they name inside work tree paths.

As far as I know, the bolted-on "work on files outside" code does not feed the paths to get_pathspec() (and it shouldn't --- that mode is not a git operation to begin with, and should work even without having a work tree, so there is no valid base to make the paths relative to).

But it needs to be checked.

Hi,

On Fri, 7 Mar 2008, Junio C Hamano wrote:

We have a few places where you have to pass a "die_on_error" flag. Why not imitate that?

Ciao, Dscho

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Before get_pathspec() was made absolute-path capable, it died on a path that is outside the work tree, so we know die() there is what the callers have expected all along. This miniseries is about fixing the earlier one that broke that expectation, nothing more.

If enough callers want different behaviours on error, it probably is worth an API enhancement so that the callers can choose to do their own thing, but that is a separate topic I haven't seen the need for, and definitely is not part of this fix.

Junio C Hamano <gitster@pobox.com> writes:

By the way, this is meant to also fix the breakage of "git clean" run with a path outside the work tree. With two patches from Dmitry:

 Make private quote_path() in wt-status.c available as quote_path_relative())
 git-clean: correct printing relative path)

on top of the series, the following tests now pass.

-- >8 -- git-clean: add tests for relative path

This adds tests for recent change by Dmitry to fix the report "git clean" gives on removed paths, and also makes sure the command detects paths that is outside working tree.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7300-clean.sh |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)