It seems that sometimes darcs follows symbolic links instead of ignoring 
them. It surely happens when substituting a recorded file with a 
symbolic link.

A description of the problem is appended below and a minimal test is 
attached.

Currently using v2.9.6, but I've found the same problem in 2.8 and, 
IIRC, also in 2.5. Could not test older versions.

Thanks,
Gian Piero.

----------- darcs version

$ darcs --exact-version
darcs compiled on Dec  4 2012, at 22:31:54
...
$ darcs --version 
2.9.6 (+ 26 patches)


----------- steps to reproduce

       # -- initialize the repository
$ darcs ini ; echo ImmutableFile > file ; darcs rec -lam init
Finished recording patch 'init'

       # -- add a test file and record the patch
$ echo TemporaryFile > maybeFile ; darcs rec -lam 'Add maybeFile'
Finished recording patch 'Add maybeFile'

       # -- remove the just added file and check that darcs recognizes the 
       # removal
$ rm maybeFile
$ darcs wh -s
R ./maybeFile
$ darcs wh -ls
R ./maybeFile

       # -- create a symbolic link with the same name of the just removed 
       # file pointing to an existent file (does not need to be in the 
       # repodir)
$ ln -s file maybeFile

       # -- now you get different opinions about what changed depending on 
       # the use of the '--look-for-adds' option. In both case 'maybeFile' 
       # is wrongly reported as changed (the current content of 'maybeFile' 
       # is assumed to be the one of the file the link points to), anyway 
       # using the '-l' option you also get the right information that 
       # 'maybeFile' has been removed.
$ darcs wh -s
M ./maybeFile -1 +1
$ darcs wh -ls
M ./maybeFile -1 +1
R ./maybeFile

       # -- trying to record the changes without the use of '-l' leads to 
       # the wrong patch being recorded
$ darcs rec -am 'Maybe remove maybeFile'
Finished recording patch 'Maybe remove maybeFile'
$ darcs cha --last 1 -s
Mon Dec 10 23:15:44 CET 2012  "Gian Piero Carrubba" <gpiero@rm-rf.it>
     * Maybe remove maybeFile

       M ./maybeFile -1 +1

       # -- unrecord the wrong patch
$ darcs unrec --last 1 -a
Finished unrecording.

       # -- passing '-l' to the record command leads to the right patch 
       # being  recorded.
$ darcs rec -lam 'Remove maybeFile'
Finished recording patch 'Remove maybeFile'
$ darcs cha --last 1 -s
Mon Dec 10 23:24:22 CET 2012  "Gian Piero Carrubba" <gpiero@rm-rf.it>
     * Remove maybeFile

       R ./maybeFile

       # -- now if you unrecord the last patch the `-l' option does not 
       # make a difference anymore
$ darcs unrec --last 1 -a
Finished unrecording.
$ darcs wh -s
R ./maybeFile
$ darcs wh -ls
R ./maybeFile

       # -- recreate the issue
$ darcs rec -lam 'Remove maybeFile'
Finished recording patch 'Remove maybeFile'
$ echo AnotherTemporaryFile > maybeFileTwo
$ darcs rec -lam 'Add maybeFileTwo'
Finished recording patch 'Add maybeFileTwo'
$ rm maybeFileTwo ; ln -s file maybeFileTwo

       # -- giving a hint to darcs about the removal of the file is another 
       # way for solving
$ darcs wh -s
M ./maybeFileTwo -1 +1
$ darcs wh -ls
M ./maybeFileTwo -1 +1
R ./maybeFileTwo
$ darcs remove maybeFileTwo
$ darcs wh -s
R ./maybeFileTwo
$ darcs wh -ls
R ./maybeFileTwo
$ darcs rec -am 'Remove maybeFileTwo'
Finished recording patch 'Remove maybeFileTwo'
$ darcs cha --last 1 -s
Tue Dec 11 00:37:10 CET 2012  "Gian Piero Carrubba" <gpiero@rm-rf.it>
     * Remove maybeFileTwo

       R ./maybeFileTwo

       # -- create again the issue, this time giving the file the same 
       # content of the file referenced by the link so that 'wh' will not 
       # report a modified file
