grpc / grpc/grpc-rust

`compile_protos` emits unreachable `cargo:rerun-if-changed` that will always result in dirty

Open
#2,239 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
12.5k
Forks
1.3k
Avg merge
4d 7h
Merged PRs (30d)
24

Description

## Bug Report

### Version

Originally:

tonic 0.12.3
tonic 0.9.2
tonic_build 0.8.4

Repro for issue:
tonic_build 0.13.0

### Platform

Windows 11 Enterprise N 23H2 22631.5039 x86_64
+
Linux 0727077831c3 6.5.0-1025-azure 26~22.04.1-Ubuntu SMP Thu Jul 11 22:33:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

### Crates

`tonic_build`

### Description

`tonic_build` emits a `cargo:rerun-if-changed` based on the literal path that is provided, however that isn't necessarily the path that will be resolved. If a given path only resolves off a passed include, the emitted path is for a non-existent file resulting in cargo determining the cache is always dirty.

Consider this crate:
```text
repro/
proto/
first.proto
build.rs
```

With build script:

```rust
fn main() -> Result<(), Box> {
tonic_build::configure()
.compile_protos(
&[
"first.proto",
],
&[
// Contrived relative path
"../repro/proto",
],
)
.expect("Failed to compile protos.");

Ok(())
}
```

This emits:

```sh
[repro 0.1.0] cargo:rerun-if-changed=first.proto
[repro 0.1.0] cargo:rerun-if-changed=../repro/proto
```

`first.proto` isn't a valid file, the real file is `proto/first.proto`. So subsequent builds are always dirty:

```sh
Fresh hyper-timeout v0.5.2
Fresh tonic v0.13.0
Dirty repro v0.1.0 (C:\repro): the file `first.proto` is missing
Compiling repro v0.1.0 (C:\repro)
```

Changing `first.proto` to `../repro/proto/first.proto` resolves the issue.

```sh
[repro 0.1.0] cargo:rerun-if-changed=../repro/proto/first.proto
[repro 0.1.0] cargo:rerun-if-changed=../repro/proto

...

Fresh tonic v0.13.0
Fresh repro v0.1.0 (C:\repro)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.41s
```

In the real build I was debugging, the relative path went to a less nested portion of a large repro, and there where multiple protos needed from each include dir. The original author looked to be writing paths like you would for C++ includes. E.g. `/src/foo/some_crate/build.rs` needed several proto files from `/src/bar/protos/`. So `../../bar/protos` was specified as an include and then each proto file as just the file name.

If this type of input isn't the right way to use the function, I do still think it's a defect to have this succeed. It would be better if the emitted path is what's used for code generation, and then we would get a build failure for entering an invalid path.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.