Good day Stefan,
Do you think you could split the first patch into a 1:1 initial import
and have a modifications commit separately?
One of my big issues with EDK II is duplicated code. I look at it, I
don't understand why it is duplicated. I look at the differences, I
don't understand why they are there. I dig deeper into the matter (e.g.
git blame), and I realise there is no reason but someone was taking a
copy+paste route, and future contributors did not know or care about the
respective other piece of code. This series standalone is basically that
out-of-the-box, with no future changes needed. I'll suggest once more to
enforce a code duplication ban, both within edk2, and between edk2 and
edk2-platforms.
It would of course be great if you submitted a follow-up series to drop
the old library from edk2-platforms. If you plan not to, and all
maintainers agree to merge this without such a series being submitted,
please CC me so I know when this is merged and can propose such a patch
set myself.
Best regards,
Marvin
On 13/08/2021 21:02, Stefan Berger wrote:
On 8/13/21 2:47 PM, Sean Brogan wrote:
Thanks for the link as i missed that message.
To me this just points out more problems with how OVMF is being
managed in the edk2 project and the uselessness of edk2 platforms as
anything more than just a dumping ground repo to hold sample code.
But that is a problem larger than this patchset.
I guess if you are going doing option 2 can we rename the library
interface you are defining in OvmfPkg so it doesn't conflict with the
existing one in edk2-platforms/minplatform. That would mean change:
I have now created v5 here with the latest code appearing in
SecurityPkg again:
https://github.com/stefanberger/edk2/commits/stefanberger/ovmf_disable_platform_hierarchy.v5
I can probably post that pretty quickly but I'll be out for a while.
If it's urgent, someone else can pick it it up from there. I tested it
on QEMU for x86 and aarch64 and test-compiled on various platforms
that I touched (some didn't compile for me before the changes).
What I wasn't sure about is whether edk2-platforms is a 'holding area'
for code to be imported ideally 1:1 into edk2. So I ended up making
those changes already in v1 to cut out a dependency. If what I have in
v5 (or also v4) is sufficient for general consumption, then let's put
it into SecurityPkg.
Stefan
* name in OvmfPkg.dec file
* header file in OvmfPkg/Include/Library
* all references in DSC file for mapping an instance
* all references in your INFs for dependency
Thanks
Sean
On 8/12/2021 3:19 PM, Stefan Berger wrote:
On 8/12/21 4:59 PM, Sean Brogan wrote:
This seems like a bad place for a general purpose lib that many
other platforms may take a dependency on.
In v1 this was SecurityPkg. OvmfPkg is a platform package and
therefore not a good place to define broad interfaces.
What caused this to move here?
Option 2 from this message:
https://listman.redhat.com/archives/edk2-devel-archive/2021-August/msg00398.html
Stefan
Thanks
Sean
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79309): https://edk2.groups.io/g/devel/message/79309
Mute This Topic: https://groups.io/mt/84845083/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