$ echo ImmutableFile > maybeFileThree
$ darcs rec -lam 'Add maybeFileThree'
Finished recording patch 'Add maybeFileThree'
$ rm maybeFileThree ; ln -s file maybeFileThree

       # -- in this case 'wh' without '-l' reports no changes
$ darcs wh -s
No changes!
$ darcs wh -ls
R ./maybeFileThree
$ darcs rec -lam 'Remove maybeFileThree'
Finished recording patch 'Remove maybeFileThree'

       # -- create again the problem, pointing the symbolic link to a 
       # not-existent file
$ echo JustAnotherTemporaryFile > maybeFileFour
$ darcs rec -lam 'Add maybeFileFour'
Finished recording patch 'Add maybeFileFour'
$ rm maybeFileFour ; ln -s not-existent maybeFileFour

       # -- this time, darcs croaks
$ darcs wh -s
darcs: ...: openBinaryFile: does not exist (No such file or directory)
$ darcs wh -ls
darcs: ...: openBinaryFile: does not exist (No such file or directory)

       # -- cannot use the same workarounds that fixed the other cases
$ darcs remove maybeFileFour
darcs: ...: openBinaryFile: does not exist (No such file or directory)
$ darcs rec -lam 'Remove maybeFileFour'
darcs: ...: does not exist (No such file or directory)

       # -- remove the link seems the only way to go
$ rm maybeFileFour
$ darcs wh -s
R ./maybeFileFour
$ darcs wh -ls
R ./maybeFileFour

Attachments

• issuexxxx.sh

Darcs Manual doesn't claim to support this use of symlinks one way or the 
other, but at a minimum, I think it would be good to make the behavior 
consistent with and without --look-for-adds.

Please let me stress that consistence with and without --look-for-adds 
is not enough and the current behaviour without --look-for-adds is 
self-inconsistent.

EDIT: I ended up with a long message beyond my initial intentions.  
Anyway the important part is the 'user experience' one (last two 
paragraphs).

-----

$ darcs ini --repo R
$ echo Sample Text > R/f
$ echo Ops, wrong file > R/g
$ darcs rec -lam init --repo R
Finished recording patch 'init'
$ darcs get R S
Copying patches, to get lazy repository hit ctrl-C...
Finished getting.
$ rm R/g ; ln -s f R/g
$ darcs rec -am WRONG --repo R
Finished recording patch 'WRONG'
$ darcs diff -u -p WRONG --repo R
Tue Dec 11 17:48:46 CET 2012  "Gian Piero Carrubba" <gpiero@rm-rf.it>
   * WRONG
diff -rN -u old-R/g new-R/g
--- old-R/g     2012-12-11 17:48:54.120648808 +0100
+++ new-R/g     2012-12-11 17:48:54.120648808 +0100
@@ -1 +1 @@
-Ops, wrong file
+Sample Text
$ darcs push -a --repo R S
Finished applying...
Push successful.

-----

Now we have a file and a symlink to it in R and two files with the same 
content in S. Assuming that
- we want to track the content and not the container
- the content of a symlink is defined as the content of the dereferenced 
   file
we can still consider the behaviour until now as correct.

Anyway now the things become really messy.

-----

$ echo A new line >> R/f
$ darcs rec -am FAIL --repo R
Finished recording patch 'FAIL'
$ darcs diff -u -p FAIL --repo R
Tue Dec 11 17:57:51 CET 2012  "Gian Piero Carrubba" <gpiero@rm-rf.it>
   * FAIL
diff -rN -u old-R/f new-R/f
--- old-R/f     2012-12-11 17:58:02.354956537 +0100
+++ new-R/f     2012-12-11 17:58:02.358956575 +0100
@@ -1 +1,2 @@
  Sample Text
+A new line
$ darcs push -a --repo R S
Finished applying...
Push successful.

-----

Now we still have a file and a symlink to it in R, but two files with 
different contents in S. Wait a moment, we still have an unrecorded 
change in R so maybe reverting it we can really have two repos 
containing the same patches and the same content.

-----

$ darcs wh -ls --repo R
R ./g
$ darcs rev -a --repo R
There are no changes to revert!
Finished reverting.

-----

Mmmh, maybe forcing ? I assume that reverting the removal of a file 
means re-adding it...

-----

$ darcs add --repo R R/g
The following file is already in the repository;
note that to ensure portability we don't allow
files that differ only in case. Use --case-ok to override this:
g

darcs failed:  No files were added

-----

No way.

At this point all sort of things can happen to your poor repo :)
Now, to be fair, just let me add a case in which darcs auto-magically 
fix the anomaly. 

-----

$ echo Another new line >> S/g
$ darcs rec -am PATCH --repo S
Finished recording patch 'PATCH'
$ darcs push -a --repo S R
Finished applying...
Push successful.

-----

Now magically the symlink R/g has been substituted by a file with the 
same name. The content still isn't the same as S/g, but we again have an 
unrecorded change in R (a different one, this time) and simply reverting 
it leads to the situation in which we have two repos containing the same 
patches and the same content.
It's great (sort of: not sure I like darcs removed my symlink w/o 
letting me know), but what if the normal flow is mono-directional from R 
to S ?

As a darcs user I know darcs doesn't support symlinks and I can cope 
with it. I assume it simply ignores them, as this is the behaviour I've 
observed almost all the times. This is a minor annoyance that can be 
fixed with a hook script when needed. Anyway I'm happy remembering the 
following rules about symlinks:
1. symlinks are ignored.

Instead we have that:
1. symlinks are ignored.
2. rule 1) is not true when they have the same name of a just deleted 
    file if the removal patch hasn't been recorded. In this case the 
    symlink is considered a sort of virtual file whose content is the 
    content of the file the link points to.
3. rule 2) is not true if the file has been deleted via 'darcs remove'.
4. when 2) is in effect, '--look-for-adds' also acts as a sort of 
    '--look-for-removals'.
5. when 2) is in effect, it applies only to some commands. I.e., pulling 
    a patch could lead to the removal of the link (w/o tracking of the 
    removal) if a file with the same name is affected (so effectively 
    acting as if the symlink is ignored by darcs).
6. ? (forgotten something?)

I don't think this scenario well into the overall usage simplicity of 
darcs.

Ciao,
Gian Piero.

I fully agree that the current behaviour is a mess, as you amply
illustrated. I am not so sure ignoring symlinks is the right idea,
however. I would say Darcs should guard against them: as soon as you try
to add one or replace a file or directory that Darcs knows about with a
symlink, or in just about any situation where Darcs accesses a file or
directory (that is or is about to become version controlled), Darcs
should fail with an error message, unless you pass an explicit switch
(--resolve-symbolic-links).

* [Wed, Mar 15, 2017 at 06:13:35PM +0000] Ben Franksen:
>I fully agree that the current behaviour is a mess, as you amply
>illustrated. I am not so sure ignoring symlinks is the right idea,
>however. I would say Darcs should guard against them: as soon as you try
>to add one or replace a file or directory that Darcs knows about with a
>symlink, or in just about any situation where Darcs accesses a file or
>directory (that is or is about to become version controlled), Darcs
>should fail with an error message, unless you pass an explicit switch
>(--resolve-symbolic-links).

I'm not sure. While at first it seems sensible to me, after some 
thoughts there are some points that led me to think this is not viable 
(or at least not the preferred option).

First of all: consistency. I don't expect darcs behaving differently wrt 
symlinks depending if they happen to be named as a just removed file or 
not. And what if the filesystem is both case insensitive and supports 
symlinks?[0]
Unless you suggest to always deal with symlinks this way. But at this 
point, why not properly support them as symlinks (so, both storing and 
recreating as such)?[1]

Another point is someway related to security concerns. The destination 
of the symlink could reside outside the repo, so at the moment this 
could lead to some unintentional info leaks. Supporting a 
resolve-symlink option put the developers in front of a choice:

a. supporting it "read-only" (only when storing a patch)
  symlinks should be checked for not linking to files outside the repo, 
  in order to prevent the unintentional info leaks.
  In general, this doesn't seem an improvement wrt what we have now: 
  repos cannot be reliably cloned, two synced repos could not be really 
  in sync, ...

b. supporting it "read-n-write" (also when cloning, apply patches,...)
  Maybe this could solve the problem regarding synced repos that go out 
  of sync, but other problems remain. Moreover, if darcs does not triple 
  check that the destination file is inside the repo, this could also 
  lead to remote code execution.

At last, I think I first noticed the current behaviour because using 
etckeeper (script for storing changes in /etc) with darcs as backend.  
Debian started to move some files out of /etc and replacing them with 
symlinks and this let me discover the issue. In this context, and I 
guess in many others, failing with an error or resolving the symlinks 
are both undesired behaviours and at least we need another option 
(ignore-symbolic-links).

Best,
Gian Piero.

[0] I didn't check for any real application of this use case, that I 
could imagine being pretty marginal, but nevertheless should be 
accounted too.

[1] The long-standing reason for not implementing support for symlinks, 
as I recall it, is that Windows does not support them. Does it still 
stand? I think I've read something during the last months about Windows 
starting implementing them. And even if this is not the case, couldn't 
darcs treat them similarly as the --case-ok option, so warning the user 
and let them decide if they want a portable repo or not?

I think it is not possible to support symlinks properly in Darcs without
changing the patch format. This is currently not practical with the
limited manpower and considering that there are other problems with the
current patch format that we might want to resolve first.

What else can we do? At the end of your last message (about similarity
to --case-ok) you seem to suggest that giving users the choice is the
best way. I am now thinking that --resolve-symlinks is a pretty bad idea
and instead we should rather have --ignore-symlinks (and still give a
warning if we see a link that targets a tracked file or directory), with
 --refuse-symlinks being the default (effectively turning the warning to
an error). The warning is important: consider the case where I remove a
file and replace it with a symlink to another file. With the regular
file system commands it looks as if I had changed the file, but Darcs
must report it as deleted (with --ignore-symlinks): this is too
surprising for my taste without a hint to the user.

Re-reading what this started with, it seems I (now) violently agree with
you :)

Except that I think we should issue a warning unconditionally.

* [Sun, Mar 26, 2017 at 08:10:30AM +0000] Ben Franksen:
>I think it is not possible to support symlinks properly in Darcs 
>without changing the patch format. This is currently not practical with 
>the limited manpower and considering that there are other problems with 
>the current patch format that we might want to resolve first.

Yeah, I guessed it. I meant it as a wishlist for a darcs-3 patch format 
(I intended to send a followup to the ml with my wishes for a darcs-3 
format, but stopped as I wanted to reconsider it more).

>I am now thinking that --resolve-symlinks is a pretty bad idea
>and instead we should rather have --ignore-symlinks (and still give a
>warning if we see a link that targets a tracked file or directory), with
> --refuse-symlinks being the default (effectively turning the warning to
>an error).

Seems good to me. I still prefer to have '--ignore-symlinks' as default, 
but that's what the defaults file is for.

Ciao,
Gian Piero.

Am 01.04.2017 um 17:37 schrieb Gian Piero Carrubba:
>> I am now thinking that --resolve-symlinks is a pretty bad idea
>> and instead we should rather have --ignore-symlinks (and still give a
>> warning if we see a link that targets a tracked file or directory), with
>> --refuse-symlinks being the default (effectively turning the warning to
>> an error).
> Seems good to me. I still prefer to have '--ignore-symlinks' as default, 
> but that's what the defaults file is for.

So now the problem is: where in the code do we insert such a check...

I tried to turn posted steps to reproduce into a more complete test 
script. I was stupefied to find that this seems to succeed. Then I 
remembered that for the tests we we set 'ALL --ignore-times' as 
default.

It appears that the inconsistent and unexpected behaviors here are 
caused solely by the index.

gpiero, if you are still interested in this, can you verify that if 
you add 'ALL --ignore-times' to your defaults, darcs behaves as 
expected?

There is now an extended test for this. It succeeds when --ignore-
times is in effect (the default only when running our test suite), but 
with --ignore-times it fails. Fixing this requires digging into the 
index code, which we haven't done yet but should.

The following patch sent by Ben Franksen <ben.franksen@online.de> updated issue issue2275 with
status=resolved;resolvedin=2.15.0 HEAD

* resolve issue2275: ignore symlinks as repo paths even when the index is used 
Ignore-this: c7f8d3b035d71bd1a0cc9834867d8717a1effe45594c066a62aadfe78ea7b75f47d1424dea6a5eeb

It was necessary to fix the last test scenario in the script (removing a
recorded file and replacing it with a symbolic link to a non-existing file)
because what the script expected here was not what we want, namely that the
symbolic link is treated as if it wouldn't exist.