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

Windows show-stopper: File.unlink doesn't work like it does on POSIX systems #66

Merged
merged 1 commit into from
Dec 8, 2011

Conversation

GregMefford
Copy link
Contributor

On Windows, File#unlink does not delete the file like it does on POSIX, so ZipFile fails to create the file, trying to validate the (empty) tempfile. File.close! closes and then unlinks it, which is fine since we aren't writing to it directly anyway.
On a more abstract level, I'm not sure why we are creating a Tempfile to begin with. Couldn't we just go ahead and write to the file we're trying to create, or at least to a "filename.temp.jar" in the same directory instead of in the temp directory?

…X, so ZipFile fails to create the file, trying to validate the (empty) tempfile. File.close! closes and then unlinks it, which is fine since we aren't writing to it directly anyway.


At a more abstract level, I'm not even sure why we are creating a Tempfile here. Couldn't we just go ahead and write to the file we're trying to create, or at least to a "filename.temp.jar" in the same directory instead of in the temp directory?
nicksieger added a commit that referenced this pull request Dec 8, 2011
Windows show-stopper: File.unlink doesn't work like it does on POSIX systems
@nicksieger nicksieger merged commit cad1fb5 into jruby:master Dec 8, 2011
@nicksieger
Copy link
Member

Thanks. Yeah we could write to the current directory, but all we need is a single empty jar file. I wrote it to use tempfile so the current directory wouldn't get cluttered.

@enebo
Copy link
Member

enebo commented Dec 8, 2011

def empty_jar_path
Tempfile.open(["empty", "jar"]) { |t| return t.path }
end

This seems mildly more safe since it guarantees tempfile will not leave around an open file in case of weird behavior with underlying filesystem. I say mildly, because any weird artifact of underlying filesystem might leak through our impl of Tempfile (JRuby has native impl and does not use Delegate ruby-based one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants