Skip to content

flowey: persist job platform/arch in pipeline static DB#3052

Merged
justus-camp-microsoft merged 1 commit intomicrosoft:mainfrom
justus-camp-microsoft:platform_arch_in_db
Apr 1, 2026
Merged

flowey: persist job platform/arch in pipeline static DB#3052
justus-camp-microsoft merged 1 commit intomicrosoft:mainfrom
justus-camp-microsoft:platform_arch_in_db

Conversation

@justus-camp-microsoft
Copy link
Copy Markdown
Contributor

Right now flowey gets its platform and arch at runtime, but since Nix has been introduced as a flavor of Linux distro that we want to be set at job level, there needs to be a way to set this per-job and have it persist. This change encodes the platform and arch as part of the generated JSON when flowey executes a pipeline and enables us to correctly select the platform if Nix has been set to be used in a job.

This is part of a series of changes needed to get to a reproducible build that runs all of our build command and dependencies through Nix.

@justus-camp-microsoft justus-camp-microsoft requested a review from a team as a code owner March 18, 2026 21:24
Copilot AI review requested due to automatic review settings March 18, 2026 21:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Persists per-job platform and architecture into Flowey’s generated pipeline static DB JSON so runtime snippet execution can use the job’s resolved environment (including Nix) instead of re-detecting host platform/arch at runtime.

Changes:

  • Add Serialize/Deserialize derives to FlowPlatformLinuxDistro, FlowPlatform, and FlowArch so they can be encoded into the pipeline static DB.
  • Populate job_platforms and job_archs maps when emitting GitHub/ADO pipelines.
  • Update ExecSnippet to read platform/arch from pipeline.json per job index.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
flowey/flowey_core/src/node.rs Make platform/arch enums serializable for persistence in the static DB JSON.
flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs Record per-job platform/arch into the pipeline static DB during GH YAML emission.
flowey/flowey_cli/src/pipeline_resolver/ado_yaml.rs Record per-job platform/arch into the pipeline static DB during ADO YAML emission.
flowey/flowey_cli/src/cli/exec_snippet.rs Load per-job platform/arch from pipeline.json and extend the static DB schema.

Comment on lines +142 to +146
pipeline_static_db
.job_platforms
.insert(job_idx.index(), platform);
pipeline_static_db.job_archs.insert(job_idx.index(), arch);

Comment on lines +136 to +139
pipeline_static_db
.job_platforms
.insert(job_idx.index(), platform);
pipeline_static_db.job_archs.insert(job_idx.index(), arch);
@github-actions
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

Comment thread flowey/flowey_cli/src/cli/exec_snippet.rs
serde_json::from_reader(pipeline_static_db)?
};

let flow_platform = *job_platforms
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like we're using platform and arch in a lot of other places, like in nodes, should they be updated too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is definitely a valid comment and probably worth looking into - in general I'm not a fan (and I don't think Daniel was either) of nodes checking backends because it makes it hard to reason about what the node will do from reading its source code alone.

In general I think it's better to make parameters that change behavior be part of the request and have higher level nodes request them based on the backend (so hoist it up as high as is reasonable) that way nodes are more readable.

Copilot AI review requested due to automatic review settings March 20, 2026 17:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

@@ -85,6 +82,13 @@ impl ExecSnippet {
serde_json::from_reader(pipeline_static_db)?
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

serde_json::from_reader(pipeline_static_db)? will surface a raw serde error if pipeline.json is missing/older-schema. Consider adding an anyhow::Context here that points to the likely remediation (ensure the job staged the correct pipeline.json for this flowey binary / regenerate in runtime mode).

Suggested change
serde_json::from_reader(pipeline_static_db)?
serde_json::from_reader(pipeline_static_db).context(
"failed to deserialize pipeline.json; ensure this flowey binary has the correct \
pipeline.json staged, or regenerate it in runtime mode",
)?

Copilot uses AI. Check for mistakes.
Comment on lines +87 to +90
.context("invalid job_idx: missing platform")?;
let flow_arch = *job_archs
.get(&job_idx)
.context("invalid job_idx: missing arch")?;
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The errors "invalid job_idx: missing platform/arch" can also occur when pipeline.json is present but was generated by an older flowey (or otherwise doesn't include these per-job entries). Consider making the message more actionable by including the job_idx value and hinting that the staged pipeline.json may be out of date for this flowey binary.

Suggested change
.context("invalid job_idx: missing platform")?;
let flow_arch = *job_archs
.get(&job_idx)
.context("invalid job_idx: missing arch")?;
.with_context(|| {
format!(
"invalid job_idx {job_idx}: missing platform (pipeline.json may be out of date for this flowey binary)"
)
})?;
let flow_arch = *job_archs
.get(&job_idx)
.with_context(|| {
format!(
"invalid job_idx {job_idx}: missing arch (pipeline.json may be out of date for this flowey binary)"
)
})?;

Copilot uses AI. Check for mistakes.
@@ -945,7 +945,7 @@ pub enum FlowPlatformKind {
}

/// The kind platform the flow is being running on, Windows or Unix.
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The doc comment above FlowPlatformLinuxDistro says it's the platform kind (Windows/Unix), but this enum actually represents the Linux distro. Updating the comment would avoid confusion for callers and keep docs aligned with the type's purpose.

Suggested change
/// The kind platform the flow is being running on, Windows or Unix.
/// The Linux distribution the flow is running on.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@github-actions
Copy link
Copy Markdown

@justus-camp-microsoft justus-camp-microsoft merged commit 077f160 into microsoft:main Apr 1, 2026
81 of 83 checks passed
moor-coding pushed a commit to moor-coding/openvmm that referenced this pull request Apr 13, 2026
Right now flowey gets its platform and arch at runtime, but since Nix
has been introduced as a flavor of Linux distro that we want to be set
at job level, there needs to be a way to set this per-job and have it
persist. This change encodes the platform and arch as part of the
generated JSON when flowey executes a pipeline and enables us to
correctly select the platform if Nix has been set to be used in a job.

This is part of a series of changes needed to get to a reproducible
build that runs all of our build command and dependencies through Nix.
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