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

Generalize to Buck/Pants/etc #8

Closed
LegNeato opened this issue Sep 12, 2017 · 7 comments
Closed

Generalize to Buck/Pants/etc #8

LegNeato opened this issue Sep 12, 2017 · 7 comments

Comments

@LegNeato
Copy link

I was planning to build something similar and see you have already done a good chunk of the work!

Any plans to make the output tool and format pluggable? I think having clearly defined interfaces could also help the project's structure.

Would you take PRs to support such a thing? If not and you prefer to just focus on Bazel for now, no worries--I will work in my own repo and maybe we can merge projects in the future.

@acmcarther
Copy link
Owner

I'd definitely love to see PRs for generalizing.... though I'm not sure what that might involve. I'm extremely interested in hearing what needs folks have as well. I've been playing with things like dependency replacement, build script support, and metadeps support, but I've been having a hard time prioritizing the work since I don't have many real world use cases.

@LegNeato
Copy link
Author

LegNeato commented Sep 12, 2017

Excellent!

This would be the rough plan I would do if it were up to me...

Generalize

  • Rename all the non-bazel/blaze specific stuff.
  • Pull out the Cargo.toml parsing into a generic module with non-tool specific types. (This could even be its own project as I bet it would be useful in other contexts like querying a pure-Rust project, etc).
  • Have a BuildToolGenerator trait. It writes out tool-specific files from the generic Cargo.toml, platform, and RazeConfig data.
  • Have a default TemplateFileWriter that BuildToolGenerators can use to write their files using tool-specific template files (as you have in your branch).

A project that has similar structure to what I am thinking is https://github.com/swagger-api/swagger-codegen/tree/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages (though not the code style and such). The logic that aggregates all the input information is shared and then it is passed to domain-specific generators to do their thing.

Add

  • Automatically run/manage cargo-vendor. That is, if someone runs cargo raze, it will vendor anything changed and generate build files for it. As raze already requires vendoring crates, I think this would be fine.
  • Add a RazeConfig type that reads config from Cargo.toml and passes to BuildToolGenerators. Anything with raze.* will be passed to all tools and raze.[tool].* will only be passed to the BuildToolGenerator with that id. Contrived example:
[raze]
# Always used by raze core and passed to every tool.
regenerate_vendor_directory = true

[raze.buck]
# Only passed to Buck's generator.
include_builddefs = "./path/to/BUILD"

[raze.bazel]
# Only passed to Bazel's generator.
skylard_rule_location = "https://blah"
  • Add a section in Cargo.toml for overrides and includes. The keys on the left are tool agnostic and the values on the right are validated by raze to exist before passing to the BuildToolGenerator. Something like:
[raze.override]
# These could be used instead of the generated files.
"//my/target/1" = "./foo/BUILD"
"//my/target/2" = "./foo2/BUILD"

[raze.include]
# These could be included in the generated files by the tool.
"//my/target/3" = "./foo3/BUILD"
"//my/target/4" = "./foo4/BUILD"
  • Let BuildToolGenerators fail on dependencies with build.rs/metadeps and/or the user specify inputs manually.
  • Do not generate files if they already exist in the vendored source.

@LegNeato
Copy link
Author

Nice, I see you are thinking about a lot of the same stuff in #6

@acmcarther
Copy link
Owner

I'm going to spike generalizing these changes tonight, with a focus on getting sensible but extendable traits. I'm going to get #6 merged as well, just so that the state of the world is consistent.

Thanks!

@acmcarther
Copy link
Owner

acmcarther commented Sep 13, 2017

One clarification:
This project isn't actually currently parsing the cargo toml... At least not in the manner you might think it is. We're actually trusting Cargo itself to parse the toml, and we're extracting the crate relationships from Cargo's own structures. I believe an approach that just parses the toml raw will be brittle, as it won't properly capture nuances in how Cargo interprets the tomls. For the record, I actually started by parsing the tomls myself, and got pretty far: cargo2bazel

That said, I think embedding build tool metadata into the workspace toml isn't a bad idea at all.

The initial stab at what you wanted starts as follows (from #6):

  • All tools share crate plan information. Those structs are detailed in context.rs. Presumably, build tools will need their own configs, but that's still on the todo list.
  • A new class "Renderer" exists in rendering.rs. It takes a PlannedBuild (from planning.rs), and produces a list of FileOutputs.
    Right now, main.rs still just goes through the bazel flow, and I havent added any pluggability in the middle of the planning workflow for tools to gather their own metadata. There's also still no means to self-configure from Cargo.toml yet (as I wasn't reading the file in the first place).

Immediately on my todo list is to get more test coverage so that people can actually contribute.

EDIT: Added reference to cargo2bazel.
EDIT 2: Updated file references to hyperlinks

@LegNeato
Copy link
Author

Sounds good! I see you landed your branch.

@acmcarther
Copy link
Owner

This repo has since moved to https://github.com/google/cargo-raze.

I'm unlikely to drive this particular forward on my own (as I don't use Buck), so I haven't reopened this in that repo. That said, I did extract the rendering into a separate struct that implements a "Renderer". Unfortunately though, the whole integration is pretty leaky as an abstraction and I think it heavily relies on Bazel-specific features. It would really be an uphill battle to be tool-agnostic, though I'd definitely be open to accepting contributions that worked toward that, provided that they didn't increase the maintenance overhead and didn't block the way toward further functionality using Bazel-specific features.

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

No branches or pull requests

2 participants