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

ui: vendor NPM modules #27035

Merged
merged 1 commit into from
Jul 3, 2018
Merged

ui: vendor NPM modules #27035

merged 1 commit into from
Jul 3, 2018

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jun 27, 2018

Vendor the NPM packages that our UI uses by checking them into a
submodule, github.com/cockroachdb/yarn-vendored. This prevents NPM or
Yarn registry flakiness from failing our CI builds, and also ensures
that a git clone --recurse-submodules is sufficient to build
CockroachDB offline.

The feature that enables this is Yarn's "offline mirror." The blog post
that introduced the feature also serves as its documentation:

https://yarnpkg.com/blog/2016/11/24/offline-mirror/

To ensure the offline mirror is really used, we pass the '--offline'
flag to all Yarn invocations in the build system.

Fix #14615.
Really fix #21432.

Release note: None

@benesch benesch requested review from vilterp, bdarnell, couchand and a team June 27, 2018 20:59
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@bdarnell
Copy link
Contributor

LGTM, but there should be some documentation about how to add/update packages here.


Review status: :shipit: complete! 0 of 0 LGTMs obtained


Comments from Reviewable

@couchand
Copy link
Contributor

Thanks for keeping up with this stuff @benesch! Please don't merge until @vilterp and I have had a chance to check it out, thanks.

@benesch
Copy link
Contributor Author

benesch commented Jun 27, 2018

Yeah, waiting on writing documentation until its proven that TeamCity can actually build with this.

@benesch benesch force-pushed the yarn-offline branch 3 times, most recently from 83a59e5 to 0ca81f6 Compare June 28, 2018 20:55
@benesch
Copy link
Contributor Author

benesch commented Jun 28, 2018

Ok, TeamCity is happy! I've added instructions on how to manage dependencies in this new world.

Copy link
Contributor

@vilterp vilterp left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and the thorough docs! Sad that the repo is so flaky that this is necessary.

@couchand pointed out that rebasing branches before and after this is going to be tricky, but might as well do it now.


```bash
$ cd yarn-vendor
$ git checkout -b YOURNAME/add-left-pad
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

@couchand
Copy link
Contributor

couchand commented Jul 2, 2018

I had to make ui-maintainer-clean to get this to build. is that just me? if it's everyone that might deserve a shout out somewhere.

Otherwise LGTM

@benesch
Copy link
Contributor Author

benesch commented Jul 2, 2018

Hopefully just you! TeamCity and my GCE worker had no problem with it, for example. You didn't happen to save the error message, did you?

@couchand
Copy link
Contributor

couchand commented Jul 2, 2018

cp pkg/ui/node_modules/protobufjs/cli/{package.standalone.json,package.json}
ln -s ../../../yarn.protobufjs-cli.lock pkg/ui/node_modules/protobufjs/cli/yarn.lock
ln: failed to create symbolic link 'pkg/ui/node_modules/protobufjs/cli/yarn.lock': File exists
Makefile:334: recipe for target 'pkg/ui/yarn.installed' failed

@benesch
Copy link
Contributor Author

benesch commented Jul 2, 2018

Aha! Thanks. I know how to fix that.

Vendor the NPM packages that our UI uses by checking them into a
submodule, github.com/cockroachdb/yarn-vendored. This prevents NPM or
Yarn registry flakiness from failing our CI builds, and also ensures
that a `git clone --recurse-submodules` is sufficient to build
CockroachDB offline.

The feature that enables this is Yarn's "offline mirror." The blog post
that introduced the feature also serves as its documentation:

    https://yarnpkg.com/blog/2016/11/24/offline-mirror/

To ensure the offline mirror is really used, we pass the '--offline'
flag to all Yarn invocations in the build system.

Fix cockroachdb#14615.
Really fix cockroachdb#21432.

Release note: None
@benesch
Copy link
Contributor Author

benesch commented Jul 3, 2018

Ok, fixed. Merging! TFTRs.

bors r=couchand,vilterp,bdarnell

craig bot pushed a commit that referenced this pull request Jul 3, 2018
27035: ui: vendor NPM modules r=couchand,vilterp,bdarnell a=benesch

Vendor the NPM packages that our UI uses by checking them into a
submodule, [github.com/cockroachdb/yarn-vendored](https://github.com/cockroachdb/yarn-vendored). This prevents NPM or
Yarn registry flakiness from failing our CI builds, and also ensures
that a `git clone --recurse-submodules` is sufficient to build
CockroachDB offline.

The feature that enables this is Yarn's "offline mirror." The blog post
that introduced the feature also serves as its documentation:

    https://yarnpkg.com/blog/2016/11/24/offline-mirror/

To ensure the offline mirror is really used, we pass the '--offline'
flag to all Yarn invocations in the build system.

Fix #14615.
Really fix #21432.

Release note: None

Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
@craig
Copy link
Contributor

craig bot commented Jul 3, 2018

Build succeeded

@craig craig bot merged commit 369a292 into cockroachdb:master Jul 3, 2018
@benesch benesch deleted the yarn-offline branch July 3, 2018 15:39
benesch added a commit to benesch/cockroach that referenced this pull request Oct 4, 2018
`yarn install` requires the yarn-vendor submodule to be available.
This was missed when we started vendoring our Yarn dependencies in
PR cockroachdb#27035.

Fix cockroachdb#30619.

Release note: None
craig bot pushed a commit that referenced this pull request Oct 8, 2018
30993: build: check out submodules before yarn install r=couchand a=benesch

`yarn install` requires the yarn-vendor submodule to be available.
This was missed when we started vendoring our Yarn dependencies in
PR #27035.

Fix #30619.

Release note: None

Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
benesch added a commit to benesch/cockroach that referenced this pull request Oct 15, 2018
`yarn install` requires the yarn-vendor submodule to be available.
This was missed when we started vendoring our Yarn dependencies in
PR cockroachdb#27035.

Fix cockroachdb#30619.

Release note: None
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.

build: make build should work while offline build: Yarn registry errors can fail the build
5 participants