From e15b7af2fb54795dd9fb702ff830ff9d97af0b4a Mon Sep 17 00:00:00 2001 From: Holden Omans Date: Mon, 6 Aug 2018 10:33:12 -0400 Subject: [PATCH 1/4] Removed tempfile class into file (#5939) --- spec/std/dir_spec.cr | 18 ++++++ spec/std/file_spec.cr | 89 ++++++++++++++++++++++++++++++ spec/std/tempfile_spec.cr | 105 ----------------------------------- src/dir.cr | 26 +++++++++ src/file.cr | 86 +++++++++++++++++++++++++++++ src/tempfile.cr | 113 -------------------------------------- 6 files changed, 219 insertions(+), 218 deletions(-) delete mode 100644 spec/std/tempfile_spec.cr delete mode 100644 src/tempfile.cr diff --git a/spec/std/dir_spec.cr b/spec/std/dir_spec.cr index a1527375b5c3..da7b38230a0f 100644 --- a/spec/std/dir_spec.cr +++ b/spec/std/dir_spec.cr @@ -437,4 +437,22 @@ describe "Dir" do Dir.rmdir("foo\0bar") end end + + describe "tempdir" do + it "returns default directory for tempfiles" do + old_tmpdir = ENV["TMPDIR"]? + ENV.delete("TMPDIR") + Dir.tempdir.should eq("/tmp") + ensure + ENV["TMPDIR"] = old_tmpdir if old_tmpdir + end + + it "returns configure directory for tempfiles" do + old_tmpdir = ENV["TMPDIR"]? + ENV["TMPDIR"] = "/my/tmp" + Dir.tempdir.should eq("/my/tmp") + ensure + ENV["TMPDIR"] = old_tmpdir if old_tmpdir + end + end end diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index b9122895b42e..0f166f3cb11b 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -1080,6 +1080,93 @@ describe "File" do end {% end %} + describe "tempname" do + it "creates a path without creating the file" do + path = File.tempname + + File.exists?(path).should be_false + end + + it "has the given extension" do + path = File.tempname ".sock" + + File.extname(path).should eq(".sock") + end + end + + describe "tempfile" do + it "creates and writes" do + tempfile = File.tempfile "foo" + tempfile.print "Hello!" + tempfile.close + + File.exists?(tempfile.path).should be_true + File.read(tempfile.path).should eq("Hello!") + ensure + File.delete(tempfile.path) if tempfile + end + + it "has given extension if passed to constructor" do + tempfile = File.tempfile "foo", ".pdf" + File.extname(tempfile.path).should eq(".pdf") + end + + it "creates and deletes" do + tempfile = File.tempfile "foo" + tempfile.close + tempfile.delete + + File.exists?(tempfile.path).should be_false + ensure + File.delete(tempfile.path) if tempfile && File.exists?(tempfile.path) + end + + it "doesn't delete on open with block" do + tempfile = File.tempfile("foo") do |f| + f.print "Hello!" + end + File.exists?(tempfile.path).should be_true + ensure + File.delete(tempfile.path) if tempfile + end + + it "has given extension if passed to open" do + tempfile = File.tempfile("foo", ".pdf") { |f| } + File.extname(tempfile.path).should eq(".pdf") + ensure + File.delete(tempfile.path) if tempfile + end + + it "creates and writes with TMPDIR environment variable" do + old_tmpdir = ENV["TMPDIR"]? + ENV["TMPDIR"] = "/tmp" + + tempfile = File.tempfile "foo" + tempfile.print "Hello!" + tempfile.close + + File.exists?(tempfile.path).should be_true + File.read(tempfile.path).should eq("Hello!") + ensure + ENV["TMPDIR"] = old_tmpdir if old_tmpdir + File.delete(tempfile.path) if tempfile + end + + it "is seekable" do + tempfile = File.tempfile "foo" + tempfile.puts "Hello!" + tempfile.seek(0, IO::Seek::Set) + tempfile.tell.should eq(0) + tempfile.pos.should eq(0) + tempfile.gets(chomp: false).should eq("Hello!\n") + tempfile.pos = 0 + tempfile.gets(chomp: false).should eq("Hello!\n") + tempfile.close + ensure + File.delete(tempfile.path) if tempfile + end + end + describe "closed stream" do it "raises if writing on a closed stream" do io = File.open(datapath("test_file.txt"), "r") @@ -1275,6 +1362,8 @@ describe "File" do File.match?("ab{{c,d}ef,}", "abcef").should be_true File.match?("ab{{c,d}ef,}", "abdef").should be_true end + + end describe File::Permissions do diff --git a/spec/std/tempfile_spec.cr b/spec/std/tempfile_spec.cr deleted file mode 100644 index 7f01d835015a..000000000000 --- a/spec/std/tempfile_spec.cr +++ /dev/null @@ -1,105 +0,0 @@ -require "spec" -require "tempfile" - -describe Tempfile do - describe "tempname" do - it "creates a path without creating the file" do - path = Tempfile.tempname - - File.exists?(path).should be_false - end - - it "has the given extension" do - path = Tempfile.tempname ".sock" - - File.extname(path).should eq(".sock") - end - end - - it "creates and writes" do - tempfile = Tempfile.new "foo" - tempfile.print "Hello!" - tempfile.close - - File.exists?(tempfile.path).should be_true - File.read(tempfile.path).should eq("Hello!") - ensure - File.delete(tempfile.path) if tempfile - end - - it "has given extension if passed to constructor" do - tempfile = Tempfile.new "foo", ".pdf" - File.extname(tempfile.path).should eq(".pdf") - end - - it "creates and deletes" do - tempfile = Tempfile.new "foo" - tempfile.close - tempfile.delete - - File.exists?(tempfile.path).should be_false - ensure - File.delete(tempfile.path) if tempfile && File.exists?(tempfile.path) - end - - it "doesn't delete on open with block" do - tempfile = Tempfile.open("foo") do |f| - f.print "Hello!" - end - File.exists?(tempfile.path).should be_true - ensure - File.delete(tempfile.path) if tempfile - end - - it "has given extension if passed to open" do - tempfile = Tempfile.open("foo", ".pdf") { |f| } - File.extname(tempfile.path).should eq(".pdf") - ensure - File.delete(tempfile.path) if tempfile - end - - it "creates and writes with TMPDIR environment variable" do - old_tmpdir = ENV["TMPDIR"]? - ENV["TMPDIR"] = "/tmp" - - tempfile = Tempfile.new "foo" - tempfile.print "Hello!" - tempfile.close - - File.exists?(tempfile.path).should be_true - File.read(tempfile.path).should eq("Hello!") - ensure - ENV["TMPDIR"] = old_tmpdir if old_tmpdir - File.delete(tempfile.path) if tempfile - end - - it "is seekable" do - tempfile = Tempfile.new "foo" - tempfile.puts "Hello!" - tempfile.seek(0, IO::Seek::Set) - tempfile.tell.should eq(0) - tempfile.pos.should eq(0) - tempfile.gets(chomp: false).should eq("Hello!\n") - tempfile.pos = 0 - tempfile.gets(chomp: false).should eq("Hello!\n") - tempfile.close - ensure - File.delete(tempfile.path) if tempfile - end - - it "returns default directory for tempfiles" do - old_tmpdir = ENV["TMPDIR"]? - ENV.delete("TMPDIR") - Tempfile.dirname.should eq("/tmp") - ensure - ENV["TMPDIR"] = old_tmpdir if old_tmpdir - end - - it "returns configure directory for tempfiles" do - old_tmpdir = ENV["TMPDIR"]? - ENV["TMPDIR"] = "/my/tmp" - Tempfile.dirname.should eq("/my/tmp") - ensure - ENV["TMPDIR"] = old_tmpdir if old_tmpdir - end -end diff --git a/src/dir.cr b/src/dir.cr index 9ef2646c5f76..3d8045a5c5fe 100644 --- a/src/dir.cr +++ b/src/dir.cr @@ -35,6 +35,32 @@ class Dir end end + # Returns the tmp dir for system. + # + # ``` + # Dir.tempdir # => "/tmp" + # ``` + def self.tempdir : String + Crystal::System::File.tempdir + end + + # Returns a fully-qualified path to a temporary directory without actually + # creating the directory. + # + # ``` + # Dir.tempname # => "/tmp/20171206-1234-449386" + # ``` + def self.tempname + time = Time.now.to_s("%Y%m%d") + rand = Random.rand(0x100000000).to_s(36) + {% if flag?(:win32) %} + # TODO: Remove this once Process is implemented + File.join(dirname, "#{time}-#{rand}") + {% else %} + File.join(dirname, "#{time}-#{Process.pid}-#{rand}") + {% end %} + end + # Calls the block once for each entry in this directory, # passing the filename of each entry as a parameter to the block. # diff --git a/src/file.cr b/src/file.cr index b4844c195d54..b0084e0c34dc 100644 --- a/src/file.cr +++ b/src/file.cr @@ -374,6 +374,92 @@ class File < IO::FileDescriptor end end + # The `tempfile` method is for managing temporary files. + # + # ``` + # tempfile = File.tempfile("foo") + # # or + # tempfile = File.tempfile("foo") do |file| + # file.print("foobar") + # end + # + # File.size(tempfile.path) # => 6 + # File.info(tempfile.path).modification_time # => 2015-10-20 13:11:12 UTC + # File.exists?(tempfile.path) # => true + # File.read_lines(tempfile.path) # => ["foobar"] + # ``` + # + # Files created from this method are stored in a directory that handles + # temporary files. + # + # ``` + # File.tempfile("foo").path # => "/tmp/foo.ulBCPS" + # ``` + # + # Also, it is encouraged to delete a tempfile after using it, which + # ensures they are not left behind in your filesystem until garbage collected. + # + # ``` + # tempfile = File.tempfile("foo") + # tempfile.delete + # ``` + # + # The optional `extension` argument can be used to force the resulting filename + # to end with the given extension. + # + # ``` + # File.tempfile("foo", ".png").path # => "/tmp/foo.ulBCPS.png" + # ``` + # + # *encoding* and *invalid* are passed to `IO#set_encoding`. + def self.tempfile(name, extension = nil, encoding = nil, invalid = nil) + fileno, path = Crystal::System::File.mktemp(name, extension) + File.new(path, fileno, blocking: true, encoding: encoding, invalid: invalid) + end + + # Creates a file with *filename* and *extension*, and yields it to the given + # block. It is closed and returned at the end of this method call. + # + # ``` + # tempfile = File.tempfile("foo") do |file| + # file.print("bar") + # end + # File.read(tempfile.path) # => "bar" + # ``` + def self.tempfile(filename, extension = nil) + tempfile = File.tempfile(filename, extension) + begin + yield tempfile + ensure + tempfile.close + end + tempfile + end + + # Returns a fully-qualified path to a temporary file without actually + # creating the file. + # + # ``` + # File.tempname # => "/tmp/20171206-1234-449386" + # ``` + # + # The optional `extension` argument can be used to make the resulting + # filename to end with the given extension. + # + # ``` + # File.tempname(".sock") # => "/tmp/20171206-1234-449386.sock" + # ``` + def self.tempname(extension = nil) + time = Time.now.to_s("%Y%m%d") + rand = Random.rand(0x100000000).to_s(36) + {% if flag?(:win32) %} + # TODO: Remove this once Process is implemented + File.join(dirname, "#{time}-#{rand}#{extension}") + {% else %} + File.join(dirname, "#{time}-#{Process.pid}-#{rand}#{extension}") + {% end %} + end + class BadPatternError < Exception end diff --git a/src/tempfile.cr b/src/tempfile.cr deleted file mode 100644 index cd2926db6b43..000000000000 --- a/src/tempfile.cr +++ /dev/null @@ -1,113 +0,0 @@ -require "c/stdlib" - -# The `Tempfile` class is for managing temporary files. -# Every tempfile is operated as a `File`, including -# initializing, reading and writing. -# -# ``` -# tempfile = Tempfile.new("foo") -# # or -# tempfile = Tempfile.open("foo") do |file| -# file.print("foobar") -# end -# -# File.size(tempfile.path) # => 6 -# File.info(tempfile.path).modification_time # => 2015-10-20 13:11:12 UTC -# File.exists?(tempfile.path) # => true -# File.read_lines(tempfile.path) # => ["foobar"] -# ``` -# -# Files created from this class are stored in a directory that handles -# temporary files. -# -# ``` -# Tempfile.new("foo").path # => "/tmp/foo.ulBCPS" -# ``` -# -# Also, it is encouraged to delete a tempfile after using it, which -# ensures they are not left behind in your filesystem until garbage collected. -# -# ``` -# tempfile = Tempfile.new("foo") -# tempfile.delete -# ``` -# -# The optional `extension` argument can be used to force the resulting filename -# to end with the given extension. -# -# ``` -# Tempfile.new("foo", ".png").path # => "/tmp/foo.ulBCPS.png" -# ``` -class Tempfile < File - # Creates a `Tempfile` with the given filename and extension. - # - # *encoding* and *invalid* are passed to `IO#set_encoding`. - def initialize(name, extension = nil, encoding = nil, invalid = nil) - fileno, path = Crystal::System::File.mktemp(name, extension) - super(path, fileno, blocking: true, encoding: encoding, invalid: invalid) - end - - # Retrieves the full path of a this tempfile. - # - # ``` - # Tempfile.new("foo").path # => "/tmp/foo.ulBCPS" - # ``` - getter path : String - - # Returns a fully-qualified path to a temporary file without actually - # creating the file. - # - # ``` - # Tempfile.tempname # => "/tmp/20171206-1234-449386" - # ``` - # - # The optional `extension` argument can be used to make the resulting - # filename to end with the given extension. - # - # ``` - # Tempfile.tempname(".sock") # => "/tmp/20171206-1234-449386.sock" - # ``` - def self.tempname(extension = nil) - time = Time.now.to_s("%Y%m%d") - rand = Random.rand(0x100000000).to_s(36) - {% if flag?(:win32) %} - # TODO: Remove this once Process is implemented - File.join(dirname, "#{time}-#{rand}#{extension}") - {% else %} - File.join(dirname, "#{time}-#{Process.pid}-#{rand}#{extension}") - {% end %} - end - - # Creates a file with *filename* and *extension*, and yields it to the given - # block. It is closed and returned at the end of this method call. - # - # ``` - # tempfile = Tempfile.open("foo") do |file| - # file.print("bar") - # end - # File.read(tempfile.path) # => "bar" - # ``` - def self.open(filename, extension = nil) - tempfile = Tempfile.new(filename, extension) - begin - yield tempfile - ensure - tempfile.close - end - tempfile - end - - # Returns the tmp dir used for tempfile. - # - # ``` - # Tempfile.dirname # => "/tmp" - # ``` - def self.dirname : String - Crystal::System::File.tempdir - end - - # Deletes this tempfile. - def delete - File.delete(@path) - end -end From e6926a65856beb0ac7064302347c8011ca4edad6 Mon Sep 17 00:00:00 2001 From: Holden Omans Date: Mon, 6 Aug 2018 10:39:53 -0400 Subject: [PATCH 2/4] remove depends on Tempfile (#5939) --- spec/compiler/codegen/private_spec.cr | 5 ++--- spec/spec_helper.cr | 4 ++-- spec/std/http/server/server_spec.cr | 5 ++--- spec/support/tempfile.cr | 7 +++---- src/compiler/crystal/tools/playground/server.cr | 1 - 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/spec/compiler/codegen/private_spec.cr b/spec/compiler/codegen/private_spec.cr index eee389178232..2bb895a35fcb 100644 --- a/spec/compiler/codegen/private_spec.cr +++ b/spec/compiler/codegen/private_spec.cr @@ -1,5 +1,4 @@ require "../../spec_helper" -require "tempfile" describe "Codegen: private" do it "codegens private def in same file" do @@ -15,7 +14,7 @@ describe "Codegen: private" do ] compiler.prelude = "empty" - tempfile = Tempfile.new("crystal-spec-output") + tempfile = File.tempfile("crystal-spec-output") output_filename = tempfile.path tempfile.close @@ -42,7 +41,7 @@ describe "Codegen: private" do ] compiler.prelude = "empty" - tempfile = Tempfile.new("crystal-spec-output") + tempfile = File.tempfile("crystal-spec-output") output_filename = tempfile.path tempfile.close diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 5ea235ae0cc6..5c10b5345b9c 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -186,13 +186,13 @@ def run(code, filename = nil, inject_primitives = true, debug = Crystal::Debug:: end def build_and_run(code) - code_file = Tempfile.new("build_and_run_code") + code_file = File.tempfile("build_and_run_code") code_file.close # write code to the temp file File.write(code_file.path, code) - binary_file = Tempfile.new("build_and_run_bin") + binary_file = File.tempfile("build_and_run_bin") binary_file.close `bin/crystal build #{code_file.path.inspect} -o #{binary_file.path.inspect}` diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index ffd157725db3..60a1dee2937c 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -1,7 +1,6 @@ require "spec" require "http/server" require "http/client/response" -require "tempfile" private class RaiseErrno < IO def initialize(@value : Int32) @@ -326,8 +325,8 @@ module HTTP {% if flag?(:unix) %} describe "#bind_unix" do it "binds to different unix sockets" do - path1 = Tempfile.tempname - path2 = Tempfile.tempname + path1 = File.tempname + path2 = File.tempname begin server = Server.new do |context| diff --git a/spec/support/tempfile.cr b/spec/support/tempfile.cr index bc465dd85dff..4dd15f511c79 100644 --- a/spec/support/tempfile.cr +++ b/spec/support/tempfile.cr @@ -1,10 +1,9 @@ -require "tempfile" require "file_utils" {% if flag?(:win32) %} - SPEC_TEMPFILE_PATH = File.join(Tempfile.dirname, "cr-spec-#{Random.new.hex(4)}").gsub("C:\\", '/').gsub('\\', '/') + SPEC_TEMPFILE_PATH = File.join(Dir.tempdir, "cr-spec-#{Random.new.hex(4)}").gsub("C:\\", '/').gsub('\\', '/') {% else %} - SPEC_TEMPFILE_PATH = File.join(Tempfile.dirname, "cr-spec-#{Random.new.hex(4)}") + SPEC_TEMPFILE_PATH = File.join(Dir.tempdir, "cr-spec-#{Random.new.hex(4)}") {% end %} SPEC_TEMPFILE_CLEANUP = ENV["SPEC_TEMPFILE_CLEANUP"]? != "0" @@ -17,7 +16,7 @@ SPEC_TEMPFILE_CLEANUP = ENV["SPEC_TEMPFILE_CLEANUP"]? != "0" # The constructed path is yielded to the block and cleaned up afterwards. # # Paths should still be uniquely chosen inside a spec file. This helper -# ensures they're placed in the temporary location (`Tempfile.dirname`), +# ensures they're placed in the temporary location (`Dir.tempdir`), # avoids name clashes between parallel spec runs and cleans up afterwards. # # The unique directory for the spec run is removed `at_exit`. diff --git a/src/compiler/crystal/tools/playground/server.cr b/src/compiler/crystal/tools/playground/server.cr index 946daa6dfcbe..460c1ec19ed5 100644 --- a/src/compiler/crystal/tools/playground/server.cr +++ b/src/compiler/crystal/tools/playground/server.cr @@ -1,5 +1,4 @@ require "http/server" -require "tempfile" require "logger" require "ecr/macros" require "markdown" From 7be6d92ef3e60b1bea5f0440f8d131732fd5a82a Mon Sep 17 00:00:00 2001 From: Holden Omans Date: Mon, 6 Aug 2018 10:43:59 -0400 Subject: [PATCH 3/4] Fixes to spec tests --- spec/std/http/server/server_spec.cr | 1 + src/file.cr | 4 ++-- src/http/formdata.cr | 3 +-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index 60a1dee2937c..4e15da9db75a 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -1,6 +1,7 @@ require "spec" require "http/server" require "http/client/response" +require "../../../support/ssl" private class RaiseErrno < IO def initialize(@value : Int32) diff --git a/src/file.cr b/src/file.cr index b0084e0c34dc..30e2af9a36b5 100644 --- a/src/file.cr +++ b/src/file.cr @@ -454,9 +454,9 @@ class File < IO::FileDescriptor rand = Random.rand(0x100000000).to_s(36) {% if flag?(:win32) %} # TODO: Remove this once Process is implemented - File.join(dirname, "#{time}-#{rand}#{extension}") + File.join(Dir.tempdir, "#{time}-#{rand}#{extension}") {% else %} - File.join(dirname, "#{time}-#{Process.pid}-#{rand}#{extension}") + File.join(Dir.tempdir, "#{time}-#{Process.pid}-#{rand}#{extension}") {% end %} end diff --git a/src/http/formdata.cr b/src/http/formdata.cr index fafdc35315a9..080b339a7e7a 100644 --- a/src/http/formdata.cr +++ b/src/http/formdata.cr @@ -10,7 +10,6 @@ require "./formdata/**" # # ``` # require "http" -# require "tempfile" # # server = HTTP::Server.new do |context| # name = nil @@ -20,7 +19,7 @@ require "./formdata/**" # when "name" # name = part.body.gets_to_end # when "file" -# file = Tempfile.open("upload") do |file| +# file = File.tempfile("upload") do |file| # IO.copy(part.body, file) # end # end From bfafff49bb720b298eb4d140a361a82e5b222a80 Mon Sep 17 00:00:00 2001 From: Holden Omans Date: Mon, 6 Aug 2018 10:46:57 -0400 Subject: [PATCH 4/4] removed support ssl reference in server spec --- spec/std/http/server/server_spec.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index 4e15da9db75a..60a1dee2937c 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -1,7 +1,6 @@ require "spec" require "http/server" require "http/client/response" -require "../../../support/ssl" private class RaiseErrno < IO def initialize(@value : Int32)