darcs

Issue 2049 Darcs regression: 2.5 creates a broken patch

Title Darcs regression: 2.5 creates a broken patch
Priority urgent Status resolved
Milestone 2.5.2 STABLE Resolved in 2.5.2 STABLE
Superseder Nosy List ganesh, lele1, mornfall
Assigned To ganesh
Topics

Created on 2011-02-28.22:06:41 by lele1, last changed 2011-03-12.12:34:58 by ganesh.

Files
File name Uploaded Type Edit Remove
unnamed iago, 2011-03-10.21:52:57 text/html
Messages
msg13761 (view) Author: lele1 Date: 2011-02-28.22:06:38
Today I faced a strange problem, and spent some time to investigate.

The source of the problem is that I added a new subtree to my
repository, and the subtree is named "something.core". 

As I discovered, by default darcs boring rules match that name:

  # core dumps
  (^|/|\.)core$

which seems wrong by its own.

Regardless, as esemplified here below, darcs creates a patch where the
root of the subtree is NOT present, and that by the rules of the game
is invalid, even if the whole repository is clonable.

The following script:

#!/bin/sh
echo
echo "*** Creating bad patch"
cd /tmp
rm -rf darcs-boring-issue darcs-boring-issue-*
mkdir darcs-boring-issue
cd darcs-boring-issue
darcs --version
darcs init
mkdir src
darcs add src
darcs record -a -m "Added src directory" src
cd src
mkdir pcommerce.core
touch pcommerce.core/__init__.py
darcs add --recur pcommerce.core
darcs record -a -m "Added 3rd party subtree" pcommerce.core
darcs changes -s

echo
echo "*** A complete clone works"
cd /tmp
darcs get darcs-boring-issue darcs-boring-issue-clone
darcs-2.4.4 get darcs-boring-issue darcs-boring-issue-clone-2.4

echo
echo "*** One patch at a time does not (darcs 2.5)"
mkdir darcs-boring-issue-one-at-a-time
cd darcs-boring-issue-one-at-a-time
darcs init
darcs pull -a -p "Added src directory" ../darcs-boring-issue
darcs pull -a ../darcs-boring-issue

echo
echo "*** One patch at a time does not (darcs 2.4)"
mkdir darcs-boring-issue-one-at-a-time-with-2.4
cd darcs-boring-issue-one-at-a-time-with-2.4
darcs-2.4.4 init
darcs-2.4.4 pull -a -p "Added src directory" ../darcs-boring-issue
darcs-2.4.4 pull -a ../darcs-boring-issue

echo
echo "*** Creating bad patch (2.4)"
cd /tmp
mkdir darcs-boring-issue-2.4
cd darcs-boring-issue-2.4
darcs-2.4.4 --version
darcs-2.4.4 init
mkdir src
darcs-2.4.4 add src
darcs-2.4.4 record -a -m "Added src directory" src
cd src
mkdir pcommerce.core
touch pcommerce.core/__init__.py
darcs-2.4.4 add --recur pcommerce.core
darcs-2.4.4 record -a -m "Added 3rd party subtree" pcommerce.core
darcs-2.4.4 changes -s

produces the following here:

$ sh darcs-boring-issue.sh

*** Creating bad patch
2.5 (release)
Recording changes in "src":

Finished recording patch 'Added src directory'
Skipping boring file src/pcommerce.core
Recording changes in "src/pcommerce.core":

Finished recording patch 'Added 3rd party subtree'
Mon Feb 28 19:37:46 CET 2011  lele@metapensiero.it
  * Added 3rd party subtree

    A ./src/pcommerce.core/__init__.py

Mon Feb 28 19:37:45 CET 2011  lele@metapensiero.it
  * Added src directory

    A ./src/

*** A complete clone works
Note: if you want to change the default remote repository to
/tmp/darcs-boring-issue,
quit now and issue the same command with the --set-default flag.
Copying patches, to get lazy repository hit ctrl-C...
Finished getting.
Copying patches, to get lazy repository hit ctrl-C...
Finished getting.

*** One patch at a time does not (darcs 2.5)
Note: if you want to change the default remote repository to
/tmp/darcs-boring-issue,
quit now and issue the same command with the --set-default flag.
Finished pulling and applying.
Note: if you want to change the default remote repository to
/tmp/darcs-boring-issue,
quit now and issue the same command with the --set-default flag.
Warning: ./src/pcommerce.core/__init__.py-0:
    takeFile ./src/pcommerce.core/__init__.py-0
    in /tmp/darcs-boring-issue-one-at-a-time: does not exist (No such
    file or directory) Finished pulling and applying. 

