Skip to content

Rust examples #477

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

Merged
merged 52 commits into from
Jul 23, 2024
Merged

Rust examples #477

merged 52 commits into from
Jul 23, 2024

Conversation

marvin-hansen
Copy link
Contributor

@marvin-hansen marvin-hansen commented Jun 25, 2024

As discussed with Alex Eagle on Slack recently,

this PR contributes multiple Rust examples that cover roughly the top ten use cases.
All examples use the new Bazelmod format. The format follows loosely the great C++ tutorial.

If there is anything that needs adjustment, please let me know.

Formalities:

  • I've signed the Google CLA
  • I've signed all commits

Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Updated main README.md.

Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

Will there be some prose to go along with this tutorial, like there is for C++ and Go and some others?

Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
@marvin-hansen
Copy link
Contributor Author

Ok, I pushed a bunch of commits resolving you suggestions.

It's possible for me to write some text to go along the tutorial.

However, would you please point me at the source of the C++ and Go tutorial text,
as I couldn't find it in this repo? I

I take a look at this at the upcoming weekend.

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented Jun 26, 2024

I've found the source of the C++ and Go tutorial text in the Bazel main repo.

Just to clarify your question, do you ask for a tutorial similar to the C++ and fill a PR
with the main repo to get a Rust getting started tutorial on the Bazel website?

In principal, I am okay with that, I just want to make sure I get this right?

@marvin-hansen marvin-hansen changed the title Rust examples with Bazelmod Rust examples Jun 26, 2024
@alexeagle
Copy link
Collaborator

The Go tutorial isn't on the Bazel site: https://bazel-contrib.github.io/SIG-rules-authors/go-tutorial.html
In the past we have not figured out where this content belongs. Google doesn't staff the project with a tech writer currently so it may be better for tutorials to live in bazel-contrib or in the ruleset repo.

I'm asking about your future plans because you named these folders "tutorial", but without accompanying text they are not part of a tutorial. If they are just examples with a README that's fine too. Writing a real tutorial is a lot of work and also easily goes out-of-date so it's a future commitment as well.

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented Jun 27, 2024

@alexeagle

I see. Perfectly valid point. And you are right, this is not a tutorial similar to the Go one you shared; not even close.
Sorry for the confusion, first time contribution to this project...

However, there is no way I can allocate the time to put something like the Go tutorial together let alone maintain it over time. This is a job for a professional tech writer, not a random contributor. Also, I am really unsure if that kind of overkill tutorial is actually needed because, in my observation, the Rust Bazel community tends to be filled with rather senior engineers and they may not even want a comprehensive first principle tutorial since they already know Bazel and only need to file some Rust specific gaps such as generating proto bindings, cross compiling, patching MUSL together etc. And that's basically the gap I'm filling.

I read so often comments in the rules_rust issue tracker that refer to, well, missing Rust Bazelmod examples. I actually filled another PR over at the rules_rust repo to address at least half a dozen open Bazelmod issues that can be closed with a simple code example or a brief Readme.

What I suggest to resolve this matter is to rename the folder to "rust-examples" and leave it there.

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

thank you for your contribution! I've only looked at the first couple of examples so far, but will continue reviewing in a bit.

marvin-hansen and others added 8 commits July 3, 2024 13:10
version 7.2.1

Signed-off-by: Marvin Hansen <[email protected]>
Fixed wrong wording.

Co-authored-by: Xùdōng Yáng <[email protected]>
Fixed formatting,.

Co-authored-by: Xùdōng Yáng <[email protected]>
Fixed typo.

Co-authored-by: Xùdōng Yáng <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
  example to CI.

Updated example Readme.

Signed-off-by: Marvin Hansen <[email protected]>
  example to CI.

Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
…of the presubmit.yml config file.

Signed-off-by: Marvin Hansen <[email protected]>
with a notice about supported platforms (Linux and Mac)

Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
Signed-off-by: Marvin Hansen <[email protected]>
@marvin-hansen
Copy link
Contributor Author

Update:

  1. Removed the LLVM example.
  2. Added all Rust examples to CI
  3. Updated Rust example Readme with a notice of supported platforms (Linux and Mac)

I've bumped into the max 80 tasks per config limit on the CI. Initially, I thought the the tutorial file was too big, so I've split that one into three (C++, Java, Rust) as suggested by the Ci error message, but as it turned out, it was the pre-submit file that imports everything and therefore exceeded the 80 task limit. In response, I've removed most Mac targets for the Rust examples except for cross compilation to stay within the limit and to give some more space to add more targets later. That's probably good since the mac targets tend to build slower.

Build is green.

Please let me know if you want any further changes to any of the examples.

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

I skimmed through the rest of the pages and saw no major concerns (other than the rules_cc thing). Thanks again for the very detailed examples!

## Setup

The setup is twofold, the Rust rules are declared in the MODULE file,
but the rules_cc are not yet available in the Bazelmod format and thus are declared in
Copy link
Member

Choose a reason for hiding this comment

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

rules_cc is on BCR: https://registry.bazel.build/modules/rules_cc

It might be a bit out of date for your use case; if so, we can submit a newer version.

In general, though, rules_cc isn't really ready for public consumption. If you're just using C++ rules like cc_import and cc_library, I believe you can just use the natively bundled rules in Bazel (that is, don't load them).

Copy link
Contributor Author

@marvin-hansen marvin-hansen Jul 13, 2024

Choose a reason for hiding this comment

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

Thanks for the hint. I am no using CC at all in my projects so I didn't know this particularity.
Let me fix this and update the PR.

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented Jul 13, 2024

While contributing to the rules_rust repo recently, I've reported a fairly hairy bug that causes the MUSL example to produce an incorrect linked binary due to an incorrect toolchain resolution. I only discovered this bug after the examples were written during some routine structural integrity testing and that way I know that the MUSL example in this PR is also affected. The other examples were fine and especially the cross compilation examples were passing all tests.

Point is, a fix is under way, but may take longer as it's not even merged so the bigger question is how to proceed?

I can think of two options:

  1. Remove the MUSL example, review, merge, and wait for the fix to re-add the MUSL example later.
    That's what we decided over at rules_rust as it's unclear when the fix lands.

  2. Convert the PR into a draft, put it on hold until the MUSL fix arrives, apply the fix, and take it from there.

Thoughts?

@alexeagle
Copy link
Collaborator

FYI @illicitonion

@Wyverald
Copy link
Member

I can think of two options:

I'm okay with either option.

Also, please let me know when you've made all necessary changes to the PR and are ready to submit. I can override the CLA check from my end. (Alternatively, feel free to remove the "Co-authored-by" lines referring to me.)

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented Jul 16, 2024

Update:

  1. Removed the WORKSPACE file with rules cc from example nr 4, FFI, and use the default rules. Also, updated the Readme.
  2. Updated all rust examples to the latest rules_rust (0.47.1)
  3. Renamed the rust-tutorial folder to rust-examples to better reflect the intention
  4. Removed the MUSL example for the time being.

I have to renovate the MUSL example anyways once a patch becomes available as the configuration changed, but more importantly I have to wait for the next rules_rust release that includes the patch and I believe that’s going to take some time, therefore it’s sensible to remove the MUSL example for the time being. I add it back later once the rules_rust have been patched and the example has been updated.

CI seems to fail only on Android and MacOS stuff, whereas the Rust examples all build.

Would be great if someone with more knowledge on Android / MacOS could take a look.

Please let me know if you want any further changes.

Otherwise, feel free to merge anytime when the CI issues are resolved.

@Wyverald Wyverald merged commit a905ed1 into bazelbuild:main Jul 23, 2024
0 of 2 checks passed
@Wyverald
Copy link
Member

CI failures are unrelated (and already failing before this PR).

CLA check failure is a false positive -- somehow my personal email address is used there.

So I went ahead and merged this. Thank you again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants