chore: update to http 1.x crates

[flokli]

This requires updating prost crates to 0.13.x, tonic to 0.12.x and prost-wkt-* to 0.6.*.

Only the examples still pull in http 0.x, due to
seanmonstar/warp#1090, but that shouldn't affect consumers of bigtable_rs.

Closes #70
Closes #77
Closes #78
Closes #79
Closes #80
Closes #81
Closes #82
Closes #84
Closes #85
Closes #87
Closes #88

[flokli]

@cbrewster helped me out on finishing this :-)

hyper doesn't use tokio types anymore, so an adapter from hyper_utils has to be used: https://github.com/hyperium/hyper/blob/master/CHANGELOG.md#breaking-changes-3 / hyperium/hyper@f9f65b7

[flokli]

This now also includes the tonic[-build] bump to 0.12.2. @liufuyang, can you take a look at this?

[liufuyang]

Thanks a lot. I tried your change locally and it works. Besides a small fix I mentioned above, if you want, you can also try uncommenting all the commented lines in the build.rs
https://github.com/liufuyang/bigtable_rs/blob/main/bigtable_rs/build.rs#L8-L88 , then run cargo build, then you will see some rs files related to google proto updated. Then you can commit those in as well.

Looks like they finally fixed an issue with the newer prost-build, so the generated google proto rust code can handle the text block in the comment nicely, then we do not need to comment build.rs script anymore.

Previously I had to run build.rs, then manually fix the problem in some comments part of the generated code, then comment out the section in build.rs (otherwise next time building overrides and brings the problem back again).

So if you want, you can also bring the updated google proto rs after enabling build.rs.

[flokli]

Sure! I removed the out-commenting in build.rs, ran cargo check and cargo fmt and committed the changes. PTAL!

[flokli]

Not sure about this comment. The git submodule is pointing to upstream.

[liufuyang]

Yeah that is a patched proto so the submodule is pointing to my patched googleapis repo.

[liufuyang]

@flokli Thanks a lot. This looks good, however the diff looks a bit different from my local branch. I also just tried to checkout your branch, run cargo build, then I can see these changes:

 bigtable_rs/src/google/google.api.rs                           |  21 ---
 bigtable_rs/src/google/google.bigtable.v2.rs                   | 357 ++++++++++++++++++++++---------------------
 bigtable_rs/src/google/google.cloud.conformance.bigtable.v2.rs |   3 -
 bigtable_rs/src/google/google.rpc.rs                           |   1 -
 4 files changed, 183 insertions(+), 199 deletions(-)

So if you run cargo build on your branch, does it look the same or your branch will still be clean?
Also make sure the googleapis submodule is the same version as in this repo googleapis @ 1a99ec0

[liufuyang]

Interesting, the build script result seems can be cached.

Do you mind doing these on your side (otherwise I can take it over if you want).

1. add a file here bigtable_rs/rustfmt.toml and put this in it:

ignore = [
    "src/google",
]

Do not add rustfmt.toml mentioned above, but update build.rs, see my comment blow.
2. Do a cargo clean and cargo build, and see changed lines git diff --stat, it should be like this (on my side):

git diff --stat
 bigtable_rs/build.rs                                           |  7 +++++
 bigtable_rs/src/google/google.api.rs                           | 21 --------------
 bigtable_rs/src/google/google.bigtable.v2.rs                   | 66 --------------------------------------------
 bigtable_rs/src/google/google.cloud.conformance.bigtable.v2.rs |  3 --
 bigtable_rs/src/google/google.rpc.rs                           |  1 -

3. If you do a cargo fmt and checkout the diff again, it should remain the same.

Let me know if it works for you.

[liufuyang]

Well, I just realized the ignore option in rustfmt.toml is not stable, so it does not work directly on stable rustc.... That is a bit annoying... Maybe we should still fall back to the idea of commenting out the build script... 😞

[liufuyang]

Okay, I've got an idea, we can do a cargo fmt in the end of the build script, so add these in the build.rs will do:

use std::process::Command;

fn main() -> Result<(), Box<dyn std::error::Error>> {
...
...
...    
    Command::new("cargo")
        .arg("fmt")
        .output()
        .expect("failed to execute cargo fmt");
    
    Ok(())
}

Then we do not need rustmft.toml anymore.

