ci(aio): freeze the lockfile for CI builds #19616

petebacondarwin · 2017-10-09T07:16:24Z

The CI will now fail if the dependencies in the AIO
package.json have been modified without the
lockfile being updated.

gkalpak

LGTM (as soon as Travis is happy).
Should we do the same for the examples boilerplate?

petebacondarwin · 2017-10-09T09:05:01Z

Oh yes. I forgot about that

On Mon, 9 Oct 2017, 09:58 George Kalpakas, ***@***.***> wrote: ***@***.**** approved this pull request. LGTM (as soon as Travis is happy). Should we do the same for the examples boilerplate? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19616 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA9J7K1fBGai54eprY5BWDTIWZtr1zZks5sqeAagaJpZM4PyFF1> .

mary-poppins · 2017-10-09T10:09:42Z

You can preview ee629ff at https://pr19616-ee629ff.ngbuilds.io/.

petebacondarwin · 2017-10-09T10:22:19Z

And the examples dependency installation uses the pure lockfile flag, so won't actually fail if the package has changed but will always use what is in the lockfile. Is this acceptable?

mary-poppins · 2017-10-09T10:26:29Z

You can preview 33f0cab at https://pr19616-33f0cab.ngbuilds.io/.

mary-poppins · 2017-10-09T10:29:50Z

You can preview 377e2a6 at https://pr19616-377e2a6.ngbuilds.io/.

gkalpak · 2017-10-09T10:58:00Z

Why not use --frozen-lockfile for examples too? Are there any downsides?

petebacondarwin · 2017-10-09T12:14:23Z

No you're right there are no downsides. I was worried about people getting errors calling example-use-npm if they had changed their package.json, but then they should also update the lock file too.

petebacondarwin · 2017-10-09T12:24:13Z

Updated to use freeze-lockfile

mary-poppins · 2017-10-09T12:31:17Z

You can preview 2546c0d at https://pr19616-2546c0d.ngbuilds.io/.

IgorMinar · 2017-10-10T02:34:02Z

scripts/ci/install.sh

@@ -52,7 +52,7 @@ if [[ ${TRAVIS} && (${CI_MODE} == "aio" || ${CI_MODE} == "aio_e2e" || ${CI_MODE}
  travisFoldStart "yarn-install.aio"
    (
      cd ${PROJECT_ROOT}/aio
-      yarn install
+      yarn install --frozen-lockfile --non-interactive


can you also update the main repo yarn install on line 40?

IgorMinar

otherwise lgtm. I didn't know about this option. Thanks for turning it on.

petebacondarwin · 2017-10-10T11:56:10Z

@IgorMinar - added freeze-lockfile to various other places in the project. PTAL

gkalpak · 2017-10-10T12:24:52Z

aio/aio-builds-setup/dockerbuild/Dockerfile

@@ -156,7 +156,7 @@ RUN find $AIO_SCRIPTS_SH_DIR -maxdepth 1 -type f -printf "%P\n" \
 # Set up the Node.js scripts
 COPY scripts-js/ $AIO_SCRIPTS_JS_DIR/
 WORKDIR $AIO_SCRIPTS_JS_DIR/
-RUN yarn install --production
+RUN yarn install --production --freeze-lockfile --non-interactive


Not sure this is worth it, because we don't run it on CI anyway.

I agree that the --non-interactive is not needed in that case. but the --freeze-lockfile still has value, no?

--non-interactive is fine actually, because this is automatically built on the preview server.
We will be testing with --freeze-lockfile on CI, so leaving it here shouldn't be an issue theoretically. Thinking about it again, it is probably better to keep the flags (worst case scenario: the preview server will remain on an older version until this is fixed).

gkalpak · 2017-10-10T12:26:08Z

docs/DEVELOPER.md

@@ -58,7 +58,7 @@ Next, install the JavaScript modules needed to build and test Angular:

 ```shell
 # Install Angular project dependencies (package.json)
-yarn install
+yarn install --freeze-lockfile


This here seems redundant. Theoretically, they've just cloned the repo.

The point is that it could automatically update the yarn.lock file if you don't do this, which could be breaking and confusing.

This would only happen if we messed up and committed package.json changes without the corresponding yarn.lock changes. In that case, using --freeze-lockfile would be fail which will probably be even more confusing 😃

This is not how I understand yarn. Imagine a dependency in package.json that is a range (e.g. ^1.2.3). This could have a lockfile dependency of 1.2.4 say. Now suppose that the owner of that dependency releases 1.2.5. If we do not freeze the lockfile, then yarn install will install 1.2.5 and then modify the lockfile accordingly. Surely this is not what we want. We want the developers of Angular to install the versions in the lockfile.

I don't think this is how yarn install works. Normally, yarn install just installs what is in the lockfile. (That's the whole point, right? 😃)

If there are "indications" that package.json has been modified without a corresponding update to yarn.lock, then and (only then) yarn install will update the lockfile (possible upgrading other dependencies in the process). Here is where the --freeze-lockfile takes effect (failing in case that happens).

So, afaict, what --freeze-lockfile protects us from is updating package.json manually (not via yarn) and forgetting to also update the lockfile.

_{(That's just my understanding; could be wrong 😁)}

Oh I get it now: yarnpkg/yarn#4147
So if I do anything to any dependency, then the yarn.lock will get modified unless the lock file is frozen. So indeed if you have just cloned then this is not needed.

mary-poppins · 2017-10-10T12:34:48Z

You can preview c175f85 at https://pr19616-c175f85.ngbuilds.io/.

mary-poppins · 2017-10-10T12:50:17Z

You can preview 4f576f9 at https://pr19616-4f576f9.ngbuilds.io/.

The CI will now fail if the dependencies in the AIO package.json have been modified without the lockfile being updated.

petebacondarwin · 2017-10-11T07:17:17Z

@IgorMinar has approved this PR, so not sure why pullapprove is still complaining...

mary-poppins · 2017-10-11T07:24:13Z

You can preview a5e0a94 at https://pr19616-a5e0a94.ngbuilds.io/.

) PR Close #19616

PR Close #19616

The CI will now fail if the dependencies in the AIO package.json have been modified without the lockfile being updated. PR Close #19616

) PR Close #19616

PR Close #19616

angular-automatic-lock-bot · 2019-09-12T21:51:08Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin added comp: aio action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 9, 2017

petebacondarwin requested review from IgorMinar and gkalpak October 9, 2017 07:16

googlebot added the cla: yes label Oct 9, 2017

gkalpak approved these changes Oct 9, 2017

View reviewed changes

petebacondarwin force-pushed the aio-freeze-lockfile branch from a65bff9 to ee629ff Compare October 9, 2017 09:57

petebacondarwin force-pushed the aio-freeze-lockfile branch from ee629ff to 33f0cab Compare October 9, 2017 10:16

petebacondarwin added this to REVIEW in docs-infra Oct 9, 2017

petebacondarwin force-pushed the aio-freeze-lockfile branch from 377e2a6 to 2546c0d Compare October 9, 2017 12:22

gkalpak approved these changes Oct 9, 2017

View reviewed changes

IgorMinar reviewed Oct 10, 2017

View reviewed changes

IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 10, 2017

IgorMinar approved these changes Oct 10, 2017

View reviewed changes

petebacondarwin force-pushed the aio-freeze-lockfile branch from 095635f to c175f85 Compare October 10, 2017 12:26

gkalpak reviewed Oct 10, 2017

View reviewed changes

petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 10, 2017

petebacondarwin force-pushed the aio-freeze-lockfile branch from c175f85 to 4f576f9 Compare October 10, 2017 12:39

petebacondarwin added 5 commits October 11, 2017 08:15

ci(aio): freeze the lockfile for CI builds

45c3344

The CI will now fail if the dependencies in the AIO package.json have been modified without the lockfile being updated.

build(aio): freeze lockfile when installing example dependencies

e4108eb

build(aio): freeze yarn lockfile for aio-builds-setup scripts

7b1bcee

build(aio): freeze lockfile when setting up examples

9e52fe7

ci: freeze yarn lockfile when installing dependencies

a5e0a94

petebacondarwin force-pushed the aio-freeze-lockfile branch from 4f576f9 to a5e0a94 Compare October 11, 2017 07:16

petebacondarwin added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 11, 2017

petebacondarwin moved this from REVIEW to MERGE in docs-infra Oct 11, 2017

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze lockfile when installing example dependencies (#19616

ad7e781

) PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze yarn lockfile for aio-builds-setup scripts (#19616)

dc03811

PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze lockfile when setting up examples (#19616)

afa2bb2

PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

ci: freeze yarn lockfile when installing dependencies (#19616)

566606e

PR Close #19616

chuckjaz closed this in 437e803 Oct 11, 2017

petebacondarwin removed this from MERGE in docs-infra Oct 11, 2017

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

ci(aio): freeze the lockfile for CI builds (#19616)

2905069

The CI will now fail if the dependencies in the AIO package.json have been modified without the lockfile being updated. PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze lockfile when installing example dependencies (#19616

6225fed

) PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze yarn lockfile for aio-builds-setup scripts (#19616)

ee3c681

PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

build(aio): freeze lockfile when setting up examples (#19616)

fc1dcff

PR Close #19616

chuckjaz pushed a commit that referenced this pull request Oct 11, 2017

ci: freeze yarn lockfile when installing dependencies (#19616)

3e3f918

PR Close #19616

petebacondarwin deleted the aio-freeze-lockfile branch November 17, 2017 13:04

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(aio): freeze the lockfile for CI builds #19616

ci(aio): freeze the lockfile for CI builds #19616

petebacondarwin commented Oct 9, 2017

gkalpak left a comment

petebacondarwin commented Oct 9, 2017 via email

mary-poppins commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

gkalpak commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

IgorMinar Oct 10, 2017

IgorMinar left a comment

petebacondarwin commented Oct 10, 2017

gkalpak Oct 10, 2017

petebacondarwin Oct 10, 2017

gkalpak Oct 10, 2017

gkalpak Oct 10, 2017

petebacondarwin Oct 10, 2017

gkalpak Oct 10, 2017

petebacondarwin Oct 10, 2017

gkalpak Oct 10, 2017

petebacondarwin Oct 10, 2017

mary-poppins commented Oct 10, 2017

mary-poppins commented Oct 10, 2017

petebacondarwin commented Oct 11, 2017

mary-poppins commented Oct 11, 2017

angular-automatic-lock-bot bot commented Sep 12, 2019

ci(aio): freeze the lockfile for CI builds #19616

ci(aio): freeze the lockfile for CI builds #19616

Conversation

petebacondarwin commented Oct 9, 2017

gkalpak left a comment

Choose a reason for hiding this comment

petebacondarwin commented Oct 9, 2017 via email

mary-poppins commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

gkalpak commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

petebacondarwin commented Oct 9, 2017

mary-poppins commented Oct 9, 2017

Choose a reason for hiding this comment

IgorMinar left a comment

Choose a reason for hiding this comment

petebacondarwin commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mary-poppins commented Oct 10, 2017

mary-poppins commented Oct 10, 2017

petebacondarwin commented Oct 11, 2017

mary-poppins commented Oct 11, 2017

angular-automatic-lock-bot bot commented Sep 12, 2019