Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[build] Fix and extend build scripting #1212

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

benyamin-codez
Copy link
Contributor

  1. Fixed various syntax issues, e.g. unquoted variables
  2. Unwrapped nested IF statements to ensure no jumping out of loops
  3. Extended colour pallette to stdout
  4. Introduced environment variable SKIP_SDV_ACTUAL to skip Static Driver Verifier (SDV) during analysis builds. CodeQL, Code Analysis (CA) and DVL operations are still executed. Default is to perform the SDV build (unchanged behaviour).
  5. Make actual SDV build conditional (Win10_SDV only) f)
  6. Introduced managed CodeQL package cache and test suite versioning control. If the CodeQL binary is detected at CODEQL_BIN a CodeQL build commences, otherwise it is skipped (unchanged behaviour). The package cache versions are set in the :config_ql_whcp function (new). Test suite versioning is via git hash of the WDK Developer Supplemental Tools repository (new, depends on git). The versions are determined by the value of the WHCP_LEVEL variable, which can be either WHCP_LEGACY or WHCP_24H2. Provision for future WHCP versions is templated as WHCP_NEXT.
  7. Introduced environment variable CODEQL_OFFLINE_ONLY to perform package cache and suite version checking but NOT download any needed updates. Exits on error expecting resolution. Requires git.
  8. Introduced environment variable CODEQL_RUN_BLIND to retain legacy behaviour and perform no package cache or suite version checking. It presumes the prerequisites are present. Removes git dependency.
  9. Pre-builds x86 viosock libraries when building virtio-win.sln or viosock.sln for amd64 in the circumstances the required x86 libraries do not already exist.
  10. If running in a Germanium EWDK environment, the script will instantiate a Cobalt EWDK environment to build Win10 SDV operations, and will return to Germanium EWDK when done.
  11. Use Germanium EWDK to sign Win11 drivers.
  12. Enforce echo off after calling SetupBuildEnv.cmd
  13. Provide some extra EWDK details when EnterpriseWDK=True
  14. Prettify last leg of build and provide SUCCESS / FAIL feedback.

a) Fixed various syntax issues, e.g. unquoted variables
b) Unwrapped nested IF statements to ensure no jumping out of loops
c) Extended colour pallette to stdout
d) Introduced environment variable SKIP_SDV_ACTUAL to skip
   Static Driver Verifier (SDV) during analysis builds. CodeQL,
   Code Analysis (CA) and DVL operations are still executed.
   Default is to perform the SDV build (unchanged behaviour).
e) Make actual SDV build conditional (Win10_SDV only)
f) Introduced managed CodeQL package cache and test suite versioning
   control. If the CodeQL binary is detected at CODEQL_BIN a CodeQL
   build commences, otherwise it is skipped (unchanged behaviour).
   The package cache versions are set in the :config_ql_whcp
   function (new). Test suite versioning is via git hash of the
   WDK Developer Supplemental Tools repository (new, depends on git).
   The versions are determined by the value of the WHCP_LEVEL variable,
   which can be either WHCP_LEGACY or WHCP_24H2. Provision for future
   WHCP versions is templated as WHCP_NEXT.
g) Introduced environment variable CODEQL_OFFLINE_ONLY to perform
   package cache and suite version checking but NOT download any needed
   updates. Exits on error expecting resolution. Requires git.
h) Introduced environment variable CODEQL_RUN_BLIND to retain legacy
   behaviour and perform no package cache or suite version checking.
   It presumes the prerequisites are present. Removes git dependency.
i) Pre-builds x86 viosock libraries when building virtio-win.sln or
   viosock.sln for amd64 in the circumstances the required x86 libraries
   do not already exist.
j) If running in a Germanium EWDK environment, the script will instantiate
   a Cobalt EWDK environment to build Win10 SDV operations, and will
   return to Germanium EWDK when done.
k) Use Germanium EWDK to sign Win11 drivers.
l) Enforce echo off after calling SetupBuildEnv.cmd
m) Provide some extra EWDK details when EnterpriseWDK=True
n) Prettify last leg of build and provide SUCCESS / FAIL feedback.

Signed-off-by: benyamin-codez <[email protected]>
@kostyanf14
Copy link
Member

kostyanf14 commented Dec 12, 2024

@benyamin-codez

  1. Please split this into several commits/PRs to simplify the review
  2. Several items are really independent and can be merged separately, so create new PRs for it:
    • 1. Fixed various syntax issues, e.g. unquoted variables
    • 2. Unwrapped nested IF statements to ensure no jumping out of loops
    • 11. Use Germanium EWDK to sign Win11 drivers.
    • 12. Enforce echo off after calling SetupBuildEnv.cmd
    • 13. Provide some extra EWDK details when EnterpriseWDK=True
  3. Move item 9. Pre-builds x86 viosock libraries when building... to separate PR for discussion
  4. Why do we need 4. Introduced environment variable SKIP_SDV_ACTUAL to skip? What is the point of building DVL without SDV results? For real certification for Win10/Win11 (before 24H2) you must submit DVL with SDV. If you want to check CodeQL results you can build a Win11_SDV target that skips SDV as it is deprecated.
  5. Why do we need 6. Introduced managed CodeQL package cache and test suite versioning control.? CodeQL results with the Win11 24H2 test suite are fully compatible with all current submissions that require CodeQL results.

