Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opencast issues#1123 Paella dependency lockdown protection #448

Closed

Conversation

karendolan
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This requests changing the name of the non-publishable "package-lock.json" to the publishable "npm-shirkwrap.json". Ref opencast/opencast#1123

What is the current behavior? (You can also link to an open issue here)

When Opencast's module/engage-paella-player runs it's npm ci (or npm install), Paella's package-lock.json file is ignored and the paella's package.json is used to retrieve the latest matching iterations of Paella's dependencies. Instead of the dependency versions described in Paella's package-lock.json.

What does this implement/fix? Explain your changes.

Changing the package-lock to npm-shrinkwrap honors Paella's locked dependency versions when paella is built within Opencast project's engage-paella-player module.

Does this close any currently open issues?

closes Opencast issue #1123 which is that Paella's lunr 2.3.7 dependency was not published with a lunr.min.js file so the Paella player's gulpfile build fails.

Does this PR introduce a breaking change? What changes might users need to make in their application due to this PR?

Unknown how this might cause issues.

Any other comments?

I'm making a companion temporary pull fix for Opencast's enage-paella-player. It hard codes a node_modules/paellalayer/npm-shrinkwrap.json file in it, so it probably (hopefully) won't be approved before this change is accepted here.

@karendolan
Copy link
Contributor Author

@miesgre I'm going to update the pull to mirror the current package-lock.json. Is there a reason the Paella package doesn't want to provide the npm-shrinkwrap file to protect projects that use Paella as an npm build dependency?

@karendolan
Copy link
Contributor Author

npm-shrinkwrap.json is a file created by "npm shrinkwrap". It is identical to package-lock.json, with one major caveat: Unlike package-lock.json, npm-shrinkwrap.json may be included when publishing a package.
https://docs.npmjs.com/configuring-npm/shrinkwrap-json.html

@karendolan
Copy link
Contributor Author

One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package. It shares a format with npm-shrinkwrap.json, which is essentially the same file, but allows publication. This is not recommended unless deploying a CLI tool or otherwise using the publication process for producing production packages.
https://docs.npmjs.com/configuring-npm/package-lock-json.html

@miesgre I don't know how to include "npm shrinkwrap" as part of the npm publish sequence of events for this project for this pull. I realize that npm-shrinkwrap.json should not be a regular part of the repo files, just created for publishing a version.

@miesgre
Copy link
Collaborator

miesgre commented Apr 24, 2020

@karendolan I think you have problems with de opencast build.

The reason is that opencast bundle downloads the source code of paella using npm.
See here: https://github.com/opencast/opencast/blob/334901ca55ccb348d02ce18f8a838be8cd86f8c3/modules/engage-paella-player/package.json#L17

When downloading with npm, the package-lock.json file is not downloaded, and then during the build other package versions can be installed.
I think we need to change the way the opencast bundle downloads the source code of paella.
We can use the download-maven-plugin in the same way the studio bundle is doing.
See: https://github.com/opencast/opencast/blob/334901ca55ccb348d02ce18f8a838be8cd86f8c3/modules/studio/pom.xml#L21-L39

@karendolan
Copy link
Contributor Author

The npm dependency issue with Opencast is resolved via opencast/opencast#1547.
An npm-shrinkwrap could not have resolved the hidden babel build config file.

@karendolan karendolan closed this May 11, 2020
@karendolan karendolan deleted the 6.2.2-shrinkwrapPubish branch May 11, 2020 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants