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

Create crypto binaries report #122

Open
wants to merge 7 commits into
base: release/202311
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .azuredevops/CryptoTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ stages:
-a $(build_archs)
-t $(build_targets)
--stop-on-fail
--report
TOOL_CHAIN_TAG=${{ parameters.toolchain }}

- task: CopyFiles@2
Expand Down Expand Up @@ -158,6 +159,7 @@ stages:
BUILDLOG_CryptoBin_*.txt
SETUPLOG.txt
UPDATE_LOG.txt
Report_STANDARD_DEBUG_VS2022.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to save the Release version as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

report_file_name = f"Report_{thebuilder.flavor}_{target}_{tool_chain}.txt"

This would appear to suggest that we need to handle flavor, target, tool_chain?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The report can be created for each of these 3 configurations (using --report flag)
I triggered it in the Crypto Test pipeline which currently just builds this specific combination.
Besides of the binary sizes, nothing else in the report is different between different flavors, target, toolchain

targetFolder: '$(Build.ArtifactStagingDirectory)/Logs'
flattenFolders: true

Expand Down
169 changes: 169 additions & 0 deletions CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
## @file BundleCrypto.py
# Plugin to Bundle the CryptoBin build output
#
##
# Copyright (c) Microsoft Corporation. All rights reserved.
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
from edk2toolext.environment.plugintypes.uefi_build_plugin import IUefiBuildPlugin
from pathlib import Path
from datetime import datetime
from git import Repo
import lzma

binary_to_type = {}
arch_and_type_to_lib = {}

class ReportCrypto(IUefiBuildPlugin):

def do_post_build(self, thebuilder):
Copy link

@inbal2l inbal2l Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Flickdm, @DorLevi95, @kenlautner - generally speaking, I think it might be worth adopting Doxygen decomentation style (for python, described here) for future projects, so that we can use that to auto-gen decomentation site for the tools. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inbal2l @DorLevi95 Great comment! Usually we inherit our python coding standards from
edk2toollib and try to adhere to google python style and use Ruff for opininated code formatting. We should be able to use mkdocs + some extensions to generate documentation like edk2-pytool-lib

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DorLevi95 could you add function comments :)


# Path to Build output
build_path = Path(thebuilder.env.GetValue("BUILD_OUTPUT_BASE"))
tool_chain = thebuilder.env.GetValue("TOOL_CHAIN_TAG")
target = thebuilder.env.GetValue("TARGET")
# Create report file in the build directory
report_file_name = f"Report_{thebuilder.flavor}_{target}_{tool_chain}.txt"
report_File = Path(thebuilder.ws) / "Build" / report_file_name
report_File.touch()
arch_list = thebuilder.env.GetValue("TARGET_ARCH")

# Time and Title
title = f"CRYPTO BINARIES REPORT - {thebuilder.flavor} FLAVOR - {target}\n\n"
time = datetime.now().strftime('%Y-%m-%d %H:%M:%S\n')
report = [title, "Build Time: " + time]

# get tool chain
report.append(f"Tool Chain: {tool_chain}\n")

# get each CryptoBin module type
self.get_module_type_for_crypto_bin(thebuilder)

report.append("=============================================\n")
# get submodules information
report.append("<------Submodules------>\n")
repo = Repo(thebuilder.ws)
for sub in repo.submodules:
report.append("--------\n")
report.append(f"Name: {sub.name}\n")
report.append(f"Branch: {sub.branch_name}\n")
report.append(f"Commit: {sub.hexsha}\n")

report.append("=============================================\n")
# For each architecture built
for arch in arch_list.split(" "):
arch_build_path = build_path / arch
# write flavor and architecture name to report file
report += ["--------------------------------\n", "ARCH: " + arch + "\n\n"]

# get built binaries
files = list(arch_build_path.glob("*.efi"))
files.sort()

# start binaries sizes report
report.append("<------Crypto binaries sizes report------>\n\n")
for file in files:
# get file size in k bytes
file_size = file.stat().st_size / 1024

# get compressed file size using LMZA
with open(file, 'rb') as input_file:
# Read the content of the input files
input_data = input_file.read()
# Compress the data using LZMA
compressed_data = lzma.compress(input_data)

# Get the number of k bytes of the compressed data
compressed_data_size = len(compressed_data) / 1024
# log to file (assuming file name won't be longer than 40 characters to keep the report clean)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe throw an exception if this assumption is invalid

report.append(f"{file.name} - " + (40-len(file.name))* " " + f"Uncompressed: {file_size:.2f} KB | LZMA Compressed: {compressed_data_size:.2f} KB\n")

# get linked openssl configuration
report.append("\n")
report.append("<------Linked Openssl configuration------>\n\n")
for file in files:
# get module type for the binary
module_type = binary_to_type.get(file.name, "UEFI_APPLICATION") # Default to UEFI_APPLICATION if not found (e.g. test binary)
opensslib = self.get_linked_lib(thebuilder, arch, module_type, "OpensslLib")
report.append(f"{file.name} - " + (40-len(file.name))* " " + f"OpensslLib: {opensslib}\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same assumption that the file name won't be longer than 40 characters



# start openssl configuration report
report.append("=============================================\n")
report.append("<------Openssl configuration report------>\n")

# get openssl library files
openssl_lib = Path(thebuilder.edk2path.GetAbsolutePathOnThisSystemFromEdk2RelativePath("OpensslPkg")) / "Library" / "OpensslLib"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this to the top in case we need to easily change this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with other instances

Copy link
Author

@DorLevi95 DorLevi95 Jan 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Flickdm @inbal2l I moved the paths initialization to the beginning of the "post_build" method to centralize them under one place.
Couldn't do the assigning in the global scope as it needs the data under "thebuilder" param in the "post_build" input.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

openssl_inf_files = list(openssl_lib.glob("OpensslLib*.inf"))
openssl_inf_files.sort()
for file in openssl_inf_files:
# write openssl library files to report file
openssl_flags = self.get_openssl_flags(file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Would it be possible to map the flags to a human readable form? Or do you think that these would change / introduce too much complexity?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be interesting if we have a list of "required" flags and if one is missing that it throws an exception

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first suggestion sounds to indeed introduce more complexity. We would need to create a mapping between all the possible flags and their human read descriptions.
Most flags can be understood for their name.

The second suggestion sounds good.
The future comparison too is the one who aims to throw warnings for each change in the flags found in PR.
I don't know if we would want to change in to throw error OR throw it already in the reporting tool. As this would fail the build/pipeline (depends on which tool), and that might be what the user intended.

Moreover, if we would go down to it, do we have such pre-defined required flags?

report.append("--------\n")
report.append("File: " + file.name + "\n")
report.extend(openssl_flags)

report.append("=============================================\n")

# write report to the report file
report_File.write_text("".join(report))

return 0

def get_openssl_flags(self, file):

# get openssl library flags
flags = []
with file.open() as f:
for line in f:
if "DEFINE OPENSSL_FLAGS" in line:
flags.append(line)
return flags

def get_module_type_for_crypto_bin(self, thebuilder):

CryptoBinPkg_Driver_path = Path(thebuilder.ws) / "CryptoBinPkg" / "Driver"
Copy link

@inbal2l inbal2l Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probebly define CryptoBinPkg_Driver_path (along side other enrvironment-dependent params) in the beginning of the program to make it visible, so that if changed can be changed in one place.

inf_files = list(CryptoBinPkg_Driver_path.glob("*.inf"))
for inf_file in inf_files:
base_name = ""
module_type = ""
with inf_file.open() as f:
for line in f:
if "BASE_NAME" in line:
base_name = line.split("=")[1].strip()
if "MODULE_TYPE" in line:
module_type = line.split("=")[1].strip()
if base_name != "" and module_type != "":
binary_to_type[f"{base_name}.efi"] = module_type # start binaries sizes report
break

def get_linked_lib(self, thebuilder, arch, module_type, lib):

cryptoBinPkg_dsc_path = Path(thebuilder.ws) / "CryptoBinPkg" / "CryptoBinPkg.dsc"
with cryptoBinPkg_dsc_path.open() as f:
current_key = None
# there are 3 possible lib configuarions for the crypto binaries: "[LibraryClasses] (default)", "LibraryClasses.common.{module_type}" and "[LibraryClasses.arch.module_type] (most specific)"
default = ""
common_type = ""
arch_type = ""
for line in f:
if "[LibraryClasses]" in line:
current_key = "Default"
elif "[LibraryClasses" in line:
current_key = line
if f"{lib}|" in line:
specific_lib = line.split("|")[1].strip()
if current_key == "Default":
default = specific_lib
else:
if f"{arch}.{module_type}" in current_key:
arch_type = specific_lib
elif f"common.{module_type}" in current_key:
common_type = specific_lib

if arch_type != "":
return arch_type
elif common_type != "":
return common_type
else:
return default
5 changes: 5 additions & 0 deletions CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto_plug_in.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"scope": "crypto-report",
"name": "Report Crypto Contents",
"module": "ReportCrypto"
}
8 changes: 8 additions & 0 deletions MultiFlavorBuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,17 @@ def validate_flavors(flavor_arg: str):
parserObj.add_argument("-b", "--bundle", dest="bundle", action="store_true",
default=False,
help="Bundles the build output into the directory structure for the Crypto binary distribution.")
parserObj.add_argument("-r", "--report", dest="report", action="store_true",
default=False,
help="produces a report regarding the built crpyto binaries.")

def RetrieveCommandLineOptions(self, args):
self.arch = args.arch
self.target = args.target
self.flavor = args.flavor
self.stop = args.stop
self.bundle = args.bundle
self.report = args.report

def GetWorkspaceRoot(self):
''' get WorkspacePath '''
Expand Down Expand Up @@ -116,6 +120,8 @@ def Go(self, WorkSpace, PackagesPath, PInHelper, PInManager):
params += ["-a", ",".join(arches)]
if self.bundle:
params += ["-b"]
if self.report:
params += ["-r"]

current_build = f"{flavor} {target}"
logging.log(edk2_logging.SECTION, f"Building {current_build}")
Expand All @@ -136,6 +142,8 @@ def Go(self, WorkSpace, PackagesPath, PInHelper, PInManager):
params += ["-a", "AARCH64"]
if self.bundle:
params += ["-b"]
if self.report:
params += ["-r"]

current_build = f"{flavor} {target}"
logging.log(edk2_logging.SECTION, f"Building {current_build}")
Expand Down
8 changes: 7 additions & 1 deletion SingleFlavorBuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,17 @@ def AddCommandLineOptions(self, parserObj):
parserObj.add_argument("-b", "--bundle", dest="bundle", action="store_true",
default=False,
help="Bundles the build output into the directory structure for the Crypto binary distribution.")
parserObj.add_argument("-r", "--report", dest="report", action="store_true",
default=False,
help="produces a report regarding the built crpyto binaries.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
help="produces a report regarding the built crpyto binaries.")
help="produces a report regarding the built crypto binaries.")



def RetrieveCommandLineOptions(self, args):
self.flavor = args.flavor
self.target = args.target
self.arch = args.arch
self.bundle = args.bundle
self.report = args.report

def GetWorkspaceRoot(self):
''' get WorkspacePath '''
Expand All @@ -61,7 +65,9 @@ def GetPackagesPath(self):
def GetActiveScopes(self):
''' return tuple containing scopes that should be active for this process '''
if self.bundle:
return CommonPlatform.Scopes + ("crypto-bundle",)
CommonPlatform.Scopes = CommonPlatform.Scopes + ("crypto-bundle",)
if self.report:
CommonPlatform.Scopes = CommonPlatform.Scopes + ("crypto-report",)
return CommonPlatform.Scopes

def GetBaseName(self):
Expand Down
Loading