Re: [PATCH] t7300: repair filesystem permissions with test_when_finished

2014-07-02 Thread Junio C Hamano
Junio C Hamano  writes:

> It tests that "clean -d" is happy if a blind rmdir(2) removes the
> directory.  If it fails for whatever reason (e.g. add "exit(0)" at
> the beginning of cmd_clean(), for example) to remove the directory,
> we do leave an empty unreadable directory behind.
>
> But as long as that directory is empty, that will not cause us to
> remove the trash directory at the end, so we should be OK.

s/remove/fail to &/; sorry for the noise.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t7300: repair filesystem permissions with test_when_finished

2014-07-02 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Jul 02, 2014 at 12:01:59PM -0700, Jonathan Nieder wrote:
>
>> Jeff King wrote:
>> 
>> > Signed-off-by: Jeff King 
>> > ---
>> >  t/t7300-clean.sh | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> Does the later "git clean -d with an unreadable empty directory" test
>> need the same treatment?
>
> I don't think so, because it is also failing during the experiments I'm
> doing and it is not causing the same problem.

It tests that "clean -d" is happy if a blind rmdir(2) removes the
directory.  If it fails for whatever reason (e.g. add "exit(0)" at
the beginning of cmd_clean(), for example) to remove the directory,
we do leave an empty unreadable directory behind.

But as long as that directory is empty, that will not cause us to
remove the trash directory at the end, so we should be OK.

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


Re: [PATCH] t7300: repair filesystem permissions with test_when_finished

2014-07-02 Thread Jeff King
On Wed, Jul 02, 2014 at 12:01:59PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Signed-off-by: Jeff King 
> > ---
> >  t/t7300-clean.sh | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Does the later "git clean -d with an unreadable empty directory" test
> need the same treatment?

I don't think so, because it is also failing during the experiments I'm
doing and it is not causing the same problem.

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


Re: [PATCH] t7300: repair filesystem permissions with test_when_finished

2014-07-02 Thread Jonathan Nieder
Jeff King wrote:

> Signed-off-by: Jeff King 
> ---
>  t/t7300-clean.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Does the later "git clean -d with an unreadable empty directory" test
need the same treatment?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t7300: repair filesystem permissions with test_when_finished

2014-07-02 Thread Jeff King
We create a directory that cannot be removed, confirm that
it cannot be removed, and then fix it like:

  chmod 0 foo &&
  test_must_fail git clean -d -f &&
  chmod 755 foo

If the middle step fails but leaves the directory (e.g., the
bug is that clean does not notice the failure), this
pollutes the test repo with an unremovable directory. Not
only does this cause further tests to fail, but it means
that "rm -rf" fails on the whole trash directory, and the
user has to intervene manually to even re-run the test script.

We can bump the "chmod 755" recovery to a test_when_finished
block to be sure that it always runs.

Signed-off-by: Jeff King 
---
 t/t7300-clean.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 74de814..04118ad 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -426,10 +426,10 @@ test_expect_success SANITY 'removal failure' '
 
mkdir foo &&
touch foo/bar &&
+   test_when_finished "chmod 755 foo" &&
(exec http://vger.kernel.org/majordomo-info.html