Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

walk-directory fails because of a single invalid link #15

Open
chalaev opened this issue Aug 16, 2016 · 15 comments
Open

walk-directory fails because of a single invalid link #15

chalaev opened this issue Aug 16, 2016 · 15 comments

Comments

@chalaev
Copy link

chalaev commented Aug 16, 2016

When I recursively scan a directory using osicat:walk-directory
(e.g., in https://github.com/chalaev/esy/blob/master/main.lisp)
the whole scan procedure fails because of a single invalid
soft link somewhere in the subdirectory tree.
I suggest a quick (and probably not the best one) patch fixing the problem, see
https://github.com/chalaev/esy/tree/master/osicat

@luismbo
Copy link
Contributor

luismbo commented Aug 17, 2016

What do other APIs (possibly in other languages) do in this scenario?

@chalaev
Copy link
Author

chalaev commented Aug 17, 2016

I do not know what other people usually do in this situation; when calling walk-directory, one provides a function to act on every file/directory it finds. I would optionally provide another function to be called on an invalid (unreadable) file. The patch adds such function (named complain-on-eaccess) to call-with-directory-iterator. May be one could add this optional function to walk-directory variable list;
then walk-directory could provide this function to call-with-directory-iterator.

@luismbo
Copy link
Contributor

luismbo commented Aug 17, 2016

I was asking to help make a decision on what the (default, or only) behaviour of walk-directory should be. Hmm, here's a simpler question: what does (directory "/path/to/your/dir/with/a/bad/link/*") do?

@chalaev
Copy link
Author

chalaev commented Aug 17, 2016

(directory "/home/shalaev/impurities_in_Si/literature/PRB/v11p1555.pdf") ; => nil
(directory "/home/shalaev/mpurities_in_Si/literature/PRB/v11p1555.pdf/"); => nil
(directory "/home/shalaev/impurities_in_Si/literature/PRB/v11p1555.pdf/*"); => nil

where /home/shalaev/impurities_in_Si/literature/PRB/v11p1555.pdf points to a non-existent file.

(directory "/home/shalaev/impurities_in_Si/literature/PRB/"); => (#P"/home/shalaev/impurities_in_Si/literature/PRB/")
(directory "/home/shalaev/impurities_in_Si/literature/PRB/*"); => nil

@luismbo
Copy link
Contributor

luismbo commented Aug 17, 2016

Interesting. What Lisp is that? Here's what I see with SBCL:

luis@nhop /t/test ❯❯❯ ln -s non-existing-target bad-link
luis@nhop /t/test ❯❯❯ ls -l
total 0
lrwxrwxrwx 1 luis luis 19 Aug 17 15:32 bad-link -> non-existing-target
luis@nhop /t/test ❯❯❯ sbcl
This is SBCL 1.3.0, an implementation of ANSI Common Lisp.
More information about SBCL is available at <http://www.sbcl.org/>.

SBCL is free software, provided as is, with absolutely no warranty.
It is mostly in the public domain; some portions are provided under
BSD-style licenses.  See the CREDITS and COPYING files in the
distribution for more information.
* (directory "*")

(#P"/tmp/test/bad-link")

Python works too:

luis@nhop /t/test ❯❯❯ python
Python 2.7.6 (default, Jun 22 2015, 18:00:18)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from os import listdir
>>> listdir(".")
['bad-link']

Can you fix osicat to do this? A pull request would be best, but a patch is OK if you don't feel like doing that dance.

@chalaev
Copy link
Author

chalaev commented Aug 17, 2016

It matters if bad-link points to an object in a directory which we can (not) read:

test> ls /mnt/dom/
ls: cannot open directory /mnt/dom/: Permission denied
test> ln -s /mnt/dom/non-existing-target bad-link
test> ls -lh
total 0
lrwxrwxrwx 1 shalaev shalaev 28 Aug 17 12:10 bad-link -> /mnt/dom/non-existing-target
test> ~/local/bin/sbcl 
This is SBCL 1.3.6, an implementation of ANSI Common Lisp.
More information about SBCL is available at <http://www.sbcl.org/>.

SBCL is free software, provided as is, with absolutely no warranty.
It is mostly in the public domain; some portions are provided under
BSD-style licenses.  See the CREDITS and COPYING files in the
distribution for more information.
* (directory "*")

NIL
* 

I will fix the issue with osicat:walk-directory. Will use pull request for that, plan to finish in 1-2 weeks.

@luismbo
Copy link
Contributor

luismbo commented Aug 17, 2016

Interesting. Python does list the bad-link nevertheless. I think I prefer that behavior. What do you think?

@chalaev
Copy link
Author

chalaev commented Aug 17, 2016

I agree that bad-link must be listed. If (shell) ls can read it, (sbcl) directory must list it too. I think that current behavior of directoryis not what sbcl-developers had in mind, and might be reason for a bug-report.

@Ferada
Copy link
Contributor

Ferada commented Aug 17, 2016

@chalaev I agree with your reasoning. I haven't checked the rest of osicat for matter, but perhaps another flag like :if-does-not-exists (like :if-cannot-access) would match the current functions a bit more? Possibly also reraising with cerror so it can be caught by user code.

@chalaev
Copy link
Author

chalaev commented Aug 17, 2016

For now the idea with :if-cannot-access seems me the best one; will think more about it later.

@luismbo
Copy link
Contributor

luismbo commented Aug 17, 2016

I'm leaning towards just listing it unconditionally, based on what Python and SBCL (mostly) are doing. If the file cannot be accessed, then that can be tested with other predicates.

@Ferada
Copy link
Contributor

Ferada commented Aug 17, 2016

Oh I meant for recursing into it, it should of course be listed anyway.

@epipping
Copy link

I'm surprised to see that a suggestion along the lines of :follow-symlinks nil hasn't been put forward (some CL implementations prefer names like :follow-links, :resolve-links, or :truenamep). That would address the larger problem of handling symlinks in general, not just the ones that are broken.

@luismbo
Copy link
Contributor

luismbo commented Sep 1, 2017

@chalaev do you have any energy to pursue this?

I think @epipping's proposal is sensible but it's orthogonal to this issue. When following symlinks we don't want to abort just because one of the links might be broken.

@chalaev
Copy link
Author

chalaev commented Sep 1, 2017

Will not be available until 2018, sorry.
And I agree with what you wrote:

When following symlinks we don't want to abort just because one of the links might be broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants