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

upgrade path for node and yarn and reproducible builds #67

Open
erickj opened this issue Mar 13, 2018 · 3 comments
Open

upgrade path for node and yarn and reproducible builds #67

erickj opened this issue Mar 13, 2018 · 3 comments

Comments

@erickj
Copy link
Contributor

erickj commented Mar 13, 2018

Hi all

As I spend more time digging into JS/bazel rule sets, rules_node seems to be garnering more and more of my attention. Given that bazel is still a nascent system and adopting a set of non-core build rules is a significant commitment I'd like to understand the longer term plans for rules_node.

Specifically the following:

  1. nodejs and yarn currently sit at 7.x.x and 1.0.1 respectively. With the current LTS node release at 8.10.x and yarn releasing a new version every ~1month or so, what are the plans for regular updates to the default versions of node and yarn? (FTR, the node 7.x branch is EOL in June.

As a quick check I verified that these versions pass against the current tests:
erickj@2ca8d6a

  1. reproducible builds: certainly switching to yarn was a good step towards slimmer/reproducible builds, however given that the yarn.lock file is part of the @yarn_modules external repo, rather than checked into version and --frozen-lockfile flags aren't used, isn't the point of the lock file lost? even if the yarn_modules repository rule is declared with only explicit versions, if the transitive deps of the NPM packages use range package versioning then these will generate inconsistent builds across bazel clean or even between CI runs, no? shouldn't the lock file live in the "including" repository as part of version control? and be passed into the yarn_modules rule for verification? similar to how checksums are used on other bazel external repository rules.

Something like this (which I partly borrowed from bazelbuild/rules_nodejs):
erickj@3e36ca9

Thoughts?

@pcj
Copy link
Contributor

pcj commented Mar 13, 2018

Hi @erickj,

Here's a little history of rules_node and node support in bazel in general. As an early bazel adopter, I found there to be a deficiency in a number of areas including protobuf and nodejs support. Getting protobufs to work broadly across languages requires at least basic support in all the popular languages, including of course node! There were a few node rules that had been announced, namely https://github.com/redfin/npm-bazel and https://github.com/wt/bazel_rules_nodejs. However, these did not work well with the proto story, so I wrote the initial ruleset to make rules_protobuf work with nodejs, as well as the various frontend and backend tasks I use node for.

The initial version of rules_node was okay, but there were a number of bugs that kept popping up, many related to runfiles / runtime path variation under various configurations. I did a few refactors to improve that, to various success.

Of course the dependency management story has been an important issue as well. The initial experiments with NPM demonstrated that yes bazel can be used to be the dependency management story, but the reproducibility was a problem, so I rewrote it using yarn.

There was another major rewrite as a result of #36. The problem was getting everything to simultaneously work in both a genrule and non-genrule context and supporting module-as-directory, module-as-file, etc. I'm pretty happy with the overall design after #41. Copying the node binary into the same folder where the node_modules tree simplified things alot due to some of the ways node deals with symlinks internally. There are some optimizations that could be easily done to reduce the number of actions being created with copying files around with the node_module rule.

Around that time @alexeagle released rules_typescript. In the name of compatibility with existing tools in the nodejs space, his vision of bazel + nodejs to was leave dependency management to existing tools like npm/yarn and focus on the build story. Now, Alex is one pretty smart dude with a ton of experience with blaze internally at google and major force in the way people are going to be typescript/angular projects now and into the future (i hope), so I pretty much trust his design decisions. rules_nodejs was borne out of that need, but certainly its main use case has been to service rules_typescript.

However, I still wanted nodejs dependency management to work well with bazel, because I want it to do everything with a bazel build, not have to run any extra steps, and take all advantage bazel's caching capabilities. So, my hope is that rules_node occupies at least a niche role in supporting nodejs users that are willing to go "whole-hog" on the bazel story. I think it has decent coverage of a number of advanced setups including polymer, electron, and now your rollup example (:+1:).

Now, getting back to your actual questions:

  1. The reason node is still on the 7.xx version was due to issues with native code and grpc with the v57 version of V8 (Failed to install grpc (version 1.0.1) grpc/grpc#8649). That deserves a second look. Although the node_repositories rule allows one to override, I agree pushing this forward faster would help discover issues that might exist with newer versions of node. I'm open to suggestions for a roadmap.

The reason we're still on yarn 1.0.1 is basically static friction; no-one has raised an issue with the current version.

  1. When I rewrote the rules to use yarn it was quite new, I learned as much as was necessary to get them functional. Your comments re: not taking advantage of the lock file are spot on; the PR in-progress looks like a potential solid addition. I still have questions as to the optimal way to maintain the yarn.lock file that should be committed to source control.

@erickj
Copy link
Contributor Author

erickj commented Mar 13, 2018

@pcj thanks for that background! That's very informative and answers my question re: the future upgrade path for nodejs

1 quick question, in the grpc thread that you linked, it seems 8.x.x may now be unblocked. is that not the case?

re: yarn lock, that branch is definitely a prototype in the current state. I also had some other ideas re: the yarn.lock contents and am not happy with the "free floating" file, as it is currently. My current thinking is that the contents of the lock file should be directly specified in the yarn_modules repository rule, but I didn't want to spend time optimizing a solution before agreeing on the problem. I'm eager to hear the alternatives you're considering.

Much more radically (and I'm not lobbying for this), I'm wondering if NPM dependency management should be handled in a more explicit step, similar to the way generate_workspace and other tools operate, where an external tool produces an explicit BUILD file that is intended to be copied into the workspace... effectively what parse_yarn_lock is doing, except extracted from yarn_modules repostiory_rule. The key difference being that the source of truth for NPM dependencies is a BUILD file rather than package.json, and dependency on yarn becomes nearly 0 during build phases (more on that below), and thus the need to manage a lock file disappears. If done properly (e.g. java_import_external) then caching and checksum management is entirely handled by bazel without the need to worry about yarn issues and the process becomes more consistent with bazel's native external repository rule implementations. However, w/o a clear method via NPM or Yarn to install individual packages in isolation w/o fetching their dependencies then this approach is probably infeasible given the complexity of the install process (bundles, production, dev deps, pre/post install scripts, etc...). But for a similar discussion see this thread from the go_rules maintainers, although in the end they decided against the idea.

re: rules_node generally, "So, my hope is that rules_node occupies at least a niche role in supporting nodejs users that are willing to go "whole-hog" on the bazel story."

This is exactly why I'm interested in investing effort into this project. IMO it strikes the right balance of doing NPM dependency management the "bazel way" (as opposed to bazelbuild/rules_nodejs) vs simplicity (as opposed to bazelbuild/rules_closure). To be fair, I have only briefly looked into some other implementations (e.g. rules_node variants like happy_co/rules_node, dropbox/rules_node and older ones like redfin/npm-bazel), but I've been quietly following the progress of the pubref repositories for a few years.

btw, thanks for your work so far.

@alexeagle
Copy link

alexeagle commented Mar 15, 2018 via email

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

No branches or pull requests

3 participants