*** One patch at a time does not (darcs 2.4)
darcs failed:  Not a
repository: /tmp/darcs-boring-issue-one-at-a-time/darcs-boring-issue
(/tmp/darcs-boring-issue-one-at-a-time/darcs-boring-issue/_darcs/inventory:
openBinaryFile: does not exist (No such file or directory))

darcs failed:  Not a
repository: /tmp/darcs-boring-issue-one-at-a-time/darcs-boring-issue
(/tmp/darcs-boring-issue-one-at-a-time/darcs-boring-issue/_darcs/inventory:
openBinaryFile: does not exist (No such file or directory))

*** Creating bad patch (2.4)
2.4.4 (release)
Recording changes in "src":

Finished recording patch 'Added src directory'
Skipping boring file src/pcommerce.core
Skipping 'src/pcommerce.core/__init__.py' ... couldn't add parent
directory 'src/pcommerce.core' to repository.

darcs failed:  No files were added
Recording changes in "src/pcommerce.core":

No changes in selected files or directories!
Mon Feb 28 19:51:53 CET 2011  lele@metapensiero.it
  * Added src directory

    A ./src/
msg13762 (view) Author: iago Date: 2011-03-01.15:14:46
A simple way to reproduce the bug:

--
darcs init
mkdir foo~
touch foo~/a.txt
darcs add foo~/a.txt
--

Since foo~ is considered a boring file Darcs will record an "AddFile 
foo~/a.txt" patch without record an "AddDir foo~" patch.
msg13813 (view) Author: iago Date: 2011-03-10.21:52:57
On Tue, Mar 1, 2011 at 3:14 PM, Iago Abal <bugs@darcs.net> wrote:

>
> Iago Abal <iago.abal@gmail.com> added the comment:
>
> A simple way to reproduce the bug:
>
> --
> darcs init
> mkdir foo~
> touch foo~/a.txt
> darcs add foo~/a.txt
> --
>
Mercurial just ignores both foo~ and foo~/a.txt, I think it makes sense. If
you have a regexp like "~$" then you should also consider "~/.*$", since
ignore a directory should also imply ignore its content.

I also would like to suggest to reduce the number of predefined boring files
(or comment out them), since some of them are rare and unknown for most
people.

>
> Since foo~ is considered a boring file Darcs will record an "AddFile
> foo~/a.txt" patch without record an "AddDir foo~" patch.
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/issue2049>
> __________________________________
> _______________________________________________
> darcs-devel mailing list
> darcs-devel@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
>



-- 
Iago Abal Rivas
Attachments
msg13814 (view) Author: ganesh Date: 2011-03-11.07:41:11
I think this is a bug in hashed-storage: writeFile "foo/bar" silently 
creates "foo", which is not how real filesystems behave.

Adding Petr in case he can see a quick fix but otherwise I'll keep looking 
at it.
msg13817 (view) Author: ganesh Date: 2011-03-11.21:31:16
I still think the behaviour of hashed-storage is a bug - it should 
refuse to add files to non-existent directories, rather than silently 
creating the directories. But I also think darcs should explicitly check 
in advance rather than asking it to do so, as the current code would 
rely on catching an exception which is a bit ugly. So that's what 
patch565 does.

BTW this regresses from 2.4 because we switched from using 
SlurpDirectory to hashed-storage for this code path between those two 
releases.
msg13819 (view) Author: ganesh Date: 2011-03-12.11:35:15
The following patch updated issue issue2049 with status=resolved;resolvedin=2.5.1 CURRENT

* resolve issue2049: explicitly check for parent dir when recording an add 
Ignore-this: 402ab1bb566da16088950519f8174076
The underlying hashed-storage routine doesn't currently throw when the
parent dir isn't present.
History
Date User Action Args
2011-02-28 22:06:41lele1create
2011-03-01 15:14:47iagosetmessages: + msg13762
2011-03-10 21:52:59iagosetfiles: + unnamed
messages: + msg13813
2011-03-11 07:41:13ganeshsetpriority: urgent
nosy: + ganesh, mornfall
milestone: 2.5.2 STABLE
messages: + msg13814
assignedto: ganesh
2011-03-11 21:31:17ganeshsetmessages: + msg13817
2011-03-12 11:35:16ganeshsetstatus: unknown -> resolved
messages: + msg13819
resolvedin: 2.5.1
2011-03-12 12:34:58ganeshsetresolvedin: 2.5.1 -> 2.5.2 STABLE
2011-05-07 12:56:46koweylinkissue2073 superseder