+ Bob for general python language level review.
more comments embedded.
> +def BuildUniversalPayload(args, MacroList):
> + build_target = args.Target
> + tool_chain_tag = args.ToolChain
> + elf_tool_chain = 'CLANGDWARF'
> + entry_module_inf = os.path.join("UefiPayloadPkg", "UefiPayloadEntry",
> "UniversalPayloadEntry.inf")
1. Lots of hardcode file/directory names here.
Can we write in below way?
entry_module_inf = "UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf"
entry_module_inf = os.path.normpath(entry_module_inf)
> + fv = os.path.join(os.environ['WORKSPACE'], "Build", "UefiPayloadPkgX64",
> "%s_%s"%(build_target, tool_chain_tag), "FV",
> "DXEFV.Fv")
> + entry = os.path.join(os.environ['WORKSPACE'], "Build",
> "UefiPayloadPkgX64", "%s_%s"%(build_target, elf_tool_chain), "X64",
> "UefiPayloadPkg", "UefiPayloadEntry", "UniversalPayloadEntry", "DEBUG",
> "UniversalPayloadEntry.dll")
2.
build_directory = os.path.join(os.environ['WORKSPACE'], os.path.normpath
("Build/UefiPayloadPkgX64")
fv = os.path.join(build_directory, "%s_%s"%(build_target, tool_chain_tag),
os.path.normpath("FV/DXEFV.fv"))
entry = ......
> + dsc_patch = os.path.join("UefiPayloadPkg", "UefiPayloadPkg.dsc")
3.
Similar change can be applied to dsc_patch (typo? dsc_path?)
> + macro_list = " "
> + for key in MacroList:
> + macro_list +="-D {0}={1} ".format(key, MacroList[key])
4. you can set macro_list = "", then insert space before each "-D" in the loop.
> +
> + #
> + # Building DXE core and DXE drivers as DXEFV.
> + #
> + build_payload = "build -p %s -b %s -a X64 -t %s -y
> UplBuildReport.txt"%(dsc_patch, build_target, tool_chain_tag)
> + build_payload += macro_list
5. you could write in following way
build_payload = f"build -p {dsc_patch} ..."
> + RunCommand(build_payload)
> + #
> + # Building Universal Payload entry.
> + #
> + build_module = "build -p %s -b %s -a X64 -m %s -t %s -y
> UplBuildReportEntry.txt"%(dsc_patch, build_target,
> entry_module_inf, elf_tool_chain)
6. can you please make sure that there is no file generated in the workspace
directory. All files are generated under build directory including the report
file and the ELF file.
> + shutil.copy (entry, os.path.join(os.environ['WORKSPACE'], 'UPL.elf'))
7. do you copy out.
> +
> +def main():
> + print ("Begin to build Universal Payload...")
8. above message is not needed when printing help. maybe just remove the above
message completely.
> + parser = argparse.ArgumentParser(description='For build Universal
> Payload')
> + parser.add_argument('-t', '--ToolChain', help="Using the ToolChain to
> build the UniversalPayload")
> + parser.add_argument('-b', '--Target', help="Using the TARGET to build
> the UniversalPayload")
9. set TARGET to DEBUG as default?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80489): https://edk2.groups.io/g/devel/message/80489
Mute This Topic: https://groups.io/mt/85504626/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
