port(turborepo): Run stub #4752

NicholasLYang · 2023-04-28T21:06:40Z

Description

This is more a proof of concept than an actual implementation. Attempts to write the beginning of run in what's hopefully semi-idiomatic Rust, with all of the helper functions stubbed out.

Testing Instructions

The stub is gated behind a rust feature flag run-stub, so to test the code add --features run-stub to your cargo command

vercel · 2023-04-28T21:06:49Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
examples-basic-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 23, 2023 9:14pm
examples-kitchensink-blog	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 23, 2023 9:14pm

7 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-cra-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	May 23, 2023 9:14pm

vercel · 2023-04-28T21:07:05Z

@NicholasLYang is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

gsoltis

This is a great start, and I think is already highlighting some areas for improvement.

Let's do the renames, and maybe the task id bit, then merge.

Can you also add a stub test that just "executes" a run?

gsoltis · 2023-05-23T22:16:35Z

crates/turborepo-lib/src/manager.rs

@@ -0,0 +1,10 @@
+#[derive(Debug)]
+pub struct Manager {


Is this the child process manager? Can you add a comment?

gsoltis · 2023-05-23T22:17:35Z

crates/turborepo-lib/src/opts.rs

+};
+
+#[derive(Debug)]
+pub struct Opts<'a> {


Do we care about the opts having a lifetime? Would it make it easier if they owned their data? It's not a ton of data...

Right now it can go either way. I think in the future I'd like a single 'run lifetime that's tied to either CommandBase or a Run struct. And that can own stuff like Args that then in turn get transformed to Opts. For now I don't think we have to make that call until this is used.

gsoltis · 2023-05-23T22:18:56Z

crates/turborepo-lib/src/run/context.rs

+
+use crate::{package_json::PackageJson, run::graph::WorkspaceCatalog};
+
+pub struct Context {


Can we name this PackageGraph? Or something like that? The Context name from Go is not great.

Yeah definitely. Context is very confusing imo

gsoltis · 2023-05-23T22:19:53Z

crates/turborepo-lib/src/run/graph.rs

+    }
+}
+
+#[derive(Default)]


Here, and elsewhere, I'm assuming we intend to remove the Default derivations in the future?

yeah definitely. Mostly for boilerplate

gsoltis · 2023-05-23T22:20:48Z

crates/turborepo-lib/src/run/mod.rs

+}
+
+impl<'a> Run<'a> {
+    pub fn new(base: &'a CommandBase, opts: Opts<'a>) -> Self {


Would it make more sense for the Run to own the CommandBase? Do we continue using it after executing a command?

I've thought about having all the commands be methods of CommandBase, that way we can have all of the relevant resources be owned by CommandBase

gsoltis · 2023-05-23T22:22:50Z

crates/turborepo-lib/src/run/pipeline.rs

+pub type Pipeline = HashMap<String, BookkeepingTaskDefinition>;
+
+#[derive(Debug, Default, Serialize, Deserialize)]
+pub struct BookkeepingTaskDefinition {


cc @mehulkar

Is there a path to dropping the Bookkeeping in a rust world? Post-port? Or something we can do as part of the porting effort?

It's main goal was to assist in merging fields. If Rust has a better way to distinguish between when a field is missing vs when it's the "empty value", we should be able to drop it. The integration tests covering this behavior are pretty good, so I'd just say "try it"

Yep! We can do Option<T> for those

gsoltis · 2023-05-23T22:24:45Z

crates/turborepo-lib/src/run/task_id.rs

+pub const TASK_DELIMITER: &str = "#";
+pub const ROOT_PKG_NAME: &str = "//";
+
+pub fn get_task_id(pkg_name: impl std::fmt::Display, target: &str) -> String {


I'm 99% certain that task id doesn't need to be a string. Can we try a simple struct?

What do you mean by simple struct here?

Thought about this a bit, and I think "simple" might not be the right word. And we maybe shouldn't block on this. I'm thinking of something like:

pub enum TaskId { SinglePackage { task: String }, Monorepo { package: Option<String>, task: String } }

gsoltis

This is a great start, and I think is already highlighting some areas for improvement.

Let's do the renames, and maybe the task id bit, then merge.

Can you also add a stub test that just "executes" a run?

NicholasLYang · 2023-05-24T19:23:53Z

crates/turborepo-lib/src/run/mod.rs

+    };
+
+    #[tokio::test]
+    async fn test_run() -> Result<()> {


This is not an integration test because I'd need some way to plumb rust features through integration tests.

gsoltis

Can you add a comment re: Manager and then let's merge + iterate

gsoltis · 2023-05-25T18:15:50Z

Merged as #5099

turbo-orchestrator bot added the team: turborepo label Apr 28, 2023

NicholasLYang force-pushed the nicholasyang/run-stub branch from c0e5924 to 5d9fd46 Compare April 28, 2023 21:08

turbo-orchestrator bot added the team: turbopack label May 1, 2023

NicholasLYang force-pushed the nicholasyang/run-stub branch from 6bd91da to e1de259 Compare May 11, 2023 15:24

Starting a run with most of the logic stubbed out

ec59ac9

NicholasLYang force-pushed the nicholasyang/run-stub branch from e1de259 to ec59ac9 Compare May 23, 2023 21:14

Fixing up code, added logging

d7a6d6e

vercel bot deployed to Preview – turbo-site May 23, 2023 21:53 View deployment

NicholasLYang marked this pull request as ready for review May 23, 2023 22:00

NicholasLYang requested review from a team as code owners May 23, 2023 22:00

NicholasLYang requested review from nathanhammond and chris-olszewski May 23, 2023 22:00

gsoltis suggested changes May 23, 2023

View reviewed changes

PR feedback

57e5f7f

NicholasLYang requested a review from gsoltis May 24, 2023 19:13

NicholasLYang commented May 24, 2023

View reviewed changes

gsoltis approved these changes May 24, 2023

View reviewed changes

gsoltis mentioned this pull request May 25, 2023

port(turborepo): Run stub #5099

Merged

gsoltis closed this May 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

port(turborepo): Run stub #4752

port(turborepo): Run stub #4752

NicholasLYang commented Apr 28, 2023 •

edited

vercel bot commented Apr 28, 2023 •

edited

vercel bot commented Apr 28, 2023

gsoltis left a comment

gsoltis May 23, 2023

gsoltis May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 23, 2023

mehulkar May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 23, 2023

NicholasLYang May 24, 2023

gsoltis May 24, 2023

gsoltis left a comment

NicholasLYang May 24, 2023

gsoltis left a comment

gsoltis commented May 25, 2023


		use crate::{package_json::PackageJson, run::graph::WorkspaceCatalog};

		pub struct Context {

port(turborepo): Run stub #4752

port(turborepo): Run stub #4752

Conversation

NicholasLYang commented Apr 28, 2023 • edited

Description

Testing Instructions

vercel bot commented Apr 28, 2023 • edited

vercel bot commented Apr 28, 2023

gsoltis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsoltis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsoltis left a comment

Choose a reason for hiding this comment

gsoltis commented May 25, 2023

NicholasLYang commented Apr 28, 2023 •

edited

vercel bot commented Apr 28, 2023 •

edited