implement bazel as build system #568

tgummerer · 2024-04-16T11:05:54Z

Use bazel as build system. Right now it's working side by side with the old Makefiles, and we're adding an extra CI job to run the builds. Eventually this can completely replace the Makefile based builds.

(I'll comment inline on some of the changes required for this)

tgummerer

Most of this was generated using gazelle, which is very convenient to work with especially for Go repos (@lunaris didn't find it as nice for python).

There's a few commands that are useful to know for bazel:

bazel build //... - builds the whole repo, and leaves the artifacts in a bazel-bin directory.
bazel test //... - run all the tests. That's probably the most convenient command for this repo at all times, but we can specify subparts of the repo as well, e.g. bazel test //pkg/pulumiyaml/codegen to run only the codegen tests
bazel run //:gazelle - re-runs gazelle, and updates the BUILD files. This is most useful when new files are added or removed.
bazel run //:gazelle-update-repos - this has to be run after updating the dependencies in the go.mod file, and it will update all the dependencies bazel sees (those are specified specially in the deps.bzl file, separate from go.mod and go.sum, this command can essentially be thought of as "sync go.mod to deps.bzl")

tgummerer · 2024-04-16T11:07:04Z

build/0001-cloudflare.patch

@@ -0,0 +1,58 @@
+diff --git a/dh/x25519/BUILD.bazel b/dh/x25519/BUILD.bazel


The cloudflare library doesn't compile correctly with the bazel build files it generations, so it needs this little patch to build.

tgummerer · 2024-04-16T11:07:46Z

go.mod

@@ -2,51 +2,52 @@ module github.com/pulumi/pulumi-yaml

 go 1.19

+replace github.com/hashicorp/vault/api v1.8.2 => github.com/hashicorp/vault/api v1.9.0


github.com/hashicorp/vault/api v1.8.2 doesn't compile correctly, v1.9.0 has the fixes.

tgummerer · 2024-04-16T11:08:18Z

go.mod

@@ -2,51 +2,52 @@ module github.com/pulumi/pulumi-yaml

 go 1.19

+replace github.com/hashicorp/vault/api v1.8.2 => github.com/hashicorp/vault/api v1.9.0
+
+replace google.golang.org/protobuf v1.33.0 => google.golang.org/protobuf v1.31.0


This is the most awkward think imho about this whole PR. The protobuf 1.33.0 library causes a circular dependency, so we need to downgrade here.

tgummerer · 2024-04-16T11:09:07Z

go.mod

+	github.com/pulumi/pulumi/pkg/v3 v3.112.1-0.20240415121624-fc23f9bb0851
+	github.com/pulumi/pulumi/sdk/v3 v3.112.1-0.20240415121624-fc23f9bb0851


Just waiting until v3.114.0 gets released, which includes pulumi/pulumi#15936, which is necessary for the tests to work.

tgummerer · 2024-04-16T11:10:21Z

pkg/pulumiyaml/run_plugin_version_test.go

@@ -29,11 +28,11 @@ resources:
 	assert.False(t, diags.HasErrors())

 	got := plugins
-	want := autogold.Want("test-plugins", []Plugin{{


The autogold librarry doesn't work with bazel. I don't feel like its usage is crucial anywhere we have it, the couple of places we're using it are barely worth having the extra dependency, especially because this shouldn't change frequently anyway.

:'(

I love snapshot testing. Why does this not work?

For the same reason this change is needed: #568 (comment)

Bazel creates a separate directory structure with symlinks to the original files to run the tests. So autogold can't copy the file back to the repo. Note that we can still work the whole PULUMI_ACCEPT thing work via just running the tests via the normal Go tooling, just the autogold library wants to doesn't like the symlinks even when it's not supposed to change anything for some reason.

tgummerer · 2024-04-16T11:12:29Z

pkg/pulumiyaml/run_test.go

@@ -409,10 +429,12 @@ outputs:
 func TestAssetOrArchive(t *testing.T) {
 	t.Parallel()

-	const text = `name: test-yaml
+	file := createTestFile(t)


Bazel symlinks all files to its own directory structure, to be able to know the exact inputs for each build. In this particular case, if we have a symlink pulumi-yaml tries to follow it, and then realizes that it's outside of the directory we're working in, and errors out. We're generating a test file here instead, which seems no worse than relying on the readme here.

tgummerer · 2024-04-16T11:13:14Z

scripts/get_schemas.sh

@@ -51,7 +57,7 @@ for s in "${schemas[@]}"; do
  URL="${URL//_VERSION_/${VERSION}}"


-  FILEPATH="pkg/pulumiyaml/testing/test/testdata/${NAME}-${VERSION}.json"
+  FILEPATH="${dir}pkg/pulumiyaml/testing/test/testdata/${NAME}-${VERSION}.json"


These files are not generated in the source tree anymore, but in the bazel directory structure, and thus we need to allow bazel to tell us where that is. The script will still work as it is.

Use bazel as build system. Right now it's working side by side with the old Makefiles, and we're adding an extra CI job to run the builds. Eventually this can completely replace the Makefile based builds.

lunaris · 2024-04-16T11:23:31Z

BUILD.bazel

+    name = "examples",
+    srcs = glob(["examples/**"]),
+    visibility = ["//visibility:public"],
+)


Do we need these filegroups? Apologies if I'm missing where they are used.

We do, these are used as testdata (examples here: https://github.com/pulumi/pulumi-yaml/pull/568/files/3faa6f2aa177fbe335417c7e28e361a950ecff86#diff-4f92936bcd4ef2a410b5b8690f0ce55344be249565741c2e27e63dab4c18e76cR31, README and go_mod here: https://github.com/pulumi/pulumi-yaml/pull/568/files/3faa6f2aa177fbe335417c7e28e361a950ecff86#diff-c980a768eafaa33a15061ead8b5880713599b0d408b918e930b94464611e6086R61)

lunaris · 2024-04-16T11:23:49Z

MODULE.bazel

+# Please consider migrating your external dependencies from WORKSPACE to MODULE.bazel.
+#
+# For more details, please check https://github.com/bazelbuild/bazel/issues/18958
+###############################################################################


Can probably delete this comment and leave the file empty

Done ✔️

lunaris · 2024-04-16T11:24:01Z

WORKSPACE

+
+load("@rules_python//python:repositories.bzl", "py_repositories")
+
+py_repositories()


Is rules_python being used in this workspace?

It's used by protobuf apparently. If I remove this protobuf complains about not being able to find python rules. Not sure why exactly that is and it doesn't specify it itself.

AaronFriel · 2024-04-16T15:24:22Z

build/0001-cloudflare.patch

+         "table.go",
+        "//math/fp25519:fp_amd64.h",
+     ],
+    cgo = True,


The diff here is quite large - does this reenable CGO_ENABLED writ large? Can we avoid that?

CGO_ENABLED writ large

Not sure what you mean with this?

This enables CGO for this library. I don't know Go tooling enough to know whether that transitively enables it in the dependent libraries though.

Just trying to page in why this was needed in the first place, it's just that gazelle (which also generates build files for external dependencies) just doesn't deal with the CGO structure of this particular dependency properly. The library needs to be build with CGO either way, so that shouldn't change anything here I believe.

I don't quite understand this - this library requires CGO but the rest of the binary build won't?

Maybe I don't understand how that could work with Go.

I do think we should not regress and switch to CGO_ENABLED builds, it makes cross compilation much more challenging and as Go developers say, CGO is not go. There are downsides to CGO_ENABLED=1, and we finally extricated ourselves from that a year or so ago. Going back will lead to surprises.

https://dave.cheney.net/2016/01/18/cgo-is-not-go

Actually turns out the cgo = True was not actually needed, and I just copy-pasta'd too much stuff from the internet 😅

I do think this needs a little bit of additional patching to make it work on non-linux platforms, but that should be fairly straightforward.

AaronFriel

As someone who hacks on multiple Pulumi repos frequently, my biggest concern is that this could make that more difficult if I currently use Go Workspaces. It seems like we'd need to keep the makefile and have two build systems to make that work.

Similar to that though, this appears to tightly couple bazel files to source filenames in this and other repos - that seems really tough to keep in sync. Is there a way to use wildcards or some mechanism so that adding new source files to packages in Pulumi YAML and others doesn't affect Bazel? Or if those src files are not tightly coupled, do we need to commit them?

AaronFriel · 2024-04-16T15:28:05Z

pkg/pulumiyaml/BUILD.bazel

+go_library(
+    name = "pulumiyaml",
+    srcs = [
+        "analyser.go",


Is it possible to more loosely couple this? It seems strange to require this here. Or, if these files are auto generated (by Gazelle?): should they be committed or gitignored?

Bazel does support glob so e.g. srcs = glob(["**/*.go"]) could be used here. Currently these are I believe generated by Gazelle, so I expect it's a choice of "do we want to maintain BUILD.bazel files ourselves" (use glob, don't use Gazelle) or "do we want to try and pretend we're 'just writing Go' and use Gazelle to generate BUILD.bazel files".

We could still use globs, gazelle will actually support that just fine and not overwrite it, it's pretty smart that way! Similarly, I had to adjust the data passed to tests in a few places. So the build files are mostly auto-generated, but they need to be tweaked by hand, because we are not always following the standard convention, so gazelle can't generate that for you.

In a way I didn't bother with globs, exactly because I expect it's easiest to just use Gazelle to write these build files for us. This is an extra command to run, but that seems like a worthy tradeoff for having faster and correct builds

tgummerer · 2024-04-16T15:58:40Z

As someone who hacks on multiple Pulumi repos frequently, my biggest concern is that this could make that more difficult if I currently use Go Workspaces. It seems like we'd need to keep the makefile and have two build systems to make that work.

I'm really curious what your workflow for this is! Do you have a go.work file that lives outside of the repos and ties them all together in some way? Do you run the make commands to do some setup inside the repo and then run go command outside, tied together by the Workspaces?

One of the major advantages of Bazel (not necessarily in this repo, but once we have more repos set up with it) is that it supports multiple languages. E.g. currently pulumi-dotnet uses a completely different build system. pulumi/pulumi uses makefiles that run various tools for each language, and the targets are not very fine grained, which makes it tricky to work with, and the onboarding experience honestly quite bad (In my 6 months working on it I've never been able to just run make in the repo and have it build everything).

AaronFriel · 2024-04-16T17:39:11Z

Yeah, I use a go.work file in a parent directory to all of my GitHub packages - though I will sometimes need to set GOWORK=off when working outside of those. The current contents are:

go 1.21.0

use (
        ./pulumi-java/pkg
        ./pulumi/pkg
        ./pulumi/sdk
        ./pulumi/tests
)

And as a result, everything I expect works in my dev tools, including (importantly) running go-to-definition, tests, process debugging. I can make install in the Pulumi repo, make provider in a provider repo, and so on.

tgummerer · 2024-04-17T05:52:47Z

And as a result, everything I expect works in my dev tools, including (importantly) running go-to-definition, tests, process debugging.

This will continue working with or without the makefiles. There's no changes in the Go tooling here, and I expect us to be able to continue running tests outside of the build system similar to how that's currently possible. In fact Gazelle expects the repo to follow the Go conventions to work, so there's no issue here.

The only difference would be here:

I can make install in the Pulumi repo, make provider in a provider repo, and so on.

make install would either be replaced by a script, or calling bazel directly, or we could even have make install just call bazel for backwards compatibility. So really the only difference would be that you need to have bazel installed. The intention is the have everything else in the dev workflow you're describing still working except without make, but with bazel instead.

AaronFriel · 2024-04-17T08:16:17Z

That's a relief and I appreciate your humoring my questions! Just want to make sure I understand the limitations, if any.

Re: make install changing, it sounds like, however, for end to end testing - building Pulumi YAML and Pulumi CLI using my workspace source - this might not work as expected? Or I would need to learn the "bazel way" to link together multiple modules from source?

tgummerer · 2024-04-17T09:25:04Z

That's a relief and I appreciate your humoring my questions! Just want to make sure I understand the limitations, if any.

For sure, I definitely don't want to break anyones workflow, or at the very least make sure there's an easy transition.

testing - building Pulumi YAML and Pulumi CLI using my workspace source - this might not work as expected? Or I would need to learn the "bazel way" to link together multiple modules from source?

Ah interesting, I hadn't considered building it that way tbh. I don't think this would work out of the box as it does right now, but before we consider removing the make targets in pulumi/pulumi we should make sure that this works as expected. This might mean writing a little bit of starlark code to make it easy for users, but it should definitely be doable to have something that either just works, or is very easy and well documented.

AaronFriel · 2024-04-17T15:21:06Z

Yeah, it's very common - because of Pulumi's distributed system nature - that we need to make cross-cutting changes to the Go Pulumi/Pulumi SDK module that affect both the engine and a plugin.

tgummerer commented Apr 16, 2024

View reviewed changes

implement bazel as build system

3faa6f2

Use bazel as build system. Right now it's working side by side with the old Makefiles, and we're adding an extra CI job to run the builds. Eventually this can completely replace the Makefile based builds.

tgummerer force-pushed the tg/add-bazel branch from 18fc8bd to 3faa6f2 Compare April 16, 2024 11:18

tgummerer added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Apr 16, 2024

tgummerer marked this pull request as ready for review April 16, 2024 11:19

tgummerer requested a review from a team as a code owner April 16, 2024 11:19

lunaris reviewed Apr 16, 2024

View reviewed changes

empty out module.bazel file

5a7a3da

AaronFriel reviewed Apr 16, 2024

View reviewed changes

tgummerer added 4 commits April 19, 2024 17:54

format

2e5ab2b

update to latest SDK and pkg

721c8cf

remove cgo=True

6e6a0d9

go mod tidy

70c56ae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement bazel as build system #568

implement bazel as build system #568

tgummerer commented Apr 16, 2024

tgummerer left a comment

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

AaronFriel Apr 20, 2024

tgummerer Apr 22, 2024

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

lunaris Apr 16, 2024

tgummerer Apr 16, 2024

lunaris Apr 16, 2024

tgummerer Apr 16, 2024

lunaris Apr 16, 2024

tgummerer Apr 16, 2024

AaronFriel Apr 16, 2024

tgummerer Apr 16, 2024

tgummerer Apr 16, 2024

AaronFriel Apr 20, 2024

tgummerer Apr 22, 2024

AaronFriel left a comment

AaronFriel Apr 16, 2024

lunaris Apr 16, 2024

tgummerer Apr 16, 2024

tgummerer commented Apr 16, 2024

AaronFriel commented Apr 16, 2024

tgummerer commented Apr 17, 2024

AaronFriel commented Apr 17, 2024

tgummerer commented Apr 17, 2024

AaronFriel commented Apr 17, 2024

		@@ -0,0 +1,58 @@
		diff --git a/dh/x25519/BUILD.bazel b/dh/x25519/BUILD.bazel

		@@ -2,51 +2,52 @@ module github.com/pulumi/pulumi-yaml

		go 1.19

		replace github.com/hashicorp/vault/api v1.8.2 => github.com/hashicorp/vault/api v1.9.0

		github.com/pulumi/pulumi/pkg/v3 v3.112.1-0.20240415121624-fc23f9bb0851
		github.com/pulumi/pulumi/sdk/v3 v3.112.1-0.20240415121624-fc23f9bb0851


		load("@rules_python//python:repositories.bzl", "py_repositories")

		py_repositories()

implement bazel as build system #568

Are you sure you want to change the base?

implement bazel as build system #568

Conversation

tgummerer commented Apr 16, 2024

tgummerer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AaronFriel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgummerer commented Apr 16, 2024

AaronFriel commented Apr 16, 2024

tgummerer commented Apr 17, 2024

AaronFriel commented Apr 17, 2024

tgummerer commented Apr 17, 2024

AaronFriel commented Apr 17, 2024