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

Use Pants to build our rust code #18688

Open
chrisjrn opened this issue Apr 5, 2023 · 5 comments
Open

Use Pants to build our rust code #18688

chrisjrn opened this issue Apr 5, 2023 · 5 comments
Assignees

Comments

@chrisjrn
Copy link
Contributor

chrisjrn commented Apr 5, 2023

While Pants's rust code currently uses a custom bootstrapper to build the native Pants code before executing Python, that bootstrapper is somewhat inconvenient to use: it is sensitive to branch changes and small changes in rust source code.

Even without dependency inference, Pants' caching approach is well suited to this problem! We should try using adhoc_tool to execute the rust toolchain rather than executing it directly.

A POC exists at: https://github.com/pantsbuild/pants/compare/main...chrisjrn:pants:chrisjrn/rusty-pants?expand=1

A few issues to reckon with:

  1. Chicken-and-egg: how do we execute Pants the first time, when there's no native engine to be had.
  2. Handling API changes during development: if the Python API for the native engine changes, the Python code will likely stop working until the rust is built.

We may wish to rely on only published versions of pants for building the rust code, however, this would involve maintaining a separate pants.toml in the meantime. Maybe that's OK?

@chrisjrn chrisjrn self-assigned this Apr 5, 2023
@huonw
Copy link
Contributor

huonw commented Apr 5, 2023

This would be great!

There was a discussion around bootstrapping using a pinned version of pants (rather than pants from sources) on slack a while ago: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1666375916738029

Going to a full bootstrapping model would be effectively separating

  1. the tool used to orchestrate the repo
  2. the artefacts the repo is producing

It'd make a lot of sense for 1 to be pants too, but some fixed non-HEAD/non-from-sources version. By separating them, the dev experience is likely better for more than just the Rust code, e.g. reducing how often the orchestration tooling needs to be restarted due to artefact changes.

@chrisjrn
Copy link
Contributor Author

chrisjrn commented Apr 5, 2023

The separation of 1 & 2 would be a huge improvement, and I agree about the improved dev experience arising from not needing to restart Pants all the time. Obviously there's the initial bootstrapping issue, which would benefit from using a fixed version of Pants, however we'll need some way for people who are working on Rust code to reliably run Pants against any adjusted Rust code. That's probably going to require some thought.

@benjyw
Copy link
Contributor

benjyw commented Apr 5, 2023

For now I think we can keep the logic in ./pants that checks if the engine needs (re-)building, but have that logic delegate to a published version of Pants (with a custom pants.toml) to do the actual building, so we get the caching?

It would be nice if invoking Pants was fast enough that we could do it every time and not need the rust invalidation logic, but we're not there yet.

@chrisjrn
Copy link
Contributor Author

chrisjrn commented Apr 6, 2023

I think @huonw makes some sense -- we don't necessarily need to run our testing and quality checks using the exact same version of Pants that's under test, and indeed, having the tooling that runs our tests be separate to the version would separate us from things like graph errors that prevent everything from running. That said, this smells like Future Work™ once we show that building Rust within Pants is a good idea.

I'll proceed with what @benjyw suggests for the time being, and if we can land that, then we can scope out further quality of life improvements.

@benjyw
Copy link
Contributor

benjyw commented Apr 6, 2023

Re "we don't necessarily need to run our testing and quality checks using the exact same version of Pants that's under test" I think we will continue to want to do that, or it'll be much harder to reason about what is happening when things fail, so I 100% agree with punting that (personally, I think, forever...)

Let's focus on the big practical win of not having to rebuild the engine when switching branches. Note that this also means that, with remote caching, we can eventually get rid of our bespoke logic for caching the engine in CI.

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

Successfully merging a pull request may close this issue.

4 participants