Thanks Laszlo. Will fix all comments next version.

Thanks
Jianyong

> -----Original Message-----
> From: Laszlo Ersek <ler...@redhat.com>
> Sent: 2023年11月18日 5:54
> To: devel@edk2.groups.io; Jianyong Wu <jianyong...@arm.com>; Sami
> Mujawar <sami.muja...@arm.com>
> Cc: ardb+tianoc...@kernel.org
> Subject: Re: [edk2-devel] [PATCH] CloudHv: Add CI for CloudHv on AArch64
>
> On 11/15/23 07:20, Jianyong Wu wrote:
> > Add the long lost CI for CloudHv on AArch64.
> >
> > Signed-off-by: Jianyong Wu <jianyong...@arm.com>
> > ---
> >  .../.azurepipelines/Ubuntu-GCC5.yml           | 13 ++++++++
> >  ArmVirtPkg/PlatformCI/CloudHvBuild.py         | 32
> +++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 ArmVirtPkg/PlatformCI/CloudHvBuild.py
>
> Please format and document patches such that they are easier to review.
> Otherwise reviewers have to dig too much.
>
> >
> > diff --git a/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > b/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > index d1772a65fc..ab8a2db530 100644
> > --- a/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > +++ b/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > @@ -140,6 +140,19 @@ jobs:
> >              Build.Target: "RELEASE"
> >              Run: false
> >
> > +          CLOUDHV_AARCH64_DEBUG:
> > +            Build.File: "$(package)/PlatformCI/CloudHvBuild.py"
> > +            Build.Arch: "AARCH64"
> > +            Build.Flags: ""
> > +            Build.Target: "DEBUG"
> > +            Run: false
> > +          CLOUDHV_AARCH64_RELEASE:
> > +            Build.File: "$(package)/PlatformCI/CloudHvBuild.py"
> > +            Build.Arch: "AARCH64"
> > +            Build.Flags: ""
> > +            Build.Target: "RELEASE"
> > +            Run: false
> > +
> >      workspace:
> >        clean: all
> >
>
> This seems like a copy of the KVMTOOL stanzas, except with (a) ARM removed,
> (b) KVMTOOL replaced by CLOUDHV, (c) KvmToolBuild.py replaced with
> CloudHvBuild.py.
>
> OK.
>
> > diff --git a/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > b/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > new file mode 100644
> > index 0000000000..0192cd6577
> > --- /dev/null
> > +++ b/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > @@ -0,0 +1,32 @@
> > +# @file
> > +# Script to Build ArmVirtPkg UEFI firmware # # Copyright (c)
> > +Microsoft Corporation.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent ## import os import
> > +sys
> > +
> > +sys.path.append(os.path.dirname(os.path.abspath(__file__)))
> > +from PlatformBuildLib import SettingsManager from PlatformBuildLib
> > +import PlatformBuilder
> > +
> > +    #
> ####################################################################
> ################### #
> > +    #                                Common Configuration
> #
> > +    #
> >
> +###################################################################
> #################### # class CommonPlatform():
> > +    ''' Common settings for this platform.  Define static data here and use
> > +        for the different parts of stuart
> > +    '''
> > +    PackagesSupported = ("ArmVirtPkg",)
> > +    ArchSupported = ("AARCH64", "ARM")
> > +    TargetsSupported = ("DEBUG", "RELEASE")
> > +    Scopes = ('armvirt', 'edk2-build')
> > +    WorkspaceRoot = os.path.realpath(os.path.join(
> > +        os.path.dirname(os.path.abspath(__file__)), "..", ".."))
> > +
> > +    DscName = os.path.join("ArmVirtPkg", "ArmVirtCloudHv.dsc")
> > +    FvQemuArg = "" # ignored
> > +
> > +import PlatformBuildLib
> > +PlatformBuildLib.CommonPlatform = CommonPlatform
>
> According to
>
>   git show --find-copies-harder
>
> this is a nearly identical copy of
> "ArmVirtPkg/PlatformCI/KvmToolBuild.py", the only difference is:
>
> -    DscName = os.path.join("ArmVirtPkg", "ArmVirtKvmTool.dsc")
> +    DscName = os.path.join("ArmVirtPkg", "ArmVirtCloudHv.dsc")
>
> It makes sense to me, but (a) this could have been documented in the commit
> message; (b) the patch could have been formatted with --find-copies-harder
> (and, indeed that option does not interfere with patch application, as long as
> the --base=master option is also given to git-format-patch -- then we know
> exactly where to check out a local branch for applying the patch, and to 
> rebase
> from.)
>
> Anyway:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111459): https://edk2.groups.io/g/devel/message/111459
Mute This Topic: https://groups.io/mt/102600602/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to