Don't install gulp-cli globally in the GitHub Actions workflows
#17913
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's recommended to always install dependencies locally in the project folder because global dependencies can easily conflict with other projects and, because they are not managed by the project, diverge from versions defined in e.g. `package.json`. Previously we installed `gulp-cli` globally because at the time we lacked a convenient mechanism to use Gulp otherwise, but nowadays NPM provides the `npx` command for that purpose and recommends using it over global installations (see https://docs.npmjs.com/downloading-and-installing-packages-globally and PR mozilla#17489 that provided the ground work for using it). This commit therefore updates our GitHub Actions workflows to no longer install `gulp-cli` globally but instead install it locally from the already existing entries in `package.json` like all other dependencies we use. Not only does this remove the special-casing for `gulp-cli` which simplifies the workflow definitions, it also ensures that the version ranges provided in `package.json` are respected. This makes the local and workflow setups more similar, but is also relevant for the upcoming upgrade to Gulp 5 which from a quick try is a bit involved and having `package.json` be the single source of truth for the dependency versions we use is therefore important.
timvandermeij
added a commit
to timvandermeij/botio-files-pdfjs
that referenced
this pull request
May 31, 2024
…d one The globally-installed version of Gulp on the bots is old, and recently we moved away from using globally-installed dependencies in favor of using locally-installed ones for maintainability, isolation and reproducibility of build environments. For more information, refer to PRs mozilla/pdf.js#17913 and mozilla/pdf.js#17489. In mozilla/pdf.js#18197 we have seen that even though locally Gulp 5 is installed the global version is preferred by the bots because the `gulp` command (available in the PATH) is used instead of `npx gulp` which prefers the locally-installed version from the `node_modules` folder. This commit fixes the issue by making all Gulp invocations use `npx gulp` instead, similar to what we already did in the GitHub Actions pipelines.
timvandermeij
added a commit
to timvandermeij/botio-files-pdfjs
that referenced
this pull request
May 31, 2024
…d one The globally-installed version of Gulp on the bots is old, and recently we moved away from using globally-installed dependencies in favor of using locally-installed ones for maintainability, isolation and reproducibility of build environments. For more information, refer to PRs mozilla/pdf.js#17913 and mozilla/pdf.js#17489. In mozilla/pdf.js#18197 we have seen that even though locally Gulp 5 is installed the global version is preferred by the bots because the `gulp` command (available in the PATH) is used instead of `npx gulp` which prefers the locally-installed version from the `node_modules` folder. This commit fixes the issue by making all Gulp invocations use `npx gulp` instead, similar to what we already did in the GitHub Actions pipelines.
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Nov 17, 2024
In PR mozilla#11300 Gitpod support got introduced, but we re-evaluated that decision in mozilla#11732. In PR mozilla#11800 the support was partially reverted, but the actual Gitpod files were kept to not outright break potential workflows because at the time we were not sure if, and if so how often, Gitpod was actually used for contributing to PDF.js. However, in addition to the concerns mentioned in mozilla#11732 after five years we haven't seen any contributions that clearly originated from Gitpod, and the Dockerfile has not been updated after e.g. PR mozilla#11807 and PR mozilla#17913 because it's not a workflow that we maintain or are able to test (nor have we seen Gitpod community contributions for this). This commit therefore removes the remaining Gitpod files too to reduce maintainance burden for PDF.js. Note that users of Gitpod can still contribute to PDF.js via the platform; we just don't provide/manage workspace files from this repository anymore.
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Nov 17, 2024
In PR mozilla#11300 Gitpod support got introduced, but we re-evaluated that decision in mozilla#11732. In PR mozilla#11800 the support was partially reverted, but the actual Gitpod files were kept to not outright break potential workflows because at the time we were not sure if, and if so how often, Gitpod was actually used for contributing to PDF.js. However, in addition to the concerns mentioned in mozilla#11732 after five years we haven't seen any contributions that clearly originated from Gitpod, and the files have not been updated after e.g. PR mozilla#11807 and PR mozilla#17913 because it's not a workflow that we maintain or are able to test (nor have we seen Gitpod community contributions for this). This commit therefore removes the remaining Gitpod files to reduce maintainance burden for PDF.js. Note that users of Gitpod can still contribute to PDF.js via the platform; we just don't provide/manage workspace files from this repository anymore.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's recommended to always install dependencies locally in the project folder because global dependencies can easily conflict with other projects and, because they are not managed by the project, diverge from versions defined in e.g.
package.json. Previously we installedgulp-cliglobally because at the time we lacked a convenient mechanism to use Gulp otherwise, but nowadays NPM provides thenpxcommand for that purpose and recommends using it over global installations (see https://docs.npmjs.com/downloading-and-installing-packages-globally and PR #17489 that provided the ground work for using it).This commit therefore updates our GitHub Actions workflows to no longer install
gulp-cliglobally but instead install it locally from the already existing entries inpackage.jsonlike all other dependencies we use. Not only does this remove the special-casing forgulp-cliwhich simplifies the workflow definitions, it also ensures that the version ranges provided inpackage.jsonare respected. This makes the local and workflow setups more similar, but is also relevant for the upcoming upgrade to Gulp 5 which from a quick try is a bit involved and havingpackage.jsonbe the single source of truth for the dependency versions we use is therefore important.Thanks to @nicolo-ribaudo for bringing this isolation measure to the attention!