Submodule problems with Gazelle when simply using go_repository

[mplzik]

go-jsonnet's release packages don't contain cpp-jsonnet/stdlib, but does contain a BUILD file referring to this directory.

This breaks building this repository using go_repository module from rules_go (and also, makes gazelle's autogenerated configuration unusable). There's a workaround proposed by gazelle maintainers (bazel-contrib/bazel-gazelle#732 (comment)), but this requires manual configuration.

[seh]

This sounds like a duplicate of #375.

[sbarzowski]

Hmm... @seh should we do something more about it? Maybe we can put the information from #375 in the README?

[seh]

Yes, that's a good idea. Just pulling in the files we need from the C++ repository would also eliminate this problem.

Note that the rules_jsonnet just tripped on this again here: bazel-contrib/rules_jsonnet#129 (comment).

[mplzik]

Note that putting information into README.md will not help to avoid the bad experience of broken build when using gazelle to generate the BUILD files.

[sbarzowski]

Just pulling in the files we need from the C++ repository would also eliminate this problem.

Do you mean committing all the files we depend on directly in this repo?

[seh]

Do you mean committing all the files we depend on directly in this repo?

Yes. I thought they were "just" the standard library files, but it's been a while since I was familiar with it. Potential heresy: Invert the dependency by committing those files in this repository, pulling in this one from the C++ port's repository as a submodule—if we really can't tolerate the duplication.

[sbarzowski]

There are currently three shared things IIRC:

• stdlib
• libjsonnet header files
• tests files (I don't think Bazel uses that)

We wanted to eventually put the common stuff in one repo and have both go-jsonnet and cpp-jsonnet depend on that.

Anyway, I'm not an expert on this stuff, but I think there should be away to use Gazelle with submodules. We really don't do anything weird here. We certainly could just duplicate the files (and I'm ok with that if there is no better way), but it seems to me like going backwards.

[seh]

I don't see a way to tell Gazelle to generate a git_repository target rather than a go_repository target, telling Bazel how it should acquire the files for this dependency. bazel-contrib/bazel-gazelle#182 is related, at least vaguely.

Gazelle works fine in this project. The problem comes when someone else tries to depend on it, but they tell Bazel to only grab some of the required files (avoiding the Git submodule initialization step). Per @mplzik's complaint, in some cases Gazelle makes this mistake for you automatically.

It's not yet possible to download an archive from GitHub via HTTP that includes Git submodules, per dear-github/dear-github#214.

[gibfahn]

An issue with the git_repository target is that it requires you to have git in the PATH in your image, whereas http_archive is implemented in Bazel itself.

Is there a workaround for folks who don't want to use the git_repository method? I'd assume you would have to download the submodules directly, and then tell go-jsonnet where to find them.

[seh]

Yes, you can use the local_repository rule to tell Bazel that go-jsonnet is sitting in a particular directory, presumably with the Git submodule fetched. You would then express a dependency on that repository, rather than the upstream (remote) repository.

[gibfahn]

Yes, you can use the local_repository rule to tell Bazel that go-jsonnet is sitting in a particular directory

Right, but that doesn't work for CI or sharing, I was looking for a way to make this repo fully usable.

It does actually seem to be fairly straightforward to patch the rules in a project depending on jsonnet_go, although it would be even nicer if this repo allowed you to substitute @jsonnet// for @jsonnet_go//cpp-jsonnet, or just did that by default, as having the dependency has other benefits (like not needing to download jsonnet twice if you use it in another place as well).

WORKSPACE

# WORKSPACE

maybe(
    http_archive,
    name = "jsonnet_go",
    sha256 = "8ca930c892d34a119c1970431d159000321fe323734f06a1253bd78fc3625b84",
    strip_prefix = "go-jsonnet-0.16.0",
    patch_args = ["-p1"],
    # Check if patches need an update when you update this repo.
    # Patches generated by:
    # ```shell
    # git clone https://github.com/google/go-jsonnet && cd go-jsonnet
    # git checkout $tag
    # # Make required changes.
    # git diff > /path/to/jsonnet/jsonnet_go-submodule.patch
    # ```
    patches = [
        # jsonnet-go expects jsonnet to be a git submodule at @jsonnet-go//cpp-jsonnet,
        # but we provide it as @jsonnet//, so patch the places this is used.
        "//path/to:jsonnet_go-submodule.patch",
    ],
    url = "https://github.com/google/go-jsonnet/archive/v0.16.0.tar.gz",
)

maybe(
    http_archive,
    name = "jsonnet",
    sha256 = "fa1a4047942797b7c4ed39718a20d63d1b98725fb5cf563efbc1ecca3375426f",
    strip_prefix = "jsonnet-0.16.0",
    url = "https://github.com/google/jsonnet/archive/v0.16.0.tar.gz",
)

jsonnet_go-submodule.patch

diff --git a/astgen/BUILD.bazel b/astgen/BUILD.bazel
index 0bec2f1..b4a1376 100644
--- a/astgen/BUILD.bazel
+++ b/astgen/BUILD.bazel
@@ -2,7 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")

 genrule(
     name = "dumpstdlibast",
-    srcs = ["//cpp-jsonnet/stdlib"],
+    srcs = ["@jsonnet//stdlib"],
     outs = ["stdast.go"],
     cmd = "./$(location //cmd/dumpstdlibast) \"$<\" > \"$@\"",
     tools = ["//cmd/dumpstdlibast"],
diff --git a/c-bindings/BUILD.bazel b/c-bindings/BUILD.bazel
index 2e1151d..48dc3c6 100644
--- a/c-bindings/BUILD.bazel
+++ b/c-bindings/BUILD.bazel
@@ -18,7 +18,7 @@ go_library(
         "libjsonnet.cpp",
     ],
     cdeps = [
-        "//cpp-jsonnet/include:libjsonnet",
+        "@jsonnet//include:libjsonnet",
     ],
     cgo = True,
     copts = ["-Wall -Icpp-jsonnet/include"],  # keep

[seh]

If you look through the commit history for Bazel integration (#290), at first I did use the submodule like that, referring to it as a local subdirectory. Then I discovered that it didn't work for fresh checkouts, and backed up and instead treated it as a proper Git submodule.

In your proposed jsonnet_go-submodule.patch file shown above, were you intending to include the full content from the submodule? It looks like you still need to download the submodule content manually.

I agree that using local_repository doesn't solve all of your desired scenarios. I was just answering your question as to whether it's possible download the submodule and use it locally.

The best solutions would be one of these:

• Eliminate use of submodules altogether, duplicating the shared files
• Invert the dependency, and have the C++ port use the Go port as a submodule
• Teach Gazelle to be able to generate git_repository-like targets (Submodule problems with Gazelle when simply using go_repository #392 (comment))

[gibfahn]

In your proposed jsonnet_go-submodule.patch file shown above, were you intending to include the full content from the submodule? It looks like you still need to download the submodule content manually.

Sorry, maybe I wasn't clear. The patch file and the workspace snippet above (which does an http_archive for jsonnet and jsonnet_go) allow me to depend on jsonnet_go in my Bazel project without resorting to git_repository. I'm not suggesting you simply apply that to your repo.

Teach Gazelle to be able to generate git_repository-like targets (#392 (comment))

That doesn't apply to me, as my issue isn't that I'm using Gazelle, but rather that we're trying to make Bazel actually hermetic, and depending on random git binaries in the PATH is antithetical to that.

In terms of how go-jsonnet could make life easier here (without having to get rid of the submodule, which I assume is generally useful), I think the BUILD.bazel files would need to be updated to say "if @jsonnet_go//cpp-jsonnet exists use that, else look for an @jsonnet". I'm not entirely sure how best to do that in Bazel though.

[seh]

The patch file and the workspace snippet above (which does an http_archive for jsonnet and jsonnet_go) allow me to depend on jsonnet_go in my Bazel project without resorting to git_repository.

Oh, now I see what you're doing there. I missed the detail that you were trying to depend on both the C++ and Go port, asking Bazel to download both of them, and then stitching them together.

We could do that unconditionally instead of using Git submodules when building with Bazel. What's delicate, though, is ensuring that you rely on the same version of the C++ port that the Git submodule does. We can't be sure that the Go port would always refer to the latest C++ port version for its submodule. We could write a script, though, to verify that the two references (Git submodule and Bazel http_archive target) use the same commit hash.

I don't know whether Gazelle would then play along with all of that. Unfortunately I don't have the spare attention now to try this.

[sbarzowski]

Perhaps there is a way to read the submodule hash directly from .git/ without going too crazy? If not, I'm fine with having the commit hash specified explicitly in some file in the repo (we can add a CI check to make sure that it matches + note in the README that it needs to be updated).

[gibfahn]

We could do that unconditionally instead of using Git submodules when building with Bazel. What's delicate, though, is ensuring that you rely on the same version of the C++ port that the Git submodule does.

I think documenting the versions in the release notes (and maybe in the README.md code snippet that shows how to use these rules) would be adequate, at least it's what other bazelbuild rulesets do.

AIUI The bazel solution (at the moment) for all versioning questions is "responsibility of the user". User has to make sure all their deps are at the right versions, even things used everywhere like skylib. AIUI the "solution" is to use the Bazel Federation to make sure your versions are correct.