proto_library: warning: directory does not exist

[jayconrod]

Description of the problem / feature request:

Building proto_library targets for Well Known Types in @com_google_protobuf emits warnings like the one below. The build succeeds, but it would be nice if the warning can be fixed or removed.

$ bazel build @com_google_protobuf//:any_proto
...
INFO: From Generating Descriptor Set proto_library @com_google_protobuf//:any_proto:
external/com_google_protobuf: warning: directory does not exist.
...

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

# WORKSPACE
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "com_google_protobuf",
    remote = "https://github.com/protocolbuffers/protobuf",
    tag = "v3.6.1.3",
)

git_repository(
    name = "bazel_skylib",
    remote = "https://github.com/bazelbuild/bazel-skylib",
    tag = "0.6.0",
)

bazel build @com_google_protobuf//:any_proto

What operating system are you running Bazel on?

Linux amd64

What's the output of bazel info release?

release 0.21.0

This does not happen in 0.20.0, but it does happen in 0.22.0rc2.

[jayconrod]

@lberki I wonder if this is related to recent source root changes?

[lberki]

/cc @haberman

Probably, I suppose? We did reshuffle the way protoc is called a bit. I'm traveling this week and next, though, so I probably won't be able to look at it closely until I get back home.

@dbabkin , can you help out?

[thaidn]

Friendly ping. This still happens in 0.22.0.

[dbabkin]

Hi,
There is no ETA when that will be fixed.
Priority of this task is low, as it is warning only.

[steeve]

Small ping, still happening in 0.25.3

[dapirian]

Ping again, this is extremely annoying and spammy.

[lberki]

Update: I have a change under review that will fix this issue. Let's see how the review goes. It's not complicated, so it shouldn't be problematic.

[lberki]

...and apologies for the delay. I know it was very annoying (in fact, the reason why I fixed it was because it annoyed me! :) )

[lberki]

Well, that was optimistic:

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1067#24a01f98-1ed2-4423-8d94-bf11cef557e4

The change will have to be rolled back. I'll take a look the next time I have some spare time.

[lberki]

Ugh. This is awful. Hear me out.

This warning messages happen when all the source files of a proto_library in an external repository are generated and therefore --proto_path=<root of external repository> is superfluous. However, the list of proto source roots comes from the same place that we use to determine the prefix we should cut off the exec path to obtain the proto import path.

Of course, we have to cut off different parts depending on whether the proto file is source or generated, but that we determine by... guesswork in guessProtoPathUnderRoot:

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java#L659

That algorithm broke when we removed the directory from the set of proto paths. The reason why only Go broke is that this line trying to cater for legacy use cases saves us, but I don't want to rely on it:

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java#L671

, but go_proto_library sensibly doesn't have its equivalent.

So that change was indeed wrong. Now, the question is, how to make it right -- eliminating the warning requires that we remove a repository root from the set of proto paths if there are only generated files, but if we remove it, the algorithm to generate -I clauses suddenly stops working.

Furthermore, it's not trivial to decouple --proto_path from -I because they are both derived from ProtoInfo, which is exposed to Starlark, so we can't just change it willy-nilly. In particular, it only has room for one direct proto source root, which is not enough: the principled fix is to make it so that ProtoInfo.proto_source_root is an execpath and not a root-relative path and if a proto_library has both generated and source .proto files, one would require both.

A saving grace is that ProtoInfo cannot currently be created from Starlark, so we may get away with adding a Java field that's not exposed to Starlark and leaving ProtoInfo.proto_source_root as a polite lie.

However, it's far from a trivial change :(

[hlopko]

Shouldn't we remove .proto_source_root by #7153?

[lberki]

This is not about the attribute, but the eponymous field of ProtoInfo.

[hlopko]

Yup, what I'm asking is if the field makes sense when the attribute is gone.

[lberki]

It does. That field tells what the effective proto source root is. That's a concept that makes sense even when the attribute will be gone.

[lberki]

So, I got a bit nerd sniped and I have another solution: simply create a virtual proto import directory when the proto_library rule has a generated .proto file just like we do when strip_import_prefix or import_prefix is set.

Granted, it's not that simple because we still need to make sure that our internal repository keeps chugging along, but I'd gladly fork the behavior of proto_library in this respect (they are different already, so no big change) if that's what I have to do to make proto_library saner for everyone else.

That's still not a tiny amount of work and it is still possible that some odd downstream project breaks, but this is certainly much simpler than the previous alternative. And it doesn't even require lying to Starlark!

[lberki]

Update: I have another fix. Learning from the previous one, I have to run in through all sorts of presubmits to make sure that it doesn't break anything. It introduces a slight incompatibility which I hope no one relies on, but there is only one way to be sure...

If that still doesn't work, I have another option (not generating virtual import directories for rules with proto_source_root= set, only for those with generated sources), which might still fail, but with a much lower probability. The downstream presubmit will tell.

[lberki]

Update: collateral damage is minimal. Unfortunately, it includes rules_rust and Bazel itself.

I'll send out changes to both then try to submit this.

[sgammon]

@lberki no worries, thank you for the update

[crutcher]

Is this ... dead?

[joshburkart]

@lberki Can we reopen this? It's still occurring in the latest version of Bazel, right?

[MegaPanchamZ]

@joshburkart

Can confirm the error still exists in Bazel 3.x.x

[phlax]

it would be great if this was fixed (and i guess the bug reopened)

we would like to make warnings into errors to prevent other proto generation issues, but i think these warnings make that not possible

[michael-lefkowitz-techlabs]

I'm still having this problem as well.

[comius]

The reproduction on this bug is ancient. I've tried reproduction with Bazel 5.0.0 and bzlmod:

MODULE.bazel

bazel_dep(name = "protobuf", version = "3.19.2", repo_name = "com_google_protobuf")

Command line:

bazel build --experimental_enable_bzlmod @com_google_protobuf//:any_proto

and I get no warnings.

Can somebody please provide a reproducer on Bazel 5.0.0? Or at least add a message to the thread with Bazel versions and protobuf versions where this occurs.

[phlax]

we are seeing this with bazel==4.2.1 and protobuf==3.19.3

[comius]

I tried with bazel 4.2.1, protobuf 3.19.2 and 3.19.3; no success reproducing. Something else is missing that's causing this.

WORKSPACE

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "com_google_protobuf",
    remote = "https://github.com/protocolbuffers/protobuf",
    tag = "v3.19.3",
)

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
protobuf_deps()

Command line:

bazel build  @com_google_protobuf//:any_proto

[alyssawilk]

We in this case is Envoy (https://github.com/envoyproxy/envoy). I've sent more detailed information internally for repro since the OSS instructions need some additions to build in-google, but our CI logs show it's still definitely a problem for our project and we're super happy to help you folks reproduce

[phlax]

hmm - grepping (envoyproxy/envoy) repo seems to suggest that we dont load protobuf_deps i wonder if that is the problem

ill, add PR and try and check...

[comius]

I see the warning here https://buildkite.com/bazel/envoy/builds/1811#0c20714c-e748-4c19-a6a5-844e9b4a0e70
I'll investigate further, thanks!

[phlax]

loading the protobuf deps doesnt seem to have hurt, but doesnt get rid of the messages

[comius]

The problem was created by proto_gen rule defined in https://github.com/protocolbuffers/protobuf project.

Correct issue is reported here: protocolbuffers/protobuf#6049
I created a PR fixing it: protocolbuffers/protobuf#9438

I'm closing this issue, because I didn't discover any warnings produced by the rules that are part of Bazel.

[phlax]

after being resolved for some time this has issue has returned (bazel: 6.1, protobuf: 23.1)

https://dev.azure.com/cncf/envoy/_build/results?buildId=143687&view=logs&j=8c169225-0ae8-53bd-947f-07cb81846cb5&t=4d0399a5-fd0d-5446-d022-32daa00183cc&l=296

tbh it seems more related to the protobuf version but the previous ticket in protobuf suggested it was a bazel issue

cc @alyssawilk @htuch

[phst]

Yeah, this is happening for me as well with Bazel 7 and Protobuf 23.1. Maybe this issue could be reopened? Or should we file a new issue?