jekyll-assets icon indicating copy to clipboard operation
jekyll-assets copied to clipboard

FileStore caching under Sprockets 4.x / Issue with `digest_path`

Open dramalho opened this issue 4 years ago • 4 comments

Description

I opened a branch on my blog to try to upgrade jekyll , jekyll-assets and sprockets along with it and I've stumbled onto two issues that I want to run by you. We can split this into two separate issues (sic) but I need to put this down while I remember the details :)

First of all, the important gems I'm running are:

  • sprockets (~> 4.0.beta7)
  • jekyll-assets (4.0.0.alpha) (rev: 47fc845853e27af1877691efceba991f20f46526)
  • jekyll (4.0.0)

Issue with FileStore

Now, the first issue I got was with the compile time , the whole thing was taking much much much longer than usual, so I took a look at the verbose output.

Turned out that for each post in my blog all the assets were recompiling, which pilled up of course. I opened both sprockets and jekyll-assets gems and narrowed it down to the cache store.

The short story is that Sprockets::Cache was running with NullStore despite me having file caching enabled. Digging as to why, it seems that the initialiser there stores a wrapper around the given cache store, but at the selection time it was always picking NullStore. The problem there is that Sprockets was expecting an instance of a cache store, but Jekyll-assets gives it a class instead . So locally what I did was replace

      def upstream
        return Upstream::NullStore unless enabled?
        return Upstream::MemoryStore if type == 'memory'
        Upstream::FileStore
      end

with

      def upstream
        return Upstream::NullStore unless enabled?
        return Upstream::MemoryStore if type == 'memory'
        Upstream::FileStore.new(@dir)
      end

And now Sprockets::Cache is getting an initialised FileStore and caching worked as expected. I assume the issue is the same for MemoryStore, NullStore same but the overall effect is the same

Issue with digest_path

The second issue I got was when Jekyll::Assets::Env called write_all , I was getting method asset_config called on nil class . Now for this one I didn't dig down enough, but the gist of it is here

      module Asset
        attr_accessor :environment

        # --
        # @note see the configuration.
        # Provides the digest path, or non-digest path.
        # @return [String]
        # --
        def digest_path
          environment.asset_config[:digest] ? super : logical_path
        end

This class overrides the digest_path method and does a little test, but for one reason or another, the environment was never set so that would fail. I actually commented it out the method just to get over the line, which it did, but I don't know the consequence of this.

I can try to give you more info, this issue is a little bit more undefined , but might be worth sharing just the same!

dramalho avatar Apr 18 '20 19:04 dramalho

Do you have a site I can use to replicate the former issue? As the only way to trace that is to introspect it since there can be a lot of t hings that hit that particular area!

envygeeks avatar Apr 29 '20 03:04 envygeeks

You said former but since you have a PR I guess for the Upstream caching, I guess you mean the digest_path issue right? I .. I have it on my blog, but I'll try to create a repo that triggers the thing - I need to remember where I was hitting it :)

Thanks for the Upstream fix, hope the stuff you've been dealing with is ... going well . Thanks Jordon

dramalho avatar Apr 29 '20 08:04 dramalho

I didn't know I had a PR for the caching issue, that's already fixed on master at https://github.com/envygeeks/jekyll-assets/commit/bc4250808072d29602af7f9c1d2e4b9187927d33 yes, I do mean the latter with digest_path.

envygeeks avatar Apr 29 '20 08:04 envygeeks

i'm running into the same digest_path issue with sprockets 4.

I narrowed it down to using //= link foo.js. I think this is the issue:

https://github.com/rails/sprockets/blob/24c97270fbf6de11b4ffff0311bb427b7a8a3a83/lib/sprockets/base.rb#L88

as far as I can tell, this is what sets environment, but it's setting it on CachedEnvironment, not on Base: https://github.com/envygeeks/jekyll-assets/blob/056d2c88719ef3b1f90967a606dd1441581dd832/lib/jekyll/assets/patches/cached_environment.rb#L35-L41

so the linked asset doesn't have environment set since a different find_asset was called.

mroch avatar Jul 25 '21 06:07 mroch