Re: [PATCH] UefiPayloadPkg: Add script to build UniversalPayload in UefiPayloadPkg

["Ni, Ray" <ray.ni@intel.com>]

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