From cfb6fbd3b7021e0ed864d37ce34a96b38730d250 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sat, 27 Nov 2021 16:39:33 +0800 Subject: [PATCH 1/3] Implement `Path.home` on Windows --- .github/workflows/win.yml | 2 +- spec/std/path_spec.cr | 23 +++++++++++++++++++ src/crystal/system/path.cr | 12 ++++++++++ src/crystal/system/unix/path.cr | 5 ++++ src/crystal/system/win32/path.cr | 17 ++++++++++++++ src/lib_c/x86_64-windows-msvc/c/combaseapi.cr | 4 ++++ .../x86_64-windows-msvc/c/knownfolders.cr | 5 ++++ .../x86_64-windows-msvc/c/shlobj_core.cr | 6 +++++ src/path.cr | 4 +++- 9 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 src/crystal/system/path.cr create mode 100644 src/crystal/system/unix/path.cr create mode 100644 src/crystal/system/win32/path.cr create mode 100644 src/lib_c/x86_64-windows-msvc/c/combaseapi.cr create mode 100644 src/lib_c/x86_64-windows-msvc/c/knownfolders.cr create mode 100644 src/lib_c/x86_64-windows-msvc/c/shlobj_core.cr diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index 42b4237961d0..c1c25b878d0f 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -243,7 +243,7 @@ jobs: cl /MT /c src\llvm\ext\llvm_ext.cc -I llvm\include /Fosrc\llvm\ext\llvm_ext.obj - name: Link Crystal executable run: | - Invoke-Expression "cl crystal.obj /Fecrystal-cross src\llvm\ext\llvm_ext.obj $(llvm\bin\llvm-config.exe --libs) libs\pcre.lib libs\gc.lib WS2_32.lib advapi32.lib libcmt.lib dbghelp.lib legacy_stdio_definitions.lib /F10000000" + Invoke-Expression "cl crystal.obj /Fecrystal-cross src\llvm\ext\llvm_ext.obj $(llvm\bin\llvm-config.exe --libs) libs\pcre.lib libs\gc.lib WS2_32.lib advapi32.lib libcmt.lib dbghelp.lib ole32.lib shell32.lib legacy_stdio_definitions.lib /F10000000" - name: Re-build Crystal run: | diff --git a/spec/std/path_spec.cr b/spec/std/path_spec.cr index 27ba30136987..b02b46e56ff6 100644 --- a/spec/std/path_spec.cr +++ b/spec/std/path_spec.cr @@ -1,5 +1,6 @@ require "spec" require "./spec_helper" +require "../support/env" private BASE_POSIX = "/default/base" private BASE_WINDOWS = "\\default\\base" @@ -889,4 +890,26 @@ describe Path do assert_paths_raw("foo.txt./", "foo.txt.", &.stem) assert_paths_raw("foo..txt/", "foo.", &.stem) end + + describe ".home" do + {% if flag?(:win32) %} + it "returns %USERPROFILE% if set" do + with_env("USERPROFILE": "foo/bar") do + Path.home.should eq(Path.new("foo/bar")) + end + end + + it "doesn't raise if %USERPROFILE% is missing" do + with_env("USERPROFILE": nil) do + Path.home + end + end + {% else %} + it "returns $HOME if set" do + with_env("HOME": "foo/bar") do + Path.home.should eq(Path.new("foo/bar")) + end + end + {% end %} + end end diff --git a/src/crystal/system/path.cr b/src/crystal/system/path.cr new file mode 100644 index 000000000000..17091ec3fc86 --- /dev/null +++ b/src/crystal/system/path.cr @@ -0,0 +1,12 @@ +module Crystal::System::Path + # Returns the path of the home directory of the current user. + # def self.home : String +end + +{% if flag?(:unix) %} + require "./unix/path" +{% elsif flag?(:win32) %} + require "./win32/path" +{% else %} + {% raise "No Crystal::System::Path implementation available" %} +{% end %} diff --git a/src/crystal/system/unix/path.cr b/src/crystal/system/unix/path.cr new file mode 100644 index 000000000000..5a570aa71dc8 --- /dev/null +++ b/src/crystal/system/unix/path.cr @@ -0,0 +1,5 @@ +module Crystal::System::Path + def self.home : String + ENV["HOME"] + end +end diff --git a/src/crystal/system/win32/path.cr b/src/crystal/system/win32/path.cr new file mode 100644 index 000000000000..829b0f0e96bc --- /dev/null +++ b/src/crystal/system/win32/path.cr @@ -0,0 +1,17 @@ +require "c/combaseapi" +require "c/knownfolders" +require "c/shlobj_core" + +module Crystal::System::Path + def self.home : String + if home_path = ENV["USERPROFILE"]? + home_path + elsif LibC.SHGetKnownFolderPath(pointerof(LibC::FOLDERID_Profile), 0, nil, out path_ptr) == 0 + home_path = String.from_utf16(path_ptr)[0] + LibC.CoTaskMemFree(path_ptr) + home_path + else + raise RuntimeError.from_winerror("SHGetKnownFolderPath") + end + end +end diff --git a/src/lib_c/x86_64-windows-msvc/c/combaseapi.cr b/src/lib_c/x86_64-windows-msvc/c/combaseapi.cr new file mode 100644 index 000000000000..431bf666619d --- /dev/null +++ b/src/lib_c/x86_64-windows-msvc/c/combaseapi.cr @@ -0,0 +1,4 @@ +@[Link("ole32")] +lib LibC + fun CoTaskMemFree(pv : Void*) +end diff --git a/src/lib_c/x86_64-windows-msvc/c/knownfolders.cr b/src/lib_c/x86_64-windows-msvc/c/knownfolders.cr new file mode 100644 index 000000000000..04c16573cc76 --- /dev/null +++ b/src/lib_c/x86_64-windows-msvc/c/knownfolders.cr @@ -0,0 +1,5 @@ +require "c/guiddef" + +lib LibC + FOLDERID_Profile = GUID.new(0x5e6c858f, 0x0e22, 0x4760, UInt8.static_array(0x9a, 0xfe, 0xea, 0x33, 0x17, 0xb6, 0x71, 0x73)) +end diff --git a/src/lib_c/x86_64-windows-msvc/c/shlobj_core.cr b/src/lib_c/x86_64-windows-msvc/c/shlobj_core.cr new file mode 100644 index 000000000000..590ba16bb69b --- /dev/null +++ b/src/lib_c/x86_64-windows-msvc/c/shlobj_core.cr @@ -0,0 +1,6 @@ +require "c/guiddef" + +@[Link("shell32")] +lib LibC + fun SHGetKnownFolderPath(rfid : GUID*, dwFlags : DWORD, hToken : HANDLE, ppszPath : LPWSTR*) : DWORD +end diff --git a/src/path.cr b/src/path.cr index 599757a149f5..5eea25e4ef22 100644 --- a/src/path.cr +++ b/src/path.cr @@ -1,3 +1,5 @@ +require "crystal/system/path" + # A `Path` represents a filesystem path and allows path-handling operations # such as querying its components as well as semantic manipulations. # @@ -1359,6 +1361,6 @@ struct Path # Returns the path of the home directory of the current user. def self.home : Path - new ENV["HOME"] + new(Crystal::System::Path.home) end end From cc17e88822dae82cf7b7bfd92b3f9d7bd9a636f4 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 28 Nov 2021 00:47:15 +0800 Subject: [PATCH 2/3] refactor specs --- spec/std/file_spec.cr | 104 ++++-------------------------------------- spec/std/path_spec.cr | 34 +++++--------- spec/support/env.cr | 6 ++- 3 files changed, 25 insertions(+), 119 deletions(-) diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index aaf5adc00cf1..83c4d4ee593a 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -558,102 +558,14 @@ describe "File" do end end - # TODO: these specs are redundant with path_spec.cr - describe "expand_path" do - pending_win32 "converts a pathname to an absolute pathname" do - File.expand_path("").should eq(base) - File.expand_path("a").should eq(File.join([base, "a"])) - File.expand_path("a", nil).should eq(File.join([base, "a"])) - end - - pending_win32 "converts a pathname to an absolute pathname, Ruby-Talk:18512" do - File.expand_path(".a").should eq(File.join([base, ".a"])) - File.expand_path("..a").should eq(File.join([base, "..a"])) - File.expand_path("a../b").should eq(File.join([base, "a../b"])) - end - - pending_win32 "keeps trailing dots on absolute pathname" do - File.expand_path("a.").should eq(File.join([base, "a."])) - File.expand_path("a..").should eq(File.join([base, "a.."])) - end - - pending_win32 "converts a pathname to an absolute pathname, using a complete path" do - File.expand_path("", "#{tmpdir}").should eq("#{tmpdir}") - File.expand_path("a", "#{tmpdir}").should eq("#{tmpdir}/a") - File.expand_path("../a", "#{tmpdir}/xxx").should eq("#{tmpdir}/a") - File.expand_path(".", "#{rootdir}").should eq("#{rootdir}") - end - - pending_win32 "expands a path with multi-byte characters" do - File.expand_path("Ångström").should eq("#{base}/Ångström") - end - - pending_win32 "expands /./dir to /dir" do - File.expand_path("/./dir").should eq("/dir") - end - - pending_win32 "replaces multiple / with a single /" do - File.expand_path("////some/path").should eq("/some/path") - File.expand_path("/some////path").should eq("/some/path") - end - - pending_win32 "expand path with" do - File.expand_path("../../bin", "/tmp/x").should eq("/bin") - File.expand_path("../../bin", "/tmp").should eq("/bin") - File.expand_path("../../bin", "/").should eq("/bin") - File.expand_path("../bin", "tmp/x").should eq(File.join([base, "tmp", "bin"])) - File.expand_path("../bin", "x/../tmp").should eq(File.join([base, "bin"])) - end - - pending_win32 "expand_path for common unix path gives a full path" do - File.expand_path("/tmp/").should eq("/tmp/") - File.expand_path("/tmp/../../../tmp").should eq("/tmp") - File.expand_path("").should eq(base) - File.expand_path("./////").should eq(File.join(base, "")) - File.expand_path(".").should eq(base) - File.expand_path(base).should eq(base) - end - - pending_win32 "converts a pathname to an absolute pathname, using ~ (home) as base" do - File.expand_path("~/", home: true).should eq(File.join(home, "")) - File.expand_path("~/..badfilename", home: true).should eq(File.join(home, "..badfilename")) - File.expand_path("..", home: true).should eq("/#{base.split('/')[0...-1].join('/')}".gsub(%r{\A//}, "/")) - File.expand_path("~/a", "~/b", home: true).should eq(File.join(home, "a")) - File.expand_path("~", home: true).should eq(home) - File.expand_path("~", "/tmp/gumby/ddd", home: true).should eq(home) - File.expand_path("~/a", "/tmp/gumby/ddd", home: true).should eq(File.join([home, "a"])) - end - - pending_win32 "converts a pathname to an absolute pathname, using ~ (home) as base (trailing /)" do - prev_home = home - begin - ENV["HOME"] = File.expand_path(datapath) - File.expand_path("~/", home: true).should eq(File.join(home, "")) - File.expand_path("~/..badfilename", home: true).should eq(File.join(home, "..badfilename")) - File.expand_path("..", home: true).should eq("/#{base.split('/')[0...-1].join('/')}".gsub(%r{\A//}, "/")) - File.expand_path("~/a", "~/b", home: true).should eq(File.join(home, "a")) - File.expand_path("~", home: true).should eq(home) - File.expand_path("~", "/tmp/gumby/ddd", home: true).should eq(home) - File.expand_path("~/a", "/tmp/gumby/ddd", home: true).should eq(File.join([home, "a"])) - ensure - ENV["HOME"] = prev_home - end - end - - pending_win32 "converts a pathname to an absolute pathname, using ~ (home) as base (HOME=/)" do - prev_home = home - begin - ENV["HOME"] = "/" - File.expand_path("~/", home: true).should eq(home) - File.expand_path("~/..badfilename", home: true).should eq(File.join(home, "..badfilename")) - File.expand_path("..", home: true).should eq("/#{base.split('/')[0...-1].join('/')}".gsub(/\A\/\//, "/")) - File.expand_path("~/a", "~/b", home: true).should eq(File.join(home, "a")) - File.expand_path("~", home: true).should eq(home) - File.expand_path("~", "/tmp/gumby/ddd", home: true).should eq(home) - File.expand_path("~/a", "/tmp/gumby/ddd", home: true).should eq(File.join([home, "a"])) - ensure - ENV["HOME"] = prev_home - end + describe ".expand_path" do + it "converts a pathname to an absolute pathname" do + File.expand_path("a/b").should eq(Path.new("a/b").expand(Dir.current).to_s) + File.expand_path("a/b", "c/d").should eq(Path.new("a/b").expand("c/d").to_s) + File.expand_path("~/b", home: "c/d").should eq(Path.new("~/b").expand(Dir.current, home: "c/d").to_s) + File.expand_path("~/b", "c/d", home: false).should eq(Path.new("~/b").expand("c/d", home: false).to_s) + + File.expand_path(Path.new("a/b")).should eq(Path.new("a/b").expand(Dir.current).to_s) end end diff --git a/spec/std/path_spec.cr b/spec/std/path_spec.cr index b02b46e56ff6..ab4aff8b69cd 100644 --- a/spec/std/path_spec.cr +++ b/spec/std/path_spec.cr @@ -2,6 +2,7 @@ require "spec" require "./spec_helper" require "../support/env" +private HOME_ENV_KEY = {% if flag?(:win32) %} "USERPROFILE" {% else %} "HOME" {% end %} private BASE_POSIX = "/default/base" private BASE_WINDOWS = "\\default\\base" private HOME_WINDOWS = "C:\\Users\\Crystal" @@ -13,15 +14,9 @@ end private def it_expands_path(path, posix, windows = posix, *, base = nil, env_home = nil, expand_base = false, home = false, file = __FILE__, line = __LINE__) assert_paths(path, posix, windows, %((base: "#{base}")), file, line) do |path| - prev_home = ENV["HOME"]? - - begin - ENV["HOME"] = env_home || (path.windows? ? HOME_WINDOWS : HOME_POSIX) - + with_env({HOME_ENV_KEY => env_home || (path.windows? ? HOME_WINDOWS : HOME_POSIX)}) do base_arg = base || (path.windows? ? BASE_WINDOWS : BASE_POSIX) path.expand(base_arg.not_nil!, expand_base: !!expand_base, home: home) - ensure - ENV["HOME"] = prev_home end end end @@ -585,7 +580,7 @@ describe Path do describe "converts a pathname to an absolute pathname" do it_expands_path("", BASE_POSIX, BASE_WINDOWS) it_expands_path("a", {BASE_POSIX, "a"}, {BASE_WINDOWS, "a"}) - it_expands_path("a", {BASE_POSIX, "a"}, {BASE_WINDOWS, "a"}) + it_expands_path("a", {BASE_POSIX, "a"}, {BASE_WINDOWS, "a"}, base: nil) end describe "converts a pathname to an absolute pathname, Ruby-Talk:18512" do @@ -626,7 +621,7 @@ describe Path do it_expands_path("/some////path", "/some/path", "\\some\\path") end - describe "expand path with" do + describe "expand path with .." do it_expands_path("../../bin", "/bin", "\\bin", base: "/tmp/x") it_expands_path("../../bin", "/bin", "\\bin", base: "/tmp") it_expands_path("../../bin", "/bin", "\\bin", base: "/") @@ -892,24 +887,19 @@ describe Path do end describe ".home" do - {% if flag?(:win32) %} - it "returns %USERPROFILE% if set" do - with_env("USERPROFILE": "foo/bar") do - Path.home.should eq(Path.new("foo/bar")) - end + it "uses home from environment variable if set" do + with_env({HOME_ENV_KEY => "foo/bar"}) do + Path.home.should eq(Path.new("foo/bar")) end + end - it "doesn't raise if %USERPROFILE% is missing" do - with_env("USERPROFILE": nil) do + # TODO: check that this is the home of the current user + {% if flag?(:win32) %} + it "doesn't raise if environment variable is missing" do + with_env({HOME_ENV_KEY => nil}) do Path.home end end - {% else %} - it "returns $HOME if set" do - with_env("HOME": "foo/bar") do - Path.home.should eq(Path.new("foo/bar")) - end - end {% end %} end end diff --git a/spec/support/env.cr b/spec/support/env.cr index 9e99f903c126..66f1549c5bea 100644 --- a/spec/support/env.cr +++ b/spec/support/env.cr @@ -1,4 +1,4 @@ -def with_env(**values) +def with_env(values : Hash) old_values = {} of String => String? begin values.each do |key, value| @@ -14,3 +14,7 @@ def with_env(**values) end end end + +def with_env(**values) + with_env(values.to_h) { yield } +end From ba6ecd4b2f62641e5859c6bafe262de57e010fa7 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 28 Nov 2021 22:25:29 +0800 Subject: [PATCH 3/3] fixup --- spec/std/file_spec.cr | 1 + src/crystal/system/win32/path.cr | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index 83c4d4ee593a..627c554e7b17 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -558,6 +558,7 @@ describe "File" do end end + # There are more detailled specs for `Path#expand` in path_spec.cr describe ".expand_path" do it "converts a pathname to an absolute pathname" do File.expand_path("a/b").should eq(Path.new("a/b").expand(Dir.current).to_s) diff --git a/src/crystal/system/win32/path.cr b/src/crystal/system/win32/path.cr index 829b0f0e96bc..e6f23789f931 100644 --- a/src/crystal/system/win32/path.cr +++ b/src/crystal/system/win32/path.cr @@ -7,7 +7,7 @@ module Crystal::System::Path if home_path = ENV["USERPROFILE"]? home_path elsif LibC.SHGetKnownFolderPath(pointerof(LibC::FOLDERID_Profile), 0, nil, out path_ptr) == 0 - home_path = String.from_utf16(path_ptr)[0] + home_path, _ = String.from_utf16(path_ptr) LibC.CoTaskMemFree(path_ptr) home_path else