Re: Improved NPM importer with blacklist

[swedebugia]

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

node-cyclic.scm
Description: Text Data