I have an almost identical PR here, which you can follow or copy, but I am using that one to test CI failing if git is dirty after build (I manually added a small line change in the generated file to test this). #91

The difference of the generated file on my side can be seen here c57655c, not sure why those #[allow(clippy::derive_partial_eq_without_eq)] get removed in my generated file but not on your initial commit above.

[flokli]

Isn't adding a shelling out to cargo fmt breaking compilation in places that don't use cargo? Maybe we should invoke rustfmt directly instead, or figure out why we're not emitting properly formatted code in first place…

I ran cargo clean && cargo test, after ensuring googleapis/ is at 1a99ec01f2a09389f7b4334140402f7a1a4eb426.

An inserted test comment further up got removed, so the file is recreated (and the #[allow(clippy::derive_partial_eq_without_eq)] are still there).

I'm using cargo/rustc 1.79.0, and protobuf 25.3, in case it matters.

[liufuyang]

That is a bit strange. Perhaps also try cargo update then do cargo clean and cargo build? I am not sure what is going on here.

Yes we can invoke rustfmt as well, I never used it directly, so feel free to implement in that way.

Otherwise, do you mind do a simple rebase of your branch to the current main branch in the upstream, and push again here? I would like to see if the build works.

[flokli]

What's the reason behind committing the files back to the repo in first place?

Wouldn't we better off just not set out_dir/ use file_descriptor_set_path, and have the generated files only exist at build time?

Requiring a runtime dependency on protoc seems to be acceptable, it's the default behaviour other gRPC clients also have (like opentelemetry-proto).

[liufuyang]

What's the reason behind committing the files back to the repo in the first place?

The reason was the generated file could not be compiled and needed a manual fix.

I think we can move on to let those files be generated at build time (essentially we will be doing this after we uncomment the build.rs), after we know the generated file will stay the same. But the problem is that I cannot verify that.

It differs from your build and my build, so I am curious how the CI will do, or what code will be generated on the CI.

[liufuyang]

I tried to use your branch, rebased on main, and let the CI generates the files and also format those, still, there is a diff, so the build failed on CI (see the second commit build message from #92)

So removing those [allow(clippy::derive_partial_eq_without_eq)] make the CI passing git dirty check. So it means my local generated files are the same as the ones generated on CI.

Not sure why you cannot have the exact same file generated in your environment. Thinking there is a chance users will generate those file differently, I think the best is probably just make the build.rs run conditionally, for example, only runs with an environment parameter set, so that only runs on CI for checking git dirty, and it will not run for any user's usual cargo build.

If you do not mind I will just take this over and fix it up with #92 and merge there.

If I merge your PR right now, it will fail the CI, unless we remove the generated code in the repo. As I explained, if there is a chance that the generated code can differ, I think perhaps it is less confusing to just commit those in for now. Also, the good part is anyone else who is building this repo does not really need protoc installed.

[flokli]

I'm not quite sure why my generated code differs, but feel free to go on and take over this PR!

If I merge your PR right now, it will fail the CI, unless we remove the generated code in the repo. As I explained, if there is a chance that the generated code can differ, I think perhaps it is less confusing to just commit those in for now. Also, the good part is anyone else who is building this repo does not really need protoc installed.

I'd assume this might be due to some rust version differences or something. It's only allows for clippy warnings and formatting, so it really shouldn't matter for functionality.

I'd personally prefer to not check in the files, to be more consistent with other crates dealing with the same - especially if the alternative is postprocessing the results and introduce dependencies to cargo or rustfmt at build time, and try to ensure it always runs, which also doesn't seem very reliable.

[liufuyang]

Whatever we do here, we'd better we make sure that anyone checkout the repo can do cargo build on their side and does not end up with dirty git. And I think it might still be good idea to be not protoc dependent for the usual build.

I have made some changes and can go like this #92

Do you want to take a look perhaps?

It does not introduce dependencies to cargo/rustfmt/protoc, those are only needed if one builds it with BUILD_BIGTABLE_RS_GOOGLE_PROTO=true.

[liufuyang]

Thanks. Ideally, I think there should be a config for rustfmt to ignore a folder (it has but not in stable rust... strangely).

Later perhaps we can move onto not checkin those generated files, especially when the generating toolchains are more mature, with stable APIs and no manual fix needed.

[flokli]

Ok, with #92 merged, nothing to be done here. Thanks!