@benyamin-codez
Copy link
Contributor Author

@kostyanf14

Thanks for having a look, Kostiantyn..!

First let me say, I've got some more changes to commit. So I'll do that first and then look at breaking it up.

I'll likely put these two together to avoid merge dependencies:

1. Fixed various syntax issues, e.g. unquoted variables
2. Unwrapped nested IF statements to ensure no jumping out of loops

I'll probably drop these together as they're in the same file [build\SetVsEnv.bat] and minor:

12. Enforce echo off after calling SetupBuildEnv.cmd
13. Provide some extra EWDK details when EnterpriseWDK=True

  1. Move item 9. Pre-builds x86 viosock libraries when building... to separate PR for discussion

Can do.
I must say, I'm quite curious to see what you might have to say about this.
I would imagine most - if not all - of your workflows would never hit this scenario, i.e. building without x86, whereas I would very rarely build for x86. Just a final build including x86 to make sure there are no compilation errors tbh.

  1. Why do we need 4. Introduced environment variable SKIP_SDV_ACTUAL to skip...? What is the point of building DVL without SDV results? For real certification for Win10/Win11 (before 24H2) you must submit DVL with SDV. If you want to check CodeQL results you can build a Win11_SDV target that skips SDV as it is deprecated.

One scenario is when testing the build system. Without having to do the actual SDV to check the tail end of the build, this is a great time saver. It's also helpful when you have already done the SDV build and you don't want to clobber it on a re-run. Another scenario is if you want to build for any target and do code analysis but not SDV. I note your mention of using Win11_SDV, but sometimes you might want to build for Win10 and not wish to expose Win11-specific options.

Obviously, these are all prototyping scenarios and not building for the WHCP.

  1. Why do we need 6. Introduced managed CodeQL package cache and test suite versioning control.? CodeQL results with the Win11 24H2 test suite are fully compatible with all current submissions that require CodeQL results.

That's certainly how we do it - and I'm in favour of that. However, I followed the MS documentation here and here in implementing this framework, and that really is what I am providing with this feature, a framework for future versions.
Let me provide an example - here is what this commit introduces:

if "%WHCP_LEVEL%"=="WHCP_LEGACY" (
  set CODEQL_PACK_DEL=microsoft/[email protected]
  set CODEQL_PACK_GET=codeql/[email protected],microsoft/[email protected]
)
if "%WHCP_LEVEL%"=="WHCP_24H2" (
  set CODEQL_PACK_DEL=
  set CODEQL_PACK_GET=codeql/[email protected],microsoft/[email protected]
)
if "%WHCP_LEVEL%"=="WHCP_NEXT" (
  rem USE THIS AS A TEMPLATE FOR FUTURE VERSIONS
  rem NOTE: Remember to populate the CODEQL_PACK_DEL variable for all WHCP_LEVELS above with __NEXTRANGE__ packs [per WHCP_LEGACY example]...
  set CODEQL_PACK_DEL=
  set CODEQL_PACK_GET=codeql/cpp-queries@__NEXTRANGE__,microsoft/windows-drivers@__NEXTRANGE__
)

...and then, with the introduction of say an imaginary WHCP_25H2 and updated CodeQL packs, we might have:

if "%WHCP_LEVEL%"=="WHCP_LEGACY" (
  set CODEQL_PACK_DEL=microsoft/[email protected],codeql/[email protected],microsoft/[email protected]
  set CODEQL_PACK_GET=codeql/[email protected],microsoft/[email protected]
)
if "%WHCP_LEVEL%"=="WHCP_24H2" (
  set CODEQL_PACK_DEL=codeql/[email protected],microsoft/[email protected]
  set CODEQL_PACK_GET=codeql/[email protected],microsoft/[email protected]
)
if "%WHCP_LEVEL%"=="WHCP_25H2" (
  set CODEQL_PACK_DEL=
  set CODEQL_PACK_GET=codeql/[email protected],microsoft/[email protected]
)
if "%WHCP_LEVEL%"=="WHCP_NEXT" (
  rem USE THIS AS A TEMPLATE FOR FUTURE VERSIONS
  rem NOTE: Remember to populate the CODEQL_PACK_DEL variable for all WHCP_LEVELS above with __NEXTRANGE__ packs [per WHCP_LEGACY example]...
  set CODEQL_PACK_DEL=
  set CODEQL_PACK_GET=codeql/cpp-queries@__NEXTRANGE__,microsoft/windows-drivers@__NEXTRANGE__
)

This would allow co-existence in the future regardless of whether or not the WHCP chooses to accept DVLs built with WHCP_25H2 for WHCP_24H2 or earlier targets. It also allows you to adjust rapidly should MS change their position.

Similarly, to pick a CodeQL test suite, specify in the CODEQL_DRIVER_SUITES_REPO_HASH environment variable, the relevant SHA256 Git commit from the microsoft/Windows-Driver-Developer-Supplemental-Tools repo.

Architecturally, I'm curious as to whether you think it worthwhile to split this framework, or the whole CodeQL part off to separate file(s). The natural extension of this line of thinking would be to consider whether it is worthwhile to split the whole _SDV build process off to separate file(s) - and maybe rename to _ANALYSIS or similar in the process. This is probably worth considering now if I'm splitting up this PR...

Best regards,
Ben

benyamin-codez added a commit to benyamin-codez/kvm-guest-drivers-windows that referenced this pull request Dec 14, 2024
Addendum to 7348ad4.

1. Refactors changes to the CustomBuildStep Command following code
   review. We now perform all steps regardless of Target OS. We also
   now create DVL files in the Project (driver) folder with version
   numbers in the file name for DVL files created with legacy EWDKs,
   and with the "latest" label for current EWDKs. These files are
   then copied to the relevant .\Install folders and renamed for
   extant WHCP submission operations.

2. New build\makeLegacyDVLs.bat now creates all legacy DVL files.
   This functionally replaces build\dvl1903.bat and extends it to
   service any legacy DVL build. It provides a framework to manage
   legacy DVL creation and a common place to lay out version-specific
   DVL operations and manipulations. Logging is also enabled to
   provide feedback when read at the tail of root\buildAll.bat.
   Please see PR virtio-win#1212 for details on these changes.

Signed-off-by: benyamin-codez <[email protected]>
Addendum to da96c89.

1. Further changes to buildAll.bat
   a) Added @ prefix to FOR do commands
   b) Extended logging around creation of COMPAT (RS1/1607) DVL files
   c) Added SDV and Legacy DVL (DVL files created with legacy EWDKs)
      reporting. The latter queries the <PROJECT>.legacy_dvl_result.txt
      log file created by build\makeLegacyDVLs.bat and reports accordingly.
      Please see PR virtio-win#1210 for more information.
   d) Consolidates exiting function (whether success or fail) to :leave
   e) Creates a scheduled task to call build\clean_build_log.bat before leaving.

2. Created build\clean_build_log.bat to remove colour pallette artefacts
   from build_log.txt created during HCK/CI using Jenkins config.
   This script also deletes the scheduled task created in 1(d) above.

Signed-off-by: benyamin-codez <[email protected]>
@benyamin-codez
Copy link
Contributor Author

I've pushed those changes:

Addendum to da96c89.

  1. Further changes to buildAll.bat:
    a) Added "@" prefix to FOR do commands
    b) Extended logging around creation of COMPAT (RS1/1607) DVL files
    c) Added SDV and Legacy DVL (DVL files created with legacy EWDKs) reporting. The latter queries the <PROJECT>.legacy_dvl_result.txt log file created by build\makeLegacyDVLs.bat and reports accordingly. Please see PR #1210 for more information.
    d) Consolidates exiting function (whether success or fail) to :leave
    e) Creates a scheduled task to call build\clean_build_log.bat before leaving.

  2. Created build\clean_build_log.bat to remove colour pallette artefacts from build_log.txt created during HCK/CI using Jenkins config. This script also deletes the scheduled task created in 1(e) above.

@kostyanf14

That log clean in (2) above, should complete while signing the drivers. There are presently 8 seconds of timeout, 3s to wait for handles to close and 5s for devs to read results in stdout. It usually completes within 10s. Signing a full tree of drivers usually takes longer than this in my environments. That being said, you might want to consider adding a short delay (perhaps using timeout or Powershell Start-Sleep) between the driver signing and the archiving of the artefacts for submission to ensure the build_log.txt is appropriately prepared prior to archiving.

Addendum to da96c89 and 74f5391.

1. Removed packaging folder from root\buildAll.bat per PR virtio-win#1209.

Signed-off-by: benyamin-codez <[email protected]>
@benyamin-codez
Copy link
Contributor Author

@YanVugenfirer @kostyanf14 @ybendito @vrozenfe

I have removed the packaging folder from root\buildAll.bat per PR #1209 and the folder itself per PR #1224.

Any final thoughts before I begin the process of splitting this up as discussed above...?

@kostyanf14
Copy link
Member

@benyamin-codez

Any final thoughts before I begin the process of splitting this up as discussed above...?

I think you can split patches for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants