On 04/09/20 21:57, Sean via groups.io wrote:
> Ard/Laszlo,
>
> On the topic of Package files.
> 1. The *.ci.yaml is designed to be at the root of the package and this
> follows all other Core ci usage. The Core CI will only look for this file at
> the root as of now.
OK. This makes sense to me. Even if we wanted to put the files away
under a subdirectory, we'd have to inform the Core CI system somehow --
and that would likely take the form of having to name the subdirectory
in a particular way. Which is not different from having a fixed name
regular file in the package root dir.
Thankfully, this file does have ".ci." in the name.
> 2. The PlatformBuild.py file is exactly like a build.bat or build.sh file
> found in many platform packages. This file is relevant for local dev as it
> will build the package and should support the feature set of the platform. I
> would hope that the package maintainers would continue to update this file to
> match the features of their package otherwise CI will become less and less
> relevant overtime. Thus I think it belongs in the same place as build.sh.
Thank you, this explanation is very helpful!
ArmVirtPkg does not have a "build.sh" of the kind that you mention --
and I strongly prefer it that way.
OvmfPkg happens to have a "build.sh" -- and if it were up to me, I would
remove it. I have stated this before, perhaps multiple times. I strongly
prefer people invoking "build" manually, or writing their *own* scripts
for wrapping the "build" utility. I never ever use "OvmfPkg/build.sh"
myself -- it doesn't do what I want (and what I want, in this aspect,
should not be in the upstream edk2 tree).
But, again, OVMF's "build.sh" was added in commit 66325870afaf
("OvmfPkg/build.sh: Add features and replace build32/64.sh",
2011-01-09), and by the time I "arrived" and had grown a distaste for
it, other users had become dependent on it. So I can not propose its
removal. I can certainly argue against spreading the practice to ArmVirtPkg.
I *do* agree that "PlatformBuild.py" is necessary for CI, and keeping it
up-to-date is also necessary for good coverage by CI. I'm just
requesting to move it out of the package root dir.
> The only other pattern we could follow is a ".pytools" folder like for Core
> CI. But again unless you strongly disagree I would prefer the first.
The project root already has a .pytool subdir, so (I think?) there would
be a precedent.
But, the leading dot (for "hiding" the subdirectory) is totally not my
point. I don't wish to hide the CI metafiles to *that* extent. I'd just
like to clearly associate them with the CI purpose -- it's fine if they
are visible when the directory is listed with default switches, but the
names should be specific.
> 3. The readme file...You tell me where? Or it could be combined with the
> existing readme.md file?
> 4. The iasl ext_dep file. I could see this moving deeper into the package.
> Where would you like it? This file is an external dependency to control the
> version of iasl used on this platform. It should be kept current with what
> the platform expects for iasl usage.
>
> Anyway, I need more actionable feedback if you don't agree with the above.
How do you feel about the following directory structure:
ArmVirtPkg/ArmVirtPkg.ci.yaml
ArmVirtPkg/PlatformCI/Ubuntu-GCC5.yml
ArmVirtPkg/PlatformCI/PlatformBuild.py
ArmVirtPkg/PlatformCI/README-pytools.md
ArmVirtPkg/PlatformCI/iasl_ext_dep.yaml
(Note that I also removed the ".azurepipelines/" prefix from
"Ubuntu-GCC5.yml", in addition to pushing it under "PlatformCI/". If
".azurepipelines/" is required, then I'm fine with it, as long as we can
also keep it under "ArmVirtPkg/PlatformCI/".)
The result would be that someone running "ls" or "dir" in ArmVirtPkg
would see two CI-related entries: the "ArmVirtPkg.ci.yaml" control file,
and the "PlatformCI" directory. Both entries clearly state "CI" in the
name, and separate CI objects from the rest of the package.
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#57421): https://edk2.groups.io/g/devel/message/57421
Mute This Topic: https://groups.io/mt/72880537/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
