darcs

Issue 1645 Darcs >2.3 follows directory symlinks.

Title Darcs >2.3 follows directory symlinks.
Priority bug Status resolved
Milestone 2.4.x Resolved in 2.4.x
Superseder Nosy List darcs-devel, dastapov, dmitry.kurochkin, ertai, kowey, mornfall, tux_rocker, twb
Assigned To mornfall
Topics Regression

Created on 2009-10-09.01:08:50 by twb, last changed 2010-06-16.14:39:24 by kowey.

Files
File name Uploaded Type Edit Remove
unnamed dastapov, 2010-04-01.13:06:05 text/html
unnamed dastapov, 2010-04-01.17:51:31 text/html
unnamed dastapov, 2010-04-06.09:52:25 text/html
unnamed dastapov, 2010-04-06.13:22:45 text/html
Messages
msg8934 (view) Author: twb Date: 2009-10-09.01:08:47
Now symlinks are treated as directories, instead of as files.
I believe this is undesirable.

    $ darcs init
    $ ln -s ~/.darcs/cache x
    $ /usr/bin/darcs w -l
    No changes!
    $ darcs w -l
    darcs: do_stat: resource exhausted (Cannot allocate memory)

This is comparing the 2.3.0 release to today's build, which has
darcs-hs merged in.

    $ /usr/bin/darcs --version
    2.3.0 (release)
    $ darcs --version
    2.3.1 (+ 251 patches)
msg8935 (view) Author: kowey Date: 2009-10-09.06:04:32
Hi Petr,

Could you help me characterise this?  Are we to treat this as a bug, or is it in
some way desirable?  Thanks!
msg8938 (view) Author: ertai Date: 2009-10-09.08:49:15
I think the behavior already changed in the past (pre 2.0).

I think that both behaviors have their own advantages. But maybe the "symlinks as 
files" is safer, since their is less ways to try to fool darcs.
msg8943 (view) Author: tux_rocker Date: 2009-10-10.14:45:17
Op vrijdag 09 oktober 2009 08:04 schreef Eric Kow:
> Eric Kow <kowey@darcs.net> added the comment:
> Are we to treat this as a bug, or is it in
> some way desirable? 

I don't think that error messages from internal functions are ever
desirable.

Reinier
msg8944 (view) Author: kowey Date: 2009-10-10.21:03:59
OK, I'm going to assume the right thing here is to restore the old symlinks as
files behaviour...
msg8948 (view) Author: twb Date: 2009-10-11.04:09:18
Reinier Lamers wrote:
> Eric Kow wrote:
>> Are we to treat this as a bug, or is it in some way desirable?
>
> I don't think that error messages from internal functions are ever
> desirable.

I agree, but understand that the OOM error is arising due to Darcs
trying to read in the entire ~/.darcs/cache tree into memory at once;
a separate issue from the fact that Darcs looked at the symlink "x"
and decided to descend into it.

I used this example because it's the case I actually ran into: once
deployed, http://twb.ath.cx/Preferences/ ends up with a symlink from
./.darcs/cache to ~/.cache/darcs in it.

An equivalent test would be:

    $ darcs init --repodir R
    $ touch f
    $ cd R
    $ ln -s ../f
    $ /usr/bin/darcs w -l
    No changes!
    $ darcs w -l
    a ./f

I'll dump this into tests/failing-issue1645.
msg8949 (view) Author: twb Date: 2009-10-11.04:10:38
On Sat, Oct 10, 2009 at 09:04:09PM +0000, Eric Kow wrote:
> OK, I'm going to assume the right thing here is to restore the old
> symlinks as files behaviour...

+1, but I'm prepared to listen to an argument for the new behaviour.
msg10046 (view) Author: twb Date: 2010-02-20.15:08:00
This issue is present in 2.3.99.2 (release candidate 2).

Here's another trivial test for this suckiness:

    $ darcs init
    $ ln -s . x
    $ darcs w -l
    darcs: ./x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x/x: changeWorkingDirectory: invalid argument (Too many levels of symbolic links)
msg10335 (view) Author: twb Date: 2010-03-20.20:07:45
Trent W. Buck wrote:
> Here's another trivial test for this suckiness:

And another:

I use etckeeper with Darcs to track the history of /etc, which has
lots of symlinks.  Etckeeper knows that Darcs ignores symlinks, and so
it handles them out-of-band with a metadata file:

    $ cd /etc
    $ sudo grep ln .etckeeper | head
    ln -sf '../t-prot/Muttrc' './Muttrc.d/t-prot.rc'
    ln -sf '/usr/bin/Xorg' './X11/X'
    ln -sf '/usr/bin/aptitude-curses' './alternatives/aptitude'
    ln -sf '/usr/bin/mawk' './alternatives/awk'
    ln -sf '/usr/share/man/man1/mawk.1.gz' './alternatives/awk.1.gz'
    ln -sf '/usr/bin/b2m.emacs23' './alternatives/b2m'
    ln -sf '/usr/share/man/man1/b2m.emacs23.1.gz' './alternatives/b2m.1.gz'
    ln -sf '/usr/bin/g++' './alternatives/c++'
    ln -sf '/usr/bin/c89-gcc' './alternatives/c89'
    ln -sf '/usr/share/man/man1/c89-gcc.1.gz' './alternatives/c89.1.gz'

On upgrading from 2.3.0-3 to 2.4-1, I am suddenly bombarded with
spurious changes:

    $ sudo darcs w -l | tail
    a ./alternatives/c89.1.gz
    a ./alternatives/c89
    a ./alternatives/c++
    a ./alternatives/b2m.1.gz
    a ./alternatives/b2m
    a ./alternatives/awk.1.gz
    a ./alternatives/awk
    a ./alternatives/aptitude
    a ./X11/X
    a ./Muttrc.d/t-prot.rc
    $ sudo darcs w -l | wc -l
    628

I'm very scared that if I leave Darcs 2.4 installed, these symlinks
will be recorded as normal files (or worse, directories), and that any
subsequent attempt at data recovery will result in a broken system.

For me, this is a show-stopper bug, because even if I only install
Darcs 2.4 somewhere like ~/.cabal/bin, it could still be used on /etc
if I accidentally let become root without resetting my $PATH.
msg10400 (view) Author: twb Date: 2010-03-21.15:43:21
With Darcs 2.4, it looks like "darcs w -l" will follow symlinks (new),
but "darcs add -qr ." ignores them (unchanged).  Also, e.g.

    $ sudo darcs add X11/X
    Sorry, file X11/X is a symbolic link, which is unsupported by darcs.

IIUC this makes the issue less critical -- while "darcs w -l" will
report false positives and possibly crash (e.g. traversing ln -s x x),
the commands that *modify* the repo aren't affected.

PS: strike that; "darcs rec -l" modifies the repo, and IS affected:

    $ sudo darcs rec --no-prehook -l X11/X <<<q
    Recording changes in "X11/X":

    addfile ./X11/X
    Shall I record this change? (1/2)  [ynWsfvplxdaqjk], or ? for help: q
    Record cancelled.
msg10508 (view) Author: dastapov Date: 2010-03-25.11:08:46
I can confirm that this is indeed a great annoyance, and breaks 
"etckeeper" and repositories with potentially-circulat symlinks (like 
./data/current -> .), even if they are not under version control at all.

Could the previous behavoir be restored sooner that next stable release? 
:)
msg10510 (view) Author: kowey Date: 2010-03-25.11:39:14
I'm setting this back to Target-2.4 (at Reinier's discretion to bump
back up). Perhaps a darcs 2.4.2 release would be good to have.

I'll report informally that this annoys me too, as I have a darcs repo
that has a symlink dir to a giant corpus on my hard disk.

Another point I'll make is that I keep getting this confused with
http://bugs.darcs.net/issue1746 (which seems to have nothing to do with
symlinks).  It looks like this is an easy one for Petr to resolve and it
may clear up any confusion between the two bugs.

Petr, what do you think?
msg10516 (view) Author: mornfall Date: 2010-03-25.14:46:53
Hi,

Eric Kow <bugs@darcs.net> writes:
> I'm setting this back to Target-2.4 (at Reinier's discretion to bump
> back up). Perhaps a darcs 2.4.2 release would be good to have.
>
> I'll report informally that this annoys me too, as I have a darcs repo
> that has a symlink dir to a giant corpus on my hard disk.
>
> Another point I'll make is that I keep getting this confused with
> http://bugs.darcs.net/issue1746 (which seems to have nothing to do with
> symlinks).  It looks like this is an easy one for Petr to resolve and it
> may clear up any confusion between the two bugs.
>
> Petr, what do you think?
well, it's fixed in h-s HEAD, as far as I can tell. It's just question
of deciding about bumping again and releasing 2.4.1 on top of 0.4.11...

Yours,
   Petr.
msg10518 (view) Author: kowey Date: 2010-03-25.17:48:47
On Thu, Mar 25, 2010 at 15:46:39 +0100, Petr Rockai wrote:
> well, it's fixed in h-s HEAD, as far as I can tell. It's just question
> of deciding about bumping again and releasing 2.4.1 on top of 0.4.11...

Oh, perhaps we should try that then.
The usual request for a test applies.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg10519 (view) Author: mornfall Date: 2010-03-25.18:08:46
Eric Kow <kowey@darcs.net> writes:
> Oh, perhaps we should try that then.
> The usual request for a test applies.
Hopefully someone from the unhappy department can supply one? : - )

Yours,
   Petr.
msg10520 (view) Author: dastapov Date: 2010-03-25.19:01:54
On Thu, Mar 25, 2010 at 8:08 PM, Petr Rockai <me@mornfall.net> wrote:
>
> Eric Kow <kowey@darcs.net> writes:
> > Oh, perhaps we should try that then.
> > The usual request for a test applies.
> Hopefully someone from the unhappy department can supply one? : - )

I'm slightly lost. Are you talking about this issue or issue 1746?

If it is the former, I can happily test for you.

Is it enough to build results of "darcs get
{http://darcs.net/repos/unstable,http://repos.mornfall.net/hashed-storage}",
or some other bleeding-edge dependencies needed as well?


--
Dmitry Astapov
msg10524 (view) Author: twb Date: 2010-03-26.10:47:26
Petr Rockai wrote:
> well, it's fixed in h-s HEAD, as far as I can tell. It's just question
> of deciding about bumping again and releasing 2.4.1 on top of 0.4.11...

I still have problems with 2.4+143 built against hashed-storage 0.4.10.
I couldn't find 0.4.11; the darcs repo for h-s seems to only just have
the 0.4.10 tag pushed to it.

    $ with-temp-dir
    with-temp-dir: entering directory `/tmp/with-temp-dir.ZY9yer'
    This directory will be deleted when you exit.
    $ darcs init
    $ ln -s x x
    $ darcs w -l
    darcs: do_stat: invalid argument (Too many levels of symbolic links)
    $ darcs --version
    2.4 (+ 142 patches)
    $ darcs --exact-version
    darcs compiled on Mar 26 2010, at 09:52:21

    Context:

    [Resolve issue1756: new h-s handles file removals correctly in Index.
    Petr Rockai <me@mornfall.net>**20100324212453
     Ignore-this: bbedaa58cc6630492efed346332ee21d
    ]
    [...]

    Compiled with:

    HTTP-4000.0.6
    array-0.3.0.0
    base-4.2.0.0
    bytestring-0.9.1.5
    containers-0.3.0.0
    directory-1.0.1.0
    extensible-exceptions-0.1.1.1
    filepath-1.1.0.3
    hashed-storage-0.4.10
    haskeline-0.6.2.2
    html-1.0.1.2
    mmap-0.4.1
    mtl-1.1.0.2
    network-2.2.1.7
    old-time-1.0.0.3
    parsec-2.1.0.1
    process-1.0.1.2
    random-1.0.0.2
    regex-compat-0.92
    tar-0.3.1.0
    terminfo-0.3.0.2
    text-0.7.1.0
    unix-2.4.0.0
    zlib-0.5.2.0
msg10527 (view) Author: dastapov Date: 2010-03-26.11:23:30
On Fri, Mar 26, 2010 at 12:47 PM, Trent W. Buck <bugs@darcs.net> wrote:
>
> Trent W. Buck <trentbuck@gmail.com> added the comment:
>
> Petr Rockai wrote:
>> well, it's fixed in h-s HEAD, as far as I can tell. It's just question
>> of deciding about bumping again and releasing 2.4.1 on top of 0.4.11...
>
> I still have problems with 2.4+143 built against hashed-storage 0.4.10.
> I couldn't find 0.4.11; the darcs repo for h-s seems to only just have
> the 0.4.10 tag pushed to it.

BTW, which repo is that?

I tried darcs get http://repos.mornfall.net/hashed-storage/ , and
hashed-storage.cabal in there lists version as 0.4.9

-- 
Dmitry Astapov
msg10529 (view) Author: twb Date: 2010-03-26.11:38:59
Dmitry Astapov wrote:
> On Fri, Mar 26, 2010 at 12:47 PM, Trent W. Buck <bugs@darcs.net> wrote:
>> I still have problems with 2.4+143 built against hashed-storage 0.4.10.
>> I couldn't find 0.4.11; the darcs repo for h-s seems to only just have
>> the 0.4.10 tag pushed to it.
>
> BTW, which repo is that?
>
> I tried darcs get http://repos.mornfall.net/hashed-storage/ , and
> hashed-storage.cabal in there lists version as 0.4.9

Check again.  It was updated just before my previous post to this ticket.
msg10598 (view) Author: kowey Date: 2010-03-31.21:35:41
Hi Reinier,

Are we postponing this to a Darcs 2.4.2 (or Darcs 2.5), or are we
waiting for hashed-storage 0.4.11?  

I think Petr is on holiday at the moment, so maybe the 2.4.1 release
should just go out the door and we wait till the next major release to
fix this regression?

Last patch open!
msg10602 (view) Author: mornfall Date: 2010-03-31.22:29:58
Couple of things (literally). First, no-one confirmed that the fix is good and has 
no bad sideeffects. Second, you can probably make 0.4.11 yourselves while I am away 
(hackage should not mind). You can just record the version bump and the tag (make 
sure you just tag 0.4.10 + the fix + the version bump, instead of HEAD) and send 
them my way. (I am about to collapse into bed, just after I sort out all the other 
stuff people need right now and leave tomorrow^Wtoday morning, you get me back mid-
next-week...)
msg10606 (view) Author: kowey Date: 2010-04-01.09:25:14
OK, let me make tick through my list to make sure this isn't falling
through any cracks.

1. Is this really a problem? Question to Dmitry and Trent: why is the
boring file mechanism not good enough of a workaround?

2. Do we have test cases?  Yes,
tests/failing-issue1645-ignore-symlinks.sh (I'm really bad about asking
for tests which actually already exist).

3. Has the code for it been reviewed.  Petr: I think you may have
forgotten to push your post hashed-storage 0.4.10 patches, so nobody can
review them.

Hmm, hope that's the right list.
msg10609 (view) Author: dastapov Date: 2010-04-01.13:06:05
On Thu, Apr 1, 2010 at 10:25 AM, Eric Kow <bugs@darcs.net> wrote:

>
> Eric Kow <kowey@darcs.net> added the comment:
>
> OK, let me make tick through my list to make sure this isn't falling
> through any cracks.
>
> 1. Is this really a problem? Question to Dmitry and Trent: why is the
> boring file mechanism not good enough of a workaround?
>

Boring file mechanism does not solve this because it does not cover all
possible use cases.
Repository could contain symlinks to non-bored files and/or directories.

I would like to reiterate through this from the beginning. Problem with new
symlink handling code is that:
1)it follows symlinks eagerly and could fall into endless loop
2)symlinks to non-boring directories (even non-recursive ones) throw darcs
off the rails.

Let's consider the looping symlink to directory:

mkdir repo
cd repo
darcs init
ln -s . loop

Now, let's run "darcs wh -l":

darcs-2.2: "No changes!"

darcs-2.4-release: "darcs:
./loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop/loop:
changeWorkingDirectory: invalid argument (Too many levels of symbolic
links)"

darcs-2.4+151 patches: "darcs: ./loop: openBinaryFile: inappropriate type
(is a directory)"

I guess that begavoir of both darcs-2.4 version could not be considered as
user-friendly and proper.


OK, what about looping symlinks to non-boring directories?

rm loop
mkdir d1
darcs add d1
darcs rec d1
cd d1
ln -s ../d1 loop2
cd ..

Let's run "darcs wh -l" once again:

darcs-2.2: "No changes!"
darcs-2.4-release: "darcs: do_stat: invalid argument (Too many levels of
symbolic links)"
darcs-2.4+151 patches: "darcs: ./d1/loop2: openBinaryFile: invalid argument
(Too many levels of symbolic links)"

If I add "loop2" to "boring" then "darcs-2.4+151 patches" stops failing and
reports "No changes!", while darcs-2.4 still fails with the same message.

Lastly, what about non-looping symlink to non-boring dir?

rm d1/loop2
ln -s d1 link1

Once again, lets run "darcs wh -l":

darcs-2.2: "No changes!"
darcs-2.4: "a ./link1/" -- which is VERY, VERY wrong!
darcs-2.4+151: "darcs: ./link1: openBinaryFile: inappropriate type (is a
directory)"

As you can see, both 2.4 and bleeding-edge version fail in this case as
well. If I add "link1" to "boring", both failing versions start reporting
"No changes!"

This basically means that I have to go and add each and every symlink to
"boring" in order to be able to do anything useful with darcs-2.4. What is
worse, I cannot use emacs and darcsum-mode for this, because darcs will fail
in "whatsnew". For me, this problem is very real, because it breaks my
"/etc" repo and three other working projects.

Considering the fact that symlinks are not properly handled by darcs (could
not be version-controlled) I personally see no benefits in new behavoir, but
see several nasty regressions + potential for more.


> 2. Do we have test cases?  Yes,
> tests/failing-issue1645-ignore-symlinks.sh (I'm really bad about asking
> for tests which actually already exist).
>

Command "runghc Setup.lhs test failing-issue1645-ignore-symlinks" fails for
me with the following:

rm -rf R S                      # Another script may have left a mess.
+ rm -rf R S
darcs init      --repo R S      # Create our test repos.
+ darcs init --repo R S

darcs failed:  Bad argument: `S'


>
> 3. Has the code for it been reviewed.  Petr: I think you may have
> forgotten to push your post hashed-storage 0.4.10 patches, so nobody can
> review them.



-- 
Dmitry Astapov
Attachments
msg10610 (view) Author: kowey Date: 2010-04-01.13:36:34
Hi Reinier,

Just pointing out that this ticket may require blocking the 2.4.1
release until Petr gets back (middle of next week) and we have time to
review hashed-storage 0.4.11 (by weekend?).  Hmm, then again maybe
that's too rushed and we'd be better off waiting for a 2.4.2 to play it
safe?  We could try to fix it ourselves urgently but that sounds like it
could be riskier :-)

Hi Dmitry,

Great reply!  This sort of thing is helpful to me because I often go
through the bug tracker in stupid-mode (throughput-maximisation over
quality) and miss things people have already said.  :-(
So having things put all in order is useful for me.

[Re-arranging some of your comments to fit my reply]

On Thu, Apr 01, 2010 at 14:05:14 +0100, Dmitry Astapov wrote:
> > 1. Is this really a problem? Question to Dmitry and Trent: why is the
> > boring file mechanism not good enough of a workaround?

> Boring file mechanism does not solve this because it does not cover
> all possible use cases.  Repository could contain symlinks to
> non-bored files and/or directories.

I meant putting the source path (eg. loop and loop2) in boring.
Would that cover those use cases?

> This basically means that I have to go and add each and every symlink to
> "boring" in order to be able to do anything useful with darcs-2.4.

Ah, so I think this answers the question I meant to ask.

Would something like

   find . -type l >> _darcs/prefs/boring

do the trick or are there shortcomings I have still failed to
anticipate?

> 1)it follows symlinks eagerly and could fall into endless loop

> mkdir repo
> cd repo
> darcs init
> ln -s . loop

...

> 2)symlinks to non-boring directories (even non-recursive ones) throw darcs
> off the rails.

> rm loop
> mkdir d1
> darcs add d1
> darcs rec d1
> cd d1
> ln -s ../d1 loop2
> cd ..

