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

fix(deps): frozen installation fails during ugprade workflow #801

Merged
merged 3 commits into from
May 18, 2021
Merged

fix(deps): frozen installation fails during ugprade workflow #801

merged 3 commits into from
May 18, 2021

Conversation

iliapolo
Copy link
Contributor

Fixes #800.

The problem was that projen itself can change package.json files due to its own requirements.

For example https://github.com/cdklabs/cdk-triggers/blob/main/.projenrc.js#L6

Which causes the package.json file to change after running yarn install - and a subsequent frozen installation would fail.

This PR uses projen itself to install the dependencies and update the lock file, so this happens after all package.json mutations have happened.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


// we use projen to install the dependencies and thus update the lock file,
// so we need to make it think its not running inside CI.
task.exec(`CI=0 ${this._project.projenCommand}`);
Copy link
Contributor

@eladb eladb May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
task.exec(`CI=0 ${this._project.projenCommand}`);
task.exec(this._project.projenCommand, {
env: { CI: '0' }
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool

@@ -118,7 +118,11 @@ export class UpgradeDependencies extends Component {
}

task.exec(ncuCommand.join(' '));
task.exec(this._project.package.renderInstallCommand(false));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean we can make this private again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2021

Codecov Report

Merging #801 (0f3d8ed) into main (377966e) will increase coverage by 5.12%.
The diff coverage is 73.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #801      +/-   ##
==========================================
+ Coverage   78.46%   83.59%   +5.12%     
==========================================
  Files          88       96       +8     
  Lines        3181     3620     +439     
  Branches      809      805       -4     
==========================================
+ Hits         2496     3026     +530     
+ Misses        683      593      -90     
+ Partials        2        1       -1     
Impacted Files Coverage Δ
src/cdk8s-app-ts.ts 91.89% <ø> (ø)
src/cdk8s-construct.ts 33.33% <ø> (ø)
src/github/workflows.ts 100.00% <ø> (ø)
src/java/pom.ts 93.84% <ø> (ø)
src/tasks/runtime.ts 15.53% <7.14%> (-0.78%) ⬇️
src/typescript/projenrc.ts 17.14% <17.14%> (ø)
src/javascript/render-options.ts 20.25% <20.25%> (ø)
src/javascript/projenrc.ts 54.83% <54.83%> (ø)
src/github/mergify.ts 91.66% <75.00%> (-8.34%) ⬇️
src/node-package.ts 71.00% <76.47%> (+3.28%) ⬆️
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dff296c...0f3d8ed. Read the comment docs.

@mergify mergify bot merged commit 06465af into projen:main May 18, 2021
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.

upgrade-deps build fails with "Your lockfile needs to be updated"
3 participants