ci: add job to verify binary size #475
+2,447
−1,164
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9089,6 +9089,7 @@ dependencies = [ | |
| "ignore", | ||
| "log", | ||
| "mbrman", | ||
| "object", | ||
| "rayon", | ||
| "serde", | ||
| "serde_json", | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //! Download a github release artifact | ||
|
|
||
| use flowey::node::prelude::*; | ||
|
|
||
| flowey_request! { | ||
| pub struct Request { | ||
| /// First component of a github repo path | ||
| /// | ||
| /// e.g: the "foo" in "github.com/foo/bar" | ||
| pub repo_owner: String, | ||
| /// Second component of a github repo path | ||
| /// | ||
| /// e.g: the "bar" in "github.com/foo/bar" | ||
| pub repo_name: String, | ||
| /// Specific artifact to download. | ||
| pub file_name: String, | ||
| /// Path to downloaded artifact. | ||
| pub path: WriteVar<PathBuf>, | ||
| /// The Github actions run id to download artifacts from | ||
| pub run_id: ReadVar<String> | ||
| } | ||
| } | ||
|
|
||
| new_simple_flow_node!(struct Node); | ||
|
|
||
| impl SimpleFlowNode for Node { | ||
| type Request = Request; | ||
|
|
||
| fn imports(ctx: &mut ImportCtx<'_>) { | ||
| ctx.import::<crate::cache::Node>(); | ||
| ctx.import::<crate::use_gh_cli::Node>(); | ||
| } | ||
|
|
||
| fn process_request(request: Self::Request, ctx: &mut NodeCtx<'_>) -> anyhow::Result<()> { | ||
| let Request { | ||
| repo_owner, | ||
| repo_name, | ||
| file_name, | ||
| path, | ||
| run_id, | ||
| } = request; | ||
|
|
||
| let gh_token = ctx.get_gh_context_var(GhContextVar::GITHUB__TOKEN); | ||
| ctx.req(crate::use_gh_cli::Request::WithAuth( | ||
| crate::use_gh_cli::GhCliAuth::AuthToken(gh_token), | ||
| )); | ||
| let gh_cli = ctx.reqv(crate::use_gh_cli::Request::Get); | ||
|
|
||
| ctx.emit_rust_step("download artifacts from github actions run", |ctx| { | ||
| let gh_cli = gh_cli.claim(ctx); | ||
| let run_id = run_id.claim(ctx); | ||
| let path = path.claim(ctx); | ||
| move |rt| { | ||
| let sh = xshell::Shell::new()?; | ||
| let gh_cli = rt.read(gh_cli); | ||
| let run_id = rt.read(run_id); | ||
|
|
||
| let path_end = format!("{repo_owner}/{repo_name}/{run_id}"); | ||
| let out_dir = std::env::current_dir()?.absolute()?.join(path_end); | ||
| fs_err::create_dir_all(&out_dir)?; | ||
| sh.change_dir(&out_dir); | ||
|
|
||
| xshell::cmd!(sh, "{gh_cli} run download {run_id} -R {repo_owner}/{repo_name} --pattern {file_name}").run()?; | ||
|
|
||
| rt.write(path, &out_dir); | ||
| Ok(()) | ||
| } | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
|
There was a problem hiding this comment. don't forget your module docs |
||
| use flowey::node::prelude::*; | ||
|
|
||
| flowey_request! { | ||
| pub struct Request { | ||
| pub repo_path: ReadVar<PathBuf>, | ||
| pub merge_commit: WriteVar<String>, | ||
| } | ||
| } | ||
|
|
||
| new_simple_flow_node!(struct Node); | ||
|
|
||
| impl SimpleFlowNode for Node { | ||
| type Request = Request; | ||
|
|
||
| fn imports(_ctx: &mut ImportCtx<'_>) {} | ||
|
|
||
| fn process_request(request: Self::Request, ctx: &mut NodeCtx<'_>) -> anyhow::Result<()> { | ||
| let Request { | ||
| repo_path, | ||
| merge_commit, | ||
| } = request; | ||
|
|
||
| let head_ref = ctx.get_gh_context_var(GhContextVar::GITHUB__HEAD_REF); | ||
| let pr_number = ctx.get_gh_context_var(GhContextVar::GITHUB__PR_NUMBER); | ||
|
|
||
| ctx.emit_rust_step("get merge commit", |ctx| { | ||
| let merge_commit = merge_commit.claim(ctx); | ||
| let head_ref = head_ref.claim(ctx); | ||
| let repo_path = repo_path.claim(ctx); | ||
| let pr_number = pr_number.claim(ctx); | ||
|
|
||
| |rt| { | ||
| let sh = xshell::Shell::new()?; | ||
| let repo_path = rt.read(repo_path); | ||
| let head_ref = rt.read(head_ref); | ||
| let pr_number = rt.read(pr_number); | ||
|
|
||
| sh.change_dir(repo_path); | ||
|
|
||
| // TODO: Make this work for non-main PRs | ||
| xshell::cmd!(sh, "git fetch origin main").run()?; | ||
| xshell::cmd!(sh, "git fetch origin pull/{pr_number}/head:{head_ref}").run()?; | ||
| let commit = xshell::cmd!(sh, "git merge-base {head_ref} origin/main").read()?; | ||
| rt.write(merge_commit, &commit); | ||
|
|
||
| Ok(()) | ||
| } | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| use flowey::node::prelude::*; | ||
|
|
||
| flowey_request! { | ||
| pub struct Request { | ||
| pub github_commit_hash: ReadVar<String>, | ||
| pub repo_path: ReadVar<PathBuf>, | ||
| pub gh_workflow_id: WriteVar<String>, | ||
| } | ||
| } | ||
|
|
||
| new_simple_flow_node!(struct Node); | ||
|
|
||
| impl SimpleFlowNode for Node { | ||
| type Request = Request; | ||
|
|
||
| fn imports(_ctx: &mut ImportCtx<'_>) {} | ||
|
|
||
| fn process_request(request: Self::Request, ctx: &mut NodeCtx<'_>) -> anyhow::Result<()> { | ||
| let Request { | ||
| repo_path, | ||
| github_commit_hash, | ||
| gh_workflow_id, | ||
| } = request; | ||
|
|
||
| let gh_token = ctx.get_gh_context_var(GhContextVar::GITHUB__TOKEN); | ||
|
There was a problem hiding this comment. hmmm, maybe not strictly required for this PR, but do we need some way API like |
||
|
|
||
| ctx.emit_rust_step("get action id", |ctx| { | ||
| let gh_workflow_id = gh_workflow_id.claim(ctx); | ||
| let github_commit_hash = github_commit_hash.claim(ctx); | ||
| let gh_token = gh_token.claim(ctx); | ||
| let repo_path = repo_path.claim(ctx); | ||
|
|
||
| |rt| { | ||
| let github_commit_hash = rt.read(github_commit_hash); | ||
| let sh = xshell::Shell::new()?; | ||
| let gh_token = rt.read(gh_token); | ||
| let repo_path = rt.read(repo_path); | ||
|
|
||
| sh.change_dir(repo_path); | ||
| // Fetches the CI build workflow id for a given commit hash | ||
| let get_action_id = |commit: String| { | ||
| xshell::cmd!( | ||
| sh, | ||
| "gh run list --commit {commit} -w '[flowey] OpenVMM CI' -s 'completed' -L 1 --json databaseId --jq '.[].databaseId'" | ||
|
There was a problem hiding this comment. we def don't want to be hard-coding openvmm specific pipeline names within a |
||
| ) | ||
| .env("GITHUB_TOKEN", gh_token.clone()) | ||
|
There was a problem hiding this comment. is this actually necessary? I would expect GitHub Actions to already have this set ambiently? |
||
| .read() | ||
| }; | ||
|
|
||
| let mut github_commit_hash = github_commit_hash.clone(); | ||
| let mut action_id = get_action_id(github_commit_hash.clone()); | ||
| let mut loop_count = 0; | ||
|
|
||
| // CI may not have finished the build for the merge base, so loop through commits | ||
| // until we find a finished build or fail after 5 attempts | ||
| while let Err(ref e) = action_id { | ||
| if loop_count > 4 { | ||
| anyhow::bail!("Failed to get action id after 5 attempts: {}", e); | ||
| } | ||
|
|
||
| github_commit_hash = xshell::cmd!(sh, "git rev-parse {github_commit_hash}^").read()?; | ||
| action_id = get_action_id(github_commit_hash.clone()); | ||
| loop_count += 1; | ||
| } | ||
|
|
||
| if let Ok(id) = action_id { | ||
| print!("Got action id: {}", id); | ||
| rt.write(gh_workflow_id, &id); | ||
| } else { | ||
| anyhow::bail!("Failed to get action id"); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to be mindful about how we choose to expose this particular class of context variables in
flowey.All other constants defined in this list are guaranteed to be valid in any pipeline run. The same cannot be said of these new
github.event.pull_requestconstants, which should only be used in PR-triggered workflows. i.e: it seems unwise to make it "trivial" to access these variables in the context of a CI-triggered workflow via the existingget_gh_context_varAPI, given that the resulting loosely-typedStringcould sometimes be empty.My gut feeling is that we want to have some API that would let us model these sorts of context-dependent variables in a type-safe manner, in order to give users a way to get a
ReadVar<Option<PullRequestRelatedThing>>.Consider the following modification to the existing
get_gh_context_varAPI:The resulting API is very fluent for end users, and to make our lives easier as implementors, we can leverage the type-state pattern to avoid an explosion of different
GhContextVarReadertypes, and instead, simply transition between various versions of a single backingGhContextVarReader<T>type.With this API, the leaf-nodes (e.g:
pull_request().head().ref()) can then encode any necessary flowey logic to read the rawStringdata, and then convert it into aOption<String>if need be.I know this is all somewhat orthogonal to the problem this PR is specifically trying to solve... but we must not forget that flowey is not a framework set in stone. It still has many rough edges and core abstractions that need to be reconsidered / reworked. This sort of flowey rework can and should be done as part of whatever feature work we are doing.
In this case, I don't actually think there's too much work to re-jig the API as I suggest here - there shouldn't be any "backend" flowey work, and it'd simply be some clever refactoring of the user-facing
NodeContextAPIs.That said - I would suggest you split this work out into a separate PR, and then rebase this PR on-top of that one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theoretically, it might even make sense to have a single
pull_request() -> ReadVar<Option<GhEventPullRequest>>method, which dumps the entiregithub.events.pull_requestvariable as JSON, and then flowey uses aserdedefn of the corresponding object to avoid needing to manually write out methods for each field. See https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_requestIts a "big" object, but its not a huge object, so I think that might be a viable approach - even if the pipeline only ends up using one or two fields of the object that gets parsed in.
And if its hard to transcribe that particular object structure in its entirety into a serde type, we can always go piecemeal (given that serde will just ignore JSON fields during deserialization that it wasn't explicitly told about)