kessler / license-report

create a short report about a project's dependencies (license, url etc)
MIT License
222 stars 39 forks source link

Fails with versions like "nightly" and "latest" #51

Closed folknor closed 2 years ago

folknor commented 3 years ago

license-report just tells me skipping package@nightly (invalid semversion) for any version that isn't parsable as a semver.

But why does it need the version anyway?

npm accepts these "versions", so it seems like license-report should as well.

Thank you!

BePo65 commented 3 years ago

Just for testing this effect: can you give me the name of a package that has a version "nightly"? license-report takes the needed data from the response of the repository (which should probably also contain "nightly").

folknor commented 3 years ago

Sure; https://www.npmjs.com/package/parcel

folknor commented 3 years ago

I believe it also fails with "next" for https://www.npmjs.com/package/bootstrap

BePo65 commented 3 years ago

ok, thanks. I see the nightly version. So give me some time to check, how this will influence the code of license-report. By now we are on the point to refactor the e2e tests, so that not every update of a package on npm will break the tests.

BePo65 commented 3 years ago

license-report uses the npm package semver for handling version checks and for finding the latest version in the registry (e.g. in npmjs). By default prerelease versions are ignored. With an additional option of semver this could be changed, but as I saw in the issues of the npm package semver and in the github page of semver.org, there are lengthy discussions about prerelease- and nightly versions.

Can You give me an example of how you use the nightly build of a package in your package.json, so that I can find out, if this could be handled by the npm package semver?

folknor commented 3 years ago

Yes, like I said in my previous comments I use parcel nightlies and bootstrap next. Here is the deps from one app that is live.

    "dependencies": {
        "@fortawesome/fontawesome-free": "latest",
        "@parcel/optimizer-cssnano": "nightly",
        "@parcel/optimizer-htmlnano": "nightly",
        "@parcel/packager-css": "nightly",
        "@parcel/packager-html": "nightly",
        "@parcel/transformer-css": "nightly",
        "@parcel/transformer-html": "nightly",
        "@parcel/transformer-image": "nightly",
        "@parcel/transformer-postcss": "nightly",
        "@parcel/transformer-posthtml": "nightly",
        "@parcel/transformer-sass": "nightly",
        "@popperjs/core": "latest",
        "@tarekraafat/autocomplete.js": "latest",
        "animate.css": "latest",
        "autoprefixer": "latest",
        "avif": "latest",
        "bootstrap": "next",
        "copyfiles": "latest",
        "dice-coefficient": "latest",
        "dompurify": "latest",
        "fabric": "latest",
        "ol": "dev",
        "opentype.js": "latest",
        "parcel": "nightly",
        "postcss": "latest",
        "sass": "latest",
        "store2": "latest"
    }
BePo65 commented 3 years ago

Thanks, so now I have something to play with - give me some time to find out, if or how to handle this.

BePo65 commented 3 years ago

semver is fun ;-) And if I look at the lengthy discussion on semver about "is nightly a prerelease or a postrelease" I might loose heart.

So I fiddled around with different examples (you can see the results in stackblitz in the console).

As an outcome in my opinion the most urgent topics are

  1. getting the installed version (by now we take the version from "dependency" in package.json that is more of a "defined version" with ranges and all; see issue #53) - this could be solved by taking the information in package-lock.json, but then we always need a 'package-lock.json' file (what we should anyway to get reproducable installs)

  2. getting the "remoteVersion" (that is the latest version on the server matching the given range in package.json) for prebuilds and special tags (like "latest") - could be solved by using the "dist-tags" in the response from npmjs (e.g. @parcel/optimizer-cssnano, hoping that other repositories have that tag too)

Is that what you expected?

folknor commented 3 years ago

Since you are asking me directly, I would say option 1 is the way it should work.

For example, you can depend on "ol": "dev" in package.json and judging by https://www.npmjs.com/package/ol you would expect to get version 6.5.1-dev.1620690698899 but the version you get will still be the latest release (6.5.0) because a package in the dependency chain forces npm to downgrade to 6.5.0.

But maybe I misunderstood your question?

BePo65 commented 3 years ago

What I wanted to say was that we need a small modification of license-report with the 2 topics above.

With "ol" you have some fine examples for problems with "nightly" ;-))

To figure it out, I made a small spike with ol@dev in "dependencies" and as you said, what gets installed is 6.5.1-dev.1620690698899 and npm ls --depth=0 will show

npm ERR! peer dep missing: ol@^6.1.0, required by ol-mapbox-style@6.3.2

but nevertheless the code will probably run anyway (but I didn't try :-).

Now with the modifications proposed above, the output of license-report will be:

So I think the output of license-report will show what we would expect - or did I miss a point?

folknor commented 3 years ago

Sounds very good :-)

BePo65 commented 3 years ago

Thanks for the fast response; I'll prepare a PR

folknor commented 3 years ago

Can you provide a diff or patch here so I can use it locally?

BePo65 commented 3 years ago

@folknor I can do that, but give me a week or 2 to prepare the pr (I didn't do that until today as I wanted to wait for responses of the owner of this package - he's the boss ;-)

folknor commented 3 years ago

Alright, thank you!

BePo65 commented 3 years ago

@folknor I just uploaded a branch named pu/nightly. This adds a new field definedVersion to the fields of license-report that contains the version as defined in the package.json file. Besides this new field, this branch will handle prerelease- (e.g. 1.0.0-alpha.1) and dist-tags (e.g. nightly). I would appreciate it, if you could use this branch for a real world test and give me some feedback.

folknor commented 3 years ago
license-report --output=csv
kessler,stuff,@fortawesome/fontawesome-free,perpetual,material,(CC-BY-4.0 AND OFL-1.1 AND MIT),git+https://github.com/FortAwesome/Font-Awesome.git,5.15.3,5.15.3,latest,Dave Gandy dave@fontawesome.com http://twitter.com/davegandy
kessler,stuff,@parcel/optimizer-cssnano,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/optimizer-htmlnano,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/packager-css,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/packager-html,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/transformer-css,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/transformer-html,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/transformer-image,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.2328,2.0.0-nightly.2328,nightly,
kessler,stuff,@parcel/transformer-postcss,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/transformer-posthtml,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@parcel/transformer-sass,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.706,2.0.0-nightly.706,nightly,
kessler,stuff,@popperjs/core,perpetual,material,MIT,git+https://github.com/popperjs/popper-core.git,2.9.2,2.9.2,latest,Federico Zivolo federico.zivolo@gmail.com
kessler,stuff,@tarekraafat/autocomplete.js,perpetual,material,Apache-2.0,git+https://github.com/TarekRaafat/autoComplete.js.git,10.1.2,10.1.2,latest,Tarek Raafat tarek.m.raafat@gmail.com https://www.tarekraafat.com
kessler,stuff,animate.css,perpetual,material,MIT,git+https://github.com/animate-css/animate.css.git,4.1.1,4.1.1,latest,Animate.css
kessler,stuff,autoprefixer,perpetual,material,MIT,git+https://github.com/postcss/autoprefixer.git,10.2.6,10.2.6,latest,Andrey Sitnik andrey@sitnik.ru
kessler,stuff,avif,perpetual,material,Apache-2.0,git://github.com/lovell/avif-cli.git,0.0.2,0.0.2,latest,Lovell Fuller npm@lovell.info
kessler,stuff,bootstrap,perpetual,material,MIT,git+https://github.com/twbs/bootstrap.git,5.0.1,5.0.1,latest,The Bootstrap Authors https://github.com/twbs/bootstrap/graphs/contributors
kessler,stuff,copyfiles,perpetual,material,MIT,git://github.com/calvinmetcalf/copyfiles.git,2.4.1,2.4.1,latest,
kessler,stuff,dice-coefficient,perpetual,material,MIT,git+https://github.com/words/dice-coefficient.git,2.0.0,2.0.0,latest,Titus Wormer tituswormer@gmail.com https://wooorm.com
kessler,stuff,dompurify,perpetual,material,(MPL-2.0 OR Apache-2.0),git://github.com/cure53/DOMPurify.git,2.2.9,2.2.9,latest,Mario Heiderich mario@cure53.de https://cure53.de/
kessler,stuff,fabric,perpetual,material,MIT,git+https://github.com/fabricjs/fabric.js.git,4.5.0,4.5.0,latest,Juriy Zaytsev kangax@gmail.com
kessler,stuff,ol,perpetual,material,BSD-2-Clause,git://github.com/openlayers/openlayers.git,6.5.1-dev.1622757582411,6.5.0,dev,
kessler,stuff,opentype.js,perpetual,material,MIT,git://github.com/opentypejs/opentype.js.git,1.3.3,1.3.3,latest,Frederik De Bleser frederik@debleser.be
kessler,stuff,parcel,perpetual,material,MIT,git+https://github.com/parcel-bundler/parcel.git,2.0.0-nightly.704,2.0.0-nightly.704,nightly,
kessler,stuff,postcss,perpetual,material,MIT,git+https://github.com/postcss/postcss.git,8.3.0,8.3.0,latest,Andrey Sitnik andrey@sitnik.ru
kessler,stuff,sass,perpetual,material,MIT,git+https://github.com/sass/dart-sass.git,1.34.1,1.34.1,latest,Natalie Weizenbaum nweiz@google.com https://github.com/nex3
kessler,stuff,store2,perpetual,material,(MIT OR GPL-3.0),git+ssh://git@github.com/nbubna/store.git,2.12.0,2.12.0,latest,Nathan Bubna nathan@esha.com http://www.esha.com/
BePo65 commented 3 years ago

Looks very good; thanks for the test. So I will create a PR for this.

folknor commented 3 years ago

Excellent!

Here's what I actually care about; license-report --output=html --fields name --fields licenseType --fields link --fields author, just if that helps you steer the project in the future. Obviously this will be used to create an attribution section in a webapp.

There's an issue where license-report --output=table --fields name --fields licenseType --fields link --fields author creates a table with the fields in the order specified in the cmdline, but --output=html reverses the order of the fields. So it looks like this:

reversed

Should I file a separate bug about that, or is it intended?

Thank you!

BePo65 commented 3 years ago

looks like we should promote you to head of project QA ;-)

But seriously: that's definitely a bug, so please create an issue for this and I will fix it - it is just a small code change.

BePo65 commented 2 years ago

Fixed in next release based on pr #65.