* [edk2-devel] Commits Spanning Packages @ 2024-01-31 18:58 Michael Kubacki 2024-01-31 19:33 ` Michael D Kinney 2024-01-31 20:41 ` Pedro Falcato 0 siblings, 2 replies; 4+ messages in thread From: Michael Kubacki @ 2024-01-31 18:58 UTC (permalink / raw) To: devel@edk2.groups.io Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Pierre Gondois, Ard Biesheuvel, Abner Chang, Min Xu, Zhiguang Liu, Tuan Phan, Yu Pu, Ray Ni Some commits have been spanning packages. For a monolithic upstream repo like edk2, this makes ingesting commits downstream more tedious if project requirements do not necessitate all packages to be maintained. Below are examples of commits spanning packages. Many of these are more problematic because "core" packages like MdePkg, MdeModule, SecurityPkg, UefiCpuPkg, etc. are involved. OvmfPkg and ArmVirtPkg seem to be a common pairing. To focus the examples below, I excluded commits with that pair. The request is to limit crossing package boundaries as much as possible and for maintainers to assess package impact when merging changes. Perhaps something like PatchCheck could be updated to help identify this. Here's some examples: - **ArmPkg:ArmPlatformPkg** - Commit: https://github.com/tianocore/edk2/commit/103fa647d159e3d76be2634d2653c2d215dd0d46 - **ArmVirtPkg:ArmPkg** - Commit: https://github.com/tianocore/edk2/commit/34969dd260b4ecaba8d4a58d32fc07bd0fee57f9 - Commit: https://github.com/tianocore/edk2/commit/7d78a86ecfc6d86244a552b1bad6fb346439116b - **ArmVirtPkg:OvmfPkg** - Commit: https://github.com/tianocore/edk2/commit/f8d0501ded2ad426cc5c6c1237ec83fc978f8373 - Commit: https://github.com/tianocore/edk2/commit/d881c6ddf556dfb5032db04733dfa86aa891e7f8 - Commit: https://github.com/tianocore/edk2/commit/77e9b3a7c68cea4eeb64af85e4f695f278172314 - **ArmVirtPkg:ArmPkg:OvmfPkg** - Commit: https://github.com/tianocore/edk2/commit/9a7509e465d648314824733200566190c8814ec6 - **ArmVirtPkg:EmbeddedPkg** - Commit: https://github.com/tianocore/edk2/commit/e40fefafa90fb3a9aca77adc697c97cf6a4dd673 - **BaseTools:ArmPkg** - Commit: https://github.com/tianocore/edk2/commit/f484427d10a5ff6c2437c2f7c671e9e552ad6766 - **DynamicTablesPkg:MdePkg** - Commit: https://github.com/tianocore/edk2/commit/4c55f6394fafe0494ec24e7c05cb68c938d7852d - **IntelFsp2WrapperPkg:IntelFsp2Pkg** - Commit: https://github.com/tianocore/edk2/commit/fa78edc57eb000f7820d5896f61053069b61eba1 - **MdeModulePkg:MdePkg** - Commit: https://github.com/tianocore/edk2/commit/5443c2dc310d2c8eb15fb8eefd5057342e78cd0d - Commit: https://github.com/tianocore/edk2/commit/53eb26b238541799134aa5846530485c915735da - Commit: https://github.com/tianocore/edk2/commit/502c01c5028038e4e6b4512e9c66be0ec4d11492 - **MdeModulePkg:OvmfPkg** - Commit: https://github.com/tianocore/edk2/commit/fd306d1dbc36d10f744306e92366d73273c82c8f - **MdeModulePkg:UefiCpuPkg** - Commit: https://github.com/tianocore/edk2/commit/e7abb94d1fb8a0e7725b983bbf5ab1334afe7ed1 - Commit: https://github.com/tianocore/edk2/commit/2a09527ebcb459b40bc3661d85aa11c3905526dc - **MdeModulePkg:MdePkg:SecurityPkg** - Commit: https://github.com/tianocore/edk2/commit/65b5dd828ef2ea5056031b239a4e7a6642f771a3 - **MdeModulePkg:UefiCpuPkg:ArmPkg** - Commit: https://github.com/tianocore/edk2/commit/0f7bccf584d93b2642c0a413a47fc821d1f5dbfd - **MdeModulePkg:ArmVirtPkg:SignedCapsulePkg:EmulatorPkg:OvmfPkg:NetworkPkg:SecurityPkg** - Commit: https://github.com/tianocore/edk2/commit/2f981bddcbd6adde5f682caf0d3812ba92bc0f73 - **MdePkg:NetworkPkg** - Commit: https://github.com/tianocore/edk2/commit/671b0cea510ad6de02ee9d6dbdf8f9bbb881f35d - **MdePkg:UefiCpuPkg** - Commit: https://github.com/tianocore/edk2/commit/8079d4dc4f8ac23e02b02996e326cac4099b3e49 - Commit: https://github.com/tianocore/edk2/commit/ea55bd8f66eeca5f4e80c3679bcf1b1007286b8a - **MdePkg:UefiPayloadPkg** - Commit: https://github.com/tianocore/edk2/commit/39f3c26e8c40e092baeb0ec4d0396498506e0a9e - **OvmfPkg:BaseTools** - Commit: https://github.com/tianocore/edk2/commit/f0b97e165e0af79ac9eb3c6ac7697f9183afc7cb - Commit: https://github.com/tianocore/edk2/commit/ff36b2550f94dc5fac838cf298ae5a23cfddf204 - **OvmfPkg:UefiCpuPkg** - Commit: https://github.com/tianocore/edk2/commit/f220dcbba86bfc1222180c61bbd31dd6023433db - Commit: https://github.com/tianocore/edk2/commit/765ba5bf050022de8c44d93e467639d7f5fa237c - **OvmfPkg:UefiCpuPkg::UefiPayloadPkg** - Commit: https://github.com/tianocore/edk2/commit/a89f558d3c56d9c16e5f5b7d395c1aa36ccd38f2 - **RedfishPkg:MdePkg** - Commit: https://github.com/tianocore/edk2/commit/7275993dc64481b1c21d7d70af434bfaafe86e81 Thanks, Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114915): https://edk2.groups.io/g/devel/message/114915 Mute This Topic: https://groups.io/mt/104081468/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] Commits Spanning Packages 2024-01-31 18:58 [edk2-devel] Commits Spanning Packages Michael Kubacki @ 2024-01-31 19:33 ` Michael D Kinney 2024-01-31 20:41 ` Pedro Falcato 1 sibling, 0 replies; 4+ messages in thread From: Michael D Kinney @ 2024-01-31 19:33 UTC (permalink / raw) To: Michael Kubacki, devel@edk2.groups.io Cc: Andrew Fish, Leif Lindholm, Pierre Gondois, Ard Biesheuvel, Abner Chang, Xu, Min M, Liu, Zhiguang, Tuan Phan, Yu Pu, Ni, Ray, Kinney, Michael D Hi Michael, I think a CI check for this is the best way to prevent patches with changes to 2 or more packages. * Use git command to retrieve list of files modified by a patch. * Look through parent directories and collect all *.dec files * If number of *.dec files > 1 then FAIL * If number of *.dec files is 0 or 1, then PASS I have also noticed that some of the Author names in patches are devel@edk2.groups.io. PatchCheck should be updated to look for that Author email address and FAIL. Best regards, Mike > -----Original Message----- > From: Michael Kubacki <mikuback@linux.microsoft.com> > Sent: Wednesday, January 31, 2024 10:58 AM > To: devel@edk2.groups.io > Cc: Andrew Fish <afish@apple.com>; Leif Lindholm > <quic_llindhol@quicinc.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; Pierre Gondois <pierre.gondois@arm.com>; > Ard Biesheuvel <ardb+tianocore@kernel.org>; Abner Chang > <abner.chang@amd.com>; Xu, Min M <min.m.xu@intel.com>; Liu, Zhiguang > <zhiguang.liu@intel.com>; Tuan Phan <tphan@ventanamicro.com>; Yu Pu > <yu.pu@intel.com>; Ni, Ray <ray.ni@intel.com> > Subject: Commits Spanning Packages > > Some commits have been spanning packages. For a monolithic upstream repo > like edk2, this makes ingesting commits downstream more tedious if > project requirements do not necessitate all packages to be maintained. > > Below are examples of commits spanning packages. Many of these are more > problematic because "core" packages like MdePkg, MdeModule, SecurityPkg, > UefiCpuPkg, etc. are involved. OvmfPkg and ArmVirtPkg seem to be a > common pairing. To focus the examples below, I excluded commits with > that pair. > > The request is to limit crossing package boundaries as much as possible > and for maintainers to assess package impact when merging changes. > > Perhaps something like PatchCheck could be updated to help identify > this. > > Here's some examples: > > - **ArmPkg:ArmPlatformPkg** > - Commit: > https://github.com/tianocore/edk2/commit/103fa647d159e3d76be2634d2653c2d > 215dd0d46 > - **ArmVirtPkg:ArmPkg** > - Commit: > https://github.com/tianocore/edk2/commit/34969dd260b4ecaba8d4a58d32fc07b > d0fee57f9 > - Commit: > https://github.com/tianocore/edk2/commit/7d78a86ecfc6d86244a552b1bad6fb3 > 46439116b > - **ArmVirtPkg:OvmfPkg** > - Commit: > https://github.com/tianocore/edk2/commit/f8d0501ded2ad426cc5c6c1237ec83f > c978f8373 > - Commit: > https://github.com/tianocore/edk2/commit/d881c6ddf556dfb5032db04733dfa86 > aa891e7f8 > - Commit: > https://github.com/tianocore/edk2/commit/77e9b3a7c68cea4eeb64af85e4f695f > 278172314 > - **ArmVirtPkg:ArmPkg:OvmfPkg** > - Commit: > https://github.com/tianocore/edk2/commit/9a7509e465d64831482473320056619 > 0c8814ec6 > - **ArmVirtPkg:EmbeddedPkg** > - Commit: > https://github.com/tianocore/edk2/commit/e40fefafa90fb3a9aca77adc697c97c > f6a4dd673 > - **BaseTools:ArmPkg** > - Commit: > https://github.com/tianocore/edk2/commit/f484427d10a5ff6c2437c2f7c671e9e > 552ad6766 > - **DynamicTablesPkg:MdePkg** > - Commit: > https://github.com/tianocore/edk2/commit/4c55f6394fafe0494ec24e7c05cb68c > 938d7852d > - **IntelFsp2WrapperPkg:IntelFsp2Pkg** > - Commit: > https://github.com/tianocore/edk2/commit/fa78edc57eb000f7820d5896f610530 > 69b61eba1 > - **MdeModulePkg:MdePkg** > - Commit: > https://github.com/tianocore/edk2/commit/5443c2dc310d2c8eb15fb8eefd50573 > 42e78cd0d > - Commit: > https://github.com/tianocore/edk2/commit/53eb26b238541799134aa5846530485 > c915735da > - Commit: > https://github.com/tianocore/edk2/commit/502c01c5028038e4e6b4512e9c66be0 > ec4d11492 > - **MdeModulePkg:OvmfPkg** > - Commit: > https://github.com/tianocore/edk2/commit/fd306d1dbc36d10f744306e92366d73 > 273c82c8f > - **MdeModulePkg:UefiCpuPkg** > - Commit: > https://github.com/tianocore/edk2/commit/e7abb94d1fb8a0e7725b983bbf5ab13 > 34afe7ed1 > - Commit: > https://github.com/tianocore/edk2/commit/2a09527ebcb459b40bc3661d85aa11c > 3905526dc > - **MdeModulePkg:MdePkg:SecurityPkg** > - Commit: > https://github.com/tianocore/edk2/commit/65b5dd828ef2ea5056031b239a4e7a6 > 642f771a3 > - **MdeModulePkg:UefiCpuPkg:ArmPkg** > - Commit: > https://github.com/tianocore/edk2/commit/0f7bccf584d93b2642c0a413a47fc82 > 1d1f5dbfd > - > **MdeModulePkg:ArmVirtPkg:SignedCapsulePkg:EmulatorPkg:OvmfPkg:NetworkPk > g:SecurityPkg** > - Commit: > https://github.com/tianocore/edk2/commit/2f981bddcbd6adde5f682caf0d3812b > a92bc0f73 > - **MdePkg:NetworkPkg** > - Commit: > https://github.com/tianocore/edk2/commit/671b0cea510ad6de02ee9d6dbdf8f9b > bb881f35d > - **MdePkg:UefiCpuPkg** > - Commit: > https://github.com/tianocore/edk2/commit/8079d4dc4f8ac23e02b02996e326cac > 4099b3e49 > - Commit: > https://github.com/tianocore/edk2/commit/ea55bd8f66eeca5f4e80c3679bcf1b1 > 007286b8a > - **MdePkg:UefiPayloadPkg** > - Commit: > https://github.com/tianocore/edk2/commit/39f3c26e8c40e092baeb0ec4d039649 > 8506e0a9e > - **OvmfPkg:BaseTools** > - Commit: > https://github.com/tianocore/edk2/commit/f0b97e165e0af79ac9eb3c6ac7697f9 > 183afc7cb > - Commit: > https://github.com/tianocore/edk2/commit/ff36b2550f94dc5fac838cf298ae5a2 > 3cfddf204 > - **OvmfPkg:UefiCpuPkg** > - Commit: > https://github.com/tianocore/edk2/commit/f220dcbba86bfc1222180c61bbd31dd > 6023433db > - Commit: > https://github.com/tianocore/edk2/commit/765ba5bf050022de8c44d93e467639d > 7f5fa237c > - **OvmfPkg:UefiCpuPkg::UefiPayloadPkg** > - Commit: > https://github.com/tianocore/edk2/commit/a89f558d3c56d9c16e5f5b7d395c1aa > 36ccd38f2 > - **RedfishPkg:MdePkg** > - Commit: > https://github.com/tianocore/edk2/commit/7275993dc64481b1c21d7d70af434bf > aafe86e81 > > Thanks, > Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114918): https://edk2.groups.io/g/devel/message/114918 Mute This Topic: https://groups.io/mt/104081468/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] Commits Spanning Packages 2024-01-31 18:58 [edk2-devel] Commits Spanning Packages Michael Kubacki 2024-01-31 19:33 ` Michael D Kinney @ 2024-01-31 20:41 ` Pedro Falcato 2024-01-31 21:03 ` Michael Kubacki 1 sibling, 1 reply; 4+ messages in thread From: Pedro Falcato @ 2024-01-31 20:41 UTC (permalink / raw) To: devel, mikuback Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Pierre Gondois, Ard Biesheuvel, Abner Chang, Min Xu, Zhiguang Liu, Tuan Phan, Yu Pu, Ray Ni On Wed, Jan 31, 2024 at 6:58 PM Michael Kubacki <mikuback@linux.microsoft.com> wrote: > > Some commits have been spanning packages. For a monolithic upstream repo > like edk2, this makes ingesting commits downstream more tedious if > project requirements do not necessitate all packages to be maintained. > > Below are examples of commits spanning packages. Many of these are more > problematic because "core" packages like MdePkg, MdeModule, SecurityPkg, > UefiCpuPkg, etc. are involved. OvmfPkg and ArmVirtPkg seem to be a > common pairing. To focus the examples below, I excluded commits with > that pair. > > The request is to limit crossing package boundaries as much as possible > and for maintainers to assess package impact when merging changes. Personally I'm all for making downstreams' lives easier, but I don't see how one could pull off many of the changes you linked, in 2+ commits, without breaking bisection. Pulling off 103fa647d159e3d76be2634d2653c2d215dd0d46 would require you to duplicate the struct, change a bunch of usages of the old struct to the new struct, then change the old one back. This is hugely annoying and complicated, and we should avoid this kind of rigamarole if possible. -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114920): https://edk2.groups.io/g/devel/message/114920 Mute This Topic: https://groups.io/mt/104081468/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] Commits Spanning Packages 2024-01-31 20:41 ` Pedro Falcato @ 2024-01-31 21:03 ` Michael Kubacki 0 siblings, 0 replies; 4+ messages in thread From: Michael Kubacki @ 2024-01-31 21:03 UTC (permalink / raw) To: devel, pedro.falcato Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Pierre Gondois, Ard Biesheuvel, Abner Chang, Min Xu, Zhiguang Liu, Tuan Phan, Yu Pu, Ray Ni On 1/31/2024 3:41 PM, Pedro Falcato wrote: > On Wed, Jan 31, 2024 at 6:58 PM Michael Kubacki > <mikuback@linux.microsoft.com> wrote: >> >> Some commits have been spanning packages. For a monolithic upstream repo >> like edk2, this makes ingesting commits downstream more tedious if >> project requirements do not necessitate all packages to be maintained. >> >> Below are examples of commits spanning packages. Many of these are more >> problematic because "core" packages like MdePkg, MdeModule, SecurityPkg, >> UefiCpuPkg, etc. are involved. OvmfPkg and ArmVirtPkg seem to be a >> common pairing. To focus the examples below, I excluded commits with >> that pair. >> >> The request is to limit crossing package boundaries as much as possible >> and for maintainers to assess package impact when merging changes. > > Personally I'm all for making downstreams' lives easier, but I don't > see how one could pull off many of the changes you linked, in 2+ > commits, without breaking bisection. > > Pulling off 103fa647d159e3d76be2634d2653c2d215dd0d46 would require you > to duplicate the struct, change a bunch of usages of the old struct to > the new struct, then change the old one back. This is hugely annoying > and complicated, and we should avoid this kind of rigamarole if > possible. > That is why the request is: "... to limit crossing package boundaries as much as possible and for maintainers to assess package impact when merging changes." Not an absolute rule. However, the rules currently seem ambiguous and susceptible to convenience. To provide a counter example of a single change that could easily be split up and couples MdePkg (highly used, mandatory, fundamental) with RedfishPkg (much less frequently used, specialized) - https://github.com/tianocore/edk2/commit/7275993dc64481b1c21d7d70af434bf. There are also dependency relationships between packages. The example you cited is between ArmPkg and ArmPlatformPkg. Which has better cohesion than something like the example I provided between MdePkg and RedfishPkg. Or, all the packages involved in https://github.com/tianocore/edk2/commit/2f981bddcbd6adde5f682caf0d3812b (which could also easily been split up). Those should have been split up and could have with minimal effort. Not looking for anything more than to request devs understand impact, package relationships/scope, and consider tooling to make this more consistent and raise awareness when it relatively rarely comes up. - Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114921): https://edk2.groups.io/g/devel/message/114921 Mute This Topic: https://groups.io/mt/104081468/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-31 21:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-31 18:58 [edk2-devel] Commits Spanning Packages Michael Kubacki 2024-01-31 19:33 ` Michael D Kinney 2024-01-31 20:41 ` Pedro Falcato 2024-01-31 21:03 ` Michael Kubacki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox