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

fix(gatsby-plugin-sharp): bail early if sharp isn't working #10677

Merged
merged 6 commits into from
Mar 20, 2019

Conversation

sidharthachatterjee
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee commented Dec 27, 2018

We've seen several issues where builds break because gatsby-plugin-sharp fails for some reason.

Typically because sharp didn't build correctly during install like in #10620 most likely due to an incorrect version of python

From https://github.com/nodejs/node-gyp

python (v2.7 recommended, v3.x.x is not supported)

This PR adds a try-catch around requiring sharp and bails early if it fails

@sidharthachatterjee
Copy link
Contributor Author

sidharthachatterjee commented Dec 27, 2018

Deleting node_modules/sharp/build/Release/sharp.node and running gatsby develop fails with an error message
screenshot 2018-12-27 20 13 33
Need to

  • add link to a docs page explaining how to resolve this on different platforms

@pieh
Copy link
Contributor

pieh commented Dec 27, 2018

I think we should also list some options that user can do to fix this:

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Left a comment, we have to remember to offer advice/info as to how to fix the issue if we're going to throw early like this.

// TODO: Add link to docs
reporter.panic(
`
"sharp" doesn't seem to have been built or installed correctly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some additional info here?

This doesn't seem like it'd be any more useful to a user as they go through debugging why this error is triggered?

e.g.

reporter.panic(
  reporter.stripIndent`
    The dependency "sharp" does not seem to have been built or installed correctly.
    Check out this guide for more info: https://gatsby.app/sharp-installation-issues
  `
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also note the link doesn't--and probably shouldn't--be something we maintain)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh absolutely, I left a TODO there to add them

I'll add these right after testing this on ubuntu 14.04 (that was the environment in the original issue)

Thanks for the comment!

@polarathene
Copy link
Contributor

In my case on alpine linux, building libvips for sharp was problematic. It's only available as a package in the edge repos, and compatibility to use that from stable release repos is unreliable(some existing alpine gatsby images ran into this issue with recent builds). Instead the alpine images are better off being built on either edge base or stable with node 8 or 10 where the sharp npm package supports pulling in binaries for many platforms/distro instead of trying to build from source. If a user decides to use current latest node v11 they'll probably miss out on the benefit of the binary support on v8 or v10, could be worth pointing out(the sharp npm package repo points this out though in their docs).

Still, in some cases I've run into the problem of running gatsby develop and even with the sharp.node file in a place an error states it could not locate the file, I was able to resolve it by adding bindings.js package to my package.json. It's a helper for finding bindings, might be worth mentioning too.

Only other issue I've run into is the supporting packages for jpeg/webp/png, but I think this is specific to alpine again and I need to look into it. I recall some errors, specifically I think mozjpeg failed to install, it built correctly but couldn't run autoconf(or similar filename, can't recall), if it's meant to be a package alpine doesn't seem to have it. When using local images and requesting fluid with webp fragment in graphql, my source jpeg files caused errors due to not being able to use mozjpeg. When I get time I'll investigate that further(if alpine specific will contribute that to the gatsby docker repo where a gatsby-cli docker container is in a current PR).

@sidharthachatterjee
Copy link
Contributor Author

Added better error messaging for this and asserted that it works fine if sharp.node isn't available due to a bad install. Wasn't able to reproduce it on Ubuntu 14.04 though (everything worked fine).

Nonetheless, this should be good to go for now.

@sidharthachatterjee sidharthachatterjee changed the title [WIP] fix(gatsby-plugin-sharp): bail early if sharp isn't working fix(gatsby-plugin-sharp): bail early if sharp isn't working Feb 26, 2019
@sidharthachatterjee
Copy link
Contributor Author

This renders the following

success open and validate gatsby-configs — 0.046 s
success load plugins — 1.688 s
⠁
      The dependency "sharp" does not seem to have been built or installed correctly.

      - Try to reinstall packages and look for errors during installation
      - Consult "sharp" installation page at http://sharp.pixelplumbing.com/en/stable/install/

      If neither of the above work, please open an issue in https://github.com/gatsbyjs/gatsby/issues

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were pairing on this and tried multiple ways to break installation of sharp package (without much success). Best we can do is to manually broke it afterwards and this message works for that case.

This is better than nothing and provide some context early without displaying cascade of errors that are caused by faulty sharp installation but also confusing as they hide root problem.

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.

4 participants