These sound like really good test cases.

Could you modify our tests/failing-issue1645... accordingly and darcs
send the results?  (it will default to patches@darcs.net and wind up in
our patch tracker)
 
> What is worse, I cannot use emacs and darcsum-mode for this, because
> darcs will fail in "whatsnew". For me, this problem is very real,
> because it breaks my "/etc" repo and three other working projects.

Even if the boring trick above works?

I'm not saying that users should *have* to use boring workaround; I just
want to make sure I have an accurate assessment of the damage.

> Considering the fact that symlinks are not properly handled by darcs (could
> not be version-controlled) I personally see no benefits in new behavoir, but
> see several nasty regressions + potential for more.

Sounds right...

> > 2. Do we have test cases?  Yes,
> > tests/failing-issue1645-ignore-symlinks.sh (I'm really bad about asking
> > for tests which actually already exist).
> >
> 
> Command "runghc Setup.lhs test failing-issue1645-ignore-symlinks" fails for
> me with the following:
> 
> rm -rf R S                      # Another script may have left a mess.
> + rm -rf R S
> darcs init      --repo R S      # Create our test repos.
> + darcs init --repo R S
> 
> darcs failed:  Bad argument: `S'

Ouch! The test was failing for the wrong reason; good catch.  I've
updated the test accordingly and now I think it fails for the right
reason.

> > 3. Has the code for it been reviewed.  Petr: I think you may have
> > forgotten to push your post hashed-storage 0.4.10 patches, so nobody can
> > review them.

This one may be the key blocker as I suspect the 0.4.11 code is just on
Petr's laptop :-P

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg10612 (view) Author: dastapov Date: 2010-04-01.17:51:31
On Thu, Apr 1, 2010 at 2:35 PM, Eric Kow <kowey@darcs.net> wrote:

> [Re-arranging some of your comments to fit my reply]
>
> On Thu, Apr 01, 2010 at 14:05:14 +0100, Dmitry Astapov wrote:
> > > 1. Is this really a problem? Question to Dmitry and Trent: why is the
> > > boring file mechanism not good enough of a workaround?
>
> > Boring file mechanism does not solve this because it does not cover
> > all possible use cases.  Repository could contain symlinks to
> > non-bored files and/or directories.
>
> I meant putting the source path (eg. loop and loop2) in boring.
> Would that cover those use cases?
>

This still seems like an extra hoop to jump through for no apparent reason.

>
> > This basically means that I have to go and add each and every symlink to
> > "boring" in order to be able to do anything useful with darcs-2.4.
>
> Ah, so I think this answers the question I meant to ask.
>
> Would something like
>
>   find . -type l >> _darcs/prefs/boring
>
> do the trick or are there shortcomings I have still failed to
> anticipate?
>

First of all, it is simply not convenient to do this for each repo. Then, if
this would become the recommended course of action, why not just make darcs
ignore simlinks by default? Oh, wait - that would be exactly how darcs-2.2
behaved ;)

Then, for me there is an issue with "etckeeper" - external tool would have
to be modified to add new symlinks to boring file before each darcs
invocation. And when I'm done with "etckeeper", i would have to patch
"darcsum" as well...

[snip]

>
> These sound like really good test cases.
>
> Could you modify our tests/failing-issue1645... accordingly and darcs
> send the results?  (it will default to patches@darcs.net and wind up in
> our patch tracker)
>

Done.


>
> > What is worse, I cannot use emacs and darcsum-mode for this, because
> > darcs will fail in "whatsnew". For me, this problem is very real,
> > because it breaks my "/etc" repo and three other working projects.
>
> Even if the boring trick above works?
>

The point of using "darcsum-mode" is exactly to avoid using darcs on
command-line. Hence, boring trick kind of defeats the purpose of
"darcsum-mode".


>
> I'm not saying that users should *have* to use boring workaround; I just
> want to make sure I have an accurate assessment of the damage.
>

In this case, yes. Current bleeding-edge darcs and darcs-2.4-release could
be used with this trick


-- 
Dmitry Astapov
Attachments
msg10616 (view) Author: twb Date: 2010-04-01.23:30:01
Eric Kow wrote:
> 1. Is this really a problem? Question to Dmitry and Trent: why is the
> boring file mechanism not good enough of a workaround?

While TECHNICALLY sufficient, it's tedious.

One can't simply add the output of "find -type l" to the boring file,
because the symlinks might contain regex special characters (such as a
period); and in a pathological case, may contain newlines.  Regexp
escaping would be needed.

For cases like etckeeper, one can't simply do it once -- it would have
to be part of a pre-hook.

Further, the current behaviour of add and --look-for-adds are
inconsistent -- the former will ignore symlinks, the latter will
traverse them.
msg10646 (view) Author: tux_rocker Date: 2010-04-03.12:15:19
I'm going to wait on this ticket with the 2.4.1 release. IMO it would be
silly to release 2.4.1 now and than another 2.4.2 only a week later.
msg10677 (view) Author: mornfall Date: 2010-04-05.18:15:05
Eric,

the patch is public for a while. There are lots of post-.10 changes, you
just need to say changes --from-tag . to see them, since they are older
than the tag...

Yours,
   Petr.
msg10678 (view) Author: kowey Date: 2010-04-05.19:48:51
DOH! Darcs developer forgets that darcs patch reordering.  Sorry, the
"last" patch in the repo was 0.4.10 so I just got confused.

Petr, is it safe then to tag hashed-storage with just this one patch?

Sun Mar 21 10:03:46 GMT 2010  Petr Rockai <me@mornfall.net>
  * Never follow symlinks.
    hunk ./Bundled/Posix.hsc 83
    -  do_stat (\p -> (fp `withCString` (`c_stat` p)))
    +  do_stat (\p -> (fp `withCString` (`lstat` p)))
msg10687 (view) Author: dastapov Date: 2010-04-06.09:52:25
On Mon, Apr 5, 2010 at 10:48 PM, Eric Kow <bugs@darcs.net> wrote:

>
> Eric Kow <kowey@darcs.net> added the comment:
>
> DOH! Darcs developer forgets that darcs patch reordering.  Sorry, the
> "last" patch in the repo was 0.4.10 so I just got confused.
>
> Petr, is it safe then to tag hashed-storage with just this one patch?
>
> Sun Mar 21 10:03:46 GMT 2010  Petr Rockai <me@mornfall.net>
>  * Never follow symlinks.
>    hunk ./Bundled/Posix.hsc 83
>    -  do_stat (\p -> (fp `withCString` (`c_stat` p)))
>    +  do_stat (\p -> (fp `withCString` (`lstat` p)))
>
> ----------
> status: need-implementation -> waiting-for
>


JFYI: this patch was present when I was testing latest darcs, and apparently
it is not enough.

-- 
Dmitry Astapov
Attachments
msg10689 (view) Author: mornfall Date: 2010-04-06.12:55:32
Dmitry Astapov <dastapov@gmail.com> writes:
> JFYI: this patch was present when I was testing latest darcs, and
> apparently it is not enough.
I have pushed one more patch to h-s, that disregards all entities that
are neither regular files nor directories, in plain trees. This makes
the test pass for me.

Yours,
   Petr.
msg10691 (view) Author: dastapov Date: 2010-04-06.13:22:45
On Tue, Apr 6, 2010 at 3:54 PM, Petr Rockai <me@mornfall.net> wrote:

> Dmitry Astapov <dastapov@gmail.com> writes:
> > JFYI: this patch was present when I was testing latest darcs, and
> > apparently it is not enough.
> I have pushed one more patch to h-s, that disregards all entities that
> are neither regular files nor directories, in plain trees. This makes
> the test pass for me.
>

I could confirm that version "2.4 (+ 163 patches)", built with latest
bleeding-edge hashed-storage passes the symlink-related tests.

-- 
Dmitry Astapov
Attachments
msg10693 (view) Author: kowey Date: 2010-04-07.17:33:02
On Tue, Apr 06, 2010 at 13:22:46 +0000, Dmitry Astapov wrote:
> I could confirm that version "2.4 (+ 163 patches)", built with latest
> bleeding-edge hashed-storage passes the symlink-related tests.

OK, great, so it looks like we need to do these things

1. Tag 0.4.11 with the two patches from Petr
2. Get 0.4.11 reviewed
3. Review Dmitry's test case and apply it to Darcs
4. Rename test case to non-failing
5. Bump hashed-storage dependency

Petr: Would it be possible for you to take on #1 and #3?

I'll request for Ganesh to do #2 by doing #4 and #5

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg10698 (view) Author: kowey Date: 2010-04-08.11:06:51
On Wed, Apr 07, 2010 at 17:33:04 +0000, Eric Kow wrote:
> 1. Tag 0.4.11 with the two patches from Petr
> 2. Get 0.4.11 reviewed
> 3. Review Dmitry's test case and apply it to Darcs
> 4. Rename test case to non-failing
> 5. Bump hashed-storage dependency
> 
> Petr: Would it be possible for you to take on #1 and #3?

I've gone ahead and done #3

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
msg10796 (view) Author: kowey Date: 2010-04-23.15:24:40
The following patch updated the status of issue1645 to be resolved:

* Resolve issue1645: bump hashed-storage to 0.4.11; don't follow symlinks. 
Ignore-this: dc680d7f37252c59035b19cbd0f27927
msg11382 (view) Author: kowey Date: 2010-06-13.17:02:16
The following patch updated the status of issue1645 to be resolved-in-stable:

* Resolve issue1645: bump hashed-storage to 0.4.11; don't follow symlinks. 
Ignore-this: dc680d7f37252c59035b19cbd0f27927
History
Date User Action Args
2009-10-09 01:08:50twbcreate
2009-10-09 06:04:36koweysetstatus: unknown -> needs-reproduction
assignedto: mornfall
topic: + Regression
messages: + msg8935
nosy: + kowey, mornfall
2009-10-09 08:49:17ertaisetnosy: + ertai
messages: + msg8938
2009-10-10 14:45:19tux_rockersetnosy: + tux_rocker
messages: + msg8943
2009-10-10 21:04:08koweysetstatus: needs-reproduction -> needs-implementation
topic: + Target-2.4
nosy: kowey, darcs-devel, twb, ertai, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8944
priority: bug
assignedto: mornfall -> (no value)
2009-10-11 04:09:25twbsetnosy: kowey, darcs-devel, twb, ertai, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8948
2009-10-11 04:10:43twbsetnosy: kowey, darcs-devel, twb, ertai, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8949
2009-10-23 22:41:01adminsetnosy: + nicolas.pouillard, - ertai
2009-10-24 00:06:07adminsetnosy: + ertai, - nicolas.pouillard
2009-11-15 16:48:49tux_rockersettopic: - Target-2.4
2010-02-20 15:08:04twbsetmessages: + msg10046
title: symlink regression after darcs-hs -> Darcs >2.3 follows directory symlinks.
2010-03-20 20:07:47twbsetmessages: + msg10335
2010-03-21 06:45:57koweysettopic: + Target-2.5
2010-03-21 15:43:23twbsetmessages: + msg10400
2010-03-25 11:08:49dastapovsetnosy: + dastapov
messages: + msg10508
2010-03-25 11:39:21koweysettopic: + Target-2.4, - Target-2.5
assignedto: mornfall
messages: + msg10510
2010-03-25 14:46:56mornfallsetmessages: + msg10516
2010-03-25 17:48:48koweysetmessages: + msg10518
2010-03-25 18:08:47mornfallsetmessages: + msg10519
2010-03-25 19:01:54dastapovsetmessages: + msg10520
2010-03-26 10:47:27twbsetmessages: + msg10524
2010-03-26 11:23:31dastapovsetmessages: + msg10527
2010-03-26 11:39:00twbsetmessages: + msg10529
2010-03-31 21:35:42koweysetmessages: + msg10598
2010-03-31 22:29:58mornfallsetmessages: + msg10602
2010-04-01 09:25:15koweysetmessages: + msg10606
2010-04-01 13:06:07dastapovsetfiles: + unnamed
messages: + msg10609
2010-04-01 13:36:35koweysetmessages: + msg10610
2010-04-01 17:51:32dastapovsetfiles: + unnamed
messages: + msg10612
2010-04-01 21:17:24koweylinkpatch203 issues
2010-04-01 23:30:02twbsetmessages: + msg10616
2010-04-03 12:15:20tux_rockersetmessages: + msg10646
2010-04-05 18:15:06mornfallsetmessages: + msg10677
2010-04-05 19:48:52koweysetstatus: needs-implementation -> waiting-for
messages: + msg10678
2010-04-06 09:52:26dastapovsetfiles: + unnamed
messages: + msg10687
2010-04-06 12:55:33mornfallsetmessages: + msg10689
2010-04-06 13:22:46dastapovsetfiles: + unnamed
messages: + msg10691
2010-04-07 17:33:04koweysetmessages: + msg10693
2010-04-08 11:06:52koweysetmessages: + msg10698
2010-04-23 15:24:41koweysetstatus: waiting-for -> resolved
messages: + msg10796
2010-06-13 17:02:16koweysetstatus: resolved -> resolved-in-stable
messages: + msg11382
2010-06-15 21:31:21adminsetmilestone: 2.4.x
2010-06-15 21:31:22adminsettopic: - Target-2.4
2010-06-15 22:14:18adminsetstatus: resolved-in-stable -> resolved
2010-06-15 22:14:18adminsetresolvedin: 2.5.0
2010-06-16 14:39:24koweysetresolvedin: 2.5.0 -> 2.4.x