vsivsi / meteor-job-collection

A persistent and reactive job queue for Meteor, supporting distributed workers that can run anywhere.
https://atmospherejs.com/vsivsi/job-collection
Other
388 stars 68 forks source link

Unable to install package with meteor 1.6 #239

Open sdarnell opened 7 years ago

sdarnell commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

vsivsi commented 7 years ago

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 commented 7 years ago

😄 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 commented 7 years ago

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

vsivsi commented 7 years ago

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

vsivsi commented 7 years ago

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 commented 7 years ago

I suspect this release will fix the Windows 10 install issues people have been encountering as well... https://github.com/vsivsi/meteor-job-collection/issues/231

hexsprite commented 7 years ago

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

hexsprite commented 7 years ago

works for me with 1.6 beta 10. thanks!

SimonSimCity commented 7 years ago

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