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

Fix Process::INITIAL_PWD for non-existent path #10525

Conversation

straight-shoota
Copy link
Member

This patch changes runtime behaviour to gracefully handle if the initial working directory does not exist.

  • Exception::CallStack just ignores it and shows full paths. The initial working directory is just used to shorten paths, no big deal if that's not possible.
  • executable_path_impl on openbsd returns nil if the initial working directory does not exist. It's impossible to find the executable that way.

Resolves #7198

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib labels Mar 19, 2021
src/exception/call_stack.cr Outdated Show resolved Hide resolved
src/exception/call_stack.cr Outdated Show resolved Hide resolved
with_tempfile("non-existent") do |path|
Dir.mkdir path
Dir.cd(path) do
# on win32 it seems not possible to remove the directory while we're cd'ed into it
Copy link
Member

Choose a reason for hiding this comment

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

So should the test be totally disabled for win32? "Pending" means it can be made to work, but seems like this one can't

Copy link
Member Author

Choose a reason for hiding this comment

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

So far we only know it apparently can't work this way. But it might be doable in a different way...? Not sure about that. And in doubt, I'd rather leave it pending than entirely forget about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

From https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/rmdir-wrmdir?view=msvc-170#remarks:

The directory must be empty, and it must not be the current working directory or the root directory.

And https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/rmdir:

You can't use the rmdir command to delete the current directory. If you attempt to delete the current directory, the following error message appears:

The process can't access the file because it is being used by another process.

If you receive this error message, you must change to a different directory (not a subdirectory of the current directory), and then try again.

@oprypin
Copy link
Member

oprypin commented Oct 26, 2021

Hmm isn't this a breaking change - changing the type of INITIAL_PWD to nilable

@straight-shoota
Copy link
Member Author

Yeah, but it's not part of the public API (nodoc). So that should be fine.

@straight-shoota straight-shoota added this to the 1.3.0 milestone Oct 27, 2021
@straight-shoota straight-shoota changed the title Fix Process::INITIAL_PWD for non-existent path Fix Process::INITIAL_PWD for non-existent path Oct 27, 2021
@straight-shoota straight-shoota merged commit c573015 into crystal-lang:master Oct 28, 2021
@straight-shoota straight-shoota deleted the fix/nonexistent-initial_pwd branch October 28, 2021 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle nonexistent PWD gracefully
4 participants