jessedoyle / prawn-icon

Easy icons for Prawn.
Other
31 stars 16 forks source link

FONTDIR not working when used by JRuby in an OSGI environment #45

Closed PartTimeDataScientist closed 4 years ago

PartTimeDataScientist commented 4 years ago

While trying to use AsciidoctorJ PDF in an OSGi environment font based icons are not rendered properly. Prawn::Icon is trying to access the .ttf-file from "uri:classloader:/gems/prawn-icon-2.5.0/data/fonts/fas/*.ttf" which doesn't work in that case (org.jruby.exceptions.StandardError: (UnknownFont) Icon font not found for set: ...).

One possible solution that seems to do the trick for me is to store the icon-fonts in an extra folder (inside the OSGi-bundle) and provide Prawn::Icon with the location. This can be done by setting a system property in Java and testing if the corresponding ENV_JAVA is set on the Ruby side. In addition, this should provide an easy way to use custom icon fonts in a configurable location (provided one has the ttf and created a suitable .yml shim). Using more (custom) icon fonts seems to be interesting as it is discussed in at least two Asciidoctor issues (#539 and #3461)

So for my case I have modified base.rb to read like follows

module Prawn
  class Icon
    module Base
      if ENV['prawn.iconfont.dir'] != nil
        FONTDIR = File.expand_path(ENV['prawn.iconfont.dir'])
      elsif ENV_JAVA['prawn.iconfont.dir'] != nil
        FONTDIR = File.expand_path(ENV_JAVA['prawn.iconfont.dir'])
      else
        FONTDIR = File.join \
        File.expand_path('../../..', __dir__), 'data/fonts'
      end
    end
  end
end

As I might have overlooked side effects or perhaps did not find the optimal solution I didn't submit it as a PR but I wanted to have my solution documented in case someone is running into a similar issue. Feel free to close the issue if you think it is not relevant for the general public as I have found a suitable workaround for me.

jessedoyle commented 4 years ago

Hi @PartTimeDataScientist - thanks for using Prawn::Icon!

Sorry you're running into problems.

The data directory is a common idiom for ruby gems to include various associated data files. In fact we made this change quite a while ago to conform to the standard (see: https://github.com/jessedoyle/prawn-icon/issues/16).

Unfortunately my familiarity with jruby/OSGI is next to zero, so I'm in unfamiliar territory here.

I'd be open to allowing configuration of the font directory, but I'd like to understand the root problem a bit more first.

In your case does the data directory for the gem exist somewhere else in the OSGI bundle? Perhaps it's just namespaced somewhat differently due to the environment?

I do see that FONTDIR is currently set in a non-portable way (i.e. it shouldn't assume the use of / characters in the pathname). One change that we should definitely make is to setup the global in a platform-independent way (we should likely use Gem.datadir as well).

If we do end up allowing configuration of the font directory, it may be worthwhile to expose a good interface something like:

Prawn::Icon.configure do |config|
  config.data_directory = '/path/to/somewhere'
end

I'll look into this over the next few weeks to see what options we have. PRs would definitely be welcome as well!

PartTimeDataScientist commented 4 years ago

Thanks for looking into it @jessedoyle, I'll try to assist where possible.

I've seen issue #16 as there aren't that many issue to browse for Prawn::Icon ;-) but I don't think that this is the root cause of the observed problem.

The OSGi bundle I am working on is actually a plugin for Eclipse where I am including asciidoctorj-pdf-1.5.3.jar which in turn contains (among others) the gems prawn-icon-2.5.0 and asciidoctor-pdf-1.5.3. Both gems are using fonts which are located in the __dir__/data/fonts directory of the gem (see image). But while asciidoctor-pdf seems to be able to load fonts or themes from the __dir_/data/* directories prawn-icon is not. image

At first glance asciidoctor is loading fonts and theme-file in asciidoctor-pdf/lib/asciidoctor/pdf/theme_loader.rb, but the code doesn't look too different. Using File.absolute_path %(#{__dir__}/../../../data/fonts) or GEM_FONTS_DIR in Prawn::Icon\base.rb to set the FONTDIR didn't improve anything. Maybe @mojavelinux can comment on what might be different in the way the two gems access their bundled data files. At least he has also already solved a related problem with the option to set the pdf-fontsdir attribute for including more and custom fonts in asciidoctor.

# asciidoctor-pdf/lib/asciidoctor/pdf/theme_loader.rb
# frozen_string_literal: true

require 'safe_yaml/load'
require 'ostruct'
require_relative 'measurements'

module Asciidoctor
  module PDF
    class ThemeLoader
      include ::Asciidoctor::PDF::Measurements
      include ::Asciidoctor::Logging

      DataDir = ::File.absolute_path %(#{__dir__}/../../../data)
      ThemesDir = ::File.join DataDir, 'themes'
      FontsDir = ::File.join DataDir, 'fonts'
      BaseThemePath = ::File.join ThemesDir, 'base-theme.yml'
# [...]
mojavelinux commented 4 years ago

This doesn't feel like the right repository to report this issue. I'd say this is more of an issue with AsciidoctorJ PDF, and maybe just Asciidoctor PDF.

PartTimeDataScientist commented 4 years ago

I know that I understand not nearly enough of the interaction between Asciidoctor and Prawn::Icon to discuss your judgment. I just thought this would be the right place as the exception is raised by Prawn::Icon and modifying the base.rb file allowed me to find a workable solution. I'll gladly report this as an issue for AsciidoctorJ PDF or Asciidoctor PDF if you think that would be the more appropriate spot.

mojavelinux commented 4 years ago

If you were using Prawn::Icon by itself in JRuby/OSGi, then I would agree the place is here. But by bringing Asciidoctor PDF examples in here, you have stepped outside the scope of what this project should have to worry about. That's not to say a code change might be required here, but we are distracting this project with concerns that really start from Asciidoctor PDF.

mojavelinux commented 4 years ago

@jessedoyle wrote:

I do see that FONTDIR is currently set in a non-portable way (i.e. it shouldn't assume the use of / characters in the pathname).

That doesn't make it non-portable. Ruby already handles path normalization across platforms.

The main issue I see is how you are getting to the base location of the gem. What we do in Asciidoctor is define the ROOT_DIR of the gem in the entrypoint script using:

ROOT_DIR = ::File.dirname ::File.absolute_path __dir__

Then we build paths from there. Within a jar, JRuby builds the absolute path in a way that allows it to reference other files in the jar. You definitely don't have to worry about that as a gem author.

mojavelinux commented 4 years ago

@jessedoyle

we should likely use Gem.datadir as well

Gem.datadir is an entirely different location which is not inside the gem. I've been tripped up by this in the past. I have never found Gem.datadir to be useful.

mojavelinux commented 4 years ago

A consumer of a gem should never have to worry about having to set up paths so that the gem can access its own files. This can be achieved by defining the ROOT_DIR correctly as I have shown (and as @PartTimeDataScientist pointed out we do in the theme loader for Asciidoctor PDF).

mojavelinux commented 4 years ago

@PartTimeDataScientist After studying this issue more closely, I see that your point about this being an issue with Prawn::Icon is likely correct. I think the fact that you were talking about Asciidoctor PDF made it seem like you were citing a problem that had to do with how it is integrated into Asciidoctor PDF, but that's not actually the case. So I retract my earlier criticism about where you filed this issue.

jessedoyle commented 4 years ago

Thanks for the input!

Yes, I think making the font directory configurable is a reasonable request - therefore I think we should keep this issue open until there's a mechanism for that.

@mojavelinux - Nice, that's good to know about Ruby path normalization. I guess I've always taken that for granted.

I've used Gem.loaded_specs[gem_name].full_gem_path in the past and it's worked quite well. I think we should try to use a similar approach.

mojavelinux commented 4 years ago

Sounds like a good approach.

PartTimeDataScientist commented 4 years ago

@PartTimeDataScientist After studying this issue more closely, I see that your point about this being an issue with Prawn::Icon is likely correct. I think the fact that you were talking about Asciidoctor PDF made it seem like you were citing a problem that had to do with how it is integrated into Asciidoctor PDF, but that's not actually the case. So I retract my earlier criticism about where you filed this issue.

Happy to hear that. :-) I already had the feeling that I always overlook something when reporting/discussing the problems I have with integrating asciidoctor in my little project.

This particular issue has actually bothered me for quite a while but with your hints it sounds as if there is a straightforward way for improving the situation and I can live with my workaround for the moment to work on other topics.

@jessedoyle With the comments of @mojavelinux I think this might also be split up in two issues

  1. Fix how the data/fonts directory is defined
  2. Make the fonts directory configurable or allow users to add an additional fonts directory
jessedoyle commented 4 years ago

@PartTimeDataScientist - I've got a PR open that I think should address both issues mentioned above.

The font directory can (optionally) be configured as follows:

Prawn::Icon.configure do |config|
  config.font_directory = '/path/to/fonts'
end

The PR should also resolve the issue with the assumptions Prawn::Icon used when finding the font directory. I think this should resolve your issue in JRuby/OSGI.

@mojavelinux - I'm unfamiliar with the environment. Is it possible to just point the Gemfile to a git branch to test the fix?

i.e.

# in Gemfile
gem 'prawn-icon', git: 'https://github.com/jessedoyle/prawn-icon.git', branch: 'gh-45'
mojavelinux commented 4 years ago

@jessedoyle I thought we worked out that the directory does not need to be configured. Sure, it might be nice to be able to configure it for running the tests in say a specialized environment like Linux packaging. But for normal usage, it should just work. As @PartTimeDataScientist pointed out, the internal gem paths in Asciidoctor PDF work fine in a JRuby/OSGi environment, so prawn-icon can do the same.

jessedoyle commented 4 years ago

@mojavelinux - For sure I definitely understand the configuration part is not needed. My PR makes the configuration piece entirely optional, with sane defaults to the gem's internal data directory.

I opted to implement the configuration of the font directory because I found @PartTimeDataScientist's argument compelling:

In addition, this should provide an easy way to use custom icon fonts in a configurable location (provided one has the ttf and created a suitable .yml shim)

Essentially one can now implement custom icon fonts given that some elbow grease has been put in to generate the necessary foundation (unicode mapping file, etc.).

The PR also implements path resolution using Gem::Specification#full_gem_path, which I'm hoping fixes the core issue that @PartTimeDataScientist was running into.

mojavelinux commented 4 years ago

Ah, thanks for clarifying. In that case, fantastic!

PartTimeDataScientist commented 4 years ago

Just did a quick test and replaced all the ruby files in the prawn-icon\lib in the asciidoctorj-pdf jar with the ones in your gh-46 branch but that doesn't seem to work as expected. The execution is now failing at the initialization of the default_font_directory

Execute failed: (NoMethodError) undefined method `full_gem_path' for nil:NilClass
org.jruby.exceptions.NoMethodError: (NoMethodError) undefined method `full_gem_path' for nil:NilClass
    at RUBY.default_font_directory(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/configuration.rb:24)
    at RUBY.font_directory(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/configuration.rb:17)
    at RUBY.<module:Base>(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/base.rb:28)
    at RUBY.<class:Icon>(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/base.rb:26)
    at RUBY.<module:Prawn>(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/base.rb:13)
    at RUBY.<main>(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/base.rb:12)
mojavelinux commented 4 years ago

I think it's a timing issue. The code is called while it is being loaded, so it is not yet loaded and thus not present in Gem.loaded_specs. The assignment would have to be deferred until first use (after the gem is loaded).

jessedoyle commented 4 years ago

Interesting, I'll look into it over the next few days. As a fallback we can definitely use @mojavelinux's suggestion.

jessedoyle commented 4 years ago

@PartTimeDataScientist - Sorry to keep bothering you, do you mind trying to run Gem.loaded_specs.keys somewhere?

Basically just throw a line like this above this line:

puts Gem.loaded_specs.keys
PartTimeDataScientist commented 4 years ago

@jessedoyle - Sure thing! Trying to help where I can. I modified base.rb and configuration.rb to read as below. Interestingly only did_you_mean seems to be loaded at that point if I understand the output correctly. In order to test if it is a timing issue as @mojavelinux suggested I also modified font_data.rb in my "workaround" version which shows a little more but still no prawn-icon.

Hope you can make sense of the results...

base.rb

        puts ">>> base.rb"
        puts Gem.loaded_specs.keys
      # @deprecated Use {Prawn::Icon.configuration.font_directory} instead
      FONTDIR = Prawn::Icon.configuration.font_directory.to_s

configuration.rb

        puts ">>> configuration.rb"
        puts Gem.loaded_specs.keys
        Pathname.new(
          Gem.loaded_specs['prawn-icon'].full_gem_path()
        ).join('data', 'fonts')

output

>>> base.rb
did_you_mean
>>> configuration.rb
did_you_mean
ERROR    KNIME-Worker-2-AsciidocWriter 0:23 Node     Execute failed: (NoMethodError) undefined method `full_gem_path' for nil:NilClass
org.jruby.exceptions.NoMethodError: (NoMethodError) undefined method `full_gem_path' for nil:NilClass

font_data.rb - "workaround version"

      def path
        puts ">>> font_data.rb"
        puts Gem.loaded_specs.keys
        ttf  = File.join(Icon::Base::FONTDIR, @set.to_s, '*.ttf')
        font = Dir[ttf].first

output

>>> font_data.rb
did_you_mean
jar-dependencies
psych
jruby-openssl
>>> font_data.rb
did_you_mean
jar-dependencies
psych
jruby-openssl
[...]
jessedoyle commented 4 years ago

Thanks for the help @PartTimeDataScientist. Rubygems should be loading all required dependencies by that point, so something is definitely a little broken. I'll do a bit of research though and should be able to come up with a solution in a few days.

jessedoyle commented 4 years ago

Hey @PartTimeDataScientist.

I'm hoping to cut a new release and (partially) resolve this issue for you.

The fact that Gem.loaded_specs doesn't contain the prawn-icon gem is odd - perhaps it's a quirk of JRuby.

With that being said, I added a failsafe mechanism in https://github.com/jessedoyle/prawn-icon/pull/46 in the event that the gemspec hasn't been loaded yet. In that case, we simply revert to the previous path resolution behaviour.

I believe the key to solving your issue will be with the configurable font directory that was added:

i.e.

Prawn::Icon.configure do |config|
  config.font_directory = '/absolute/path/to/font/directory'
end

My suggestion would be to utilize the configuration of the font directory above to be able to set up the proper pathing for your environment.

For now I'm going to close this issue. Feel free to comment and we can reopen if necessary.

Thanks!

mojavelinux commented 4 years ago

To my knowledge, there is no such quirk in JRuby. I verified using JRuby 9.2 with Asciidoctor PDF 1.5.3 and the following call works when placed at the very top of the prawn/icon.rb.

Gem.loaded_specs.fetch('prawn-icon').full_gem_path

Unless it can be proved that this call is problematic on JRuby with a sample project, we must assume that there is a misconfiguration elsewhere that would cause this to fail.

While allowing the font directory to be configured is certainly a nice enhancement to accommodate certain use cases, I would not worry about adding a defensive callback if Gem.loaded_specs doesn't find prawn-icon.

PartTimeDataScientist commented 3 years ago

Hi @jessedoyle: I just found the time to test your new release. In order to run the test I've replaced all praw-icon ruby files in the Asciidoctor PDF 1.5.3 JAR with the ones from the new release and ended up with exactly the same behavior as before: :-/

Looks like the Asciidoctor way of getting the full_gem_path is the most stable way of getting the relevant directory in the JRuby case...

Caused by: org.jruby.exceptions.StandardError: (UnknownFont) Icon font not found for set: far
    at RUBY.path(uri:classloader:/gems/prawn-icon-2.5.0/lib/prawn/icon/font_data.rb:72)
PartTimeDataScientist commented 3 years ago

While updating the libraries on my module I figured that the root cause of the font-files not being found (at least for the 3.0.0 version - didn't test for the old one as of yet) is not the way how the gem folder is defined but the ".glob" statement searching for .ttf-files in line 68 of font_data.rb which is not working inside a .jar file! I've found some issues regarding DIR.* not working inside .jar-files but most old and often claimed to be fixed.

def path
  font = Icon.configuration.font_directory
  .join(@set.to_s)
  .glob('*.ttf')
  .first

Nonetheless this allows to propose a generic solution to the problem. I've hard-coded the .ttf-file belonging to each set in my internal version by introduing a tiny accessory function font_path() and a modification of the path() function:

def font_path(key)
  font_files = {
    fab: "fa-brands.ttf",
    far: "fa-regular.ttf",
    fas: "fa-solid.ttf",
    fi: "foundation-icons.ttf",
    pf: "paymentfont-webfont.ttf"
  }
  path = Icon.configuration.font_directory.to_s + "/" + key.to_s + "/" + font_files.fetch(key)
  File.expand_path(path)
end

def load_fonts(document)
  document.font_families[@set.to_s] ||= { normal: path }
  self
end

def path
  font = font_path(@set)

  if font.nil?
    raise Prawn::Errors::UnknownFont,
      "Icon font not found for set: #{@set}"
  end

  @path ||= font.to_s
end

That works stable for my application where prawn-icon is included within asciidoctorj-pdf.jar for all icon sets. I envision that this approach should work for the other usecases as well and might even be more stable and flexible e.g. regarding .otf font files for specific sets.

@jessedoyle Please let me know what you think about this idea. I didn't submit it as a PR as of yet as I don't have the ability to test the code outside my usecase but could easily do so if you agree with this approach.

jessedoyle commented 3 years ago

Hey @PartTimeDataScientist, I'm glad to hear that you've narrowed in on the issue (and that you've found a monkeypatch/solution that works for you)!

I am surprised that Pathname#glob appears to be the issue as that method is part of the Ruby Standard Library. From the code snippet above, I do agree with your conclusion though! There's a really old jRuby issue that mentions something about the ClassLoader API not being able to list files in a directory - maybe that's at play here?

Prawn::Icon's font path resolution methods are already more complex than I was hoping they'd get. Also, I'd prefer to avoid hard-coding the list of font files directly as it's possible for users to define and configure their own icon fonts that don't ship with the gem. Therefore, I'd like to avoid changing the font path resolution functionality at this point in time.

Are you able to include the code snippet above in your own app/code as a monkeypatch? I don't suspect that it will cause any issues for you in the future.

Thanks again!

PartTimeDataScientist commented 3 years ago

Hi @jessedoyle, thanks for looking into it. I did find the same issue that you refer to but that is claimed to be fixed since years ago. There is a newer issue but that only refers to using the classpath: notation and not the uri:classloader: notation so that shouldn't be the case here. Nonetheless Dir.* and jRuby and .jar-Files don't seem to be best friends...

I can live with monkey patching the gem inside the asciidoctorj-pdf.jar but nonetheless would prefer to have a more general solution. One other idea that I had after finding the "glob" as root cause was to define the required font file in the set.yml file directly. Although I guess that this would also be a not too complex solution it was out of reach of my limited ruby skills and understanding of the library. :-/ I was already happy that I could debug the issue using plenty of "puts" statements throughout the whole code... :-)

This would still allow users to add their own font (they would have to create the shim-file anyway) without the need for a code change while getting rid of the glob statement. It would also facilitate geting rid of the font subdirectories as you could place all the .yml together with the required .ttf files in a single /fonts directory:

Thinking this even further it should be possible to get rid of the various shim files and subdirectories and have a single "sets.yml" in a fonts directory together with the required .ttf or .otf files which IMHO would notably decrease the complexity of font path resolution.

---
far:
  __font_version__: 5.15.1
  __font_file__: "fa-regular.ttf"
  address-book: ""
  address-card: ""
  [...]
fab:
  __font_version__: 5.15.1
  __font_file__: "fa-brands.ttf"
  500px: ""
  accessible-icon: ""
  accusoft: ""
  [...]
fa:
  fa-500px: fab-500px
  fa-address-book-o: far-address-book
  fa-address-card-o: far-address-card