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

[LittleBoxOfSunshine]

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:

repro/
    proto/
        first.proto
    build.rs

With build script:

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

    Ok(())
}

This emits:

[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:

       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.

[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.

[LittleBoxOfSunshine]

Similar topic to #1070 which concerns overinclusivity in what's emitted wheras this is a resolution issue

[LucioFranco]

Yeah, seems like a problem I am open to PRs.