search

tianocore / edk2-pytool-extensions

Extensions to the edk2 build system allowing for a more robust and plugin based build system and tool execution environment
Other
60 stars 40 forks source link

[Bug]: stuart_setup etc. report ERRORs if Linux-x86, Windows-x86 etc. directories are deleted, and doesn't recover #549

Closed bcran closed 1 year ago

bcran commented 1 year ago

Contact Details

rebecca@bsdio.com

Describe the Bug

If the directories for external dependencies are somehow deleted, it appears there's no way to recover except by knowing to delete the extdep_state.yaml file.

ERROR - Could not find appropriate folder for mu-uncrustify-release. Host(os='Linux', arch='x86', bit='64')
ERROR - Could not find appropriate folder for edk2-acpica-iasl. Host(os='Linux', arch='x86', bit='64')
ERROR - Could not find appropriate folder for mu-uncrustify-release. Host(os='Linux', arch='x86', bit='64')
ERROR - Could not find appropriate folder for edk2-acpica-iasl. Host(os='Linux', arch='x86', bit='64')

What Python version are you using?

Python 3.11

Reproduction steps

  1. git clone https://github.com/tianocore/edk2.git
  2. cd edk2 && python -m venv .venv
  3. . .venv/bin/activate
  4. stuart_setup -c .pytool/CISettings.py
  5. stuart_update -c .pytool/CISettings.py
  6. rm -rf ./BaseTools/Bin/edk2-acpica-iasl_extdep/Linux-x86 ./OvmfPkg/PlatformCI/iasl_extdep/Linux-x86 ./.pytool/Plugin/UncrustifyCheck/mu-uncrustify-release_extdep/Linux-x86
  7. stuart_setup -c .pytool/CISettings.py
  8. stuart_update -c .pytool/CISettings.py

Expected behavior

There should either be a flag to reset back to initial state, or stuart_setup should see there's a state mismatch and recover.

Execution Environment

Ubuntu 23.04

Pip packages

Package                Version
---------------------- --------
antlr4-python3-runtime 4.7.1
cffi                   1.15.1
edk2-basetools         0.1.43
edk2-pytool-extensions 0.21.9
edk2-pytool-library    0.14.0
lcov-cobertura         2.0.2
pefile                 2023.2.7
pip                    23.0.1
pycparser              2.21
pygit2                 1.12.1
PyYAML                 6.0
semantic-version       2.10.0
setuptools             66.1.1

Additional context

No response

Javagedes commented 1 year ago

Hi @bcran, What situation brought about an external dependency being half-way deleted? I can probably make some changes to catch this when calling stuart update (which is responsible for updating the external dependnecies) by re-downloading any external dependency that throws the error mentioned, but I'm hoping to figure out the situation that this happens in and try and prevent that instead.

The expectation for external dependencies is that they are to remain untouched as they are dependencies that are used, not co-developed (think of them as pip packages, we download them, and use them, but we don't really ever edit them)

Thanks, Joey

bcran commented 1 year ago

I can't remember why I deleted them, but it's possible I was trying to clean up the clone and hadn't realized the extdep_state.yaml wasn't part of the repo.

Javagedes commented 1 year ago

There are two reasons that this error can occur:

  1. The extdep has the "host_specific" flag which signals that the top-level directory of the external dependency contains folders specific to combination of os/arch/bit (see here) and that particular host combination is not supported.
  2. the extdep has the "host_specific" flag and that particular host combination was supported, but the folder was deleted.

My original intent was to add sha256 hashing to all dependencies as default by hashing all files in the dependency as follows:

  1. Download the external dependency
  2. Hash all files inside the external dependency and set the commit hash in the extdep_state.yaml file
  3. When environment verification is completing, re-hash the files and compare the commit hash.
  4. If the hashes don't match the external dependency is redownloaded

Unfortunately, this breaks down when the extdep has an extdep inside of it. In this situation, the old hash becomes incorrect when the internal extdep is downloaded. There are a few ways we could work around this, but honestly it is extra complexity for an extremely uncommon situation.

In light of keeping the detection of this issue simple, this will be the new approach:

If the external dependency is "host_specific", the supported host combinations will be recorded in the extdep_state.yaml file and verified all directories still exist. While this won't catch changes internal to those files, it will at least be able to:

  1. keep the current error, updating it slightly to say that the detected host combination is unsupported by this external dependency
  2. Detect when a particular host combination was deleted, and re-download the external dependency.