Convert repository to Bazel #10
Conversation
.bazelrc
Outdated
| # your project directory, which you must exclude from version control and your | ||
| # editor's search path. | ||
| build --symlink_prefix=dist/ | ||
| # To disable the symlinks altogether (including bazel-out) you can use |
There was a problem hiding this comment.
I imagine you can prune some of the instructions for alternatives you didn't elect
| # https://docs.travis-ci.com/user/languages/javascript-with-nodejs#Travis-CI-supports-yarn | ||
| - curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 1.13.0 | ||
| - export PATH="$HOME/.yarn/bin:$PATH" | ||
| - wget https://github.com/bazelbuild/bazel/releases/download/0.28.1/bazel_0.28.1-linux-x86_64.deb |
There was a problem hiding this comment.
this seems expensive and sad and maybe wrong.
on most projects we recommend Bazel just being an npm dependency in the package.json, assuming that developers on the project want to approach it as they would any frontend project.
Also you want to ensure that developers have the same version of Bazel as the CI. things are changing quickly enough that version skew will be a frequent breakage for devs
if you really want to use Bazel natively installed on the machine for some reason, then ideally you would do that in the docker image that the VM is booted with, not do a network fetch and install in each build, it's slow.
This is a lot of the motivation for us moving to CircleCI where you can pick your base docker image.
There was a problem hiding this comment.
Why would it be wrong? I do check the checksum. Also, a yarn install would also fetch Bazel from the network. Though I agree with you that developers may be using a different version.
What would be the alternative workflow? Is this the list below what you suggest?
- We assume the machine has yarn already installed. (But then, they may be using a different yarn version)
- Call
yarnto install dependencies - Use
yarn run bazelwhenever we want to call Bazel
BUILD.bazel
Outdated
| # Note: if you move the tsconfig.json file to a subdirectory, you can add an alias() here instead | ||
| # so that ts_library rules still use it by default. | ||
| # See https://www.npmjs.com/package/@bazel/typescript#installation | ||
| exports_files(["tsconfig.json"], visibility = ["//:__subpackages__"]) |
There was a problem hiding this comment.
have you run buildifier to format all the bazel files? you'll probably want to do that eventually so that code reviews don't devolve into whitespace fashion critique
There was a problem hiding this comment.
I just did now. I didn't think about doing it before. This is something one should mention in an end-to-end tutorial for Bazel. (Critical User Journeys!!!)
It's so complicated to add buildifier to the workspace: https://github.com/bazelbuild/buildtools/tree/master/buildifier#setup-and-usage-via-bazel. I wish I could do something like yarn add --dev buildifier.
I feel we need a package.json for Bazel WORKSPACES :-)
In any case, it's unclear I would want to pull all those dependencies. It will probably slow down my CI.
| @@ -1,3 +1,4 @@ | |||
| /// <amd-module name="outline_shadowsocksconfig/src/shadowsocks_config" /> | |||
There was a problem hiding this comment.
in some repos we do some stripping of this thing.
We typically use an npm_package target so that the thing that's distributed on npm isn't identical to the sources. And that way we don't check in any generated artifacts, which are confusing if the repo is in a state where the sources are updated but the generated artifacts are not
There was a problem hiding this comment.
FYI, I get 404 for https://bazelbuild.github.io/rules_nodejs/npm_package/npm_package.html and https://github.com/bazelbuild/rules_nodejs/blob/master/docs/npm_package/npm_package.html, the top 2 results for [bazel npm_package]
I never quite know where to look for rules documentation. I finally found it at https://bazelbuild.github.io/rules_nodejs/Built-ins.html. However, it doesn't really say what npm_package does. This is another example where an end-to-end tutorial could have helped a lot.
I ended up digging the source code, which has more useful documentation: https://github.com/bazelbuild/rules_nodejs/blob/master/internal/npm_package/npm_package.bzl
There was a problem hiding this comment.
On our specific use case, the reason for this repo is to share code between outline-server and outline-client. We don't really publish this as a public npm, but we do use it as a npm dependency, using a github specification.
I'm converting outline-server to Bazel, and having this repo as Bazel will probably help, since I'm still having issues with ShadowsocksConfig there.
There was a problem hiding this comment.
How do I strip the /// header? Is that an option in the build rule?
|
|
||
| ts_library( | ||
| name = "shadowsocks_config", | ||
| srcs = ["shadowsocks_config.ts"], |
There was a problem hiding this comment.
do you actually get benefits from using Bazel at such a small scope?
There was a problem hiding this comment.
Not much. The main reason is to:
- Make sure I'm doing the right thing, so I can apply the same in the bigger repos
- Make sure CI works
- Make it easier to consume this repo in outline-server that I'm converting to Bazel.
This will make it easier to use from outline-server.
@alexeagle, @rupertks: Can you sanity-check? Is there anything that we can do better?