Re: [PATCH] Repeatability when copying a whole directory into a new one.

[Jani Nikula]

On Sat, 05 Nov 2011, Austin Clements  wrote:
> On Thu, Sep 29, 2011 at 7:26 PM, Thomas Schwinge  wrote:
>> This new test currently fails -- but it shouldn't.
>> ---
>>
>> Hi!
>>
>> I found this while manually copying directories and running notmuch new.
>>
>> Am I just too sleepy at this time, or is it another DB vs. directory
>> mtime issue?
>
> Nice catch.  I haven't verified this, but I believe the problem is
> that notmuch never deletes directory documents.  In fact, there isn't
> even an API to do so.  Even though it detects the deleted directory
> and deletes all messages under it, the directory document sticks
> around.  When the directory comes back, notmuch finds the old
> directory document with the old directory mtime and thinks it doesn't
> have to rescan the directory because the cp -a reproduced the same
> mtime the directory used to have.
>
> So, I guess part of the answer is "don't cp -a" because that mucks
> with mtimes and mtimes are all notmuch has to go by.  But that's no
> excuse for not removing the directory documents when the directory is
> removed.
>
> Fixing this is slightly tricky.  I feel like there *shouldn't* be an
> API to simply remove a directory document because that would let you
> violate database consistency.  Instead, I think the API should
> recursively remove everything under the removed directory, exactly
> like what notmuch-new.c:_remove_directory does right now (plus
> removing the directory documents).  But _remove_directory depends on
> remove_filename, which currently has notmuch-new-specific logic in it.
>  I feel like there must be a nice solution to this, and I'm just not
> thinking of it.

Stumbled upon this one while checking old bug reports. Maybe the fix in

commit e26d99dc7b02f33299c281f97a13deaef802bc7a
Author: Jani Nikula 
Date:   Fri Sep 25 23:48:46 2015 +0300

cli: delete directory documents on directory removal

is inelegant, as it adds the API to remove directory document, but it's
there now. I was unaware of this bug report and your analysis at the
time.

BR,
Jani.

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] Repeatability when copying a whole directory into a new one.

[Tomi Ollila]

On Fri, 30 Sep 2011 01:26:46 +0200, Thomas Schwinge  
wrote:
> This new test currently fails -- but it shouldn't.
> ---
> 
> Hi!
> 
> I found this while manually copying directories and running notmuch new.
> 
> Am I just too sleepy at this time, or is it another DB vs. directory
> mtime issue?

I played a bit with this... making directories early and touching and...

... and always got the same output as below.

>  BROKEN Repeatability when copying a whole directory into a new one
> --- new.18.expected 2011-09-29 23:23:39.0 +
> +++ new.18.output   2011-09-29 23:23:39.0 +
> @@ -1,2 +1 @@
> -Processed 51 total files in almost no time.
>  No new mail.

Then, finally, the following line changes

+output1=$(notmuch new; notmuch search --format=files '*')
...
+output2=$(notmuch new; notmuch search --format=files '*')

we see this

 BROKEN Repeatability when copying a whole directory into a new one
--- new.18.expected 2011-12-18 13:01:47.0 +
+++ new.18.output   2011-12-18 13:01:47.0 +
@@ -1,104 +1,52 @@
-Processed 51 total files in almost no time.
 No new mail.
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/50:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/50:2,
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/49:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/49:2,
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/48:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/48:2,
...


We see that after second time the files are not added to the message
(again). 

I cannot see that the issue is with directory mtimes, but perhaps that
the directory .../2nd/... is not forgotten...

... but whatever the reason will eventually be I suggest to make
this change to the test -- it provides more concrete information
to anyone who will be looking fix for this case.

> 
> 
> Gr??e,
>  Thomas

Tomi

> 
> ---
>  test/new |   21 +
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/test/new b/test/new
> index 49f390d..0afb04c 100755
> --- a/test/new
> +++ b/test/new
> @@ -153,4 +153,25 @@ rm -rf "${MAIL_DIR}"/two
>  output=$(NOTMUCH_NEW)
>  test_expect_equal "$output" "No new mail. Removed 3 messages."
>  
> +
> +test_begin_subtest 'Repeatability when copying a whole directory into a new 
> one'
> +
> +add_email_corpus
> +
> +mkdir "$MAIL_DIR"/2nd
> +cp -a "$MAIL_DIR"/cur "$MAIL_DIR"/2nd/
> +output1=$(notmuch new)
> +
> +rm -rf "$MAIL_DIR"/2nd
> +notmuch new > /dev/null
> +
> +# This was quite enjoyable.  Let's do it again.
> +mkdir "$MAIL_DIR"/2nd
> +cp -a "$MAIL_DIR"/cur "$MAIL_DIR"/2nd/
> +output2=$(notmuch new)
> +
> +test_subtest_known_broken
> +test_expect_equal "$output2" "$output1"
> +
> +
>  test_done
> -- 
> 1.7.5.4
> 

Re: [PATCH] Repeatability when copying a whole directory into a new one.

[Tomi Ollila]

On Fri, 30 Sep 2011 01:26:46 +0200, Thomas Schwinge tho...@schwinge.name 
wrote:
 This new test currently fails -- but it shouldn't.
 ---
 
 Hi!
 
 I found this while manually copying directories and running notmuch new.
 
 Am I just too sleepy at this time, or is it another DB vs. directory
 mtime issue?

I played a bit with this... making directories early and touching and...

... and always got the same output as below.

  BROKEN Repeatability when copying a whole directory into a new one
 --- new.18.expected 2011-09-29 23:23:39.0 +
 +++ new.18.output   2011-09-29 23:23:39.0 +
 @@ -1,2 +1 @@
 -Processed 51 total files in almost no time.
  No new mail.

Then, finally, the following line changes

+output1=$(notmuch new; notmuch search --format=files '*')
...
+output2=$(notmuch new; notmuch search --format=files '*')

we see this

 BROKEN Repeatability when copying a whole directory into a new one
--- new.18.expected 2011-12-18 13:01:47.0 +
+++ new.18.output   2011-12-18 13:01:47.0 +
@@ -1,104 +1,52 @@
-Processed 51 total files in almost no time.
 No new mail.
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/50:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/50:2,
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/49:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/49:2,
 /home/too/vc/ext/notmuch/test/tmp.new/mail/cur/48:2,
-/home/too/vc/ext/notmuch/test/tmp.new/mail/2nd/cur/48:2,
...


We see that after second time the files are not added to the message
(again). 

I cannot see that the issue is with directory mtimes, but perhaps that
the directory .../2nd/... is not forgotten...

... but whatever the reason will eventually be I suggest to make
this change to the test -- it provides more concrete information
to anyone who will be looking fix for this case.

 
 
 Grüße,
  Thomas

Tomi

 
 ---
  test/new |   21 +
  1 files changed, 21 insertions(+), 0 deletions(-)
 
 diff --git a/test/new b/test/new
 index 49f390d..0afb04c 100755
 --- a/test/new
 +++ b/test/new
 @@ -153,4 +153,25 @@ rm -rf ${MAIL_DIR}/two
  output=$(NOTMUCH_NEW)
  test_expect_equal $output No new mail. Removed 3 messages.
  
 +
 +test_begin_subtest 'Repeatability when copying a whole directory into a new 
 one'
 +
 +add_email_corpus
 +
 +mkdir $MAIL_DIR/2nd
 +cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
 +output1=$(notmuch new)
 +
 +rm -rf $MAIL_DIR/2nd
 +notmuch new  /dev/null
 +
 +# This was quite enjoyable.  Let's do it again.
 +mkdir $MAIL_DIR/2nd
 +cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
 +output2=$(notmuch new)
 +
 +test_subtest_known_broken
 +test_expect_equal $output2 $output1
 +
 +
  test_done
 -- 
 1.7.5.4
 
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] Repeatability when copying a whole directory into a new one.

[Austin Clements]

On Thu, Sep 29, 2011 at 7:26 PM, Thomas Schwinge  
wrote:
> This new test currently fails -- but it shouldn't.
> ---
>
> Hi!
>
> I found this while manually copying directories and running notmuch new.
>
> Am I just too sleepy at this time, or is it another DB vs. directory
> mtime issue?

Nice catch.  I haven't verified this, but I believe the problem is
that notmuch never deletes directory documents.  In fact, there isn't
even an API to do so.  Even though it detects the deleted directory
and deletes all messages under it, the directory document sticks
around.  When the directory comes back, notmuch finds the old
directory document with the old directory mtime and thinks it doesn't
have to rescan the directory because the cp -a reproduced the same
mtime the directory used to have.

So, I guess part of the answer is "don't cp -a" because that mucks
with mtimes and mtimes are all notmuch has to go by.  But that's no
excuse for not removing the directory documents when the directory is
removed.

Fixing this is slightly tricky.  I feel like there *shouldn't* be an
API to simply remove a directory document because that would let you
violate database consistency.  Instead, I think the API should
recursively remove everything under the removed directory, exactly
like what notmuch-new.c:_remove_directory does right now (plus
removing the directory documents).  But _remove_directory depends on
remove_filename, which currently has notmuch-new-specific logic in it.
 I feel like there must be a nice solution to this, and I'm just not
thinking of it.

Re: [PATCH] Repeatability when copying a whole directory into a new one.

[Austin Clements]

On Thu, Sep 29, 2011 at 7:26 PM, Thomas Schwinge tho...@schwinge.name wrote:
 This new test currently fails -- but it shouldn't.
 ---

 Hi!

 I found this while manually copying directories and running notmuch new.

 Am I just too sleepy at this time, or is it another DB vs. directory
 mtime issue?

Nice catch.  I haven't verified this, but I believe the problem is
that notmuch never deletes directory documents.  In fact, there isn't
even an API to do so.  Even though it detects the deleted directory
and deletes all messages under it, the directory document sticks
around.  When the directory comes back, notmuch finds the old
directory document with the old directory mtime and thinks it doesn't
have to rescan the directory because the cp -a reproduced the same
mtime the directory used to have.

So, I guess part of the answer is don't cp -a because that mucks
with mtimes and mtimes are all notmuch has to go by.  But that's no
excuse for not removing the directory documents when the directory is
removed.

Fixing this is slightly tricky.  I feel like there *shouldn't* be an
API to simply remove a directory document because that would let you
violate database consistency.  Instead, I think the API should
recursively remove everything under the removed directory, exactly
like what notmuch-new.c:_remove_directory does right now (plus
removing the directory documents).  But _remove_directory depends on
remove_filename, which currently has notmuch-new-specific logic in it.
 I feel like there must be a nice solution to this, and I'm just not
thinking of it.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] Repeatability when copying a whole directory into a new one.

[Pieter Praet]

On Mon, 31 Oct 2011 23:59:49 -0300, David Bremner  wrote:
> On Fri, 30 Sep 2011 01:26:46 +0200, Thomas Schwinge  
> wrote:
> > This new test currently fails -- but it shouldn't.
> > ---
> > 
> > Hi!
> > 
> > I found this while manually copying directories and running notmuch new.
> > 
> > Am I just too sleepy at this time, or is it another DB vs. directory
> > mtime issue?
> > 
> >  BROKEN Repeatability when copying a whole directory into a new one
> > --- new.18.expected 2011-09-29 23:23:39.0 +
> > +++ new.18.output   2011-09-29 23:23:39.0 +
> > @@ -1,2 +1 @@
> > -Processed 51 total files in almost no time.
> >  No new mail.
> 
> I'm a bit confused here too. When the files are removed, the "notmuch new"
> sent to /dev/null in your test detects the deletes as renames. Shouldn't
> the copies be detected as duplicates or something?
> 

They should.

Applying any one of the attached patches makes the test pass;
I vote "directory mtime issue".


-- next part --
A non-text attachment was scrubbed...
Name: update-dir-mtime.patch
Type: text/x-patch
Size: 329 bytes
Desc: not available
URL: 

-- next part --
A non-text attachment was scrubbed...
Name: cp-without-archive-opt.patch
Type: text/x-patch
Size: 612 bytes
Desc: not available
URL: 

-- next part --
A non-text attachment was scrubbed...
Name: cp-to-other-dir.patch
Type: text/x-patch
Size: 391 bytes
Desc: not available
URL: 

-- next part --
A non-text attachment was scrubbed...
Name: cp-dir-content.patch
Type: text/x-patch
Size: 622 bytes
Desc: not available
URL: 

-- next part --


> d
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

Re: [PATCH] Repeatability when copying a whole directory into a new one.

[Pieter Praet]

On Mon, 31 Oct 2011 23:59:49 -0300, David Bremner da...@tethera.net wrote:
 On Fri, 30 Sep 2011 01:26:46 +0200, Thomas Schwinge tho...@schwinge.name 
 wrote:
  This new test currently fails -- but it shouldn't.
  ---
  
  Hi!
  
  I found this while manually copying directories and running notmuch new.
  
  Am I just too sleepy at this time, or is it another DB vs. directory
  mtime issue?
  
   BROKEN Repeatability when copying a whole directory into a new one
  --- new.18.expected 2011-09-29 23:23:39.0 +
  +++ new.18.output   2011-09-29 23:23:39.0 +
  @@ -1,2 +1 @@
  -Processed 51 total files in almost no time.
   No new mail.
 
 I'm a bit confused here too. When the files are removed, the notmuch new
 sent to /dev/null in your test detects the deletes as renames. Shouldn't
 the copies be detected as duplicates or something?
 

They should.

Applying any one of the attached patches makes the test pass;
I vote directory mtime issue.


diff --git a/test/new b/test/new
index 0afb04c..32b058f 100755
--- a/test/new
+++ b/test/new
@@ -168,6 +168,7 @@ notmuch new  /dev/null
 # This was quite enjoyable.  Let's do it again.
 mkdir $MAIL_DIR/2nd
 cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+touch $MAIL_DIR/2nd/cur
 output2=$(notmuch new)
 
 test_subtest_known_broken
diff --git a/test/new b/test/new
index 0afb04c..0627d69 100755
--- a/test/new
+++ b/test/new
@@ -159,7 +159,7 @@ test_begin_subtest 'Repeatability when copying a whole directory into a new one'
 add_email_corpus
 
 mkdir $MAIL_DIR/2nd
-cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+cp $MAIL_DIR/cur $MAIL_DIR/2nd/
 output1=$(notmuch new)
 
 rm -rf $MAIL_DIR/2nd
@@ -167,7 +167,7 @@ notmuch new  /dev/null
 
 # This was quite enjoyable.  Let's do it again.
 mkdir $MAIL_DIR/2nd
-cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+cp $MAIL_DIR/cur $MAIL_DIR/2nd/
 output2=$(notmuch new)
 
 test_subtest_known_broken
diff --git a/test/new b/test/new
index 0afb04c..a3c270c 100755
--- a/test/new
+++ b/test/new
@@ -166,8 +166,8 @@ rm -rf $MAIL_DIR/2nd
 notmuch new  /dev/null
 
 # This was quite enjoyable.  Let's do it again.
-mkdir $MAIL_DIR/2nd
-cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+mkdir $MAIL_DIR/3rd
+cp -a $MAIL_DIR/cur $MAIL_DIR/3rd/
 output2=$(notmuch new)
 
 test_subtest_known_broken
diff --git a/test/new b/test/new
index 0afb04c..f600983 100755
--- a/test/new
+++ b/test/new
@@ -159,7 +159,7 @@ test_begin_subtest 'Repeatability when copying a whole directory into a new one'
 add_email_corpus
 
 mkdir $MAIL_DIR/2nd
-cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+cp -a $MAIL_DIR/cur/* $MAIL_DIR/2nd/
 output1=$(notmuch new)
 
 rm -rf $MAIL_DIR/2nd
@@ -167,7 +167,7 @@ notmuch new  /dev/null
 
 # This was quite enjoyable.  Let's do it again.
 mkdir $MAIL_DIR/2nd
-cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+cp -a $MAIL_DIR/cur/* $MAIL_DIR/2nd/
 output2=$(notmuch new)
 
 test_subtest_known_broken


 d
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] Repeatability when copying a whole directory into a new one.

[David Bremner]

On Fri, 30 Sep 2011 01:26:46 +0200, Thomas Schwinge  
wrote:
> This new test currently fails -- but it shouldn't.
> ---
> 
> Hi!
> 
> I found this while manually copying directories and running notmuch new.
> 
> Am I just too sleepy at this time, or is it another DB vs. directory
> mtime issue?
> 
>  BROKEN Repeatability when copying a whole directory into a new one
> --- new.18.expected 2011-09-29 23:23:39.0 +
> +++ new.18.output   2011-09-29 23:23:39.0 +
> @@ -1,2 +1 @@
> -Processed 51 total files in almost no time.
>  No new mail.

I'm a bit confused here too. When the files are removed, the "notmuch new"
sent to /dev/null in your test detects the deletes as renames. Shouldn't
the copies be detected as duplicates or something?

d

[PATCH] Repeatability when copying a whole directory into a new one.

[Thomas Schwinge]

This new test currently fails -- but it shouldn't.
---

Hi!

I found this while manually copying directories and running notmuch new.

Am I just too sleepy at this time, or is it another DB vs. directory
mtime issue?

 BROKEN Repeatability when copying a whole directory into a new one
--- new.18.expected 2011-09-29 23:23:39.0 +
+++ new.18.output   2011-09-29 23:23:39.0 +
@@ -1,2 +1 @@
-Processed 51 total files in almost no time.
 No new mail.


Grüße,
 Thomas

---
 test/new |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/test/new b/test/new
index 49f390d..0afb04c 100755
--- a/test/new
+++ b/test/new
@@ -153,4 +153,25 @@ rm -rf ${MAIL_DIR}/two
 output=$(NOTMUCH_NEW)
 test_expect_equal $output No new mail. Removed 3 messages.
 
+
+test_begin_subtest 'Repeatability when copying a whole directory into a new 
one'
+
+add_email_corpus
+
+mkdir $MAIL_DIR/2nd
+cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+output1=$(notmuch new)
+
+rm -rf $MAIL_DIR/2nd
+notmuch new  /dev/null
+
+# This was quite enjoyable.  Let's do it again.
+mkdir $MAIL_DIR/2nd
+cp -a $MAIL_DIR/cur $MAIL_DIR/2nd/
+output2=$(notmuch new)
+
+test_subtest_known_broken
+test_expect_equal $output2 $output1
+
+
 test_done
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch