Right now EccCheck is destructive when it calculates a diff. EccCheck should either not be destructive, or fail to run in a dirty environment. Its current behavior may destroy work also makes it difficult to incrementally solve ECC errors. Ideally, the "what files have changed since X" would be generalized and added to Edk2ToolLib or Edk2ToolExt, since a number of CI and PR gate checks implement their own versions of this functionality.
Agree this issue. Bret: have you any proposal to fix this issue?
I suppose my first question would be: Why does it do that now? Is it a bug, or intentional?
I can definitely agree that this is a pretty painful experience as a local developer. I ran CI to validate my changes and had all my changes wiped out.
This is the implementation limitation. In open CI, ECC supports to skip some source files or directories to be scanned. Current implementation checkouts the code, then remove the skipped source files and directories, next run ECC checker to get ECC scan result, last type "git reset --hard" to restore all files. To avoid the issue to run open CI in the local environment, the developer needs to do the local commit message, then run open CI.
Assign to owner
There are a number of functional and performance issues with EccCheck 1) For the git state corruption issue, a design change is required. Instead of modifying the local git source tree to implement the IgnoredFiles feature, the package that will be checked with ECC should be copied to a temporary directory and the IgnoredFiles should be removed from the temporary directory. a) Python supports a tempfile module that can return a temporary directory. This module works well when used locally, but does not seem to work when used with azure pipelines windows agents. This may be related to the use of drive letters and the drive letter for the sources are D: and the temporary directory created is on C: b) Instead of using python tempfile module, create a temporary directory in the Build output directory called ecctemp. 2) The ECC configuration file called exception.xml in BaseTools/Source/Python/Ecc is updated in place with additional exceptions from the EccCheck tool and from the IgnoreErrors section of the package YAML file. This shows up as a modified file by git. Instead, make a copy of exception.xml into the temporary directory and update that copy with the additional exceptions leaving the BaseTools directory unmodified. 3) Performance issues and log size issues running the git diff commands. The git diff command is used to determine the set of modified directories in a package and the set of modified lines to include in the scan results. The current implementation redirects the git diff output to stdout, which for large patch sets is slow and generates a large build log file. The recommended solution is to use the --output of git diff to generate the git diff output into a file in the temporary directory. 4) When the ECC command line tool is run, it creates some temporary output files in the current working directory. If the current working directory is in one of the git repo local directories these temp files will show up as untracked files. In order to prevent the creation of untracked files, the working directory when ECC is run should be set to the ecctemp directory in the Build output directory. The Build output directory is already ignored by .gitignore. 5) Managing the temporary directory a) If Build/ecctemp is present when EccCheck is started, then remove the existing temporary directory b) When EccCheck exits, always remove the temporary directory. c) Add try/except to RunPlugin to guarantee that normal exits, exceptions from Keyboard Interrupt, and all unexpected exceptions are caught and the temp directory is removed. 6) Remove duplicate scans of same folders in GetModifyDir(). If a lib/module has multiple directory levels and there are source file changes al multiple levels, then ECC will be invoked multiple times and will scan the same source files more than once. Update GetModifyDir() to filter out these the redundant set of modified directories.
GitHub PR Merged: https://github.com/tianocore/edk2/pull/2216 Commits: https://github.com/tianocore/edk2/pull/2216/commits/db62ddb8c34e176d51909a1214f7669705a79a0a https://github.com/tianocore/edk2/pull/2216/commits/53d0df7942a604a2bc966e50117cbfa502d211af https://github.com/tianocore/edk2/pull/2216/commits/0603fd28a58f93d9e1d12b0d456c067f9d87af2b Email code review V2: https://edk2.groups.io/g/devel/message/83952 Email code review V1: https://edk2.groups.io/g/devel/message/83924
