microsoft / mu_devops

Project Mu Developer Operations
https://microsoft.github.io/mu/
Other
26 stars 22 forks source link

Introduce extra_build_args to Jobs/PrGate.yml. Move extra_install_steps to after stuart_update call. #292

Closed apop5 closed 7 months ago

apop5 commented 9 months ago

extra_install_step is moved to after stuart_update.

extra_install_steps can be used to pass a call to another stuart_update to include the --codeql parameter.

Javagedes commented 9 months ago

Does this need to be present for setup / ci_setup? Typically, you pass all args to all three commands.

apop5 commented 9 months ago

Does this need to be present for setup / ci_setup? Typically, you pass all args to all three commands.

I added this in the new push.

makubacki commented 9 months ago

@apop5, I assume your platform is directly using the Steps/PrGate.yml template and going to pass the parameter value?

apop5 commented 9 months ago

@apop5, I assume your platform is directly using the Steps/PrGate.yml template and going to pass the parameter value?

I have one of each. The Platform repo is using Steps/PrGate, The submodule repo is using Jobs/PrGate, which does not have a way to extend the extra_build_args.

Would it be useful to extend Jobs/PrGate to also include a extra_build_args parameter as well that is sent into the Steps/PrGate template?

makubacki commented 9 months ago

@apop5, I assume your platform is directly using the Steps/PrGate.yml template and going to pass the parameter value?

Would it be useful to extend Jobs/PrGate to also include a extra_build_args parameter as well that is sent into the Steps/PrGate template?

My initial thought is to simply move execution of extra_install_step in Steps/PrGate.yml after the normal stuart_update operation happens (similar to its place in Steps/BuildPlatform.yml). Then pass in a extra_install_step that runs update again with just the CodeQL flag. That will pull down the CodeQL CLI.

We're normally doing things like installing QEMU in some situations in those steps that I can recall. A quick check across repos would help ensure this is fine.

That keeps extra_build_args scoped to the build steps not impacting that parameter contract with current users.

apop5 commented 8 months ago

Marking as draft so I can test recommended changes.

apop5 commented 8 months ago

@apop5, I assume your platform is directly using the Steps/PrGate.yml template and going to pass the parameter value?

Would it be useful to extend Jobs/PrGate to also include a extra_build_args parameter as well that is sent into the Steps/PrGate template?

My initial thought is to simply move execution of extra_install_step in Steps/PrGate.yml after the normal stuart_update operation happens (similar to its place in Steps/BuildPlatform.yml). Then pass in a extra_install_step that runs update again with just the CodeQL flag. That will pull down the CodeQL CLI.

We're normally doing things like installing QEMU in some situations in those steps that I can recall. A quick check across repos would help ensure this is fine.

That keeps extra_build_args scoped to the build steps not impacting that parameter contract with current users.

For projects that are directly consuming Jobs/PrGate.yml, such as mu_plus's DevOpsWrapper attempting to insert an additional call to stuart_update in the extra_install_step will adds the overhead of the parameters not being resolved (a lot of repos reply on the default values contained in the PrGate.yml files)

Was this how you were envisioning this working, or do you think we could go about this in a different manor?

makubacki commented 8 months ago

For projects that are directly consuming Jobs/PrGate.yml, such as mu_plus's DevOpsWrapper attempting to insert an additional call to stuart_update in the extra_install_step will adds the overhead of the parameters not being resolved (a lot of repos reply on the default values contained in the PrGate.yml files)

Was this how you were envisioning this working, or do you think we could go about this in a different manor?

extra_install_steps flows into Steps/PrGate.yml either directly or via Jobs/PrGate.yml (directly through, not impacting that template). In any case, the default will remain an empty step list so nothing new will run unless a wrapper chooses to pass something.

I meant to move this block in Steps/PrGate.yml:

# Potential Extra steps
- ${{ if eq(parameters.self_host_agent, false) }}:
  - ${{ parameters.extra_install_step }}
- ${{ else }}:
  - bash: echo "##[warning]A self-hosted agent build requested extra install steps. Those are not supported right now."

After the existing call in Steps/PrGate.yml to stuart_update:

- task: CmdLine@2
  displayName: Update ${{ parameters.build_pkgs }} ${{ parameters.build_archs}}
  inputs:
    script: stuart_update -c ${{ parameters.build_file }} -p $(pkgs_to_build) -t ${{ parameters.build_targets}} -a ${{ parameters.build_archs}} TOOL_CHAIN_TAG=${{ parameters.tool_chain_tag}}
  condition: and(gt(variables.pkg_count, 0), succeeded())

# Potential Extra steps
- ${{ if eq(parameters.self_host_agent, false) }}:
  - ${{ parameters.extra_install_step }}
- ${{ else }}:
  - bash: echo "##[warning]A self-hosted agent build requested extra install steps. Those are not supported right now."

So, the normal stuart_update call will always run and a wrapper can optionally pass in follow on steps which, in your case, will be another call to stuart_update ... --codeql that will simply pull down the CodeQL binary. All existing Mu repos should not need their wrappers changed because they already run CodeQL in a separate GitHub workflow.

apop5 commented 8 months ago

@makubacki

Could you give this latest set of changes a quick look?

makubacki commented 7 months ago

@makubacki Could you give this latest set of changes a quick look?

Sorry I missed this message earlier. This looks good to me. I assume it meets your needs after testing?

apop5 commented 7 months ago

Sorry I missed this message earlier. This looks good to me. I assume it meets your needs after testing?

Yes, using a branch with these changes, codeql was able to run on the ado repos.