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

Convert repository to Bazel #10

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .bazelignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules
59 changes: 59 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Common Bazel settings for JavaScript/NodeJS workspaces
# This rc file is automatically discovered when Bazel is run in this workspace,
# see https://docs.bazel.build/versions/master/guide.html#bazelrc
#
# The full list of Bazel options: https://docs.bazel.build/versions/master/command-line-reference.html

# Bazel will create symlinks from the workspace directory to output artifacts.
# Build results will be placed in a directory called "dist/bin"
# Other directories will be created like "dist/testlogs"
# Be aware that this will still create a bazel-out symlink in
# 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

Choose a reason for hiding this comment

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

I imagine you can prune some of the instructions for alternatives you didn't elect

Copy link
Author

Choose a reason for hiding this comment

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

Removed

# build --symlink_prefix=/
# however this makes it harder to find outputs.

# Specifies desired output mode for running tests.
# Valid values are
# 'summary' to output only test status summary
# 'errors' to also print test logs for failed tests
# 'all' to print logs for all tests
# 'streamed' to output logs for all tests in real time
# (this will force tests to be executed locally one at a time regardless of --test_strategy value).
test --test_output=errors

# Support for debugging NodeJS tests
# Add the Bazel option `--config=debug` to enable this
test:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=9999 --nocache_test_results

# Turn off legacy external runfiles
# This prevents accidentally depending on this feature, which Bazel will remove.
run --nolegacy_external_runfiles
test --nolegacy_external_runfiles

# Turn on the "Managed Directories" feature.
# This allows Bazel to share the same node_modules directory with other tools
# NB: this option was introduced in Bazel 0.26
# See https://docs.bazel.build/versions/master/command-line-reference.html#flag--experimental_allow_incremental_repository_updates
common --experimental_allow_incremental_repository_updates

# Turn on --incompatible_strict_action_env which was on by default
# in Bazel 0.21.0 but turned off again in 0.22.0. Follow
# https://github.com/bazelbuild/bazel/issues/7026 for more details.
# This flag is needed to so that the bazel cache is not invalidated
# when running bazel via `yarn bazel`.
# See https://github.com/angular/angular/issues/27514.
build --incompatible_strict_action_env
run --incompatible_strict_action_env

# Load any settings specific to the current user.
# .bazelrc.user should appear in .gitignore so that settings are not shared with team members
# This needs to be last statement in this
# config, as the user configuration should be able to overwrite flags from this file.
# See https://docs.bazel.build/versions/master/best-practices.html#bazelrc
# (Note that we use .bazelrc.user so the file appears next to .bazelrc in directory listing,
# rather than user.bazelrc as suggested in the Bazel docs)
try-import %workspace%/.bazelrc.user

5 changes: 3 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules/
/build/*.spec.*
/yarn-error.log
dist
bazel-out
node_modules
20 changes: 13 additions & 7 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.

language: node_js
dist: trusty
language: minimal

node_js:
- 10
addons:
apt:
sources:
- ubuntu-toolchain-r-test
packages:
- wget
- pkg-config

cache:
yarn: true
fortuna marked this conversation as resolved.
Show resolved Hide resolved

before_install:
# 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

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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?

  1. We assume the machine has yarn already installed. (But then, they may be using a different yarn version)
  2. Call yarn to install dependencies
  3. Use yarn run bazel whenever we want to call Bazel

- sha256sum -c tools/bazel_0.28.1-linux-x86_64.deb.sha256
- sudo dpkg -i bazel_0.28.1-linux-x86_64.deb

script:
- yarn test
- bazel --bazelrc=tools/travis.bazelrc test //src/...
23 changes: 23 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright 2019 The Outline Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Add rules here to build your software
# See https://docs.bazel.build/versions/master/build-ref.html#BUILD_files

# Allow any ts_library rules in this workspace to reference the config
# 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__"])

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.


52 changes: 52 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright 2019 The Outline Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Bazel workspace created by @bazel/create 0.35.0

# Declares that this directory is the root of a Bazel workspace.
# See https://docs.bazel.build/versions/master/build-ref.html#workspace
workspace(
# How this workspace would be referenced with absolute labels from another workspace
name = "outline_shadowsocksconfig",
# Map the @npm bazel workspace to the node_modules directory.
# This lets Bazel use the same node_modules as other local tooling.
managed_directories = {"@npm": ["node_modules"]},
)

# Install the nodejs "bootstrap" package
# This provides the basic tools for running and packaging nodejs programs in Bazel
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = "build_bazel_rules_nodejs",
sha256 = "6625259f9f77ef90d795d20df1d0385d9b3ce63b6619325f702b6358abb4ab33",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/0.35.0/rules_nodejs-0.35.0.tar.gz"],
)

# The yarn_install rule runs yarn anytime the package.json or yarn.lock file changes.
# It also extracts and installs any Bazel rules distributed in an npm package.
load("@build_bazel_rules_nodejs//:defs.bzl", "yarn_install")
yarn_install(
# Name this npm so that Bazel Label references look like @npm//package
name = "npm",
package_json = "//:package.json",
yarn_lock = "//:yarn.lock",
)

# Install any Bazel rules which were extracted earlier by the yarn_install rule.
load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")
install_bazel_dependencies()

# Setup TypeScript toolchain
load("@npm_bazel_typescript//:index.bzl", "ts_setup_workspace")
ts_setup_workspace()
9 changes: 0 additions & 9 deletions bower.json

This file was deleted.

1 change: 1 addition & 0 deletions build/shadowsocks_config.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/// <amd-module name="outline_shadowsocksconfig/src/shadowsocks_config" />

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

How do I strip the /// header? Is that an option in the build rule?

export declare class ShadowsocksConfigError extends Error {
constructor(message: string);
}
Expand Down
Loading