Create crypto binaries report #122

DorLevi95 · 2025-01-12T10:43:31Z

Description

Creating a reporting script to run in post build.
This script will generate a report about the crypto binaries containing useful info:

Binaries sizes
Submodules info
Linked openssllib*.inf to each binary
Openssllib*.inf build configuration flags

The report will be placed in the build folder with the name "Report_" and published during Crypto tests pipeline.
A follow up PR needs to be added, which will compare reports during each PR (latest release branch report as the baseline report) and produce necessary warnings.
The goal is to catch risky changes that can break functionality or influence the memory consumption (e.g. increase or decrease in size).

Currently the report is only generated and publish during the Crypto tests pipeline. i.e. STANDARD_DEBUG flavor.
This mainly influences only the binaries sizes and might be enough to catch abnormal increase/decrease.

Please refer to this draft PR to view and example of the report and its comparison during a PR: #118

inbal2l · 2025-01-13T11:50:11Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+
+    def get_module_type_for_crypto_bin(self, thebuilder):
+
+        CryptoBinPkg_Driver_path = Path(thebuilder.ws) / "CryptoBinPkg" / "Driver"


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.

inbal2l · 2025-01-13T11:54:19Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+
+class ReportCrypto(IUefiBuildPlugin):
+
+    def do_post_build(self, thebuilder):


@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?

@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

@DorLevi95 could you add function comments :)

Flickdm

Thanks Dor! This looks great and super useful!

Flickdm · 2025-01-13T19:00:10Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+
+class ReportCrypto(IUefiBuildPlugin):
+
+    def do_post_build(self, thebuilder):


@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

Flickdm · 2025-01-13T19:33:59Z

SingleFlavorBuild.py

@@ -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.")


Suggested change

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

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

Flickdm · 2025-01-13T20:10:33Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+        report.append("<------Openssl configuration report------>\n")
+
+        # get openssl library files 
+        openssl_lib = Path(thebuilder.edk2path.GetAbsolutePathOnThisSystemFromEdk2RelativePath("OpensslPkg")) / "Library" / "OpensslLib"


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

same with other instances

@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.

Flickdm · 2025-01-16T19:38:12Z

.azuredevops/CryptoTest.yml

@@ -158,6 +159,7 @@ stages:
          BUILDLOG_CryptoBin_*.txt
          SETUPLOG.txt
          UPDATE_LOG.txt
+          Report_STANDARD_DEBUG_VS2022.txt


Do we need to save the Release version as well?

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?

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

Flickdm · 2025-01-16T19:43:22Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+
+                    # 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)


Maybe throw an exception if this assumption is invalid

Flickdm · 2025-01-16T19:44:31Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+                # 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")


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

Flickdm · 2025-01-16T19:46:24Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+        openssl_inf_files.sort()
+        for file in openssl_inf_files:
+            # write openssl library files to report file
+            openssl_flags = self.get_openssl_flags(file)


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?

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

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?

Flickdm · 2025-01-16T19:49:05Z

CryptoBinPkg/Plugin/ReportCrypto/ReportCrypto.py

+
+class ReportCrypto(IUefiBuildPlugin):
+
+    def do_post_build(self, thebuilder):


@DorLevi95 could you add function comments :)

DorLevi95 added 2 commits January 12, 2025 11:11

create a post build report to report built binaries info

52e2c57

create and publish report in CryptoTest pipeline

196145f

github-actions bot added the language:python Pull requests that update Python code label Jan 12, 2025

DorLevi95 changed the title ~~Create build report about the crypto binaries~~ Create crypto binaries report Jan 12, 2025

inbal2l reviewed Jan 13, 2025

View reviewed changes

Flickdm self-requested a review January 13, 2025 18:10

Flickdm requested changes Jan 16, 2025

View reviewed changes

Flickdm requested review from kenlautner and makubacki January 16, 2025 19:58

DorLevi95 added 5 commits January 19, 2025 12:44

centralize paths initializations

0a6f35a

fix offset alignment

76c8e23

add docs for class and methods

96beeac

undo the use of global variables, pass them as params

6ff13f2

fix typo

f3fa744

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create crypto binaries report #122

Create crypto binaries report #122

DorLevi95 commented Jan 12, 2025

inbal2l Jan 13, 2025 •

edited

Loading

inbal2l Jan 13, 2025 •

edited

Loading

Flickdm Jan 13, 2025

Flickdm Jan 16, 2025

Flickdm left a comment

Flickdm Jan 13, 2025

Flickdm Jan 13, 2025

Flickdm Jan 13, 2025

Flickdm Jan 13, 2025

DorLevi95 Jan 19, 2025 •

edited

Loading

inbal2l Jan 23, 2025

Flickdm Jan 16, 2025

Flickdm Jan 16, 2025

DorLevi95 Jan 19, 2025

Flickdm Jan 16, 2025

Flickdm Jan 16, 2025

Flickdm Jan 16, 2025

Flickdm Jan 16, 2025

DorLevi95 Jan 19, 2025

Flickdm Jan 16, 2025


		def get_module_type_for_crypto_bin(self, thebuilder):

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


		class ReportCrypto(IUefiBuildPlugin):

		def do_post_build(self, thebuilder):

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

Create crypto binaries report #122

Are you sure you want to change the base?

Create crypto binaries report #122

Conversation

DorLevi95 commented Jan 12, 2025

Description

inbal2l Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

inbal2l Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Flickdm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DorLevi95 Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inbal2l Jan 13, 2025 •

edited

Loading

inbal2l Jan 13, 2025 •

edited

Loading

DorLevi95 Jan 19, 2025 •

edited

Loading