Skip to content

Commit

Permalink
ui: vendor NPM modules
Browse files Browse the repository at this point in the history
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
  • Loading branch information
benesch committed Jul 3, 2018
1 parent fd0ba94 commit 369a292
Show file tree
Hide file tree
Showing 6 changed files with 544 additions and 5 deletions.
3 changes: 3 additions & 0 deletions .gitmodules
Expand Up @@ -20,3 +20,6 @@
[submodule "c-deps/googletest"]
path = c-deps/googletest
url = https://github.com/cockroachdb/googletest
[submodule "pkg/ui/yarn-vendor"]
path = pkg/ui/yarn-vendor
url = https://github.com/cockroachdb/yarn-vendored
11 changes: 6 additions & 5 deletions Makefile
Expand Up @@ -330,15 +330,16 @@ $(GITHOOKSDIR)/%: githooks/%
endif

.SECONDARY: pkg/ui/yarn.installed
pkg/ui/yarn.installed: pkg/ui/package.json pkg/ui/yarn.lock
$(NODE_RUN) -C pkg/ui yarn install
pkg/ui/yarn.installed: pkg/ui/package.json pkg/ui/yarn.lock pkg/ui/yarn.protobufjs-cli.lock
$(NODE_RUN) -C pkg/ui yarn install --offline
# Prevent ProtobufJS from trying to install its own packages because a) the
# the feature is buggy, and b) it introduces an unnecessary dependency on NPM.
# Additionally pin a known-good version of jsdoc.
# See: https://github.com/dcodeIO/protobuf.js/issues/716.
# We pin the ProtobufJS dependencies by linking in a lock file, which notably
# avoids depending on a buggy version of jsdoc.
cp pkg/ui/node_modules/protobufjs/cli/{package.standalone.json,package.json}
$(NODE_RUN) -C pkg/ui/node_modules/protobufjs/cli yarn add jsdoc@3.4.3
$(NODE_RUN) -C pkg/ui/node_modules/protobufjs/cli yarn install
ln -sf ../../../yarn.protobufjs-cli.lock pkg/ui/node_modules/protobufjs/cli/yarn.lock
$(NODE_RUN) -C pkg/ui/node_modules/protobufjs/cli yarn install --offline
@# We remove this broken dependency again in pkg/ui/webpack.config.js.
@# See the comment there for details.
rm -rf pkg/ui/node_modules/@types/node
Expand Down
1 change: 1 addition & 0 deletions pkg/ui/.yarnrc
@@ -0,0 +1 @@
yarn-offline-mirror yarn-vendor
142 changes: 142 additions & 0 deletions pkg/ui/README.md
Expand Up @@ -114,6 +114,148 @@ To run the tests outside of CI:
$ make test
```

## Managing dependencies

The NPM registry (and the Yarn proxy in front of it) have historically proven
quite flaky. Errors during `yarn install` were the leading cause of spurious CI
failures in the first half of 2018.

The solution is to check our JavaScript dependencies into version control, like
we do for our Go dependencies. Checking in the entire node_modules folder is a
non-starter: it clocks in at 230MB and 28k files at the time of writing.
Instead, we use a Yarn feature called the [offline mirror]. We ship a [.yarnrc]
file that instructs Yarn to save a tarball of each package we depend on in the
[yarn-vendor] folder. These tarballs are then checked in to version control. To
avoid cluttering the main repository, we've made the yarn-vendor folder a Git
submodule that points at the [cockroachdb/yarn-vendored] repository.

### Adding a dependency

Let's pretend you want to add a dependency on `left-pad`. Just use `yarn add`
like you normally would:

```bash
$ cd $GOPATH/src/github.com/cockroachdb/cockroach/pkg/ui
$ yarn add FOO
```

When Yarn finishes, `git status` will report something like this:

```bash
$ git status
Changes not staged for commit:
modified: pkg/ui/package.json
modified: pkg/ui/yarn-vendor (untracked content)
modified: pkg/ui/yarn.lock
```

The changes to package.json and yarn.lock are the normal additions of the new
dependency information to the manifest files. The changes to yarn-vendor are
unique to the offline mirror mode. Let's look more closely:

```bash
$ git -C yarn-vendor status
Untracked files:
left-pad-1.3.0.tgz
```

Yarn has left you a tarball of the new dependency. Perfect! If you were adding
a more complicated dependency, you'd likely see some transitive dependencies
appear as well.

The process from here is exactly the same as updating any of our other vendor
submodules. Make a new branch in the submodule, commit the new tarball, and push
it:

```bash
$ cd yarn-vendor
$ git checkout -b YOURNAME/add-left-pad
$ git add .
$ git commit -m 'Add left-pad@1.3.0'
$ git push origin add-left-pad
```

Be sure to push to [cockroachdb/yarn-vendored] directly instead of a personal
fork. Otherwise TeamCity won't be able to find the commit.

Then, return to the main repository and commit the changes, including the new
submodule commit. Push that and make a PR:

```bash
$ cd ..
$ git checkout -b add-left-pad
$ git add pkg/ui
$ git commit -m 'ui: use very smart approach to pad numbers with zeros'
$ git push YOUR-REMOTE add-left-pad
```

This time, be sure to push to your personal fork. Topic branches are not
permitted in the main repository.

When your PR has been approved, please be sure to merge your change to
yarn-vendored to master and delete your topic branch:

```bash
$ cd yarn-vendor
$ git checkout master
$ git merge add-left-pad
$ git push origin master
$ git push origin -d add-left-pad
```

This last step is extremely important! Any commit in yarn-vendored that is
referenced by the main repository must remain forever accessible, or it will be
impossible for future `git clone`s to build that version of Cockroach. GitHub
will garbage collect commits that are not accessible from any branch or tag, and
periodically, someone comes along and cleans up old topic branches in
yarn-vendored, potentially removing the only reference to a commit. By merging
your commit on master, you ensure that your commit will not go missing.

### Verifying offline behavior

Our build system is careful to invoke `yarn install --offline`, which instructs
Yarn to exit with an error if it would need to reach out to the network. Running
CI on your PR is thus sufficient to verify that all dependencies have been
vendored correctly.

You can perform the verification locally if you'd like, though:

```bash
$ cd $GOPATH/src/github.com/cockroachdb/cockroach/pkg/ui
$ rm -r node_modules yarn.installed
$ yarn cache clean
$ make
```

If `make` succeeds, you've vendored dependencies correctly.

### Removing a dependency

To remove a dependency, just run `yarn remove`.

Note that removing a dependency will not remove its tarball from the yarn-vendor
folder. This is not as bad as it sounds. When Git fetches submodules, it always
performs a full clone, so it would wind up downloading deleted tarballs anyway
when it fetched older commits.

TODO(benesch): Yarn's offline mode has an additional option,
`yarn-offline-mirror-pruning`, that cleans up removed dependencies' tarballs.
Look into using this once [dcodeIO/protobuf.js#716] is resolved. At the moment,
ProtobufJS tries to install some dependencies at runtime by invoking `npm
install` itself (!); we avoid this by running `yarn install` on its behalf, but
this means two separate packages share the same yarn-vendor folder and this
confuses Yarn's pruning logic.

If the size of the yarn-vendored repository becomes problematic, we can look
into offloading the large files into something like [Git LFS]. This is
contingent upon resolving the above TODO.

[cockroachdb/yarn-vendored]: https://github.com/cockroachdb/yarn-vendored
[dcodeIO/protobuf.js#716]: https://github.com/dcodeIO/protobuf.js#716
[main app bundle]: ./webpack.app.js
[Git LFS]: https://git-lfs.github.com
[offline mirror]: https://yarnpkg.com/blog/2016/11/24/offline-mirror/
[protos DLL]: ./webpack.protos.js
[vendor DLL]: ./webpack.vendor.js
[.yarnrc]: ./yarnrc
[yarn-vendor]: ./yarn-vendor
1 change: 1 addition & 0 deletions pkg/ui/yarn-vendor
Submodule yarn-vendor added at 13c980

0 comments on commit 369a292

Please sign in to comment.