[Bug]: Attributions out-of-date #28412
Assignees
Labels
regression-prod-12.3.0
Regression bug that was found in production in release 12.3.0
release-12.8.0
Issue or pull request that will be included in release 12.8.0
Sev2-normal
Normal severity; minor loss of service or inconvenience.
team-extension-platform
type-bug
Comments
Gudahtt
added
type-bug
Sev2-normal
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
The script `attributions:check` was silently failing when the attempt to generate attributions failed. This led to the attributions in v12.6.0 being out-of-date. Relates to #28412
metamaskbot
added
regression-prod-12.3.0
7 tasks
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. Fixes #28412
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. Fixes #28412
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. Fixes #28412
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. Fixes #28412
Gudahtt
added a commit
that referenced
this issue
Nov 11, 2024
Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. Fixes #28412
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 12, 2024
## **Description** The script `attributions:check` was silently failing when the attempt to generate attributions failed. This led to the attributions in v12.6.0 being out-of-date. [](https://codespaces.new/MetaMask/metamask-extension/pull/28413?quickstart=1) ## **Related issues** Relates to #28412 ## **Manual testing steps** 1. Run `yarn attributions:check`. See that the exit code is non-zero * on `develop` it also fails, but with a zero exit code, indicating success despite the fact that it failed. ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
](https://codespaces.new/MetaMask/metamask-extension/pull/28413?quickstart=1){kind=link}
github-merge-queue bot
pushed a commit
that referenced
this issue
Nov 14, 2024
## **Description** Fix the `attribution:generate` command by ensuring that it is possible to install just production dependencies. Previously the command `yarn workspaces focus --production` (used to discard development dependencies, keeping just production dependencies installed) would fail because `rimraf` was not found. `rimraf` was a development dependency used in the `postinstall` script. This was resolved by replacing `rimraf` with a Node.js script that does the same thing without needing any dependency. Once that failure was resolved, another was revealed. The `allow-scripts` step of the installation began failing because there was a package detected that had an install script that was missing from our configuration. This package was in our configuration already, but the `allow-scripts` configuration is sensitive to changes in the directory structure of `node_modules`, and that structure changed due to differences in which packages were hoisted in the production-only install. That failure was resolved by updating `generate-attributions.sh` to remove the `allow-scripts` plugin while generating attributions. We don't need `postinstall` scripts to run in order to read licences from disk. [](https://codespaces.new/MetaMask/metamask-extension/pull/28415?quickstart=1) ## **Related issues** Fixes #28412 ## **Manual testing steps** 1. Run `yarn attributions:generate`, and see that it completes successfully * Locally, it should also re-install the `allow-scripts` plugin and development dependencies * If this command is run with `CI=true` (e.g. `CI=true yarn attributions:generate`), it will skip the step of re-installing the `allow-scripts` plugin and development dependencies. This is what would happen on CI, where the environment gets discarded after this is run so there is no point in re-installing things. ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
](https://codespaces.new/MetaMask/metamask-extension/pull/28415?quickstart=1){kind=link}
metamaskbot
added
the
release-12.8.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The
attributions.txtfile linked in v12.3.0 onwards is outdated. It still reflects the attributions in v12.2.4.Expected behavior
The
attributions.txtfor each release should match the dependencies in that release.Screenshots/Recordings
No response
Steps to reproduce
The failure was unnoticed because the
attributions:checkcommand that we rely on can silently fail when the file fails to generate. The generation failed due to the addition of thepostinstallscript.This problem can be demonstrated with these steps:
postinstallscript, so that the attribution check succeedsyarn attributions:generate, and see that the attributions file is different.Error messages or log output
No response
Detection stage
In production (default)
Version
12.3.0
Build type
None
Browser
Other (please elaborate in the "Additional Context" section)
Operating system
Other (please elaborate in the "Additional Context" section)
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: