[edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #5 - Completed - Hard Freeze Lifted
Michael Kubacki
mikuback at linux.microsoft.com
Tue Dec 7 20:35:43 UTC 2021
I will send a patch for the Tianocore Wiki shortly.
Thanks,
Michael
On 12/7/2021 2:27 PM, Andrew Fish via groups.io wrote:
>
>
>> On Dec 7, 2021, at 11:22 AM, Michael D Kinney <michael.d.kinney at intel.com> wrote:
>>
>> Hello,
>>
>> Thank you to everyone for their patience and for everyone who helped with the development
>> and review of this important update to the edk2 repository.
>>
>> The last 2 PR series have completed review, passed EDK II CI checks, passed CompareBuild
>> verifications, and been pushed.
>>
>> https://github.com/tianocore/edk2/pull/2242
>> https://github.com/tianocore/edk2/pull/2244
>>
>> The hard freeze has ended and normal development activities can continue.
>>
>> 1) If you have code reviews that started before the uncrustify changes, please
>> update them. The recipes are included below and will be added to the Wiki.
>>
>> 2) For all new patch review, please make sure you run the uncrustify tool or
>> use it as a plugin with your editor. If PRs submitted to EDK II CI do not
>> match the uncrustified version, they will not pass CI.
>>
>
> Mike,
>
> How is this documented outside of this patch set? Is there a Tianocore Wiki page on configuring uncrustify? Does that Readme.rst link to that?
>
> Thanks,
>
> Andrew Fish
>
>> Changes from Update #4
>> ----------------------------------------------------------------------------
>> * Pushed PR (6)
>> * Pushed PR (7)
>> * Change uncrustify configuration align_assign_thresh from 4 to 0 (no limit).
>> * Update all use of ', OPTIONAL' to ' OPTIONAL,' for function params.
>> * Change complex DEBUG_CODE() to DEBUG_CODE_BEGIN/END()
>>
>> Changes from Update #3
>> ----------------------------------------------------------------------------
>> * Pushed PR (5)
>> * Added link to PR(6). EDK II CI Status is PASS. Build Compare PASS.
>> * Waiting for review of PR (6)
>> * Review of PR (7) completed and waiting for review of PR (6)
>> ----------------------------------------------------------------------------
>>
>> Changes from Update #2
>> ----------------------------------------------------------------------------
>> * Changed order of PRs swapping (4) and (5). The PR that activates
>> increases the max CI agent job time is independent of all the other
>> PRs and its review is complete, so it can be committed now.
>> * Pushed PRs (1), (2), (3), (4).
>> * Waiting for review to complete for PRs (5) and (6)
>> * Reviews complete for PR (7)
>> * Identifies steps using git filter-branch to apply uncrustify changes to a
>> code review patch series that was generated before the uncrustify changes
>> avoiding manual merge.
>> * Identified steps using git filter-repo to generate an alternate history of
>> the edk2 repo with uncrustify changes applied on every commit. This may
>> be useful when evaluating changes to files using tools like git blame
>> without the large uncrustify patch series.
>> ---------------------------------------------------------------------------
>>
>> Changes from Update #1
>> ----------------------------------------------------------------------------
>> * Changed order of PRs swapping (6) and (7). The PR that activates
>> EDK II CI check UncrustifyCheck has to be last because it unconditionally
>> checks all C/H files in all packages. Not just files that have been
>> modified like some of the other checkers.
>> * Updated link to the branch with the UncrustifyCheck plugin that has been
>> updated with a one line change and Reviewed-by and Tested-by tags.
>> https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_v6
>> * Reviews complete for (1), (2), (3), (5), and (7)
>> ---------------------------------------------------------------------------
>>
>> Michael Kubacki and I have prepared the patches required to apply the
>> uncrustify changes and enable EDK II CI to check all submitted
>> patches have been run through uncrustify.
>>
>> We have verified through the CompareBuild GitHub Action that the
>> format changes performed by uncrustify have no functional changes.
>> All of the OBJ, LIB, DLL, EFI, FFS, FV, and FD files match 100%
>> across 70 VS2019/GCC5 builds of all package/platform DSC files in
>> the edk2 repo.
>>
>> The hard freeze will be extended after the edk2-stable202111 tag until
>> all uncrustify related changes are committed. We do not expect this
>> to take more than a few days. Do not push any PRs until the hard
>> freeze is lifted.
>>
>> The changes are broken up into 7 patch series/PRs. The PRs are ordered
>> so they can be submitted using the normal submission process and EDK II
>> CI will pass for each one. Details are listed below.
>>
>> Uncrustify 73.0.3 for EDK II
>> =============================
>> * Sources: https://dev.azure.com/projectmu/_git/Uncrustify
>> * Documentation: https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
>> * Download: https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed=mu_uncrustify&package=mu-uncrustify-release&protocolType=NuGet&version=73.0.3
>>
>> Installing Uncrustify
>> ======================
>> The Uncrustify tool is installed automatically when the Pytools
>> environment is used and the stuart* commands are run to complete the
>> environment setup. Please see:
>>
>> https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-locally
>>
>> Uncrustify can also be installed from the download page listed above
>> or built from sources from the source link above.
>>
>> The Documentation link provides instruction on how to run uncrustify from
>> the command line or install as a Visual Studio Code plugin. The main
>> uncrustify documentation also describes how to integrate with a few other
>> editors.
>>
>> We have also discussed a client side githook. That effort has not started.
>> Let us know if that is a feature you would find useful.
>>
>> Developer impact for new code reviews
>> ======================================
>> Once the uncrustify checker is active in EDK II CI, developers must
>> make sure their patches are run through the uncrustify tool before
>> sending the patches for review.
>>
>> Developers must install and run uncrustify against changes files before
>> sending patch review emails or submitting PR for EDK II CI. If EDK II CI
>> detects and differences in source formatting, then EDK II CI will fail
>> and the developer must run uncrustify and resubmit the patches.
>>
>> Developer impact to patch series/PRs reviewed during edk2-stable201121 soft/hard freeze
>> =======================================================================================
>> Developers must rebase their changes after the uncrustify source changes are
>> committed(latest edk2/master)
>>
>> The following steps can be used to update an existing branch with the
>> required uncrustify format. This is the Windows version. I will add
>> the Linux version soon.
>>
>> 1) Fetch and checkout and rebase to latest edk2/master
>>
>> git fetch origin
>> git checkout master
>> git rebase origin/master
>>
>> 2) Make a backup copy of plugin UncrustifyCheck outside WORKSPACE.
>> (e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable and
>> EDK II specific uncrustify configuration file available when working
>> with a branch that does not have those tools in its scope.
>>
>> xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck
>>
>> 3) Check out the patch series branch (e.g. MyBranch)
>>
>> git checkout MyBranch
>>
>> 4) Rebase patch series against edk2-stable202111
>>
>> git rebase edk2-stable202111
>>
>> 5) Create new branch for the uncrustifed version (e.g. MyBranch_Uncrustified)
>>
>> git checkout -b MyBranch_Uncrustified
>>
>> 6) Use git filter-branch to uncrustify all the commits in the series
>> between the rebase target from (2) and HEAD of the branch. A filter
>> can be used to scope the uncrustify operations to only the C/H files
>> in the specific package the patch series is against. (e.g. DynamicTablesPkg).
>> BaseTools should always be excluded. If the package scoped filter is
>> not used, it will still work, but will take longer to run because
>> uncrustify will rescan every C/H files in the whole repo.
>>
>> git filter-branch --tree-filter "git ls-files DynamicTablesPkg*.c DynamicTablesPkg*.h :!BaseTools/* | c:\\Temp\\UncrustifyCheck\\mu-uncrustify-release_extdep\\Windows-x86\\uncrustify.exe -c c:\\Temp\\UncrustifyCheck\\uncrustify.cfg -F - --replace --no-backup --if-changed" edk2-stable202111..HEAD
>>
>> 7) Now that all the individual patches in the branch are uncrustified,
>> rebase against latest edk2/master that is already uncrustified.
>>
>> git rebase master
>>
>> 8) Verify the patches in this new branch.
>>
>> Impacts to tracing history across the uncrusity changes
>> =======================================================
>> Tools the view file and line history do work with the large uncrustify
>> patch series. One impact is that the operations can be very slow due
>> to the large uncrustify patches.
>>
>> One option to provide a faster experience is to provide an alternate
>> version of the edk2 repository as "documentation" that has the
>> entire history re-written with uncrustify run on every commit.
>> The tool called git-filter-repo can be used to perform this
>> transformation and runs in a reasonable period of time (a few hours)
>>
>> https://github.com/newren/git-filter-repo
>> https://github.com/newren/git-filter-repo/blob/main/contrib/filter-repo-demos/lint-history
>>
>> The following steps can be used to perform this transformation.
>> This is the Windows version. I will add the Linux version soon.
>>
>> ** WARNING ** This operation modifies(rewrites) all the commits
>> in the local copy of the repo. Do not perform
>> these steps on a local repo you are using for
>> active development.
>>
>> 1) Clone edk2 into a new directory (see **WARNING**)
>>
>> git clone https://github.com/tianocore/edk2.git edk2-uncrustified
>> cd edk2-uncrustified
>>
>> 2) Setup python virtual env, install pytools, and run stuart commands
>> to setup build environment which includes installing uncrustify tools.
>>
>> https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-locally
>>
>> 3) Make a backup copy of plugin UncrustifyCheck outside WORKSPACE.
>> (e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable and
>> EDK II specific uncrustify configuration file available when working
>> with a branch that does not have those tools in its scope.
>>
>> xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck
>>
>> 4) Use lint-history.py from git-filter-repo examples
>>
>> https://github.com/newren/git-filter-repo
>> https://github.com/newren/git-filter-repo/blob/main/contrib/filter-repo-demos/lint-history
>>
>> Line #127 - Add try except around subprocess.check_call() with except
>> being pass. This is required because there are a few commits of C
>> files in the edk2 repo that have incorrect C syntax and do not
>> build with a C compiler and break the uncrustify parser. Skip reformat
>> of C files that can not be parsed by uncrustify. These rare instances
>> are addressed in the commit that fixes the C syntax error.
>>
>> Run this slightly modified version of lint-history. Include only
>> C/H files and exclude directories that start with 'Tools' or 'BaseTools'.
>> This step took about 2.2 hours on a laptop.
>>
>> lint-history.py
>> --relevant "return (not filename.startswith(b'Tools') and not filename.startswith(b'BaseTools') and (filename.endswith(b'.c') or filename.endswith(b'.h')))"
>> c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\mu-uncrustify-release_extdep\\Windows-x86\\uncrustify.exe -c c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\uncrustify.cfg --replace --no-backup --if-changed
>>
>> Order of PRs to apply during extended hard freeze
>> ==================================================
>> 1) Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS to not force debug information
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3747
>> * https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_ReproducibleBuild
>> * https://github.com/tianocore/edk2/pull/2215
>> * Required for EmulatorPkg to pass CompareBuild for VS2019 IA32/X64 builds.
>> * Status: Review complete. PR pushed.
>>
>> 2) EccCheck should not revert staged and local changes
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=2986
>> * https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRevert_V2
>> * https://github.com/tianocore/edk2/pull/2216
>> * Required for EDK II CI to complete in a reasonable period of time when
>> processing the 4000+ source file style changes made by uncrustify.
>> * Also fixes critical bugs that can potentially corrupt git state when
>> EccCheck is run locally.
>> * Status: Review complete. PR pushed.
>>
>> 3) Update pytool LicenseCheck plugin to use temp directory for diff output file
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3746
>> * https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutputFile_V2
>> * https://github.com/tianocore/edk2/pull/2217
>> * Required to reduce EDK II CI build times.
>> * Status: Review complete. PR pushed.
>>
>> 4) Update max job time from 60 min to 120 minutes in .azurepipelines/templates
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3750
>> * https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTimeout
>> * https://github.com/tianocore/edk2/pull/2219
>> * Required to allow EccCheck of uncrustify changes to complete on Azure
>> Pipelines CI agents without timing out.
>> * Status: Review complete. PR pushed.
>>
>> 5) Update Package YAML to ignore specific ECC files/errors
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3749
>> * https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors
>> * https://github.com/tianocore/edk2/pull/2218
>> * Required to pass EccCheck
>> * Status: Review complete. PR pushed
>>
>> 6) Uncrustify Source Changes
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3737
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3767
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3760
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3739
>> * https://github.com/mdkinney/edk2/tree/Bug_3737_3767_3760_3739_ApplyUncrustifyChanges_V7
>> * https://github.com/tianocore/edk2/pull/2242
>> * Build comparison result PASS: https://github.com/mdkinney/edk2/actions/runs/1550132292
>> * EFI_D_ -> DEBUG changes required to pass PatchCheck
>> * , OPTIONAL -> OPTIONAL, changes required for proper formatting
>> * DEBUG_CODE() -> DEBUG_CODE_BEGIN/END() required for complex code blocks
>> for proper formatting.
>> * Uncrustify format changes required to pass UncrustifyCheck
>> * Status: Review complete. PR pushed
>>
>> 7) UncrustifyCheck EDK II CI Plugin
>> * https://bugzilla.tianocore.org/show_bug.cgi?id=3748
>> * https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_v7
>> * Required to enforce all PRs submitted to EDK II CI match uncrustify format.
>> * Unconditionally checks all packages. Cannot be committed until all C/H
>> source files have been updated.
>> * Status: Review complete. PR pushed.
>>
>> Combined Branch/PR for Review/Test
>> ==================================
>> * Build Comparison results must pass 100% across the full set of PRs before
>> the individual PRs can be pushed in the order listed above.
>> * Branch: https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series
>> * PR: https://github.com/tianocore/edk2/pull/2229
>> Status = PASS
>> * CompareBuild:
>> Branch: https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChanges_V5
>> --ref1: ef9a059cdb15844fe52a49af2bf7d86b9dd3e9bf
>> --ref2: Bug_3737_3739_ApplyUncrustifyChanges_V5
>> Extra Options: -n 4 --quiet
>> Results: https://github.com/mdkinney/edk2/actions/runs/1521618836
>> 30 VS2019 build comparisons PASS
>> 40 GCC5 build comparisons PASS
>> 100% PASS
>>
>> The following git log shows the set of patches from --ref1 to --ref 2across
>> which there are no differences in any of the OBJ/LIB/DLL/EFI/FFS/FV/FD files.
>>
>> --ref2
>> b7d4bf0675b7 (HEAD -> Bug_3737_3739_ApplyUncrustifyChanges_V5) UnitTestFrameworkPkg: Apply uncrusitify changes
>> 7f03d25f60e7 UefiPayloadPkg: Apply uncrusitify changes
>> 0bfd8d9b5ac9 UefiCpuPkg: Apply uncrusitify changes
>> e1cd9bfb9dea StandaloneMmPkg: Apply uncrusitify changes
>> 5da2f65be378 SourceLevelDebugPkg: Apply uncrusitify changes
>> 95b86de07e5d SignedCapsulePkg: Apply uncrusitify changes
>> fe71d97246c4 ShellPkg: Apply uncrusitify changes
>> 54c21c952992 SecurityPkg: Apply uncrusitify changes
>> 187a3785f12b RedfishPkg: Apply uncrusitify changes
>> 810100002a46 PcAtChipsetPkg: Apply uncrusitify changes
>> 276a695c0cf2 OvmfPkg: Apply uncrusitify changes
>> 303c0a91ab07 NetworkPkg: Apply uncrusitify changes
>> bc80792cd1b1 MdePkg: Apply uncrusitify changes
>> 3ea86be17a2a MdeModulePkg: Apply uncrusitify changes
>> c70ef11ed0cd IntelFsp2WrapperPkg: Apply uncrusitify changes
>> c0291221f252 IntelFsp2Pkg: Apply uncrusitify changes
>> 6a479952a690 FmpDevicePkg: Apply uncrusitify changes
>> 3a7c05b7070d FatPkg: Apply uncrusitify changes
>> b789f98c8959 EmulatorPkg: Apply uncrusitify changes
>> 952d7a1c9220 EmbeddedPkg: Apply uncrusitify changes
>> a1cc9881bab6 DynamicTablesPkg: Apply uncrusitify changes
>> 50654dfe5785 CryptoPkg: Apply uncrusitify changes
>> ed965a02dfa1 ArmVirtPkg: Apply uncrusitify changes
>> 9744023fbc46 ArmPlatformPkg: Apply uncrusitify changes
>> 7a1cde5f5bba ArmPkg: Apply uncrusitify changes
>> 19d17e0913e8 UefiCpuPkg: Change use of EFI_D_* to DEBUG_*
>> ffa718b4f994 SourceLevelDebugPkg: Change use of EFI_D_* to DEBUG_*
>> b86cb3c5e5b4 ShellPkg: Change use of EFI_D_* to DEBUG_*
>> c7c42204dc07 SecurityPkg: Change use of EFI_D_* to DEBUG_*
>> 16b8e6f958e4 PcAtChipsetPkg: Change use of EFI_D_* to DEBUG_*
>> 0ac3f8b2dac5 OvmfPkg: Change use of EFI_D_* to DEBUG_*
>> bc5004b8d294 NetworkPkg: Change use of EFI_D_* to DEBUG_*
>> 6f671a8e2377 MdePkg: Change use of EFI_D_* to DEBUG_*
>> a10c610ff9a3 MdeModulePkg: Change use of EFI_D_* to DEBUG_*
>> 09a3bddba390 FatPkg: Change use of EFI_D_* to DEBUG_*
>> 59c61318246a EmulatorPkg: Change use of EFI_D_* to DEBUG_*
>> 3a80367dda3b EmbeddedPkg: Change use of EFI_D_* to DEBUG_*
>> 23eb1aaf80ca ArmVirtPkg: Change use of EFI_D_* to DEBUG_*
>> 875914b45c54 ArmPlatformPkg: Change use of EFI_D_* to DEBUG_*
>> eb2eca82b451 ArmPkg: Change use of EFI_D_* to DEBUG_*
>> f0f3f5aae7c4 (origin/master, origin/HEAD, master) UnitTestFrameworkPkg: Update YAML to ignore specific ECC files/errors
>> c05734797790 UefiPayloadPkg: Update YAML to ignore specific ECC files/errors
>> c30c40d6c63d StandaloneMmPkg: Update YAML to ignore specific ECC files/errors
>> 9944508e85f1 ShellPkg: Update YAML to ignore specific ECC files/errors
>> 60fa40be458d SecurityPkg: Update YAML to ignore specific ECC files/errors
>> df790cd6b37e MdePkg: Update YAML to ignore specific ECC files/errors
>> 9deb9370766e MdeModulePkg: Update YAML to ignore specific ECC files/errors
>> d7d30e8f219f EmulatorPkg: Update YAML to ignore specific ECC files/errors
>> d5744ecba813 CryptoPkg: Update YAML to ignore specific ECC files/errors
>> c97fee87f0f9 ArmVirtPkg: Update YAML to ignore specific ECC files/errors
>> 1939fc9569f2 ArmPlatformPkg: Update YAML to ignore specific ECC files/errors
>> 365dced2c37a ArmPkg: Update YAML to ignore specific ECC files/errors
>> 76a1ce4d5fec .azurepipelines/templates: Update max pipeline job time to 2 hours
>> 99f84ff47390 .pytools/Plugin/LicenseCheck: Use temp directory for git diff output
>> 3019f1bbabf1 .pytool/Plugin/EccCheck: Add performance optimizations
>> 854462bd3479 .pytool/Plugin/EccCheck: Remove temp directory on exception
>> 69877614fdee .pytool/Plugin/EccCheck: Remove RevertCode()
>> --ref1
>> ef9a059cdb15 EmulatorPkg/Win/Host: Update CC_FLAGS
>> bb1bba3d7767 (tag: edk2-stable202111) NetworkPkg: Fix invalid pointer for DNS response token on error
>>
>> Best regards,
>>
>> Mike
>>
>>
>>
>>
>>
>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84462): https://edk2.groups.io/g/devel/message/84462
Mute This Topic: https://groups.io/mt/87572511/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list