Redmine

LGTM.

Gregor, I guess you have to update your patch because of this new feature: https://github.com/redmine/redmine/commit/bf81c96b79cc80bc4ffb08714a3d3f92e68e254a

You can find some hits here: http://www.redmine.org/attachments/16039/pdf_thumbnails.patch

And in addition, I guess you can add tiff support ([0] is also necessary here, so I'd suggest you to add "[0]" for all images, not only for pdf, it works in my patch as well), and add some tests.

상경 김 rus: Thanks for additionally suggesting TIFF support in #22915. I did not include your changes into my patch to keep this issue focused. I assume, adding tiff after this change was applied will be just as easy.

And after that, PDF support is still marked as unavailable in admin/infos.

I played around with this a bit and it seems that Ghostscript on Windows always opens this window, so it's pretty useless to use as a check.

As Gregor mentioned checking for presence of the GS executable in itself is a bit unreliable as well, since ImageMagick still might be built without PDF support. Unfortunately convert -list format, which lists ImageMagick's supported file types, still lists PDF even with Ghostscript not present, it merely checks what options it was compiled with but does not actually look up the actual runtime dependencies (ghostscript libraries). That's consistent with the information I found on imagemagick.org . The only reliable way to determine if a certain file type can be handled is to actually try it out.

We could either:

- try to actually thumbnail a PDF once in gs_available?
- or resort to just checking the presence / executability of the Ghostscript binary
- or introduce an option to manually enable PDF thumbnailing with proper documentation ("install ghostscript!") and completely remove the Ghostscript availability check. The more I think about it this might be the best thing to do.

Thoughts?

Jens Krämer wrote:

- or introduce an option to manually enable PDF thumbnailing with proper documentation ("install ghostscript!") and completely remove the Ghostscript availability check. The more I think about it this might be the best thing to do.

+1 for this. This is the most efficient and reliable way in the options, I think.

Jens Krämer wrote:

We could either:

- try to actually thumbnail a PDF once in gs_available?
- or resort to just checking the presence / executability of the Ghostscript binary
- or introduce an option to manually enable PDF thumbnailing with proper documentation ("install ghostscript!") and completely remove the Ghostscript availability check. The more I think about it this might be the best thing to do.

Another idea. I think there are not many people run Redmine for production on Windows, so we can simply reject Windows.

    def self.gs_available?
      return @gs_available if defined?(@gs_available)

      if ENV['OS'] == 'Windows_NT'
        @gs_available = false
      else
        @gs_available = system("gs -version") rescue false
        @gs_available ||= system("gswin32 -version") rescue false
        @gs_available ||= system("gswin64 -version") rescue false
      end

      @gs_available

My experience with imagemagick ist, that it is not very reliable with respect to predictability of its capabilities on different platforms. Why not let the user try on a setup page, if imagemagick is capable to handle pdf?

On GitHub and Redmine.org have shared two plugins [[https://www.redmine.org/plugins/redmine_preview_pdf]] and [[https://www.redmine.org/plugins/redmine_thumbnail_pdf]] that use the pdf capabilities of imagemagick. I will introduce a try-if-pdf-is-availbale option on it's setup page.

Go MAEDA wrote:

Another idea. I think there are not many people run Redmine for production on Windows, so we can simply reject Windows.

I’m in favour of rejecting Windows.

I found that Redmine has Redmine::Platform.mswin? method. We don't have to check ENV['OS'].

This should be OK.

    def self.gs_available?
      return @gs_available if defined?(@gs_available)

      unless Redmine::Platform.mswin?
        @gs_available = system("gs -version") rescue false
      else
        @gs_available = false
      end

      @gs_available
    end

For windows, use "gswin64c(32c).exe"(command line version) instead of "gswin64(32).exe".
Now it's working on windows.

--- org/lib/redmine/thumbnail.rb
+++ patch/lib/redmine/thumbnail.rb
@@ -77,5 +77,6 @@

       if Redmine::Platform.mswin?
-        @gs_available = false
+        @gs_available = system("gswin64c -version > nul 2>&1") rescue false
+        @gs_available ||= system("gswin32c -version > nul 2>&1") rescue false
       else
         begin

Created a separate issue: #33283.