Dmitry Astapov <[email protected]> added the comment:
Trent W. Buck wrote:
> Trent W. Buck <[email protected]> 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 <[email protected]>
<http://bugs.darcs.net/patch203>
__________________________________
Sun Apr 4 22:16:13 EEST 2010 Dmitry Astapov <[email protected]>
* Implemented all 13 possible symlink handling scenarios that I could think of for issue 1645
diff -rN -u old-darcs-unstable/tests/failing-issue1645-ignore-symlinks.sh new-darcs-unstable/tests/failing-issue1645-ignore-symlinks.sh
--- old-darcs-unstable/tests/failing-issue1645-ignore-symlinks.sh 2010-04-04 22:17:52.424110498 +0300
+++ new-darcs-unstable/tests/failing-issue1645-ignore-symlinks.sh 2010-04-04 22:17:52.608109316 +0300
@@ -1,8 +1,12 @@
#!/usr/bin/env bash
-## Test for issue1646 - Darcs should not follow symlinks, ESPECIALLY
-## symlinks to directories outside the repository.
+## Test for issue1645 - Since Darcs does not version-contol symlinks,
+## it should not follow them, ESPECIALLY symlinks to directories
+## outside the repository. All these tests are passed with darcs-2.2
##
-## Copyright (C) 2010 Trent W. Buck
+## See path_resolution(7) and symlink(7) for more info, especially
+## the former.
+##
+## Copyright (C) 2010 Trent W. Buck, Dmitry Astapov
##
## Permission is hereby granted, free of charge, to any person
## obtaining a copy of this software and associated documentation
@@ -26,35 +30,159 @@
. lib # Load some portability helpers.
rm -rf R S # Another script may have left a mess.
-darcs init --repo R S # Create our test repos.
+darcs init --repo R
+darcs init --repo S # Create our test repos.
+
+darcs --version
+
+add_to_boring() {
+ echo "$1" >> _darcs/prefs/boring
+}
## These are the simple does-the-wrong-thing errors.
cd R
-echo 'Example content.' > f
-darcs rec -lamf # business as usual...
-ln -s f l # darcs should just ignore l
-not darcs add -qr . # add -r never had -l's problem.
-darcs w -l >log # w -l shouldn't "see" the link.
-grep 'No changes!' log
-darcs rec -laml >log # rec -l shouldn't record it.
-grep 'No changes!' log
-cd ..
-
-## These are pathological cases
-cd S
-echo 'Example content.' > f
+touch log
+add_to_boring '^log$'
+
+unset pwd # Since this test is pretty much linux-specific, hspwd.hs is not needed
+
+# Case 1: looping symlink to non-recorded non-boring dir
+mkdir non-recorded-dir
+ln -s ../non-recorded-dir ./non-recorded-dir/loop # relative symlink
+ln -s "`pwd`"/non-recorded-dir ./non-recorded-dir/loop2 # absolute symlink
+darcs w -l >log 2>&1 # should not loop
+darcs rec -alm "added ./non-recorded-dir" >>log 2>&1 # should not loop
+darcs changes -s --patch="added ./non-recorded-dir" >>log 2>&1 # should report only dir, not symlink
+not grep -vE "(^ *$|^\+|[0-9]:[0-9][0-9]:[0-9]|./non-recorded-dir)" log
+
+# Case 2: looping symlink to recorded dir
+mkdir recorded-dir
+darcs add recorded-dir
+darcs rec -am "added recorded-dir"
+ln -s ../recorded-dir ./recorded-dir/loop # relative symlink
+ln -s "`pwd`"/recorded-dir ./recorded-dir/loop2 # absolute symlink
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+
+# Case 3: looping symlink to boring dir
+mkdir boring-dir
+add_to_boring '^boring-dir$'
+ln -s ../boring-dir ./boring-dir/loop
+ln -s "`pwd`"/boting-dir ./boring-dir/loop2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+
+# Case 4: non-looping symlink to non-recorded non-boring dir
+mkdir non-recorded-dir2
+ln -s ./non-recorded-dir2 link
+ln -s "`pwd`"/non-recorded-dir2 ./link2
+darcs w -l >log 2>&1 # should report only "non-recorded-dir2"
+darcs rec -alm "added ./non-recorded-dir2" >>log 2>&1 # should add only dir, not symlink
+darcs changes -s --patch="added ./non-recorded-dir2" >>log 2>&1 # should report only dir, not symlink
+not grep -vE "(^ *$|^\+|[0-9]:[0-9][0-9]:[0-9]|./non-recorded-dir2)" log
+rm link link2
+
+# Case 5: non-looping symlink to recorded dir
+ln -s ./recorded-dir ./link
+ln -s "`pwd`"/recorded-dir ./link2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 6: non-looping symlink to boring dir
+ln -s ./boring-dir ./link
+ln -s "`pwd`"/boring-dir ./link2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 7: symlink pointing outside the repo
+ln -s ../S link
+(cd ..; ln -s "`pwd`"/S ./R/link2)
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 8: symlink to non-recorded non-boring file
+touch non-recorded-file
+ln -s ./non-recorded-file ./link
+ln -s "`pwd`"/non-recorded-file ./link2
+darcs w -l >log 2>&1 # should report only "non-recorded-file"
+darcs rec -alm "added ./non-recorded-file" >>log 2>&1 # should add only file, not symlink
+darcs changes -s --patch="added ./non-recorded-file" >>log 2>&1 # should report only file, not symlink
+not grep -vE "(^ *$|^\+|[0-9]:[0-9][0-9]:[0-9]|./non-recorded-file)" log
+rm link link2
+
+# Case 9: symlink to recorded file
+echo "some content" > recorded-file
+darcs add recorded-file
+darcs rec -am "added recorded-file" recorded-file
+ln -s ./recorded-file ./link
+ln -s "`pwd`"/recorded-file ./link2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 10: symlink to boring file
+ln -s ./log ./link
+ln -s "`pwd`"/log ./link2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 11: dangling symlink
+ln -s /completely/bogus/path ./link
+ln -s ../../../../not/exist ./link2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm link link2
+
+# Case 12: self-referencing link
ln -s l l
-darcs w -l
+ln -s "`pwd`"/l2 ./l2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm l l2
-rm l
+# Case 13: link to device file outside the repo
ln -s /dev/zero l
-darcs w -l
-
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
rm l
-ln -s /dev/nonexistent l
-darcs w -l
+# Case 14: link to fifo
mkfifo f
-rm l
ln -s f l
-darcs w -l
+ln -s "`pwd`"/f ./l2
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm f l l2
+
+# Case 15: case-folding link to non-recorded file
+touch non-recorded-file2
+ln -s ./non-recorded-file2 ./Non-Recorded-File2
+ln -s "`pwd`"/non-recorded-file2 ./Non-ReCoRdEd-File2
+darcs w -l >log 2>&1 # should report only "non-recorded-file"
+darcs rec -alm "added ./non-recorded-file2" >>log 2>&1 # should add only file, not symlink
+darcs changes -s --patch="added ./non-recorded-file2" >>log 2>&1 # should report only file, not symlink
+not grep -vE "(^ *$|^\+|[0-9]:[0-9][0-9]:[0-9]|./non-recorded-file2)" log
+rm Non-Recorded-File2 ./Non-ReCoRdEd-File2
+
+# Case 16: case-folding link to recorded file
+ln -s ./recorded-file ./Recorded-File
+ln -s "`pwd`"/recorded-file ./ReCorded-File
+not darcs w -l >log 2>&1 # expecting "No changes!"
+darcs rec -alm "should not happen" >>log 2>&1 # expecting "No changes!" as well
+not grep -vE "(^ *$|^\+|No changes!)" log
+rm Recorded-File ReCorded-File
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
