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

stat.writable? incorrectly reporting false for some directories on Windows 7 #3505

Closed
aschmied opened this issue Nov 30, 2015 · 12 comments
Closed
Labels
Milestone

Comments

@aschmied
Copy link

In 1.7.22:

irb(main):031:0> dir = File.expand_path(ENV['TMP'])
"C:/Users/ANTHON~1/AppData/Local/Temp"
irb(main):032:0> stat = File.stat(dir)
#<File::Stat dev=0x2, ino=0, mode=040755, nlink=1, uid=0, gid=0, rdev=0x2, size=393216, blksize=512, blocks=0, atime=2015-11-30 18:19:32 -0500, mtime=2015-11-30 18:19:32 -0500, ctime=2015-11-16 11:18:42 -0500>
irb(main):033:0> stat.writable?
true

In 1.7.23:

irb(main):039:0> dir = File.expand_path(ENV['TMP'])
"C:/Users/ANTHON~1/AppData/Local/Temp"
irb(main):040:0> stat = File.stat(dir)
#<File::Stat dev=0x2, ino=0, mode=040555, nlink=1, uid=-1, gid=, rdev=0x2, size=393216, blksize=0, blocks=, atime=2015-11-30 06:45:17 -0500, mtime=2015-11-30 06:45:17 -0500, ctime=2015-11-15 23:43:54 -0500>
irb(main):041:0> stat.writable?
false

I noticed this because it broke Dir.tmpdir on this system: every candidate temp directory is skipped.

@nirvdrum
Copy link
Contributor

nirvdrum commented Dec 1, 2015

@enebo It looks like we have more work.

@enebo enebo added this to the JRuby 1.7.24 milestone Dec 1, 2015
@enebo enebo added the windows label Dec 1, 2015
@enebo
Copy link
Member

enebo commented Dec 1, 2015

@aschmied @nirvdrum crud. I spent the most time on mode logic with the new stat code. I guess there is some missing logic for directories in particular.

@atambo
Copy link
Member

atambo commented Dec 9, 2015

I'm also seeing this pop up when upgrading from jruby-1.7.22 to jruby-1.7.23. This is what the error looks like to a user when running rspec or anything that uses Dir.tmpdir:

ArgumentError: Unable to find a non world-writable directory for Dir::tmpdir. Consider setting ENV['TMPDIR'], ENV['TMP'] or ENV['TEMP'] to a non world-writable directory.
  occurred at C:/jruby-1.7.23/lib/ruby/shared/tmpdir.rb:41:in `tmpdir'

@perlun
Copy link
Contributor

perlun commented Dec 16, 2015

@olleolleolle and @rallenecraft - this may be the issue we've been seeing also, or related to it.

@perlun
Copy link
Contributor

perlun commented Dec 17, 2015

@nirvdrum, @aschmied, @enebo - I did a bit of R&D on the subject in jnr/jnr-posix#67. Unfortunately far from closing this yet, but we are getting started. More details there.

I guess I may have to clone the whole JRuby code and add some more debugging there... it could actually be an idea. Thus far, I've only tried debugging it on the jnr-posix side.

@enebo
Copy link
Member

enebo commented Dec 18, 2015

@perlun I mentioned in a comment in that PR that I think this is a generic mode issue and not platform specific. I also thought (cannot 100% remember for sure :| ) that I saw this while only step-debugging through jnr-posix itself. It would make sense that this would be jnr-posix only issue since the JRuby side is not modifying the mode value. getting mode value to match should fix all issues...

@headius
Copy link
Member

headius commented Jan 6, 2016

@perlun Now that we're past the holidays, do you have any updates on this? We can get your jnr-posix PR merged in, but it sounds like we have more work to do.

@perlun
Copy link
Contributor

perlun commented Jan 7, 2016

@perlun Now that we're past the holidays, do you have any updates on this?

Unfortunately not, yet.

We can get your jnr-posix PR merged in, but it sounds like we have more work to do.

I think so yes. My main problem was that I wasn't able to reproduce the problem, in jnr-posix so I think I would more or less have to run JRuby against my "local jnr-posix code". I'm rather unfamiliar with Maven projects etc. so any pointers on how to get this done would be helpful.

I have a Windows 8 and a Windows 10 VM so testing any suggested fix is rather easy.

@headius
Copy link
Member

headius commented Jan 8, 2016

Ahh I can help you run against an updated jnr-posix.

First, in jnr-posix, run:

mvn clean install

This should install a SNAPSHOT of jnr-posix in your local repository.

Then in JRuby, edit core/pom.rb so it references the same version of jnr-posix.

Do a full mvn clean package of JRuby and you should be running on the updated version.

enebo added a commit that referenced this issue Jan 11, 2016
@enebo
Copy link
Member

enebo commented Jan 11, 2016

This is crazy. 1.7.23 was not released with the latest version of jnr-posix. I was hurrying because of my impending travel but there is no excuse for this. I am sorry I did not update this as jnr-posix 3.0.22 had some serious issues. Strangely none of our acceptance tests hit it and we still do not have CI properly working on appveyor yet. Appveyor will happen soon since I don't ever want to repeat this...

@perlun can I ask you to build and verify that things work on your various windows boxen? It is possible there is still an issue but the basic issue I can see was fixed between 3.0.22 and 3.0.23 of jnr-posix.

@enebo enebo closed this as completed Jan 11, 2016
@perlun
Copy link
Contributor

perlun commented Jan 12, 2016

Great @enebo - that would explain things, since I was absolutely unable to repro any of this w/ jnr-posix in itself. 😉

I rebuilt now from the latest jruby-1_7 HEAD and it worked fine. 👏 Both on Windows 8.1 and Windows 10.

Any chance we can have a JRuby 1.7 released w/ this fix included in the near future? Many thanks.

irb(main):004:0> require 'tmpdir'
=> true
irb(main):005:0> Dir.tmpdir
=> "C:/Users/PLUNDB~2/AppData/Local/Temp"
irb(main):006:0> File.stat(Dir.tmpdir)
=> #<File::Stat dev=0x2, ino=0, mode=040755, nlink=1, uid=0, gid=0, rdev=0x2, si
ze=70074368, blksize=nil, blocks=nil, atime=2016-01-12 08:44:21 +0200, mtime=201
6-01-12 08:44:21 +0200, ctime=2013-10-22 18:00:23 +0300>
irb(main):007:0> File.stat(Dir.tmpdir).writable?
=> true

@enebo
Copy link
Member

enebo commented Jan 12, 2016

@perlun one more fix involving stat on windows and we should be good for another release. Looking at this week.

killbillio added a commit to killbill/killbill-admin-ui-standalone that referenced this issue Apr 18, 2016
The build contains jruby-jars 1.7.24 which should fix Windows
deployments (see jruby/jruby#3505).

Signed-off-by: Kill Bill core team <contact@killbill.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants