Merge pull request #20 from dominykas/cycles

Support cycles

.travis.yml

@@ -5,6 +5,10 @@ node_js:
   - "10"
   - "node"
 
+install:
+  - "npm install --ignore-scripts"
+  - "node bin/allow-scripts.js"
+
 jobs:
   include:
     - stage: release

lib/index.js

@@ -6,50 +6,31 @@ const Fs = require('fs');
 const Npm = require('libnpm');
 const Path = require('path');
 const Semver = require('semver');
-const Topo = require('topo');
 
 
 const internals = {};
 
 
-internals.scan = (tree, parent, map = new Map(), scanned = new Set()) => {
+internals.queue = (tree, queue = [], scanned = new Set()) => {
 
     for (const [, v] of tree.dependencies) {
 
-        if (v.hasCycle()) {
-            console.warn(`==========> skip ${v.path()} (because it has a cycle in dependencies)`);
-            continue;
-        }
-
         const path = v.path();
-        if (!map.has(path)) {
-            map.set(path, []);
-        }
-
-        const node = map.get(path);
-        node.push(parent);
 
-        if (!scanned.has(v)) {
-            scanned.add(v);
-            internals.scan(v, v.path(), map, scanned);
+        if (scanned.has(path)) {
+            // prevent cycles
+            continue;
         }
-    }
-
-    return map;
-};
 
-
-internals.queue = (tree) => {
-
-    const map = internals.scan(tree);
-    const topo = new Topo();
-    for (const [group, before] of map) {
-        topo.add(group, { group, before });
+        scanned.add(path);
+        internals.queue(v, queue, scanned);
+        queue.push(v.path()); // adding deepest deps into the queue first
     }
 
-    return topo.nodes;
+    return queue;
 };
 
+
 internals.runScript = (stage, { pkg, path, cwd, unsafePerm }, options) => {
 
     if (!pkg.scripts || !pkg.scripts[stage]) {

package.json

@@ -18,8 +18,7 @@
   "dependencies": {
     "ansicolors": "0.3.x",
     "libnpm": "2.x.x",
-    "semver": "6.x.x",
-    "topo": "3.x.x"
+    "semver": "6.x.x"
   },
   "devDependencies": {
     "@commitlint/cli": "7.x.x",

test/fixtures/cycle-a.json

@@ -4,5 +4,8 @@
   "version": "0.0.0",
   "dependencies": {
     "@example/cycle-b": "*"
+  },
+  "scripts": {
+    "install": "echo install from cycle-a >> ${OUTPUT}"
   }
 }

test/fixtures/cycle-b.json

@@ -3,6 +3,9 @@
   "name": "@example/cycle-b",
   "version": "0.0.0",
   "dependencies": {
-    "@example/cycle-a": "*"
+    "@example/cycle-c": "*"
+  },
+  "scripts": {
+    "install": "echo install from cycle-b >> ${OUTPUT}"
   }
 }

test/fixtures/cycle-c.json

@@ -0,0 +1,11 @@
+{
+  "private": true,
+  "name": "@example/cycle-c",
+  "version": "0.0.0",
+  "dependencies": {
+    "@example/cycle-a": "*"
+  },
+  "scripts": {
+    "install": "echo install from cycle-c >> ${OUTPUT}"
+  }
+}

test/fixtures/index.js

@@ -19,7 +19,8 @@ internals.readFile = (path) => Fs.readFileSync(path).toString().trim();
 exports.expectedResults = {
     basicFull: internals.readFile(Path.join(__dirname, 'basic.full.txt')),
     basicDryRun: internals.readFile(Path.join(__dirname, 'basic.dry-run.txt')),
-    deep: internals.readFile(Path.join(__dirname, 'deep.txt'))
+    deep: internals.readFile(Path.join(__dirname, 'deep.txt')),
+    withCycles: internals.readFile(Path.join(__dirname, 'with-cycles.txt'))
 };
 
 

test/fixtures/with-cycles.json

@@ -8,6 +8,9 @@
     "@example/with-install-script": "*"
   },
   "allowScripts": {
-    "@example/with-install-script": "*"
+    "@example/with-install-script": "*",
+    "@example/cycle-a": "*",
+    "@example/cycle-b": "*",
+    "@example/cycle-c": "*"
   }
 }

test/fixtures/with-cycles.txt

@@ -0,0 +1,4 @@
+install from cycle-c
+install from cycle-b
+install from cycle-a
+install from with-install-script

test/index.js

@@ -88,14 +88,13 @@ describe('allow-scripts', () => {
             const fixture = Fixtures.setup('with-cycles', [
                 'cycle-a',
                 'cycle-b',
+                'cycle-c',
                 'with-install-script'
             ]);
 
             await Allow.run({});
 
-            expect(fixture.getActualResult()).to.equal('install from with-install-script');
-            expect(fixture.getLog()).to.contain('skip node_modules/@example/cycle-a (because it has a cycle in dependencies)');
-            expect(fixture.getLog()).to.contain('skip node_modules/@example/cycle-b (because it has a cycle in dependencies)');
+            expect(fixture.getActualResult()).to.equal(Fixtures.expectedResults.withCycles);
         });
 
         it('executes allowed scripts (existing shrinkwrap)', async () => {