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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 28, 2021
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77e2f49
Fix Process::INITIAL_PWD for non-existent path
straight-shoota Mar 19, 2021
7c52f0d
Disable spec on win32
straight-shoota Mar 19, 2021
dceb771
Avoid `/` as CURRENT_DIR when INITIAL_PWD does not exist
straight-shoota Mar 20, 2021
eb7d7e2
Let CallStack::CURRENT_DIR be nilable
straight-shoota Mar 22, 2021
91e3b5f
Merge branch 'master' into fix/nonexistent-initial_pwd
straight-shoota Oct 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions spec/std/exception/call_stack_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,21 @@ describe "Backtrace" do
output.to_s.empty?.should be_true
error.to_s.should contain("Invalid memory access")
end

pending_win32 "print exception with non-existing PWD" do
source_file = datapath("blank_test_file.txt")
compile_file(source_file) do |executable_file|
output, error = IO::Memory.new, IO::Memory.new
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.

Dir.delete(path)
status = Process.run executable_file

status.success?.should be_true
end
end
end
end
end
9 changes: 6 additions & 3 deletions src/exception/call_stack.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ struct Exception::CallStack
# Compute current directory at the beginning so filenames
# are always shown relative to the *starting* working directory.
CURRENT_DIR = begin
dir = Process::INITIAL_PWD
dir += File::SEPARATOR unless dir.ends_with?(File::SEPARATOR)
dir
if dir = Process::INITIAL_PWD
dir += File::SEPARATOR unless dir.ends_with?(File::SEPARATOR)
dir
else
""
end
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
end

@@skip = [] of String
Expand Down
15 changes: 13 additions & 2 deletions src/process/executable_path.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ class Process
INITIAL_PATH = ENV["PATH"]?

# :nodoc:
INITIAL_PWD = Dir.current
#
# Working directory at program start. Nil if working directory does not exist.
#
# Used for `Exception::CallStack::CURRENT_DIR`
# and `Process.executable_path_impl` on openbsd.
INITIAL_PWD = begin
Dir.current
rescue File::NotFoundError
nil
end

# Returns an absolute path to the executable file of the currently running
# program. This is in opposition to `PROGRAM_NAME` which may be a relative or
Expand Down Expand Up @@ -166,7 +175,9 @@ end
# openbsd, ...
class Process
private def self.executable_path_impl
find_executable(PROGRAM_NAME, INITIAL_PATH, INITIAL_PWD)
if pwd = INITIAL_PWD
find_executable(PROGRAM_NAME, INITIAL_PATH, pwd)
end
end
end
{% end %}