Automatic detection of NPM binaries

[moufmouf]

Hi @nsams ,

Following your comment on #7, here is a proposal to add the feature you suggested: automatically detecting NPM binaries and creating a script in composer's binary directory to launch them.

I've tested with gulp on Windows and Linux, and it seems to work great :)

My composer.json test file:

{
      "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/moufmouf/composer-extra-assets"
        }
    ],
  "require": {
    "koala-framework/composer-extra-assets": "dev-master"
  },
  "extra": {
    "require-npm": {
      "gulp": "*"
    }
  },
  "minimum-stability": "dev"
}

Run gulp with: vendor/bin/gulp

A quick note about a detail in the implementation: I integrated in the LinkWriter class a piece of code from Symfony's FileSystem component (rather than adding a Composer dependency on Symfony's FileSystem). This is the function to find a relative path from an absolute path. I did this because I want composer-extra-assets to be compatible with any package (including future versions of Symfony, like Symfony 3, that might put different constraints on the symfony/filsesystem package).

[nsams]

Fantastic! I didn't have time to test so far, but will in the coming week.

One question: I think it is enough to create links for root package dependencies, is your code ensuring that? (npm also does it that way)

[nsams]

I just tested it, working great so far.

Two things:

• I would only create links for root package dependencies (above question)
• local installed node is not used

[moufmouf]

Hi Niko,

local installed node is not used

I'm trying to reproduce but on my environment, local node seems to be used. Are you sure it is not used? What OS are you performing your test on? Could you give me steps so I can reproduce the problem?

I would only create links for root package dependencies (above question)

What is the rationale for this? If I understand correctly, you want NPM binaries from Composer dependencies to be hidden, is that it? Are there no cases where we might want those binaries to be exposed? For instance, I'd like to build a mouf/getting-started-with-gulp composer package that installs gulp in vendor/bin/gulp and also offers a friendly gulp.js file generator to get started using gulp. That seems to me a valid use case to expose more than just the root package binaries. Another solution if you want to keep only root binaries by default: could we put a flag in composer.json extra section that let's a package expose the bin dependencies in vendor/bin directory?

For instance:

{
  "extra": {
    "require-npm": {
      "gulp": "*"
    },
    "expose-npm-binaries": true
}

[nsams]

local installed node is not used

I'm trying to reproduce but on my environment, local node seems to be used. Are you sure it is not used? What OS are you performing your test on? Could you give me steps so I can reproduce the problem?

Sorry it is working correctly. my fault. (Difficult to test this...)

I would only create links for root package dependencies (above question)

What is the rationale for this? If I understand correctly, you want NPM binaries from Composer dependencies to be hidden, is that it?

Well, I just thought binaries I'm not interested in (as it's internal stuff used by a composer package) should not crowd vendor/bin.

I do however understand your use case. And composer also does the same, npm doesn't though.

If you are happy with a "expose-npm-binaries" I'm all for it.

[moufmouf]

Hi @nsams ,

Ok, I tried to add support for the "expose-npm-binaries" parameter (https://github.com/moufmouf/composer-extra-assets/commit/3dd04841a9f246bec6a82da23283152b6ce06922). I just realized this is a deadend.

Indeed, in order to run "gulp", not only gulp must be available in vendor/bin/gulp, but also, the gulp files must be available in [root]/node_modules/gulp.

Here is my test:

{
"repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/thecodingmachine/gulp-installer"
        },
        {
            "type": "vcs",
            "url": "https://github.com/moufmouf/composer-extra-assets"
        }
    ],
    "require": {
        "mouf/gulp-installer": "~1.0",
        "koala-framework/composer-extra-assets": "dev-master as 1.2.x-dev"
    },
    "extra": {
        "require-npm": {
            "gulp-load-plugins": "~0.8.0",
            "main-bower-files": "~2.5.0",
            "gulp-filter": "~2.0.1",
            "gulp-concat": "~2.4.3",
            "gulp-uglify": "~1.1.0",
            "gulp-sourcemaps": "~1.3.0",
            "gulp-livereload": "~3.7.0",
            "gulp-order": "~1.1.1",
            "gulp-minify-css": "~0.4.5",
            "gulp-less": "~3.0.0",
            "run-sequence": "~1.0.2"
        }
    },
"minimum-stability": "dev"
}

The mouf/gulp-installer is a simple package that contains only a composer.json file with a dependency on gulp.

Now, put a gulpfile.js with:

var gulp = require('gulp');

Run vendor/bin/gulp, you will get:

[15:15:01] Local gulp not found in ~/projects/mouf/gulp-installer-test
[15:15:01] Try running: npm install gulp

I will try to change the "expose-npm-binaries" into a "expose-npm-package" that simply adds the package into the root package.json if you are ok with this.

[nsams]

Ok, I understand your troubles.

Thing is that this works currently like npm does. You would face the exact same issue when creating a gulp-installer npm package.

Why do we need a gulp-installer package at all? Given that composer-extra-assets installs vendor/bin/gulp script - it's already installed and fully working.

You wrote about helper scripts for creating gulp.js - why not implement that as own package that doesn't depend on gulp itself - giving the root package the possibility for it's own gulp version constraint. (Downside is obviously that you need gulp-helper-scripts dependency plus gulp dependency in root package)

[moufmouf]

(Downside is obviously that you need gulp-helper-scripts dependency plus gulp dependency in root package)

Yes, that's exactly my point. You would need both "gulp-helper-scripts" and "gulp" in root package which defeats the purpose of having dependencies at all (in my opinion).

For instance, I could think of a scenario where my "gulp-helper-scripts" could be requested by another package, that would itself be a dependency of another package... and I don't want the end user of the root package to have to declare the dependency on all NPM modules that might be needed in all the packages...

On the other end, I completely understand you argument that this is currently like npm does. I guess we are at the crossing of 2 dependency managers (Composer and NPM) and we need to choose which one we want to implement.

I just submitted another commit (https://github.com/moufmouf/composer-extra-assets/commit/47e3768bdaa77e56ec2ecccd2d5583d745e6331e) that removes the "expose-npm-binaries" and adds a "expose-npm-package". This can be great with gulp I can have a bunch of dependencies declared in various files, and still use a "regular" gulpfile.js (I believe including using "gulp-load-plugins" that detects gulp plugins in node_modules, altough I did not test that yet).

You might want to have a look at the mergeNpmVersions method that merges dependencies version correctly and that could be reused on the Bower part too.

What do you think? Is it so weird?

[nsams]

Generally I would say if this feature is optional (and even disabled by default) it won't hurt anyone - and usecases like yours can be implemented.

How does mergeNpmVersions work? What if two packages require two different (possibly incompatible) versions?

The great thing about npm is that it doesn't have this "dependency hell" at all as each package can have it's own dependencies.

[moufmouf]

About mergeNpmVersions, it's kind of very simple. I'm simply appending versions of the same package (which does a "AND" by default).

For instance,

{
  "gulp": ">=1.0"
}

and

{
  "gulp": ">=1.2"
  "gulp-less": "~1.2.3"
}

will be merged into:

{ "gulp": ">=1.0 >=1.2" "gulp-less": "~1.2.3" }

This looks "weird" but is perfectly valid :) If there are conflicting requirements, we can rely on NPM to tell us.

[nsams]

wow, what a great and simple idea!

I just tested with bower - and we can also use it there - and dump the VersionMatcher.

[nsams]

....so how do we continue with this PR?

[moufmouf]

Good question. This PR brings an answer to my use case. The behaviour is optional, and disabled by default, so you have to really want to enable a special option to get NPM dependencies merged into the main packages.json file. So I think it is as good as it can be.

Do you see any thing I could do to improve this PR? Are you ok to merge it or would you rather not do it because you think it is too far away from the "normal" way NPM works?

[nsams]

thanks a lot...