Skip to content

Commit

Permalink
Omit client and server modules if no services declared in Buck target
Browse files Browse the repository at this point in the history
Summary:
In {D53166614} and {D53166618}, we need to make either of 2 choices:

1. **Every** `thrift_library` gets a clients and services crate, regardless of whether the target contains any service definitions

2. The clients and services crates only exist if the target defines nonzero services

As far as I know, it's impossible to do #2 based on whether the target *actually* contains services in its Thrift source code, only based on whether the `thrift_library` macro call lists any service names.

https://www.internalfb.com/code/fbsource/[0fa600724c20b87b38d536611b742c8287322f75]/fbcode/zippydb/if/TARGETS?lines=9-16%2C32-34%2C46

So far for Rust we have not paid attention to the listed service names in the Buck target, but the other languages look at it for a similar purpose as this.

On the basis that only 1252 out of 6042 Thrift targets in fbcode today contain services, I think limiting the clients and services crates is the right call to keep the Buck graph small. We can revisit down the line if there is any reason this turns out to be annoying.

This diff eliminates the `client` and `server` modules of Thrift library targets that do not contain any services according to Buck. Previously these used to be either empty modules, or actually nonempty modules if there were services defined in the Thrift source code but not declared in the Buck target.

Based on check_all.sh, there are zero occurrences of Rust code importing a service from a Thrift library for which the Buck target does not declare any services.

Reviewed By: zertosh, shayne-fletcher

Differential Revision: D53189031

fbshipit-source-id: e4fa7ffee4ead76802bebbc92ad514ff1d24d3b2
  • Loading branch information
David Tolnay authored and facebook-github-bot committed Jan 29, 2024
1 parent b126718 commit 37d5f09
Show file tree
Hide file tree
Showing 19 changed files with 32 additions and 63 deletions.
22 changes: 20 additions & 2 deletions thrift/compiler/generate/t_mstch_rust_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ namespace compiler {
namespace rust {

struct rust_codegen_options {
// Name that the main crate uses to refer to its dependency on the types
// crate.
// Names that the main crate uses to refer to its dependency on the other
// crates.
std::string types_crate;
std::string clients_crate;
std::string services_crate;

// Key: package name according to Thrift.
// Value: rust_crate_name to use in generated code.
Expand Down Expand Up @@ -529,6 +531,8 @@ class rust_mstch_program : public mstch_program {
{
{"program:types?", &rust_mstch_program::rust_has_types},
{"program:types", &rust_mstch_program::rust_types},
{"program:clients?", &rust_mstch_program::rust_has_clients},
{"program:servers?", &rust_mstch_program::rust_has_servers},
{"program:structsOrEnums?",
&rust_mstch_program::rust_structs_or_enums},
{"program:nonexhaustiveStructs?",
Expand Down Expand Up @@ -595,6 +599,10 @@ class rust_mstch_program : public mstch_program {
return types;
}

mstch::node rust_has_clients() { return !options_.clients_crate.empty(); }

mstch::node rust_has_servers() { return !options_.services_crate.empty(); }

mstch::node rust_structs_or_enums() {
return !program_->structs_and_unions().empty() ||
!program_->enums().empty() || !program_->xceptions().empty();
Expand Down Expand Up @@ -2192,6 +2200,16 @@ void t_mstch_rust_generator::generate_program() {
boost::algorithm::replace_all_copy(*types_crate_flag, "-", "_");
}

if (auto clients_crate_flag = get_option("clients_crate")) {
options_.clients_crate =
boost::algorithm::replace_all_copy(*clients_crate_flag, "-", "_");
}

if (auto services_crate_flag = get_option("services_crate")) {
options_.services_crate =
boost::algorithm::replace_all_copy(*services_crate_flag, "-", "_");
}

if (auto cratemap_flag = get_option("cratemap")) {
auto cratemap = load_crate_map(*cratemap_flag);
options_.multifile_mode = cratemap.multifile_mode;
Expand Down
20 changes: 12 additions & 8 deletions thrift/compiler/generate/templates/rust/lib.rs.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ pub use self::types::*;{{!
}}{{!
}}{{#program:types?}}

pub use {{program:types}} as types;{{!
}}{{/program:types?}}{{#program:constants?}}
pub use {{program:types}}::consts as consts;{{!
}}{{/program:constants?}}{{!
pub use {{program:types}} as types;
{{/program:types?}}{{#program:constants?}}
pub use {{program:types}}::consts as consts;
{{/program:constants?}}{{!
}}{{#program:structs}}{{!
}}{{#struct:has_adapter?}}{{!
}}{{>lib/adapter/struct}}{{!
Expand All @@ -55,12 +55,16 @@ pub use {{program:types}}::consts as consts;{{!
#[doc(hidden)]
pub mod dependencies;
pub use {{program:types}}::services as services;
{{#program:clients?}}
pub mod client;
pub mod mock;
{{/program:clients?}}
{{#program:servers?}}
pub mod server;
pub mod mock;{{!
}}{{/program:services?}}
pub use {{program:types}}::errors as errors;{{!
}}{{#program:nonstandardTypes?}}
{{/program:servers?}}
{{/program:services?}}
pub use {{program:types}}::errors as errors;
{{#program:nonstandardTypes?}}

pub(crate) mod r#impl {{>lib/block}}{{>lib/mod.impl}}}{{!
}}{{/program:nonstandardTypes?}}
3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/adapter/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/basic/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/doctext/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/exceptions/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions thrift/compiler/test/fixtures/inheritance/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/interactions/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions thrift/compiler/test/fixtures/params/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/rust-noserver/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/stream/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions thrift/compiler/test/fixtures/types/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 37d5f09

Please sign in to comment.