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

Use Ruby implementation for which #315

Merged
merged 1 commit into from Jul 12, 2017

Conversation

davispuh
Copy link
Contributor

In Windows there's no /dev/null and it causes error "The System cannot find the path specified". Also in Windows there's no native which and it works only because it's included in DevKit.

This Ruby implementation should correctly work for all platforms.

@larskanis
Copy link
Member

Thanks for your contributions! I'll find some time at the weekend to inspect your requests.

@larskanis
Copy link
Member

The output "The System cannot find the path specified" appears for every rake run, but does not break the build if make is called make on the system, which is the case for the Windows RubyInstaller. Nevertheless it's misleading to have such an output.

I think this is a proper cross platform solution, that should be merged.

@davispuh
Copy link
Contributor Author

I just rebased (just in case) and that previous Travis failure was due something else.

@davispuh
Copy link
Contributor Author

davispuh commented Nov 8, 2014

rebased again, why this haven't been merged?

@djberg96
Copy link
Contributor

djberg96 commented Jun 4, 2015

Please merge.

@tduehr
Copy link
Member

tduehr commented Jun 4, 2015

Why iterate over the path and extensions rather than use a glob?

@djberg96
Copy link
Contributor

djberg96 commented Jun 4, 2015

@tduehr wouldn't work on Windows with their file extensions, e.g. notepad.exe, not notepad.

@tduehr
Copy link
Member

tduehr commented Jun 4, 2015

Dir.glob("*.{exe,bat}") works for me in 2.2.2 (compiled w/ VC) under win8.1. C:\Ruby\bin

Dir.glob("**/*{#{ENV["PATHEXT"].tr(';',',')}}") works in the directory above it C:\Ruby

@djberg96
Copy link
Contributor

djberg96 commented Jun 4, 2015

Hm, for some reason I thought that approach had some issue, but perhaps I'm mistaken. Anyway, it's not like we need to hyper optimize for this little bit of functionality. I'm happy with whatever works. :)

@davispuh
Copy link
Contributor Author

davispuh commented Jun 5, 2015

I'm don't really understand why glob would be preferable? I'm not sure if there would be any performance benefits.
Here's implementation using Dir.glob and it seems to be more complicated...

def which(name)
  exts = ENV['PATHEXT'] ? '{' + ENV['PATHEXT'].split(';').join(',') + '}' : ''
  paths = '{' + ENV['PATH'].split(File::PATH_SEPARATOR).join(',').gsub('\\', '/') + '}'
  Dir.glob(paths + '/' + name + exts).each do |app|
    return app if File.executable?(app)
  end
  nil
end

In PATH there can be multiple executables with same name and some might not even be executable so we've to check all in correct order.

@tduehr
Copy link
Member

tduehr commented Jun 5, 2015

I was wondering if there was a specific reason is all... Then i got to way over optimizing. I'll pull this when I get a chance to write something to use which normally then failover to this.

@tduehr tduehr merged commit a78689d into ffi:master Jul 12, 2017
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.

None yet

4 participants