📄 Generate licenses info when shrinkwrapping

[grdryn]

Any time a dependency changes (added, updated, removed), we need to
shrinkwrap that change. It makes sense that we update the license info
of our dependencies at this time also.

[grdryn]

Due to a limitation in the license-reporter tool, this currently only gives info on the dependencies declared in the package.json, and not their dependencies. This is being looked into now, so in a later version we should get a fuller picture.

[grdryn]

I've added this block in here to match the main Dockerfile. This Dockerfile is currently broken for other reasons though (since at least early September I guess?), so I'm not sure if it's even worthwhile to keep this file at all?

[vibe13]

I am not up to date with that file either, but if it's going to be there, it's better to keep them as close as possible +1

[grdryn]

This now makes npm shrinkwrap dependent on a devDependency. Previously, because of a limitation on how we used to build our tar archives with grunt-fh-build, we needed to tell people to run npm shrinkwrap with only production dependencies present. Now though, our minimum target is Node.js 6 which comes with npm 3+, and with that change we had to rethink how we pack our archives, so we no longer have the limitation of needing to run shrinkwrap without devDependencies.

If this change is accepted in all of our components, I'll notify people of the new way.

[pb82]

Is license-reporter installed in the Jenkins environment? It looks like this is using a globally installed binary of it instead of the one from node_modules? And if that's the case, why do we need it in devDependencies?

[grdryn]

@pb82 When you run things from npm scripts, then ./node_modules/.bin is added to the start of the PATH, so it's made available from there. It's not included globally in the Jenkins environment.

https://docs.npmjs.com/misc/scripts#path

[grdryn]

This is the latest commit on master of the license-reporter repo. It's not yet published to npmjs, and specifying master is not stable (from experience -- our Jenkins builds already run this tool, and previously depended on master, and broke frequently as a result).

[vibe13]

Yes, +1 to publish the component to npmjs, once it reaches 1.0 milestone :-)

[lgriffin]

That's the plan! Jay is looking after this as we might want to name it appropriately to show it's Red Hat supported (legal get involved) and / or namespace it.

[grdryn]

Currently this (and the html equivalent) only contains info on the Node.js/npm dependencies. In our container image layer, we also install RPMs for dejavu-sans-fonts, and phantomjs. We should have license info recorded on those also. It'll be a lot easier if that's a separate bunch of xml/html files, but even then, I'm not sure how best to keep those up-to-date. I'll see what I can do!

[vibe13]

Perfectly fine to keep the files separate for Node and RPM licenses. RPMs change very rarely, so updating them would be not an usual task (and could be manual), while the Node files would be updated much more frequently (and with automation).

[lgriffin]

We can also have multiple license.xml files within the root directory. The structure is:
./licenses/PRODUCT/*

This was designed around the potential scenario where we ship a product made up of products, so you could have RHMAP, RHOAR, EAP all in the one product and they each have their own folder for all the local file info + the HTML + the XML file.

So we could get creative there and differentiate between RHMAP licenses and RPM licenses (if that's a requirement). The naming requirements aren't hard enforced so you could do RHMAP and RHMAP_MISC for example -- we have no formal request that the names have to be a certain way.

[lgriffin]

I think that it is a sensible approach to do this. The official stance on the license reporter is 'anything that becomes customer facing'. So any patch for example would need an updated license. As we move towards RC generation it makes sense to have everything up to date prior to that. One less potential failure point.

[grdryn]

I've updated the version of license-reporter here, and now it scans and reports on the entire dependency graph. As you can see, the result of this is that there are now 673 files in this PR (and over 39 thousand line changes) -- this is because the html links to a local copy of each of the license files. I think the overhead of this is worth it, if it means that everything will always be up-to-date when someone changes a dependency and shrinkwraps (and we'll see any changes in licensing info as those dependencies change).

[grdryn]

Also, thanks to @helio-frota for the updates to license-reporter!

[lgriffin]

That sounds good Ger :)

I'm wondering what the footprint impact might be like on Docker now that we go down the tree and have a lot of local licenses. The reason I mention this is our local licenses correctly point towards the actual file in the dependency. Other products (EAP) literally pull down the remote license text and create a local license file and simply point each dependency to the same text so they have a finite number of local files which equals the number of licenses they work with. Legal said they would prefer our way of doing it but that's not mandated yet. Any footprint considerations we should be thinking about here?

(let's take that to email perhaps not to derail this PR)

[grdryn]

@pb82 I've updated this again, mostly around the preshrinkwrap and postshrinkwrap hooks in the package.json. This is to ensure that only production dependencies (and license-reporter) are present when the license-reporter tool is run at shrinkwrap time -- this significantly reduces the amount of files checked in (from 673 to 420).

Please take a look again and let me know if you think this is a good idea! The downside is that now running npm shrinkwrap takes around 3m30s on my machine.

[grdryn]

@lgriffin The licenses directory is 2.3MB here (might be larger for other components), which I think is fine. I also think this way is better, since we have the potential to catch any changes to license files (even for the same license) on dependency version updates. This will hopefully just be copyright changes, such as adding new years, or contributors; but there could be other changes that we should know about.

[pb82]

if we install license reporter in the preshrinkwrap stage do we still need it here? Or is this to guarantee that we install the correct version?

[grdryn]

Yeah, it gets the version from here. We're discussing on the license-reporter repo to see if we can make this cleaner by not including the devDependencies license info, even if they're present in node_modules -- similar to how shrinkwrap just includes production dependencies; so in future, the preshrinkwrap and postshrinkwrap hooks might be cleaner.

[grdryn]

I've changed this again now, after some new changes in the tool. There are now only 350 new files in the licenses dir. It's at a point now where I'm happy with it, so I'll start to make the same change in all of the other components.

I'll merge this at some point this afternoon (Irish time), so if any of you have other concerns, please let me know before then. Thanks!

[pb82]

LGTM

[pb82]

@grdryn I've noticed that you removed the npm install license-reporter from the preshrinkwrap stage. So should we do a full npm install (as opposed to only prod dependencies) when we update the shrinkwrap file?

[grdryn]

@pb82 Yes, npm i && npm shrinkwrap. In a previous iteration, I had the preshrinkwrap doing rm -rf ./node_modules && npm i --production && npm i license-reporter to try to reduce the amount of unnecessary things that were included in ./licenses. Now that we've changed the tool to only work on production dependencies by default, those extra steps aren't needed.

I'll merge all of the changes to the nodejs components together when I've got them all ready, and send a mail to say that you just need to do npm shrinkwrap (and prefix that with npm i && if you don't have all the dependencies (it'll fail at the npm ls in the preshrinkwrap if you don't have enough)).