From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web08.2704.1638909346041045312 for ; Tue, 07 Dec 2021 12:35:46 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=FUpTfoRI; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [10.0.0.19] (c-73-27-179-174.hsd1.fl.comcast.net [73.27.179.174]) by linux.microsoft.com (Postfix) with ESMTPSA id BA83A20B7179; Tue, 7 Dec 2021 12:35:44 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com BA83A20B7179 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1638909345; bh=l2kEC9Kng52iAeca3GQ1ZJ0eCjQQn3rtd1lVp0gLwac=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=FUpTfoRI0wWeYN8ht0fjjGRYUwfgW4hGVOuWvUZA+H/laTHpumhnuCPJ1kQtDlaS/ MOTO5lV9Mjdux99T8cyUMamn4tg53XiByC+j6JWaAc0sSsZBrzRwzvI+ZtijhTkLV4 yidhWMujsEBItCn712R2dGfrepq03zZWrG25QnSg= Message-ID: Date: Tue, 7 Dec 2021 15:35:43 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #5 - Completed - Hard Freeze Lifted To: devel@edk2.groups.io, afish@apple.com, Mike Kinney Cc: Michael Kubacki , Leif Lindholm References: From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 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 >> >> >> >> >> >> > > > > >