Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce rust_common struct #575

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Conversation

hlopko
Copy link
Member

@hlopko hlopko commented Feb 3, 2021

This PR introduces a rust_common struct that will be used to access the Rust Sandwich API (the term first introduced in https://bazel.build/designs/2016/08/04/extensibility-for-native-rules.html and used ever since, for example https://blog.bazel.build/2017/03/07/java-sandwich.html :)

The motivation for this change:

  • to provide a single place for other rule authors where they can see what APIs are available.
  • to enable rule authors to make their custom rules interacting with Rust rules backwards and forwards compatible using the hasattr function. hassattr only works on struct fields, not on globals.

The plan is to eventually refactor uses of all private APIs to go trough rust_common.

Credit goes to brave owners of rules_swift who discovered how useful this approach is the hard way.

@UebelAndre
Copy link
Collaborator

Can you also add this to the docs in ./docs?

Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the naming a bit strange, as providers tend to take the form FooInfo (e.g. JavaInfo) - in the Java example, java_common.compile returns a JavaInfo and is typically referenced thus, rather than as java_common.compile...

I understand moving CrateInfo to a specific providers file, and making more clear what's public, but I'm not so clear on why rust_common is a useful wrapper layer?

@@ -430,7 +430,7 @@ _common_attrs = {
List of `rust_library` targets with kind `proc-macro` used to help build this library target.
"""),
cfg = "exec",
providers = [CrateInfo],
providers = [rust_common.crate_info],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this works, but I guess I can see how it would...!

@hlopko
Copy link
Member Author

hlopko commented Feb 3, 2021

I understand moving CrateInfo to a specific providers file, and making more clear what's public, but I'm not so clear on why rust_common is a useful wrapper layer?

My thinking:

  • having rust_common for functions is imho idiomatic way of structuring the sandwich in Bazel rules (C++, Java, proto, swift, objc all do the equivalent)
  • the reason for above is also the hasattr (as noted in the PR description). hassattr only works on struct fields, not on globals.
  • By hiding provider symbol behind it allows us to query the existence of provider in the far future when we decide to remove CrateInfo. This is mostly stockholm syndrome I have from updating Bazel rules :)

Am I making any sense?

@illicitonion
Copy link
Collaborator

  • By hiding provider symbol behind it allows us to query the existence of provider in the far future when we decide to remove CrateInfo. This is mostly stockholm syndrome I have from updating Bazel rules :)

This is the only one I'm not super clear on, but I'll defer to your experience! :)

@hlopko hlopko merged commit d468cfa into bazelbuild:main Feb 3, 2021
@UebelAndre
Copy link
Collaborator

Would still appreciate having docs updated 😅

@dfreese
Copy link
Collaborator

dfreese commented Feb 3, 2021

Would still appreciate having docs updated

I was looking for a way to handle that in an automated fashion on submit. There's probably a way to setup a github action to push to a "docs" branch and then serve the pages from there, but I need to look into it further.

@hlopko
Copy link
Member Author

hlopko commented Feb 4, 2021

Discussed with Andre offline, right now the docs tooling doesn't support generating docs for structs, only rules and macros it seems.

@hlopko hlopko deleted the fix_rust_common branch April 1, 2021 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants