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 support #24

Closed
Arcovion opened this issue Jan 14, 2014 · 11 comments
Closed

Windows support #24

Arcovion opened this issue Jan 14, 2014 · 11 comments

Comments

@Arcovion
Copy link

Would be really nice to have this in windows, all the binaries exist for windows in some form.
Currently I tried setting line 47 of bin_resolver.rb to true and now all I get is The system cannot find the path specified. with no traceroute or easy way to debug it.
It's probably because it can't find the .exe files even though they are in PATH and gems\image_optim-0.10.0\vendor.
Also bin_resolver.rb uses symlinks which are not supported in ruby on windows.

Maybe it's possible to extensively patch that one file and fix everything?
If not then I guess windows support may never happen, but I accept that. 😢

@toy
Copy link
Owner

toy commented Jan 29, 2014

I knew that eventually someone would ask about windows support.
Does it work using Cygwin or something similar?
Worker#run_command will require the same amount of patching as BinResolver.
It should not be hard, if you have time and knowledge — patches/pull requests are welcome.

@Arcovion
Copy link
Author

Ok I'm running through the code and almost have it working, but how do I check the temp file?
It seems like it's immediately removed - and when I do optimize_image! it says permission denied (probably because the temp file doesn't exist so it can't do utime)

optimize.image(original).temp_path gives me a valid path but the file doesn't exist there.
How do I change the temp path?

@toy
Copy link
Owner

toy commented Jan 30, 2014

temp_path creates an instance of ImagePath which also holds reference to an instance of TempFile so it is not garbage collected (temp file is removed when TempFile instance is garbage collected). So temp file should if you assign result of temp_path to some variable.

@Arcovion
Copy link
Author

The code I'm using is:

require 'image_optim'
x = ImageOptim.new
y = x.optimize_image('example.jpg').temp_path
puts y
=> %AppData%/../Local/Temp/example20140130-12380-1xvewn20140130-12380-lqb47j.jpg

%AppData%/../Local/Temp/ directory exists and has various unrelated temp files, but not the /example.*\.jpg/ image.
Can you show me an example of the code I need to keep the temp file so I can view it? Thanks.

@toy
Copy link
Owner

toy commented Jan 30, 2014

Temp file will exist only until script finishes:

require 'image_optim'
x = ImageOptim.new
y = x.optimize_image('example.jpg').temp_path
puts y
puts "Exists: #{y.exist?}" # should print true

Please note that temp_path method creates just an empty file with name starting with basename of original and with same extension.
To persist optimized version for analysis try following:

require 'image_optim'
if optimized = ImageOptim.new.optimize_image('example.jpg')
  optimized.rename('optimized.jpg')
  puts 'Optimized'
else
  puts 'Did not optimize'
end

Also there are specs to insure that everything works.

@Arcovion
Copy link
Author

Ok #rename worked perfectly, output was 377kb -> 260kb

I'm making all these changes without any backward compatibility or specs atm, lots of stuff to change like .shellescape/.shelljoin won't work on windows, permissions errors etc.
I'll get it working but it's going to be very difficult to make a pull request, I'll put up a windows-only fork though.

@Arcovion
Copy link
Author

https://github.com/Arcovion/image_optim/commits/master

Status: Fails all tests and doesn't work 😁

However, this works:

require 'image_optim'
x = ImageOptim.new.optimize_image 'example.jpg'
# Overwrite in-place unless nothing was optimized (optimize_image! doesn't work yet)
x.rename 'example.jpg' unless x.nil?

x = ImageOptim.new.optimize_image 'example.png'
# Overwrite in-place unless nothing was optimized (optimize_image! doesn't work yet)
x.rename 'example.png' unless x.nil?

Saves 377kb -> 260kb (jpg) & 434kb -> 411kb (png)

I need to know how to ensure the filepaths for the binary commands are always put in quotes instead of escaped (windows requires any paths with spaces in to be quoted, and spaces not be escaped with backslashes), and I'm not sure what the -nice command does.
Once those few things are sorted it should be close to working.

How would I go about refactoring #replace to do what my script does above?
It gives me Errno::EACCES for image_path.rb:36:in 'copy', meaning the file is already in use.
Once replace works, quotes work, and the 'nice' option works then it'll be compatible.

@toy
Copy link
Owner

toy commented Jan 31, 2014

Changes should be only in BinResolver and Worker#run_command so making it back into main repo should not be hard, but first it must work.

I've already learned that Shellwords is only for POSIX, but for the first you can just try system with multiple arguments instead of constructed command.
nice allows to change process priority by changing its scheduling.
Not sure what to do with Errno::EACCES, you can try closing tempfile without removing it before copying.

@Arcovion
Copy link
Author

So it essentially works now
https://github.com/Arcovion/image_optim/compare/toy:master...master

The only thing it requires is that the binaries are in PATH. (bin_resolver is ignored, windows users simply need all the binaries - which isn't difficult, can add download links to readme.md if needed)

success = system "#{command} > nul 2>&1"
is used instead of
success = system "env PATH=#{@image_optim.env_path.shellescape} nice -n #{@image_optim.nice} #{command} > /dev/null 2>&1"
I still don't really understand the nice keyword, what is it being passed to?
Edit: Ok nice isn't on windows, so the system command is fine as it is.

Now I was able to run middleman-imageoptim and everything worked perfectly. Success!

Currently, I can't work out how to fix optimize_image!:

fileutils.rb:1383:in `utime': Permission denied - ./example20140131-6936-7y5n4p.jpg (Errno::EACCES)
image_path.rb:36:in `copy'
image_path.rb:42:in `replace'
image_optim.rb:88:in `optimize_image!'

Windows users can use the .rename trick above for now.

Edit: Windows doesn't let you move/copy/rename files while they're open, this is the issue I think. Would probably just need to patch #replace

@Arcovion
Copy link
Author

Closing, having all the binaries in image_optim_pack is much better and not something you can really replicate on Windows without a lot of effort.

@toy
Copy link
Owner

toy commented Jul 9, 2016

Should work on windows, have a look at ae01fef

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

No branches or pull requests

2 participants