$ darcs init
$ mkdir a
$ darcs add a
$ echo file > a/file
$ darcs add a/file
$ darcs record -a
$ mkdir b
$ darcs add b
$ darcs mv a/file b
$ darcs record
n
$ rm -r b
$ darcs record

It keeps trying to add directory b, even though it doesn't exist.  I'm using
version 2.1.0 from Debian sid.

On Mon, Jan 19, 2009 at 01:59:37AM +0000, Marco Túlio Gontijo e Silva wrote:
> $ darcs init
> $ mkdir a
> $ darcs add a
> $ echo file > a/file
> $ darcs add a/file
> $ darcs record -a
> $ mkdir b
> $ darcs add b
> $ darcs mv a/file b
> $ darcs record
> n
> $ rm -r b
> $ darcs record
> 
> It keeps trying to add directory b, even though it doesn't exist.  I'm using
> version 2.1.0 from Debian sid.

I can't reproduce this with Darcs 2.2.0.  Marco, can you test with that version?

Em Sáb, 2009-03-28 às 10:50 +0000, Trent Buck escreveu:
> On Mon, Jan 19, 2009 at 01:59:37AM +0000, Marco Túlio Gontijo e Silva
> wrote:
> > $ darcs init
> > $ mkdir a
> > $ darcs add a
> > $ echo file > a/file
> > $ darcs add a/file
> > $ darcs record -a
> > $ mkdir b
> > $ darcs add b
> > $ darcs mv a/file b
> > $ darcs record
> > n
> > $ rm -r b
> > $ darcs record
> > 
> > It keeps trying to add directory b, even though it doesn't exist.
> I'm using
> > version 2.1.0 from Debian sid.
> 
> I can't reproduce this with Darcs 2.2.0.  Marco, can you test with
> that version?

I can reproduce it with 2.2.0 and with darcs.net.

I've made a small script:

darcs=${HOME}/trabalho/debian/darcs/darcs.net/darcs
${darcs} init
mkdir a
${darcs} add a
echo file > a/file
${darcs} add a/file
${darcs} record -am patch
mkdir b
${darcs} add b
${darcs} mv a/file b
echo n | ${darcs} record
rm -r b
${darcs} record

It should not ask to add the directory b on darcs record.  I think this
is related to my last patch to coalesce, I'll take a look.

Greetings.

Em Sáb, 2009-03-28 às 09:39 -0300, Marco Túlio Gontijo e Silva escreveu:
> I think this
> is related to my last patch to coalesce, I'll take a look.

I took a look, and this doesn't seem to hold.  I think it's related to
something else.

Greetings.

On Sat, Mar 28, 2009 at 09:39:13 -0300, Marco Túlio Gontijo e Silva wrote:
> I can reproduce it with 2.2.0 and with darcs.net.
> 
> I've made a small script:

Nice! Could you turn this into a script for use in the bug/ directory?
It should be the same thing, except you can call 'darcs' directly (the
shell harness is smart enough to point to the right darcs).  Note that
darcs shell tests work by failing on the first non-zero exit status.

Hi Marco,

Attached is a version of the test which uses darcs whatsnew instead of darcs
record.  Do you think this is the same bug?  Also, what happens when you
simplify this test even more, for example, removing the 'a' directory and just
adding a file instead?

It's interesting to inspect the contents of the pending patch too to see what
happens.

Part of my difficulty with this bug is not actually being sure if it's really
undesirable behaviour.  Perhaps we want the user to have to explictly remove the
directory?  As a general principle, what do we want to happen when we darcs add
a directory and subsequently remove it by hand?

Anyway, Marco if you could confirm that this test matches your expectations
about how darcs should behave, we can move this forward a notch.

Thanks!

Attachments

• failing-issue1325.sh

Hello Eric.

(...)
> Attached is a version of the test which uses darcs whatsnew instead of darcs
> record.  Do you think this is the same bug?

Yes, it is.

> Also, what happens when you simplify this test even more, for example,
> removing the 'a' directory and just adding a file instead?

I could not understand what you mean.  Can you give a shell script with the
idea?

> Part of my difficulty with this bug is not actually being sure if it's really
> undesirable behaviour.  Perhaps we want the user to have to explictly remove the
> directory?  As a general principle, what do we want to happen when we darcs add
> a directory and subsequently remove it by hand?

Conceptually, I have the opinion that anything done between records should not
be stored, and must be simplified at most to create patches.  If the user want
to record the creation and deletion of a directory, it can record two patches
for these changes.

> Anyway, Marco if you could confirm that this test matches your expectations
> about how darcs should behave, we can move this forward a notch.

This test is right for me.

Greetings.

On Thu, Aug 20, 2009 at 15:54:35 +0000, Marco Túlio Gontijo e Silva wrote:
> > Also, what happens when you simplify this test even more, for example,
> > removing the 'a' directory and just adding a file instead?
> 
> I could not understand what you mean.  Can you give a shell script with the
> idea?

Here it is:  The difference here is that we just create a file 'file'
without putting it in a subdirectory

. ../tests/lib                  # Load some portability helpers.
rm -rf R                        # Another script may have left a mess.
darcs init   --repo=R

echo file > R/file             # we need a hunk to make this interesting
darcs add    --repo=R file
darcs record --repo=R -am ' file'

mkdir R/b
darcs add    --repo=R b
darcs mv     --repo=R file b

rm -r R/b
darcs whatsnew --repo=R | not grep adddir

> > Part of my difficulty with this bug is not actually being sure if it's really
> > undesirable behaviour.  Perhaps we want the user to have to explictly remove the
> > directory?  As a general principle, what do we want to happen when we darcs add
> > a directory and subsequently remove it by hand?
> 
> Conceptually, I have the opinion that anything done between records should not
> be stored, and must be simplified at most to create patches.  If the user want
> to record the creation and deletion of a directory, it can record two patches
> for these changes.

So with the above test, you get this darcs whatsnew output:

adddir ./b
move ./file ./b/file
hunk ./b/file 1
-file
rmfile ./b/file
rmdir ./b

Interestingly, if you change the 'echo file > R/file' to a touch, you
get something much more expected:

rmfile ./file

So I guess what you would find much more reasonable in the above example
is simply

hunk ./file 1
-file
rmfile ./file

Does it still sound like we're talking about the same test?
I have a nagging feeling that this may be impossible to fix if so,
but let's see for now if we're still talking about the same thing.

Hi Eric.

Em Ter, 2009-08-25 às 01:51 +0000, Eric Kow escreveu:
> Eric Kow <kowey@darcs.net> added the comment:
> 
> On Thu, Aug 20, 2009 at 15:54:35 +0000, Marco Túlio Gontijo e Silva wrote:
> > > Also, what happens when you simplify this test even more, for example,
> > > removing the 'a' directory and just adding a file instead?
> > 
> > I could not understand what you mean.  Can you give a shell script with the
> > idea?
> 
> Here it is:  The difference here is that we just create a file 'file'
> without putting it in a subdirectory
> 
> . ../tests/lib                  # Load some portability helpers.
> rm -rf R                        # Another script may have left a mess.
> darcs init   --repo=R
> 
> echo file > R/file             # we need a hunk to make this interesting
> darcs add    --repo=R file
> darcs record --repo=R -am ' file'
> 
> mkdir R/b
> darcs add    --repo=R b
> darcs mv     --repo=R file b
> 
> rm -r R/b
> darcs whatsnew --repo=R | not grep adddir

Ok, I got it now.

(...)
> So with the above test, you get this darcs whatsnew output:
> 
> adddir ./b
> move ./file ./b/file
> hunk ./b/file 1
> -file
> rmfile ./b/file
> rmdir ./b
> 
> Interestingly, if you change the 'echo file > R/file' to a touch, you
> get something much more expected:
> 
> rmfile ./file
> 
> So I guess what you would find much more reasonable in the above example
> is simply
> 
> hunk ./file 1
> -file
> rmfile ./file
> 
> Does it still sound like we're talking about the same test?

Yes, this is a smaller test case for the same thing.  I would expect this last
output to be generated by darcs.

> I have a nagging feeling that this may be impossible to fix if so,
> but let's see for now if we're still talking about the same thing.

Yes, I think we are.

Greetings.

Thanks, Marco.  I've been meaning to submit your patch to the list for a while
now and finally got around to it.

We basically need a good idea for how to improve pending patch minimisation.  I
suspect this is the kind of problem that we can't really fix in the general
case.  There will always be a pending patch somewhere that doesn't quite mean
100% what the user wants.  But I'll assume for now that we're treating this as a
bug.

Huh! This sounds awfully similar to issue1316.  Perhaps it's not so 
much a problem with the pending patch itself, but our look-for-adds 
stuff?

1. When you darcs add, you put the adddir in pending
2. When you record, you then remove said addir from pending (because 
you are transferring it to the newly created patch)

So if you delete the directory before recording, you never get around 
to doing this record.

Would the right fix then be to introduce a pending filtering step that 
verifies that the desired pending patches still exist in the pristine-
to-working diff?  That sort of thing sounds eminently doable.

I had more luck than gpiero tracking down which patch fixes this. For reference, the 
command I used was

darcs test --backoff /home/ben/src/darcs/screened/trackdown.sh

with the test script being

if ! cabal build --enable-tests --disable-optimization; then
  exit 125
else
  ! cabal run darcs-test --disable-optimization -- -s=sf -f=2 -t=issue1325
fi

It told me:

Last recent patch that fails the test (assuming monotony in the given range):
patch 4caf10dd7d378f34d9249518dd02158f6a78346b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Aug  3 01:38:01 CEST 2019
  * refactor and extend tryToShrink and sortCoalesceFL

  [...long comment elided...]

I have no idea yet *why* this patch fixes the problem.