On 4/15/20 7:18 PM, Laszlo Ersek wrote:
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.
Agreed. I never added one because I never felt the need for it. 'build'
has a standard interface across platforms and works in a uniform way.
build.sh are bespoke helpers, and so I don't think they should have been
added in the first place.
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.
Agreed. And if it has a uniform interface across packages, even better.
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
This looks good to me, and could be used for other packages as well, afaict.
--
Ard.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#57423): https://edk2.groups.io/g/devel/message/57423
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]
-=-=-=-=-=-=-=-=-=-=-=-