For MdePkg, MdeModulePkg, FmpDevicePkg Reviewed-by: Liming Gao <gaolim...@byosoft.com.cn>
> -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael D > Kinney > 发送时间: 2021年12月1日 6:34 > 收件人: devel@edk2.groups.io; Kinney, Michael D > <michael.d.kin...@intel.com>; Michael Kubacki > <michael.kuba...@microsoft.com>; Andrew Fish (af...@apple.com) > <af...@apple.com>; Leif Lindholm <l...@nuviainc.com> > 主题: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard > Freeze Update #4 > > Hello, > > Thank you for your patience during this extended hard freeze. > > Just one more step to go. There has been a delay in the review of > the patch series with the uncrustify source changes. PR(6). This > patch series was not sent out as patch review email because of its > very large size. It only contains source style changes and the > CompareBuild tool and GitHub action has shown there are no binary > differences introduced with these source style changes. > > If you are a package maintainer, then please review the following > branch/PR for your package contents and review the EDK II CI results > and BuildCompare results. I do not expect a line by line review > because we already had time to provide feedback on the source style > performed by uncrustify. Instead, a Reviewed-by for your package > indicates that you have reviewed the EDK II CI results and CompareBuild > tool functionality and results and you accept the source style > changes to your package. > > * > https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCh > anges_V5 > * https://github.com/tianocore/edk2/pull/2229 > * https://github.com/mdkinney/edk2/actions/runs/1521618836 > > Additional details on this update below. > > Thank you, > > Mike > > > 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/P > roject-Mu-(EDK-II)-Fork-Readme > * Download: > https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed= > mu_uncrustify&package=mu-uncrustify-release&protocolType=NuGet&versio > n=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. The branch with a preview of the uncrustify changes can be > used > to start this rebase work. > > > https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCh > anges_V5 > > 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-dem > os/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-dem > os/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_R > eproducibleBuild > * 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_EccCheckRemoveGitRev > ert_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_LicenseCheckUseDiffOut > putFile_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_IncreaseAzurePipelinesTi > meout > * 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_EccCheckIgnoreFilesErro > rs > * 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=3739 > * > https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCh > anges_V5 > * https://github.com/tianocore/edk2/pull/2229 > * Build comparison result PASS: > https://github.com/mdkinney/edk2/actions/runs/1521618836 > * EFI_D_ -> DEBUG changes required to pass PatchCheck > * Uncrustify format changes required to pass UncrustifyCheck > * Status: > Waiting for review > > 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 > _v6 > * Required to enforce all PRs submitted to EDK II CI match uncrustify > format. > * Unconditionally checks all packages. Can not be committed until all > C/H > source files have been updated. > * Status: Review complete > > 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_ApplyUncrustifyCh > anges_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 (#84180): https://edk2.groups.io/g/devel/message/84180 Mute This Topic: https://groups.io/mt/87419078/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
