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

Unable to install package with meteor 1.6 #239

Open
sdarnell opened this issue Jun 6, 2017 · 16 comments
Open

Unable to install package with meteor 1.6 #239

sdarnell opened this issue Jun 6, 2017 · 16 comments

Comments

@sdarnell
Copy link

sdarnell commented Jun 6, 2017

OK, I know meteor 1.6 (1.6-alpha.1 to be precise) is at a very early stage, but it uncovers an issue with this package.

The problem is that the package includes an embedded copy of 'fibers' in the node_modules subdirectory, and this needs to be recompiled because meteor 1.6 has switched to node 8.x.

The first time around, the installation repeatedly tried to recompile but eventually gave up. I believe this is because the version of fibers is not the latest. For reference the first error is:

../src/fibers.cc:49:34: error: no template named 'WeakCallbackData' in namespace 'v8'; did you mean 'WeakCallbackInfo'?
   void WeakCallbackShim(const v8::WeakCallbackData<T, P>& data) {
   ~~~~^~~~~~~~~~~~~~~~
   WeakCallbackInfo
   /Users/steve/.node-gyp/8.0.0/include/node/v8.h:7723:16: note: 'WeakCallbackInfo' declared here
   friend class WeakCallbackInfo;
   ^

This is on macOS sierra 10.12.5 running with the command line tools (not full xcode).

If I hack out the fibers from the embedded node_modules things work.

@sebakerckhof
Copy link

Would it be a good idea to Meteor's new functionality and depend on the meteor-job package instead of using a git submodule?

@sdarnell
Copy link
Author

sdarnell commented Jun 6, 2017

I agree that a regular package dependency sounds as if it would be better, but I'm not sure how this error is related to the use of a git submodule?
The problem seems to be that due to the way the package is built, it includes fibers in the node_modules directory of the distributed package.

@sdarnell
Copy link
Author

sdarnell commented Jun 6, 2017

Aha... I see that the meteor-job package has a DEV dependency on fibers:

  "dependencies": {
  },
  "devDependencies": {
    "coffee-script": "^1.12.5",
    "mocha": "^3.2.0",
    "rewire": "^2.5.2",
    "sinon": "^1.17.7",
    "chai": "^3.5.0",
    "fibers": "^1.0.15"
  },

and all of these seem to be embedded in the resulting collections package.

@vsivsi
Copy link
Owner

vsivsi commented Jun 6, 2017

fibers is dev dependency because meteor-job supports running in node.js, outside of Meteor, but with Fibers. The dev dependency is required for code coverage of the Fibers dependent functionality that module's unit tests. That's literally all it is used for. Code dependent on the meteor-job package is 100% responsible for setting up its own Fibers environment (optionally, if it wishes to use Fibers...)

As for why meteor-job is included in this Atmosphere package as a git submodule, rather than as a vanilla npm package dependency... There is (or at least was) a good reason, but that was like 2.5 years ago, so the details escape me at the moment.

@vsivsi
Copy link
Owner

vsivsi commented Jun 6, 2017

Right, I remember now. This was literally issue #1 on this package.

The client side of JobCollection in Meteor also has a dependency on the meteor-job npm package, and prior to Meteor 1.4(?) it was not really possible to satisfy client dependencies with an npm package directly. My understanding is that works now (provided said package has no native compiled dependencies, of course).

I haven't changed this because it would break backward compatibility of this package with older versions of Meteor. I don't have time to maintain and merge bug fixes etc. into multiple release branches of this package for compatibility with every historical version of Meteor it once supported, so I've been very conservative making changes that aren't backward compatible.

@sdarnell
Copy link
Author

sdarnell commented Jun 6, 2017

OK, sounds fair enough.
It seems very strange that the dev dependency is leaking out into the final collections package distribution. I don't know whether that is a result of how it is built or how meteor's process for building packages works - but it seems very odd that any/all dev dependencies get shipped.

@vsivsi
Copy link
Owner

vsivsi commented Jun 6, 2017

If Meteor's package publishing process causes npm dev-only dependencies to be included, that is a bug in Meteor.

@vsivsi
Copy link
Owner

vsivsi commented Jun 6, 2017

Hmm, it's possible that maybe it picked it up because I had run unit tests out of the same directory I published from... Unfortunately my dev box is about 4000 miles away and I won't be back there for 2 weeks, so this will need to wait.

@sdarnell
Copy link
Author

sdarnell commented Jun 6, 2017

😄 I don't think it is urgent as meteor 1.6 is probably at least 2 weeks away, but I wanted to give you a heads up on the problem.

@hexsprite
Copy link

Any chance you can try a new release with a clean build @vsivsi? 1.6 is in beta.4 now.

@vsivsi
Copy link
Owner

vsivsi commented Jul 3, 2017

Yes, I'm planning a quick release in the next day or two.

@vsivsi
Copy link
Owner

vsivsi commented Jul 7, 2017

job-collection 1.5.2 is on Atmosphere now. I have verified that it was published without any dev dependencies installed in the meteor-job submodule (including fibers), so hopefully this will clear-up the Meteor 1.6-rc install issues you all have been seeing.

@vsivsi
Copy link
Owner

vsivsi commented Jul 17, 2017

I suspect this release will fix the Windows 10 install issues people have been encountering as well...
#231

@hexsprite
Copy link

I'll test this with my app shortly and let you know

@hexsprite
Copy link

works for me with 1.6 beta 10. thanks!

@SimonSimCity
Copy link

Can confirm it as well for 1.6 beta 13, the latest beta-release as of today.

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

No branches or pull requests

5 participants