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

npm install error: cairo not found #3515

Open
mojoaxel opened this Issue Oct 2, 2017 · 26 comments

Comments

Projects
None yet
4 participants
@mojoaxel
Member

mojoaxel commented Oct 2, 2017

Since #3262 i get the following error trying npm install on linux:

> canvas@1.6.7 install /home/delphin/workspaces/workspace_vis/vis/node_modules/canvas
> node-gyp rebuild

gyp WARN download NVM_NODEJS_ORG_MIRROR is deprecated and will be removed in node-gyp v4, please use NODEJS_ORG_MIRROR
gyp WARN download NVM_NODEJS_ORG_MIRROR is deprecated and will be removed in node-gyp v4, please use NODEJS_ORG_MIRROR
Package cairo was not found in the pkg-config search path.
Perhaps you should add the directory containing `cairo.pc'
to the PKG_CONFIG_PATH environment variable
No package 'cairo' found
gyp: Call to './util/has_lib.sh freetype' returned exit status 0 while in binding.gyp. while trying to load binding.gyp
...

@mojoaxel mojoaxel added this to the Minor Release v4.21 milestone Oct 2, 2017

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

How about this?

@mojoaxel Please note that this is a 'confirmed bug' on your system. I haven't heard others complaining about it (perhaps they did a manual install of cairo?), but most importantly, it works out of the box on the travis unit tests.

Contributor

wimrijnders commented Oct 2, 2017

How about this?

@mojoaxel Please note that this is a 'confirmed bug' on your system. I haven't heard others complaining about it (perhaps they did a manual install of cairo?), but most importantly, it works out of the box on the travis unit tests.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

@mojoaxel I saw the assignment. How can I solve this if it works on my linux? I would think the best person to fix it is you, since it happens on yours.

Contributor

wimrijnders commented Oct 2, 2017

@mojoaxel I saw the assignment. How can I solve this if it works on my linux? I would think the best person to fix it is you, since it happens on yours.

@mojoaxel mojoaxel assigned mojoaxel and wimrijnders and unassigned wimrijnders Oct 2, 2017

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

How about disabling the specific unit tests of the canvas is not present under node.js? This would at least prevent problems with running them locally.

Edit: This can be done. Also see the further solutions there.

Contributor

wimrijnders commented Oct 2, 2017

How about disabling the specific unit tests of the canvas is not present under node.js? This would at least prevent problems with running them locally.

Edit: This can be done. Also see the further solutions there.

@mojoaxel

This comment has been minimized.

Show comment
Hide comment
@mojoaxel

mojoaxel Oct 2, 2017

Member

perhaps they did a manual install of cairo?

Of course this fixes the problem. But still this is an external dependency. How do you plan to communicated this to the user?

As far as I am concerned we should absolut not use node-canvasas long it has this external dependency. As long as there is no node-cairo I think we should look around for other test-solutions.

Member

mojoaxel commented Oct 2, 2017

perhaps they did a manual install of cairo?

Of course this fixes the problem. But still this is an external dependency. How do you plan to communicated this to the user?

As far as I am concerned we should absolut not use node-canvasas long it has this external dependency. As long as there is no node-cairo I think we should look around for other test-solutions.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

I disagree. It's supremely useful for the unit tests and I consider those very important.

But I am willing to consider canvas alternatives for node.js, do you know any?

Contributor

wimrijnders commented Oct 2, 2017

I disagree. It's supremely useful for the unit tests and I consider those very important.

But I am willing to consider canvas alternatives for node.js, do you know any?

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

But still this is an external dependency.

There are setups which can handle the dependency automatically. How about checking out how that works? See also link above about pkgconfig.

Contributor

wimrijnders commented Oct 2, 2017

But still this is an external dependency.

There are setups which can handle the dependency automatically. How about checking out how that works? See also link above about pkgconfig.

@mojoaxel

This comment has been minimized.

Show comment
Hide comment
@mojoaxel

mojoaxel Oct 2, 2017

Member

How about disabling the specific unit tests of the canvas is not present under node.js? This would at least prevent problems with running them locally.

This could be a workaround, but developers still must be informed that Cairo is an requirement to develop tests. To be honest i'm not happy with this dependency but I also do not have the time at the moment to develop a better test solution. 😿

I propose: Let's treat this issue as a blocker (no new releases until we found a solution) and think about how to fix this. My preferred solution would be to use a ui-testing framework that did not have any non-npm dependencies, but as I said time is an issue...

Member

mojoaxel commented Oct 2, 2017

How about disabling the specific unit tests of the canvas is not present under node.js? This would at least prevent problems with running them locally.

This could be a workaround, but developers still must be informed that Cairo is an requirement to develop tests. To be honest i'm not happy with this dependency but I also do not have the time at the moment to develop a better test solution. 😿

I propose: Let's treat this issue as a blocker (no new releases until we found a solution) and think about how to fix this. My preferred solution would be to use a ui-testing framework that did not have any non-npm dependencies, but as I said time is an issue...

@mojoaxel mojoaxel added the PRIORITY label Oct 2, 2017

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

This could be a workaround, but developers still must be informed that Cairo is an requirement to develop test.

This can be combined. Disable the tests by default, in the README explain that in order to run them, you will need to install cairo. That way, you have a non-blocking setup that can be extended.

... i also do not have the time at the moment to develop a better test solution. 😿

That's because there isn't any. I was hoping you would take the hint.

Let's treat this issue as a blocker

No. But see my solution at the top of this comment.

Contributor

wimrijnders commented Oct 2, 2017

This could be a workaround, but developers still must be informed that Cairo is an requirement to develop test.

This can be combined. Disable the tests by default, in the README explain that in order to run them, you will need to install cairo. That way, you have a non-blocking setup that can be extended.

... i also do not have the time at the moment to develop a better test solution. 😿

That's because there isn't any. I was hoping you would take the hint.

Let's treat this issue as a blocker

No. But see my solution at the top of this comment.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

everyone: I'd like to know if you installed cairo by hand to get the unit tests to work. Is this issue also an issue for you?

Contributor

wimrijnders commented Oct 2, 2017

everyone: I'd like to know if you installed cairo by hand to get the unit tests to work. Is this issue also an issue for you?

@mojoaxel

This comment has been minimized.

Show comment
Hide comment
@mojoaxel

mojoaxel Oct 2, 2017

Member

See also link above about pkdconfig.

As I sayed: Installing Cairo (as descripe here) would solve this issue for me, of course. But I'm very sure this is a big issue for anybody who wants to develop vis.js and I really don't think that this kind of dependencies are a good idea!

Member

mojoaxel commented Oct 2, 2017

See also link above about pkdconfig.

As I sayed: Installing Cairo (as descripe here) would solve this issue for me, of course. But I'm very sure this is a big issue for anybody who wants to develop vis.js and I really don't think that this kind of dependencies are a good idea!

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

Your point is noted; however, consider all the things you need to do to develop for vis.js in the first place. In the big picture, it's not such a big deal.

Is the solution, interim or not, of disabling cairo-based tests accetable? And that they will run if cairo is installed?

Contributor

wimrijnders commented Oct 2, 2017

Your point is noted; however, consider all the things you need to do to develop for vis.js in the first place. In the big picture, it's not such a big deal.

Is the solution, interim or not, of disabling cairo-based tests accetable? And that they will run if cairo is installed?

@mojoaxel

This comment has been minimized.

Show comment
Hide comment
@mojoaxel

mojoaxel Oct 2, 2017

Member

Is the solution, interim or not, of disabling cairo-based tests accetable? And that they will run if cairo is installed?

Yes of course! I'm absolutely ok with treating this cairo-based tests as a interims solution untill we have a better testing framework. But than the cairo dependency, and how to install it, must be really well documented.

Member

mojoaxel commented Oct 2, 2017

Is the solution, interim or not, of disabling cairo-based tests accetable? And that they will run if cairo is installed?

Yes of course! I'm absolutely ok with treating this cairo-based tests as a interims solution untill we have a better testing framework. But than the cairo dependency, and how to install it, must be really well documented.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

Documenting this is something which I do not contest; I hope that is clear.

What I do react against is the implication that cairo should not be used at all and that production should be blocked until this is somehow resolved. IMHO, way over the top.

And the fact that you seem to be the only having a problem with installing cairo and feel that this is sufficient to make all the above statements. But I am waiting to be corrected by others on this point.

Contributor

wimrijnders commented Oct 2, 2017

Documenting this is something which I do not contest; I hope that is clear.

What I do react against is the implication that cairo should not be used at all and that production should be blocked until this is somehow resolved. IMHO, way over the top.

And the fact that you seem to be the only having a problem with installing cairo and feel that this is sufficient to make all the above statements. But I am waiting to be corrected by others on this point.

@yotamberk

This comment has been minimized.

Show comment
Hide comment
@yotamberk

yotamberk Oct 2, 2017

Contributor

I haven't had any problems with Cairo.
I'm with @wimrijnders about not blocking it. We can explain that for now, in-order of running tests, Cairo is required. I really don't think it's a show stopper that requires stopping any releases.

Contributor

yotamberk commented Oct 2, 2017

I haven't had any problems with Cairo.
I'm with @wimrijnders about not blocking it. We can explain that for now, in-order of running tests, Cairo is required. I really don't think it's a show stopper that requires stopping any releases.

@mojoaxel

This comment has been minimized.

Show comment
Hide comment
@mojoaxel

mojoaxel Oct 2, 2017

Member

OK, that please add documentation that Cairo is now a requirement for vis.js development.
I'll see that I find a better solution for the future...

Member

mojoaxel commented Oct 2, 2017

OK, that please add documentation that Cairo is now a requirement for vis.js development.
I'll see that I find a better solution for the future...

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

Thanks for support. I've cooled down a bit now. While I don't think it's a show-stopper either, I appreciate that @mojoaxel's core points are valid. My interpretation:

  • The unit tests should run out of the box with a default install - perhaps not all of them run, but at least the developer does not get slapped around with unexpected errors.
  • Developers should be able to find out what to do to enable full unit tests

I believe that this is the least intrusive. My consideration are:

  • Users of vis.js do not have to deal with this at all (duh).
  • Any developer not working with Network does not have to do anything special.
  • Any developer working with Network may or may not have to enable them. It just depends on what they're working on
  • Travis has no problems at all with running cairo. This is the most important for me

Still I would like to know from more people if they had to manually install cairo.

Contributor

wimrijnders commented Oct 2, 2017

Thanks for support. I've cooled down a bit now. While I don't think it's a show-stopper either, I appreciate that @mojoaxel's core points are valid. My interpretation:

  • The unit tests should run out of the box with a default install - perhaps not all of them run, but at least the developer does not get slapped around with unexpected errors.
  • Developers should be able to find out what to do to enable full unit tests

I believe that this is the least intrusive. My consideration are:

  • Users of vis.js do not have to deal with this at all (duh).
  • Any developer not working with Network does not have to do anything special.
  • Any developer working with Network may or may not have to enable them. It just depends on what they're working on
  • Travis has no problems at all with running cairo. This is the most important for me

Still I would like to know from more people if they had to manually install cairo.

@justinharrell

This comment has been minimized.

Show comment
Hide comment
@justinharrell

justinharrell Oct 2, 2017

Contributor

On Windows I had to manually install GTK into c:\GTK before vis would npm install which to took me some time to track down and figure out, I also got lucky because I already have Visual Studio installed, without that I don't believe it will build.

I eventually tracked down the docs for windows here: https://github.com/Automattic/node-canvas/wiki/Installation---Windows

This was not a great experience since there was no mention of it in the docs and a little frustrating to have a native module dependency blocking me from working on vis that was only used in the test framework not even needed for the library itself.

I always dread having to deal with a native node module on windows, getting it to build is hardly ever painless.

Contributor

justinharrell commented Oct 2, 2017

On Windows I had to manually install GTK into c:\GTK before vis would npm install which to took me some time to track down and figure out, I also got lucky because I already have Visual Studio installed, without that I don't believe it will build.

I eventually tracked down the docs for windows here: https://github.com/Automattic/node-canvas/wiki/Installation---Windows

This was not a great experience since there was no mention of it in the docs and a little frustrating to have a native module dependency blocking me from working on vis that was only used in the test framework not even needed for the library itself.

I always dread having to deal with a native node module on windows, getting it to build is hardly ever painless.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

The keyword here is 'frustrating'. OK, noted. In fact, having just one more person dealing with this is enough motivation for me to take preventive measures.

Anyone else had a bad time with cairo?

Contributor

wimrijnders commented Oct 2, 2017

The keyword here is 'frustrating'. OK, noted. In fact, having just one more person dealing with this is enough motivation for me to take preventive measures.

Anyone else had a bad time with cairo?

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

OK figured out how to skip tests if canvas not installed. In the main describe-block of the Network unit tests:

describe('Network', function () {
  before(function() {
...
    var ctx = window.document.createElement('canvas').getContext('2d');
    this.canvasPresent = (typeof ctx !== 'undefined' && ctx !== null);

    if (!this.canvasPresent) this.skip();
...
  });
...

And in every contained describe block:

describe('Network', function () {
  before(function() {
    if (!this.canvasPresent) this.skip();
  });
...

The interesting thing here is that the unit tests are still displayed, but with a different color.
This is actually not a bad thing; it's an indication that something out of the ordinary is going on.

Then, with some strategically placed documentation, a developer can figure out for his/herself WTF is happening. And then know if this is something to worry about or not.


Next up: how to not installl canvas by default, except when running on Travis. Suggestions are welcome.

Contributor

wimrijnders commented Oct 2, 2017

OK figured out how to skip tests if canvas not installed. In the main describe-block of the Network unit tests:

describe('Network', function () {
  before(function() {
...
    var ctx = window.document.createElement('canvas').getContext('2d');
    this.canvasPresent = (typeof ctx !== 'undefined' && ctx !== null);

    if (!this.canvasPresent) this.skip();
...
  });
...

And in every contained describe block:

describe('Network', function () {
  before(function() {
    if (!this.canvasPresent) this.skip();
  });
...

The interesting thing here is that the unit tests are still displayed, but with a different color.
This is actually not a bad thing; it's an indication that something out of the ordinary is going on.

Then, with some strategically placed documentation, a developer can figure out for his/herself WTF is happening. And then know if this is something to worry about or not.


Next up: how to not installl canvas by default, except when running on Travis. Suggestions are welcome.

@justinharrell

This comment has been minimized.

Show comment
Hide comment
@justinharrell

justinharrell Oct 2, 2017

Contributor

If you are not worried about testing the canvas output, something like this would be much lightier with no native dependencies: https://github.com/Cristy94/canvas-mock

Contributor

justinharrell commented Oct 2, 2017

If you are not worried about testing the canvas output, something like this would be much lightier with no native dependencies: https://github.com/Cristy94/canvas-mock

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

Thanks for link, will seriously consider that. However, some minimal behavior should be present.
OTOH, it's really basic. It's not overkill to copy over the idea and suit it for own purposes.

Contributor

wimrijnders commented Oct 2, 2017

Thanks for link, will seriously consider that. However, some minimal behavior should be present.
OTOH, it's really basic. It's not overkill to copy over the idea and suit it for own purposes.

@justinharrell

This comment has been minimized.

Show comment
Hide comment
@justinharrell

justinharrell Oct 2, 2017

Contributor

https://github.com/gliffy/canvas2svg might also be an option that actually does produce an artifact that can be viewed. It does mention running in node and may not need node-canvas with newer versions of jsdom and it has not native dependencies.

Contributor

justinharrell commented Oct 2, 2017

https://github.com/gliffy/canvas2svg might also be an option that actually does produce an artifact that can be viewed. It does mention running in node and may not need node-canvas with newer versions of jsdom and it has not native dependencies.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

I take my hat off to this plug, which as it happens is entirely appropriate. You see a way to make it work?

Contributor

wimrijnders commented Oct 2, 2017

I take my hat off to this plug, which as it happens is entirely appropriate. You see a way to make it work?

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

Come on guys! One more person with a bad experience on installing cairo, and I'll put back the Priority label myself. Bring it on!

Contributor

wimrijnders commented Oct 2, 2017

Come on guys! One more person with a bad experience on installing cairo, and I'll put back the Priority label myself. Bring it on!

@justinharrell

This comment has been minimized.

Show comment
Hide comment
@justinharrell

justinharrell Oct 2, 2017

Contributor

I will not lie it would make me happy to see vis.js be dependent on canvas2svg, it just so happens one side effect of doing svg is you don't need a native drawing library to produce it, its just text :).

I don't fully understand why they mention node-canvas being needed on older jsdoms and recommend otherwise, that would be the main issue to figure out, otherwise I already know canvas2svg works pretty well with vis.js.

Contributor

justinharrell commented Oct 2, 2017

I will not lie it would make me happy to see vis.js be dependent on canvas2svg, it just so happens one side effect of doing svg is you don't need a native drawing library to produce it, its just text :).

I don't fully understand why they mention node-canvas being needed on older jsdoms and recommend otherwise, that would be the main issue to figure out, otherwise I already know canvas2svg works pretty well with vis.js.

@wimrijnders

This comment has been minimized.

Show comment
Hide comment
@wimrijnders

wimrijnders Oct 2, 2017

Contributor

I am fully prepared to go with your project, if only because you've amply proven to me that you're a capable developer. Your PR's thus far inspire plenty confidence.

From another viewpoint, we need to make it work anyway for the upcoming SVG export, so it doesn't feel like additional work to me. I suppose clearing out the drawing routines grafted into canvas (shapes.js) applies here as well?

Hm. I'm jumping the gun here as usual. I propose first getting this, err, 'mess' cleared out. Then get the svg export on the roll, then see if it can be used for transplanting canvas. I'm warming up to the canvas-dummy idea now. I've actually been using something of the sort for 2d-context in the Label unit tests.

Contributor

wimrijnders commented Oct 2, 2017

I am fully prepared to go with your project, if only because you've amply proven to me that you're a capable developer. Your PR's thus far inspire plenty confidence.

From another viewpoint, we need to make it work anyway for the upcoming SVG export, so it doesn't feel like additional work to me. I suppose clearing out the drawing routines grafted into canvas (shapes.js) applies here as well?

Hm. I'm jumping the gun here as usual. I propose first getting this, err, 'mess' cleared out. Then get the svg export on the roll, then see if it can be used for transplanting canvas. I'm warming up to the canvas-dummy idea now. I've actually been using something of the sort for 2d-context in the Label unit tests.

wimrijnders added a commit to wimrijnders/vis that referenced this issue Oct 3, 2017

Use mock canvas object replacing `canvas`
Fixes #3515.

A mock canvas object is added to the unit tests, which makes usage of module `canvas` voluntary.

The issue with `canvas` is that it requires an external dependency to `cairo`. This complicates setting up a develop environment for `vis.js`

- Removed `canvas` from `package.json`
- Added section to README.md with instructions on how to install `canvas` instead.

yotamberk added a commit that referenced this issue Oct 13, 2017

Use mock canvas object replacing `canvas` (#3518)
* Use mock canvas object replacing `canvas`

Fixes #3515.

A mock canvas object is added to the unit tests, which makes usage of module `canvas` voluntary.

The issue with `canvas` is that it requires an external dependency to `cairo`. This complicates setting up a develop environment for `vis.js`

- Removed `canvas` from `package.json`
- Added section to README.md with instructions on how to install `canvas` instead.

* Removed debugger statements

* Updates for review

* Fixes for review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment