Flowey: Use explicit parameter names instead of auto generated. (#606)

This change moves from auto-generated parameter names to requiring the caller to specify a variable name. This makes for easier to read generated pipeline files and allows for better behavior if another pipeline wants to use queue-time variables to control pipeline behavior. --------- Co-authored-by: Ben Hillis <[email protected]>
microsoft · Jan 9, 2025 · 636fefa · 636fefa
1 parent 4a4c584
commit 636fefa
Show file tree

Hide file tree

Showing 8 changed files with 238 additions and 130 deletions.
diff --git a/.github/workflows/openvmm-ci.yaml b/.github/workflows/openvmm-ci.yaml
diff --git a/.github/workflows/openvmm-pr.yaml b/.github/workflows/openvmm-pr.yaml
diff --git a/flowey/flowey_cli/src/pipeline_resolver/ado_yaml.rs b/flowey/flowey_cli/src/pipeline_resolver/ado_yaml.rs
@@ -284,6 +284,8 @@ echo "$(FLOWEY_TEMP_DIR)/work" | {var_db_insert_working_dir}
                 flowey_core::pipeline::internal::Parameter::Bool { .. }
             );
 
+            let name = parameters[*pipeline_param_idx].name();
+
             // ADO resolves bools as `True` and `False`, _sigh_
             let with_lowercase = if is_bool {
                 r#" | tr '[:upper:]' '[:lower:]'"#
@@ -296,7 +298,7 @@ echo "$(FLOWEY_TEMP_DIR)/work" | {var_db_insert_working_dir}
             let cmd = format!(
                 r#"
 cat <<'EOF'{with_lowercase} | {var_db_inject_cmd}
-${{{{ parameters.param{pipeline_param_idx} }}}}
+${{{{ parameters.{name} }}}}
 EOF
 "#
             )
@@ -530,20 +532,26 @@ EOF
                     })
                     .collect()
             },
-            variables:  {
-                let mut ado_variables: Vec<schema_ado_yaml::Variable> = ado_variables.clone().into_iter()
-                        .map(|(name, value)| schema_ado_yaml::Variable { name, value })
-                        .collect();
+            variables: {
+                let mut ado_variables: Vec<schema_ado_yaml::Variable> = ado_variables
+                    .clone()
+                    .into_iter()
+                    .map(|(name, value)| schema_ado_yaml::Variable { name, value })
+                    .collect();
 
-                ado_variables.push(schema_ado_yaml::Variable {name: "FLOWEY_TEMP_DIR".into(), value: FLOWEY_TEMP_DIR.into()});
+                ado_variables.push(schema_ado_yaml::Variable {
+                    name: "FLOWEY_TEMP_DIR".into(),
+                    value: FLOWEY_TEMP_DIR.into(),
+                });
 
-                Some(
-                    ado_variables
-                )
+                Some(ado_variables)
             },
             steps: ado_steps,
             condition: Some(if let Some(cond_param_idx) = cond_param_idx {
-                format!("and(eq('${{{{ parameters.param{cond_param_idx} }}}}', 'true'), succeeded(), not(canceled()))")
+                format!(
+                    "and(eq('${{{{ parameters.{} }}}}', 'true'), succeeded(), not(canceled()))",
+                    parameters[cond_param_idx].name()
+                )
             } else {
                 "and(succeeded(), not(canceled()))".into()
             }),
@@ -644,35 +652,41 @@ EOF
         parameters: if !parameters.is_empty() {
             Some(
                 parameters
+                    .clone()
                     .into_iter()
-                    .enumerate()
-                    .map(|(idx, param)| match param {
+                    .map(|param| match param {
                         flowey_core::pipeline::internal::Parameter::Bool {
+                            name,
                             description,
+                            kind: _,
                             default,
                         } => schema_ado_yaml::Parameter {
-                            name: format!("param{idx}"),
+                            name,
                             display_name: description,
                             ty: schema_ado_yaml::ParameterType::Boolean { default },
                         },
                         flowey_core::pipeline::internal::Parameter::String {
+                            name,
                             description,
+                            kind: _,
                             default,
                             possible_values,
                         } => schema_ado_yaml::Parameter {
-                            name: format!("param{idx}"),
+                            name,
                             display_name: description,
                             ty: schema_ado_yaml::ParameterType::String {
                                 default,
                                 values: possible_values,
                             },
                         },
                         flowey_core::pipeline::internal::Parameter::Num {
+                            name,
                             description,
+                            kind: _,
                             default,
                             possible_values,
                         } => schema_ado_yaml::Parameter {
-                            name: format!("param{idx}"),
+                            name,
                             display_name: description,
                             ty: schema_ado_yaml::ParameterType::Number {
                                 default,
@@ -710,7 +724,8 @@ EOF
                                     r#ref: match git_ref {
                                         AdoResourcesRepositoryRef::Fixed(s) => s,
                                         AdoResourcesRepositoryRef::Parameter(idx) => {
-                                            format!("${{{{ parameters.param{idx} }}}}")
+                                            let name = parameters[idx].name();
+                                            format!("${{{{ parameters.{name} }}}}")
                                         }
                                     },
                                     r#type: match repo_type {

diff --git a/flowey/flowey_cli/src/pipeline_resolver/direct_run.rs b/flowey/flowey_cli/src/pipeline_resolver/direct_run.rs
@@ -221,22 +221,28 @@ fn direct_run_do_work(
         {
             let (desc, value) = match &parameters[*pipeline_param_idx] {
                 Parameter::Bool {
+                    name: _,
                     description,
+                    kind: _,
                     default,
                 } => (
                     description,
                     default.as_ref().map(|v| serde_json::to_vec(v).unwrap()),
                 ),
                 Parameter::String {
+                    name: _,
                     description,
+                    kind: _,
                     default,
                     possible_values: _,
                 } => (
                     description,
                     default.as_ref().map(|v| serde_json::to_vec(v).unwrap()),
                 ),
                 Parameter::Num {
+                    name: _,
                     description,
+                    kind: _,
                     default,
                     possible_values: _,
                 } => (
@@ -302,7 +308,9 @@ fn direct_run_do_work(
 
         if let Some(cond_param_idx) = cond_param_idx {
             let Parameter::Bool {
+                name: _,
                 description: _,
+                kind: _,
                 default,
             } = &parameters[cond_param_idx]
             else {

diff --git a/flowey/flowey_cli/src/pipeline_resolver/generic.rs b/flowey/flowey_cli/src/pipeline_resolver/generic.rs
@@ -202,9 +202,7 @@ pub fn resolve_pipeline(pipeline: Pipeline) -> anyhow::Result<ResolvedPipeline>
         let parameters_used: Vec<_> = parameters_used
             .into_iter()
             .map(|param_idx| ResolvedJobUseParameter {
-                flowey_var: flowey_core::pipeline::internal::consistent_param_runtime_var_name(
-                    param_idx,
-                ),
+                flowey_var: parameters[param_idx].name().to_string(),
                 pipeline_param_idx: param_idx,
             })
             .collect();

diff --git a/flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs b/flowey/flowey_cli/src/pipeline_resolver/github_yaml/mod.rs
@@ -365,10 +365,12 @@ echo "{RUNNER_TEMP}/work" | {var_db_insert_working_dir}
 
             let var_db_inject_cmd = bootstrap_bash_var_db_inject(flowey_var, is_string);
 
+            let name = parameters[*pipeline_param_idx].name();
+
             let cmd = format!(
                 r#"
 cat <<'EOF' | {var_db_inject_cmd}
-${{{{ inputs.param{pipeline_param_idx} != '' && inputs.param{pipeline_param_idx} || '{default}' }}}}
+${{{{ inputs.{name} != '' && inputs.{name} || '{default}' }}}}
 EOF
 "#
             )
@@ -587,13 +589,14 @@ EOF
             inputs: github_yaml_defs::Inputs {
                 inputs: parameters
                     .into_iter()
-                    .enumerate()
-                    .map(|(idx, param)| {
+                    .map(|param| {
                         (
-                            format!("param{idx}"),
+                            param.name().to_string(),
                             match param {
                                 flowey_core::pipeline::internal::Parameter::Bool {
+                                    name: _,
                                     description,
+                                    kind: _,
                                     default,
                                 } => github_yaml_defs::Input {
                                     description: Some(description.clone()),
@@ -602,7 +605,9 @@ EOF
                                     ty: github_yaml_defs::InputType::Boolean,
                                 },
                                 flowey_core::pipeline::internal::Parameter::String {
+                                    name: _,
                                     description,
+                                    kind: _,
                                     default,
                                     possible_values: _,
                                 } => github_yaml_defs::Input {
@@ -614,7 +619,9 @@ EOF
                                     ty: github_yaml_defs::InputType::String,
                                 },
                                 flowey_core::pipeline::internal::Parameter::Num {
+                                    name: _,
                                     description,
+                                    kind: _,
                                     default,
                                     possible_values: _,
                                 } => github_yaml_defs::Input {