Re: Improved NPM importer with blacklist

[swedebugia <swedebugia@riseup.net>]

[-- Attachment #1: Type: text/plain, Size: 4946 bytes --]

On 2018-11-30 17:24, Julien Lepiller wrote:
> Le 2018-11-30 17:13, swedebugia a écrit :

snip

> Hi,
> 
> I never used the recursive importer, so I didn't know it wasn't very good.
> 
> I wonder if we really need to import every version of the packages. That 
> doesn't seem very practical. There are a few cyclic dependencies issues 
> in Java packages too, and they are dealt with in a case-by-case basis. 
> Most often, we made a degraded version of one of the packages, the 
> second can use to build itself, then we rebuild the first with the 
> second package.

Sounds good.

> Sometimes, we also have to adapt some of our packages for the newer 
> versions of the dependencies we have. If we didn't, we'd have a lot of 
> versions of every package, and most of them would be outdated, probably 
> buggy or contain security holes. I'd prefer using the latest versions of 
> dependencies, and contribute patches back to upstream, so they can use 
> the latest and greatest too :)
> 
> That's obviously a lot more work, but that's also probably a saner way 
> of doing things.

Agreed, this seems better. With a good tree browser we can probably 
avoid importing more than 2-5 versions of the worst packages.

I collected a few cyclic devdeps. See attached. (these definitions is of 
little value as the versions of deps and devdeps are discarded)

>> TODO:
>> * make npm-recursive-import work by not fetching blacklisted packages
> 
> Let's be careful though: we don't want to fetch blacklisted packages 
> when they are devDependencies, but we still want them if they are 
> runtime dependencies.

Totally agree. This is exactly why I only implemented blacklisting of 
native-inputs.

>> * implement keyword blacklisting based on the descriptions
> 
> We can probably use tags instead of the description : '("test" "testing" 
> "check" "doc" "coverage" "unit") seem like a good approximation of what 
> we want to blacklist.

Fewer that half the npm packages have tags to my knowledge. We can do 
both though :D

> 
>>
>> * match not just the whole string of blacklisted packages:
>>   e.g. match also "rollup-plugin" when "rollup" is in the blacklist.
>>
>> * get the tarballs from npm-registry instead as they are never missing
>>   (githubs sometimes are) and likely reproducible.
> 
> Are they actual source tarballs, or are they somewhat different than the 
> source used to build the "binary" npm package? With maven (for java) for 
> instance, some sources are hosted, but they aren't supposed to be used 
> to build the package, they're only here for the debugger.

Fortunately it seems it is the full source. :D
See
https://registry.npmjs.org/underscore/-/underscore-1.9.1.tgz
https://registry.npmjs.org/nodeunit/-/nodeunit-0.5.5.tgz
https://registry.npmjs.org/async/-/async-0.9.0.tgz

> 
>>
>> * Output a (define-public <guixname> (inherit <guixname>-<version>)) for
>>   all imported npm-packages.
> 
> I don't think that's a good idea: if we have multiple versions of a 
> package, we'll have multiple <guixname> packages...

Ok, got it. I thought the define-publics would collide, but I guess not.

> 
>>
>> * Make it possible to specify a specific version to import (and perhaps
>>   the latest of all minor versions of a package :D).
>> (For async that would be "0.1.22", "0.2.10", "0.3.0", etc all the way
>> up to "2.6.1" which is the current beast. This would mean that we in
>> total import about 477.000 packages times the number of minor releases
>> (mean ~10?) that equals 4,7 mio. npm-packages :p) Then we will
>> definitely need to speed up guile. My guess is that we will have to
>> import at least 1,5 versions for every npm package to mitigate cyclic
>> dependencies (this means 477.000*1,5 = 715.500 npm-package-versions).
> 
> Again, I'm more in favor of patching them, rather than importing more 
> versions. Do we still have as many cyclic deps with the blacklist?

No, the blacklist makes a BIG difference (but only to the cycdevdeps. 
The deps still introduce just as many cycles. These can be avoided by 
carefully choosing a version just before the cycdep was added :) (or by 
patching but I know nothing about JS so I leave that to others)

>> * Make it easy to analyze a given npm-package to see when deps/devdeps
>> were added. In the case async, propose we import 0.9.0 first which is
>> the last version without lodash as devdep. From 1.0.0 more devdeps
>> were added. (source: https://registry.npmjs.org/async)
>>
>> Perhaps some kind of tree output for these complex packages with
>> versions as branches and dependencies as subbranches would be nice?

I will try parsing the registry to output something intelligently to 
help the user choose which version to import.

> Thanks for your work!

Thanks for sharing so we can improve this together :)

-- 
Cheers
Swedebugia

[-- Attachment #2: node-cyclic.scm --]
[-- Type: text/x-scheme, Size: 5635 bytes --]

(define-public node-rimraf
  (package
   (name "node-rimraf")
   (version "2.6.2")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/isaacs/rimraf/archive/v2.6.2/rimraf-v2.6.2.tar.gz")
      (sha256
       (base32
        "0bmssxz3s30nhq5f8ldssf6s8ga5w0aarn71wjsmvqb1j15b2r6d"))))
   (build-system node-build-system)
   (inputs `(("node-glob" ,node-glob)))
   (native-inputs
    `(
      ;; tests("node-tap" ,node-tap)
      ("node-mkdirp" ,node-mkdirp)))
   (synopsis
    "A deep deletion module for node (like `rm -rf`)")
   (description
    "A deep deletion module for node (like `rm -rf`)")
   (home-page
    "https://github.com/isaacs/rimraf#readme")
   (license license:isc)))


(define-public node-glob
  (package
   (name "node-glob")
   (version "7.1.3")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/isaacs/node-glob/archive/v7.1.3/node-glob-v7.1.3.tar.gz")
      (sha256
       (base32
        "0qcymwljbm947gvfn7g7871dnwv5s0jq0r8c8ih9xgrfcynfw3hx"))))
   (build-system node-build-system)
   (inputs
    `(("node-inflight" ,node-inflight)
      ("node-once" ,node-once)
      ("node-path-is-absolute" ,node-path-is-absolute)
      ("node-minimatch" ,node-minimatch)
      ("node-fs.realpath" ,node-fs.realpath)
      ("node-inherits" ,node-inherits)))
   (native-inputs
    `(
      ;; benchm ("node-tick" ,node-tick) 
      ;; tests ("node-tap" ,node-tap)
      ("node-rimraf" ,node-rimraf)
      ("node-mkdirp" ,node-mkdirp)))
   (synopsis "a little globber")
   (description "a little globber")
   (home-page
    "https://github.com/isaacs/node-glob#readme")
   (license license:isc)))

(define-public node-jasmine-core
  (package
   (name "node-jasmine-core")
   (version "3.3.0")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/jasmine/jasmine/archive/v3.3.0/jasmine-v3.3.0.tar.gz")
      (sha256
       (base32
        "1rg4p487hf8mlxcj99wywzwp7jp3s4d114n4j12r3mkh8qyi8nck"))))
   (build-system node-build-system)
   (inputs `())
   (native-inputs
    `(
      ;; ("node-grunt" ,node-grunt)
      ;; ("node-grunt-contrib-compass"
      ;;  ,node-grunt-contrib-compass)
      ("node-jsdom" ,node-jsdom)
      ("node-shelljs" ,node-shelljs)
      ("node-jasmine" ,node-jasmine)
      ;; ("node-load-grunt-tasks" ,node-load-grunt-tasks)
      ;; ("node-grunt-contrib-compress"
      ;;  ,node-grunt-contrib-compress)
      ;; ("node-grunt-contrib-concat"
      ;;  ,node-grunt-contrib-concat)
      ;; ("node-grunt-cli" ,node-grunt-cli)
      ("node-temp" ,node-temp)
      ("node-glob" ,node-glob)
      ;; 
      ;; ("node-grunt-contrib-jshint"
      ;;  ,node-grunt-contrib-jshint)
      ))
   (synopsis
    "Official packaging of Jasmine's core files for use by Node.js projects.")
   (description
    "Official packaging of Jasmine's core files for use by Node.js projects.")
   (home-page "https://jasmine.github.io")
   (license license:expat)))


(define-public node-jasmine
  (package
   (name "node-jasmine")
   (version "3.3.0")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/jasmine/jasmine-npm/archive/v3.3.0/jasmine-npm-v3.3.0.tar.gz")
      (sha256
       (base32
        "1b6mgxmxv71bpr4fg75azfyh1v0m469prb7srg990fkf7i5bszw9"))))
   (build-system node-build-system)
   (inputs
    `(("node-jasmine-core" ,node-jasmine-core)
      ("node-glob" ,node-glob)))
   (native-inputs
    `(("node-grunt" ,node-grunt)
      ("node-shelljs" ,node-shelljs)
      ("node-grunt-cli" ,node-grunt-cli)
      ("node-grunt-contrib-jshint"
       ,node-grunt-contrib-jshint)))
   (synopsis "Command line jasmine")
   (description "Command line jasmine")
   (home-page "http://jasmine.github.io/")
   (license license:expat)))

(define-public node-domhandler
  (package
   (name "node-domhandler")
   (version "2.4.2")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/fb55/DomHandler/archive/v2.4.2/DomHandler-v2.4.2.tar.gz")
      (sha256
       (base32
        "16hi0vapmavw9g9s321b4c9nvwfg06cclj7pjnvjzk0imnzxjngp"))))
   (build-system node-build-system)
   (inputs
    `(("node-domelementtype" ,node-domelementtype)))
   (native-inputs
    `(("node-htmlparser2" ,node-htmlparser2)
      ("node-jshint" ,node-jshint)
      ("node-mocha" ,node-mocha)))
   (synopsis
    "handler for htmlparser2 that turns pages into a dom")
   (description
    "handler for htmlparser2 that turns pages into a dom")
   (home-page
    "https://github.com/fb55/DomHandler#readme")
   (license #f)))

(define-public node-htmlparser2
  (package
   (name "node-htmlparser2")
   (version "3.10.0")
   (source
    (origin
      (method url-fetch)
      (uri "https://github.com/fb55/htmlparser2/archive/v3.10.0/htmlparser2-v3.10.0.tar.gz")
      (sha256
       (base32
        "1qvsv4aixmgnh4h7q726wapg7qnk7srw4z9nmy71jc5r2krimnvn"))))
   (build-system node-build-system)
   (inputs
    `(("node-readable-stream" ,node-readable-stream)
      ("node-domhandler" ,node-domhandler)
      ("node-domelementtype" ,node-domelementtype)
      ("node-inherits" ,node-inherits)
      ("node-domutils" ,node-domutils)
      ("node-entities" ,node-entities)))
   (native-inputs
    `(("node-eslint" ,node-eslint)
      ("node-coveralls" ,node-coveralls)
      ("node-istanbul" ,node-istanbul)
      ("node-mocha-lcov-reporter"
       ,node-mocha-lcov-reporter)
      ("node-mocha" ,node-mocha)))
   (synopsis "Fast & forgiving HTML/XML/RSS parser")
   (description
    "Fast & forgiving HTML/XML/RSS parser")
   (home-page
    "https://github.com/fb55/htmlparser2#readme")
   (license license:expat)))