Add temporary directory creation on Dir class #2911

marceloboeira · 2016-06-25T06:06:51Z

Usage:

Without prefix:

puts Dir.mktmpdir
# => /tmp/c.a39bF4

With prefix:

puts Dir.mktmpdir("foo")
# => /tmp/foo.a39bF4

The solution uses the LibC.mkdtemp bind.
The mkdtemp function creates a directory with a unique name. If it
succeeds, it overwrites template with the name of the directory, and
returns template. As with mktemp and mkstemp, template should be a
string ending with ‘XXXXXX’.

Reference:

http://www.gnu.org/software/libc/manual/html_node/Temporary-Files.html

Dir class assumes the responsability of resolving the system tmpdir from the Tempfile class. Reference: - http://rxr.whitequark.org/mri/source/lib/tmpdir.rb

marceloboeira · 2016-06-25T15:59:01Z

@jhass for some reason the tests are failing only on linux builds, could you take a look to check what I'm doing wrong?

jhass · 2016-06-25T16:48:51Z

spec/std/dir_spec.cr

+      original_tmpdir = ENV["TMPDIR"]?
+      ENV["TMPDIR"] = "/my/tmp"
+      Dir.tmpdir.should eq("/my/tmp")
+      ENV["TMPDIR"] = original_tmpdir if original_tmpdir


If TMPDIR wasn't set, this keeps it set at "/my/tmp".

Thanks, I'll fix that!

marceloboeira · 2016-06-25T18:19:10Z

I was trying to create a better interface to work with temporary directories, something like:

Dir.mktmpdir do |dir|
....
end

On which the tmp dir only exists within the block lifecycle. However for that I'll need a Dir.rm_rf function, which I'll implement later on another pull request.

asterite · 2016-06-25T18:56:20Z

There's FileUtils.rm_r

marceloboeira · 2016-06-25T19:06:44Z

@asterite thanks, I didn't remembered to check FileUtils.

marceloboeira · 2016-06-25T21:52:02Z

@jhass @asterite done, can you check it?

asterite · 2016-06-25T22:10:14Z

Thanks! Before merging this, I would try to think of a different name, mktmpdir is pretty cryptic and reminiscent of libc.

The mkdtemp function creates a directory with a unique name. If it succeeds, it overwrites template with the name of the directory, and returns template. As with mktemp and mkstemp, template should be a string ending with ‘XXXXXX’. The directory is created using mode 0700 References: - http://www.gnu.org/software/libc/manual/html_node/Temporary-Files.html - http://ruby-doc.org/stdlib-2.0.0/libdoc/tmpdir/rdoc/Dir.html

marceloboeira · 2016-06-25T22:25:09Z

@asterite I was trying to keep Ruby's syntax, I would happily take your name suggestions.

luislavena · 2016-06-28T08:32:22Z

I'm not sure if is good that we add FileUtils as dependency for Dir, which is part of prelude and means that FileUtils will be parsed, analyzed and potentially included as part of every compiled program.

Perhaps this helper method can be treated in similar way as Tempfile? (ala: a separate class).

Just a thought.

marceloboeira · 2016-06-28T12:11:31Z

@luislavena you are absolutely correct, I was actually wondering why the paths were being concatenated that way (a + b...) but considering your comment, which haven't crossed my mind until now, makes sense.

However I think we should find a more elegant way of doing this.

Either way, for now I will fix this.

asterite · 2016-06-28T13:02:40Z

Ideally we should have a way to do rm_r in Dir, with a better name, and have FileUilts.rm_r use that method, like in my comment here.

marceloboeira · 2016-07-01T23:49:33Z

@asterite can I refactor this and move rm_r to Dir ?

As discussed on #2911

asterite · 2016-07-02T00:02:30Z

@marceloboeira Ideally I'd like this method to be in Dir and in FileUtils. FileUtils would have methods names similar to bash functions, but Dir would have less cryptic names.

Problem is, I don't know what a good name for Dir is in this case.

ysbaddaden · 2016-07-02T10:02:45Z

What about:

module Dir
  def self.delete(path, *, recursive= false)
  end
end

marceloboeira · 2016-07-06T11:23:35Z

Could you accept this pull request and we rename the methods later? I really would like this feature for some projects and I would not like to "monkey patch". Also I think this is something important for several projects, not only mine.

asterite · 2016-07-06T13:39:47Z

@marceloboeira There's nothing wrong with "monkey patching" until this gets merged with time

grosser · 2016-10-16T18:53:53Z

so the only thing missing here is rename rm_r to delete ... or what is ?

... would love to use this feature :)

marceloboeira · 2016-11-01T14:37:12Z

@grosser not sure anymore. I need this feature, I believe I will create a temp shard to use it while I wait for it to be merged.

TL;DR - This is a monkey-patch to create temporary directories with Crystal standard library. The PR is open, but still solving some issues. crystal-lang/crystal#2911 The mkdtemp function creates a directory with a unique name. If it succeeds, it overwrites template with the name of the directory, and returns template. As with mktemp and mkstemp, template should be a string ending with ‘XXXXXX’. The directory is created using mode 0700 References: - http://www.gnu.org/software/libc/manual/html_node/Temporary-Files.html - http://ruby-doc.org/stdlib-2.0.0/libdoc/tmpdir/rdoc/Dir.html

sdogruyol · 2016-11-05T20:06:50Z

This is pretty useful, especially for writing tests 👍

As discussed on crystal-lang#2911

bramswenson · 2017-03-02T04:54:18Z

I've submitted a PR that is simply a rebase of this one to master. #4096

marceloboeira · 2017-03-02T09:13:31Z

@bramswenson thx!

Move system tmpdir logic to Dir class

6205580

Dir class assumes the responsability of resolving the system tmpdir from the Tempfile class. Reference: - http://rxr.whitequark.org/mri/source/lib/tmpdir.rb

marceloboeira changed the title ~~Add temporary directory file creation on Dir class~~ Add temporary directory creation on Dir class Jun 25, 2016

jhass added the topic:stdlib label Jun 25, 2016

jhass reviewed Jun 25, 2016
View reviewed changes

Move rm_r to Dir

93b3440

As discussed on #2911

Remove FileUtils from Dir

d58b559

bramswenson pushed a commit to bramswenson/crystal that referenced this pull request Mar 2, 2017

Move rm_r to Dir

717b0f9

As discussed on crystal-lang#2911

bramswenson mentioned this pull request Mar 2, 2017

Feature/mktmpdir #4096

Closed

marceloboeira closed this Mar 2, 2017

straight-shoota mentioned this pull request Nov 17, 2017

FileUtils: refactoring #5231

Open

marceloboeira deleted the feature/mktmpdir branch September 14, 2018 08:17

apainintheneck mentioned this pull request Jan 16, 2024

Add a method to create and clean up a temporary directory to the standard library #14244

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add temporary directory creation on Dir class #2911

Add temporary directory creation on Dir class #2911

marceloboeira commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

jhass Jun 25, 2016 •

edited

marceloboeira Jun 25, 2016

marceloboeira commented Jun 25, 2016 •

edited

asterite commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

asterite commented Jun 25, 2016

marceloboeira commented Jun 25, 2016 •

edited

luislavena commented Jun 28, 2016 •

edited

marceloboeira commented Jun 28, 2016 •

edited

asterite commented Jun 28, 2016

marceloboeira commented Jul 1, 2016

asterite commented Jul 2, 2016

ysbaddaden commented Jul 2, 2016

marceloboeira commented Jul 6, 2016

asterite commented Jul 6, 2016

grosser commented Oct 16, 2016

marceloboeira commented Nov 1, 2016

sdogruyol commented Nov 5, 2016

bramswenson commented Mar 2, 2017 •

edited

marceloboeira commented Mar 2, 2017

Add temporary directory creation on Dir class #2911

Add temporary directory creation on Dir class #2911

Conversation

marceloboeira commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

jhass Jun 25, 2016 • edited

Choose a reason for hiding this comment

marceloboeira Jun 25, 2016

Choose a reason for hiding this comment

marceloboeira commented Jun 25, 2016 • edited

asterite commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

marceloboeira commented Jun 25, 2016

asterite commented Jun 25, 2016

marceloboeira commented Jun 25, 2016 • edited

luislavena commented Jun 28, 2016 • edited

marceloboeira commented Jun 28, 2016 • edited

asterite commented Jun 28, 2016

marceloboeira commented Jul 1, 2016

asterite commented Jul 2, 2016

ysbaddaden commented Jul 2, 2016

marceloboeira commented Jul 6, 2016

asterite commented Jul 6, 2016

grosser commented Oct 16, 2016

marceloboeira commented Nov 1, 2016

sdogruyol commented Nov 5, 2016

bramswenson commented Mar 2, 2017 • edited

marceloboeira commented Mar 2, 2017

jhass Jun 25, 2016 •

edited

marceloboeira commented Jun 25, 2016 •

edited

marceloboeira commented Jun 25, 2016 •

edited

luislavena commented Jun 28, 2016 •

edited

marceloboeira commented Jun 28, 2016 •

edited

bramswenson commented Mar 2, 2017 •

edited