> On Dec 7, 2021, at 12:35 PM, Michael Kubacki wrote: > > I will send a patch for the Tianocore Wiki shortly. > OK thanks. I can test on Linux and macOS. Maybe I’ll try using VS Code as my editor now…. Thanks, Andrew Fish > 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 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 >>> >>> >>> >>> >>> >>> > > >