Re: [PATCH v3 00/17] NPM refactoring

[Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>]

Hi Richard,

On Nov 21, 2019, at 9:25 PM, Richard Purdie richard.purdie@linuxfoundation.org wrote:
> On Thu, 2019-11-21 at 14:53 -0500, Jean-Marie LEMETAYER wrote:
>> On Nov 21, 2019, at 7:26 PM, Richard Purdie
>> richard.purdie@linuxfoundation.org wrote:
>> > On Wed, 2019-11-20 at 10:33 +0100, Jean-Marie LEMETAYER wrote:
>> > > The current NPM support have several issues:
>> > >  - The current NPM fetcher downloads the dependency tree but not
>> > > the other
>> > >    fetchers. The 'subdir' parameter was used to fix this issue.
>> > >  - They are multiple issues with package names (uppercase, exotic
>> > > characters,
>> > >    scoped packages) even if they are inside the dependencies.
>> > >  - The lockdown file generation have issues. When a package
>> > > depends on
>> > >    multiple version of the same package (all versions have the
>> > > same checksum).
>> > > 
>> > > This patchset refactors the NPM support in Yocto:
>> > >  - As the NPM algorithm for dependency management is hard to
>> > > handle, the new
>> > >    NPM fetcher downloads only the package source (and not the
>> > > dependencies,
>> > >    like the other fetchers) (patch submitted in the bitbake-devel
>> > > list).
>> > 
>> > I'm a little confused by this item.
>> > 
>> > If the NPM fetcher only downloads a given package's source and it
>> > has
>> > dependencies, where/when are the dependencies downloaded?
>> > 
>> > My worry here is that if the fetcher isn't downloading them, DL_DIR
>> > can't contain all the needed pieces needed to reproduce a build at
>> > a
>> > later date?
>> > 
>> > I suspect I'm missing something obvious...
>> 
>> The magic is in the bbclass, in the do_fetch_append() function:
>> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/meta/classes/npm.bbclass#L51
>> 
>> Which calls:
>> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/bitbake/lib/bb/fetch2/npm.py#L312
>> 
>> Which runs the npm fetcher for each dependencies described in the
>> shrinkwrap file. This is the same behavior for the do_unpack task.
>> 
>> 
>> To be more clear, a recipe like this:
>> 
>> ```
>> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
>> S = "${WORKDIR}/npm"
>> ```
>> 
>> Will only fetch/unpack the package source without handling the
>> dependencies.
>> 
>> But a recipe like this:
>> 
>> ```
>> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
>> S = "${WORKDIR}/npm"
>> 
>> NPM_SHRINKWRAP = "${THISDIR}/${BPN}/npm-shrinkwrap.json"
>> inherit npm
>> ```
>> 
>> Will fetch/unpack the package and all the dependencies which are
>> described in the shrinkwrap file.
> 
> That does remove some of my worries, thanks for explaining!
> 
> It does make me wonder if we shouldn't have two fetch classes and then
> support a url like:
> 
> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0 \
>           npmsw://${THISDIR}/${BPN}/npm-shrinkwrap.json"
> 
> where the "npmsw" handler would handle the shrinkwrap file?
> 
> Basically I don't like the way the current code has to tag the
> shrinkwrap file handling on to the fetcher...

This definitely sounds like a great idea !

I based my work on the current implementation, but I never thought of
refactoring as much. I have however some concerns about the management
of the registry and the development dependencies. I will try to come
back with a v4.

>> Bonus tips: you can have your base package SRC_URI using another
>> fetcher than npm (e.g. git) without breaking anything. The behavior
>> will still be the same.
> 
> That does start to make it clearer why this is advantageous.

Regards,
Jean-Marie