Skip to content

Conversation

@StefanKarpinski
Copy link
Member

@StefanKarpinski StefanKarpinski commented Sep 5, 2019

This is a pretty niche issue but it allows readdir() to work after the current working direcotory has been deleted, whereas when join=true you need a path which no longer exists, so pwd() fails and therefore readdir(join=true) also fails.

I can't find a way to get a current working directory that is both deleted and non-empty:

  • if you create files first, then you can't delete the directory (unless you use rm -r in which case it deletes the file before deleting the directory)
  • trying to create files after deleting the directory doesn't work either since all the ways I tried of creating files in a deleted directory failed.

test/file.jl Outdated
@test !ispath(d)
@test isempty(readdir())
@test_throws SystemError readdir(d)
@test_throws Base.IOError readdir(join=true)
Copy link
Member Author

@StefanKarpinski StefanKarpinski Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of nasty that these throw different errors: the SystemError comes from trying to call readdir on a non-existend directory whereas the Base.IOError comes from trying to do pwd() in a deleted directory.

@StefanKarpinski
Copy link
Member Author

What is going on with the tests? The failures do not seem related to this change.

This is a pretty niche issue but it allows `readdir()` to work after the current
working direcotory has been deleted, whereas when `join=true` you need a path
which no longer exists, so `pwd()` fails and so `readdir(join=true)` also fails.
@StefanKarpinski
Copy link
Member Author

I don't see how this Win32 failure could be caused by this change, but I don't want to merge this and break Win32. Anyone know if this is a thing that's been happening otherwise?

@StefanKarpinski StefanKarpinski merged commit 3424d2c into master Sep 7, 2019
@delete-merged-branch delete-merged-branch bot deleted the sk/readdir branch September 7, 2019 22:55
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

Successfully merging this pull request may close these issues.

3 participants