Tools and CI Meeting - Date 10/21

[spbrogan]

Agenda

1. Updates on github issue transition (WIP)
2. Review of urgent CI issues
3. Updates on github cve fix process

Past Information

1. 8/5 meeting with great write up - Tools and CI Meeting - Date 8/5 #6047
2. 9/30 Meeting - Tools & CI meeting - September 30th, 2024 - Euro friendly edition #6208

Microsoft Teams meeting

Join on your computer or mobile app
Click here to join the meeting
Or call in (audio only)
+1 425-616-0754,,330776518# United States, Seattle
Phone Conference ID: 330 776 518#
Find a local number | Reset PIN
Learn More | Meeting options

[os-d]

It would be good to discuss STATIC vs static w.r.t GoogleTest here.

[makubacki]

10/21 Meeting Notes

1. Updates on github issue transition (WIP)
   • @makubacki: Make sure the plan to distribute issues across various GitHub repos is captured in RFC: Transitioning from Bugzilla to GitHub Issues #5926
     • In particular, identify the BZ fields use to direct the issues to the other Tianocore repos
2. Review of urgent CI issues
   • Note: For all CI related improvements. We need to define metrics to track how effective improvements are.
     • Someone needs to own this task - define the metrics and how they will get implemented.
     • @mdkinney - Can look at mergify metrics. Summarize what information is available and check permissions for others to access the information.
       • Todo: Add metrics to edk2 Readme with CI health and status info
   • Staged pipeline build - simpler checks earlier in the pipeline
     • Perform the simplest compilation upfront - GCC (fast) - basic compilation checks
     • 10/28: @makubacki - Submit as an issue in a new CI project in the edk2 repo
   • Reduce the total number of jobs to <= 30 (under the number of parallel job agents)
     • ArmVirtPkg, OvmfPkg, EmulatorPkg reduce number of jobs to a total of 10 for all three packages
     • Also consider load balancing work within jobs to get the overall minimum job execution duration
     • Also consider how to split work across Azure DevOps and GitHub agents
   • Next step: Consider where to place jobs - in PR, post-PR (master branch merge), nightly, etc.
   • Tasks:
     1. Platform Owner - (@os-d will do the initial reach out to platform owners):
        • What are the most essential builds necessary to include in PR status checks?
        • What are the most necessary checks to run for the overall package? Can checks happening in different steps/jobs today be combined in the future?
        • Do individual platforms (e.g. bhyve, Xen, etc.) under OVMF need to stay versus go to edk2-platforms?
        • Can you provide CI agents to reduce the impact of your platform CI on the overall project's agents?

        • Note: Tianocore CI group will need to evaluate these responses to create/propose a set of jobs for these platforms.

        • Note: 10/28: @mdkinney's suggestion: Create the PR with the changes and add the maintainers to the PR as reviewers.

     2. CI Team:
        • Make a staged pipeline proposal taking considerations into account
3. Updates on GitHub CVE fix process
   • Was not able to create a new branch (e.g. security-advisories/cve) from an existing PR that would then be used to create a new PR into edk2/main
   • @Flickdm - New plan: Create a new PR that will target a new branch (e.g. security-advisories/cve). Then take it from there to edk2/main.
     • Note: Will need @mdkinney to create the branch
     • We need infosec members or edk2 maintainers to have permission to be able to create branches with the name pattern security-advisories/*
       • Note: Might not be possible to assign permissions to teams at this level of granularity in the free version of GitHub
         • Maintainers have write access to the repo except in freeze periods. Write permission allows the branch to be created. Will need to apply branch rules to restrict the branch names for branches that can be created. Ultimately, this will mean that anyone with write access will be able to create these branches.
4. It would be good to discuss STATIC vs static w.r.t GoogleTest here.
   • Google Test compatibility: Disallow the use of local variables declared as static
     • Todo: Completely remove the use of static being used today for local variables

     • Note: Can define lowercase static to nothing for unit tests

   • EDK II style spec & code update: Only allow use of lowercase static moving forward for functions and globals. Explicitly state that static cannot be used for local variables. Use an actual global variable following global variable naming convention.
   • @bexcran - Do you plan to make the static changes discussed above?
     • 10/28: Rebecca will make the changes
5. Removal of VS2015
   • Need to define a process for enabling and retiring toolchains
     • What community input is needed to retire a given toolchain?
   • Set a defined schedule to review toolchains
   • @bexcran - Can you please send an email to the mailing list and make a GitHub discussion to propose removing VS2015? Please include a proposed deadline.
     • 10/28 - Sent an email about VS2015 and other older toolchains like GCC5. Will revisit in mid-November.
       • @mdkinney - Also follow up with Intel owners about dropping VS2017 as well.
6. Updating the build system so cross building to x86 works similar to other platforms
   • @bexcran - We moved this to next week's agenda. Please join so we can discuss further.
     • 10/28: @kuqin12 - Give feedback on cross compiling issues and experience from work earlier done.
     • 10/28: @bexcran - Highlight the impact to downstream consumers.
7. Improving edk2 PR review time
   • Official Tianocore guidelines are documented here: https://github.com/tianocore/tianocore.github.io/wiki/TianoCore-Who-we-are
   • Consider a bot that regularly checks for PRs that have push labels that haven't been merged in a long time to notify maintainers.
   • Today, reviews are monolithic across the entire PR. Some reviewers don't feel confident they can leave an approval on the whole PR.
   • 10/28: The group agrees it is a problem. We didn't have enough time to spend in detail on the topic. The suggestion is for participants to think more about this, and we dedicate more time to the topic next time.

---

Note: There have been no volunteers to work on the highest priority task (staged pipelines) in ~4 weeks now.

[bexcran]

@makubacki Yes, I'll make the static changes. Though I disagree with disallowing the use of local static variables. How about we find a more suitable unit test framework instead?

I'll join the call next week so we can discuss x86 cross-building.

[makubacki]

Yes, I'll make the static changes.

Thanks

Though I disagree with disallowing the use of local static variables. How about we find a more suitable unit test framework instead?

I've been partial to that direction as well since this goes against intuitive use of C language semantics, but I also haven't invested as much time exploring this issue as others. @mdkinney, do you have feedback?

[bexcran]

I've been partial to that direction as well since this goes against intuitive use of C language semantics, but I also haven't invested as much time exploring this issue as others. @mdkinney, do you have feedback?

Not to derail the discussion, but I'd like to see us move (slowly?) away from EDK2 types like UINT8, UINT32 etc. and instead just use standard C (C99, C11 etc.) types uint8_t, uint32_t instead. It'll make it so much easier for anyone wanting to join the project. Today, I think the coding style is far too unique compared to other open source projects.

[mdkinney]

I agree that use of 'static' for local variables is allowed by the C language. It is just not used much in edk2 and also makes the code harder to support because it is a rarely used feature and the code can always be implemented without the use of a 'static' local variable.