[darcs-users] [patch203] Implemented all 13 possible symlink handling scenarios...

Dmitry Astapov bugs at darcs.net
Sun Apr 4 19:54:39 UTC 2010


Dmitry Astapov <dastapov at gmail.com> added the comment:

Trent W. Buck wrote:
> Trent W. Buck <trentbuck at gmail.com> added the comment:
>
> Thanks for extending these tests, Dmitry.
>
>   
>> +darcs --version
>>     
>
> Superfluous.
>   
I put this in to make sure I am testing the version I think I am 
testing. It slipped into final commit by error.

>> +# Rationale: Since darcs does not version-control symlinks, it should do a
>> +# Sensible Thing handling them.
>> +# All these tests are passed with darcs-2.2
>>     
>
> This belongs in the introductory comment at the top of the file.
>   
Sure. Done.

>> +touch log
>> +add_to_boring '^log$'
>>     
>
> It's a reasonable idea to do this up-front to avoid log causing false
> positives/negatives.  A better idea might be to simply use ../log
> (i.e. store the buffer outside the repo).
>   
I decided that I'd better keep all the relevant info inside ./R.
> Above, I made sure to check add, w -l *and* rec -l, since currently
> their behaviour seems inconsistent.  The new version only tests w -l;
> I think that is insufficient.
>   
I did not know that "rec" could behave differently. I'm extending the 
tests to cover it as well.
>   
>> +[ -z "$(grep -vE "(^ *$|a ./non-recorded-dir)" log)" ]
>>     
>
> grep's exit status is meaningful; you should trust it instead of
> wrapping in a [ -z "$()" ].
>   
Right. Totally forgot about that.
>   
>> hunk ./tests/failing-issue1645-ignore-symlinks.sh 129
>> +# Case 12: link to device file
>> +ln -s /dev/zero l
>>     
>
> Should also test what happens when the device file occurs within the
> repo (cp -a it in, I guess).
>   
You mean, test the handling of the device file itself, not symlink to it?
I believe this would be better placed into a separate test.

>                                 * * *
>
> Suggest ref. symlink(7) and path_resolution(7) in introduction.
> Suggest testing
>
>  - dangling symlinks.
>   
Already covered - "Case 10"
>  - absolute (cf. relative) links.
>   
Right. Adding ...
>  - historical drive letter idiom "/../C/vms".
>  - links that cross filesystems.
>   
What could possibly (additionaly) go wrong in those two cases?
>  - symlinks to hard links.
>   
Could you please elaborate on this one?
>  - case-folding links, e.g. ln -s Makefile makefile.
>   
Adding this as well.
>  - links to char vs. block devices.
>  - links to Solaris "door" inodes?
>  - links to sockets.
>   
Don't you think that those are kinda superfluous?

What is the best way to sumbit amended patch right now? Should I "darcs 
amend" what I already have and re-sent it, or record additional patch 
and send two of them once again, or ...?

For now, I would attach amended patch to this letter.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch203>
__________________________________
-------------- next part --------------
A non-text attachment was scrubbed...
Name: failing-issue1645-ignore-symlinks.sh.patch
Type: text/x-patch
Size: 9039 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100404/b28fb386/attachment-0001.bin>


More information about the darcs-users mailing list