Skip to content

[WIP] [internal] introduce Rust backend with rustfmt support#15093

Closed
tdyas wants to merge 4 commits intopantsbuild:mainfrom
tdyas:rust_backend
Closed

[WIP] [internal] introduce Rust backend with rustfmt support#15093
tdyas wants to merge 4 commits intopantsbuild:mainfrom
tdyas:rust_backend

Conversation

@tdyas
Copy link
Contributor

@tdyas tdyas commented Apr 11, 2022

Introduce Rust backend with rustfmt and tailor support.

The rust_crate target type represents a Rust crate. The source globs currently only capture src/**/*.rs and tests/**/*.rs. The backend will eventually need a better way to capture all files that are part of the crate and not part of crates in subdirectories.

Rustup is required to be installed. The backend will query Rustup for the path to a specific toolchain and binary (e.g., cargo, rustfmt, etc.)

@tdyas tdyas added the category:internal CI, fixes for not-yet-released features, etc. label Apr 11, 2022
@tdyas tdyas requested review from Eric-Arellano and stuhood April 11, 2022 05:22
Comment on lines +7 to +29
class RustCrateSourcesField(MultipleSourcesField):
default = ("Cargo.toml", "src/**/*.rs", "tests/**/*.rs")
uses_source_roots = False
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is pretty tricky to model. We really have two types of sources fields going on - cargo.toml and the normal .RS files. It is very useful for those to be seen as completely separate fields, so that we can do things like be confident every single file is a rust file and it's safe to feed into rustc and rustfmt.

The issue is that we strongly assume there is only one sources field per target type. That is pretty crucial to have the target API Works when you say tgt.get(SourcesField). The safer thing to do would have been to have that return a tuple, but we intentionally return a single field or none.

There is one workaround I can think of, which is to use SecondaryOwnerMixin with a dedicated field for the cargo.toml. It is hacky, but I think would work.

--

How do you think we should handle cargo.lock?

@stuhood
Copy link
Member

stuhood commented Apr 15, 2022

Thanks a lot for this: both this one and #15092 are really exciting, but I feel like I need to do some research around prior art before diving in to review. This weekend hopefully?

@tdyas
Copy link
Contributor Author

tdyas commented Apr 15, 2022

Thanks a lot for this: both this one and #15092 are really exciting, but I feel like I need to do some research around prior art before diving in to review. This weekend hopefully?

This is low priority, so take your time. Can elaborate on my choices as needed.

@tdyas
Copy link
Contributor Author

tdyas commented May 7, 2022

Target type modeling: https://docs.google.com/document/d/1Q5h0PTusb2WyD3EScgeqUxBPF1IuEXCaa3qO31HEHjQ/edit#heading=h.wj9ig1p17iq0

@tdyas tdyas marked this pull request as draft June 10, 2022 16:34
@tdyas tdyas changed the title [internal] introduce Rust backend with rustfmt support [WIP] [internal] introduce Rust backend with rustfmt support Jun 10, 2022
Tom Dyas added 4 commits December 17, 2022 14:33
[ci skip-rust]

[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:internal CI, fixes for not-yet-released features, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants