build: make build should work while offline

[a-robinson]

I tried running make build while offline today, and it failed due to the build/node-run.sh -C ./pkg/ui yarn install step of the build. I was able to work around it by ripping everything related to YARN_INSTALLED_TARGET out of the Makefile, and the resulting binary's UI worked just fine.

It's possible that it works normally and only needed the network because I changed between branches that had different versions of the UI code on them, but even so it'd be nice if there was an easier way to build cockroach while offline than having to do manual Makefile surgery.

[benesch]

You can run make buildshort to build a binary without a UI, which will skip running everything associated with building the UI, including running yarn install.

I'm also inclined to fold this into #14615. Whatever solution we come up with for avoiding hitting the Yarn registry in CI will likely make building offline possible, too.

[a-robinson]

Ah, I wasn't aware of buildshort. Thanks!

[benesch]

Yeah, buildshort is a relatively recent development :)

[flokli]

@benesch I also ran into this issue while trying to build current master to test for any follow-up bugs after #19472 (comment) .

My distro (NixOS) completely blocks network access during builds, for reproducibility. I'm sure this is also the case on other distributions as well.

Couldn't we move the yarn install task into a separate Makefile step? Most likely we could also get rid of all the yarn add hacks in there, as there already is a yarn.lock file available…

[couchand]

I don't think there's anything actionable to do here.

Regular users should usually be building from tarball, which includes the prebuilt bindata.go.

If a dev wants to build a binary without the network, they run make buildshort and don't build the UI. If you need the UI, just make sure to have built it once before while on the network, and the yarn cache should take care of you.

@benesch any reason we shouldn't just close this issue?

[flokli]

@couchand So the yarn.lock being present in the repo doesn't yet solve the problem we're trying to fix near https://github.com/cockroachdb/cockroach/blob/master/Makefile#L324 ?

My idea about stripping this into separate build phases also goes out into the direction of distro packaging, as some packager build systems fetch source on their own and keep the build phase isolated from network access.

That's why I suggested a separate target to fetch external dependencies, and one to build everything assuming external deps are already there. The build target could then just execute those sequentially.

[benesch]

You could try `make pkg/ui/yarn.installed`, which will pull down the necessary NPM deps without actually building. As @couchand mentioned, though, your best bet is the source tarball, which includes a precompiled version of the UI.

…

On Wed, Feb 7, 2018 at 10:27 AM Florian Klink ***@***.***> wrote: @couchand <https://github.com/couchand> So the yarn.lock being present in the repo doesn't yet solve the problem we're trying to fix near https://github.com/cockroachdb/cockroach/blob/master/Makefile#L324 ? My idea about stripping this into separate build phases also goes out into the direction of distro packaging, as some packager build systems fetch source on their own and keep the build phase isolated from network access. That's why I suggested a separate target to fetch external dependencies, and one to build everything assuming external deps are already there. The build target could then just execute those sequentially. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21432 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA15IGLwkgnOslZQYm_tl6Xgq4kXLurZks5tScD1gaJpZM4RdAY5> .

[benesch]

Going to close based on the arguments presented by @couchand.

[bdarnell]

I think this is still worth doing something about (although not before 2.0). It's true that the source tarballs provide reproducible offline builds, but only at the go level. If I want to make a custom build with changed Go code, I can do that with the tarball. If I want to make a change to the UI, I need to pull in network resources to rebuild bindata.go (which makes it vulnerable to left-pad nonsense, etc).

I think the source tarballs should include the node_modules dependency subtree, so you have the option to rebuild the UI from them too (it's fine if they use a baked-in bindata.go by default).

[benesch]

@bdarnell I think that’s covered by #14615.

[bdarnell]

Yeah, this is a dupe of #14615 (although my point about including node_modules in source tarballs is kind of independent)

[couchand]

While I think the use case of someone wanting to pull the source tarball, modify the UI and rebuild without a network connection is awfully marginal, I'm not opposed to adding the node_modules directory to ARCHIVE_EXTRAS.

I don't think this is the solution to concerns about the left-pad problem (or should we be calling it the gobindata problem now?). Perhaps we should add node_modules to https://github.com/cockroachdb/vendored/, as that seems like it would actually solve it for mainline development, where it's much more critical.

[bdarnell]

I don't think this is the solution to concerns about the left-pad problem (or should we be calling it the gobindata problem now?).

Fair enough, but we solve the gobindata problem with our vendor repo. Here we're talking about node_modules, so it's the left-pad problem :-P

Perhaps we should add node_modules to https://github.com/cockroachdb/vendored/, as that seems like it would actually solve it for mainline development, where it's much more critical.

I'm not sure I agree that it's more critical for mainline development than for release archives, but I agree that adding node_modules to the vendor repo would be a good move. I'm just making a cost/benefit argument. Adding the node_modules source to the tarball is (hopefully) trivial, while changing our js dependency workflow to incorporate the submodule is more of a pain.