[edk2-devel] RFC for Edk2-Library
Laszlo Ersek
lersek at redhat.com
Mon May 13 20:28:46 UTC 2019
On 05/13/19 20:20, Kinney, Michael D wrote:
> Laszlo,
>
> On Windows build systems, we have to install OpenSSL command line
> utilities. For all host systems, the OpenSSL command line
> utilities need to be in the system path. My point is that this
> is similar to other dependencies like iASL and NASM.
OK. I think I must have misunderstood you at some point. Sorry about that.
> For the patch discussion, I did not mean to confuse things. From
> one perspective, there are two types of patch submissions. Single
> commit (no series) and multiple commits (patch series). The squash
> merge of a PR works just fine for the single commit (no series) type.
> There may be feedback/comments that require code changes and once the
> PR is accepted, the history in the repository shows a single commit.
> As the PR evolves, commits are made on the PR to address each piece
> of feedback. Squashing all of these together at the time the PR is
> accepted is the correct action and matches what we do the for email
> based review process. The final result for both PR and email is a
> single commit with a cleaned up commit message.
OK.
> You may consider the single commit (no series) type more rare than the
> multiple commit (patch series) type. However, there may be cases where
> a multiple commit (patch series) was used where the changes could have
> been submitted as a set of single commit (no series) changes.
OK.
Thanks
Laszlo
>
> Best regards,
>
> Mike
>
>> -----Original Message-----
>> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io]
>> On Behalf Of Laszlo Ersek
>> Sent: Monday, May 13, 2019 3:46 AM
>> To: Kinney, Michael D <michael.d.kinney at intel.com>;
>> devel at edk2.groups.io; sean.brogan at microsoft.com
>> Subject: Re: [edk2-devel] RFC for Edk2-Library
>>
>> On 05/10/19 02:01, Kinney, Michael D wrote:
>>> Laszlo,
>>>
>>> 1) We also use OpenSSL command line tool to locally
>> sign
>>> capsules and recovery images for local testing. So
>> both
>>> tool dependency and source dependency apply to the
>> OpenSSL
>>> content.
>>
>> I haven't used the tools yet that you refer to, so I'm
>> unsure how
>> exactly they invoke the "openssl" utility. If they just
>> rely on the PATH
>> environment variable, then what they invoke comes from
>> the system-wide
>> openssl package.
>>
>>>
>>> 2) If a dev submits a PR, and there are many review
>> comments
>>> that require code changes, then those code changes
>> are
>>> added to that PR until all feedback is addressed.
>>
>> I don't understand how. Let's say the pull request refers
>> to a branch
>> with three commits, and the majority of the review
>> comments request
>> updates for patch #2 (i.e., in the middle). How can the
>> submitter "add
>> changes to the PR"? It is patch #2 that needs to be
>> reworked, which will
>> require rebases, and so the hash of the HEAD commit of
>> the topic branch
>> (patch #3) will change as well.
>>
>>> At that
>>> point, if the change is something that should be in
>> a single
>>> commit, then doing a squash merge with a cleaned up
>> commit
>>> message would be appropriate.
>>
>> I don't understand -- we modify only patch #2, yes, to
>> address review
>> comments, but why does that justify squashing #1 through
>> #3 into a
>> single commit?
>>
>>> And the history of all the
>>> review feedback preserved in the PR.
>>
>> That's good (but it could be better -- see the lacking
>> email
>> integration. Anyway this is not strictly tied to my
>> concern with
>> squash-on-merge).
>>
>>>
>>> If we create a 2nd PR with the cleaned up content,
>> then the
>>> connection to the 1st PR feedback may be lost. I
>> agree this
>>> matches what we do in email reviews to create a V2
>> patch series.
>>> The V1 and V2 threads are in the email archive.
>>
>> Indeed. And the blurb for both threads reference the
>> bugzilla, and the
>> bugzilla has (if the submitter is careful) comments
>> linking the v1 and
>> v2 threads from the email archive.
>>
>>> I wonder if
>>> there is a way to link a 2nd PR to the 1st PR and
>> guarantee
>>> that both PRs are preserved? This would also allow
>> the 2nd
>>> cleaned up PR to contain a series, and if there was
>> a way to
>>> make the squash merge optional, then the developer
>> can choose
>>> the way the patches are committed.
>>
>> This sounds great! I think both PRs can reference the
>> bugzilla ticket,
>> and the bugzilla ticket can reference both PRs too (as
>> comments). The
>> first PR can be rejected & closed when the second one is
>> submitted.
>>
>>> Just a few ideas to explore...Perhaps Sean can
>> provide some
>>> additional ideas to manage complex changes in PRs.
>>
>> Thanks!
>> Laszlo
>>
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek at redhat.com]
>>>> Sent: Thursday, May 9, 2019 3:56 PM
>>>> To: Kinney, Michael D <michael.d.kinney at intel.com>;
>>>> devel at edk2.groups.io; sean.brogan at microsoft.com
>>>> Subject: Re: [edk2-devel] RFC for Edk2-Library
>>>>
>>>> On 05/09/19 20:06, Kinney, Michael D wrote:
>>>>> Hello,
>>>>>
>>>>> It is difficult to tell if the repo name edk2-library
>>>> is for firmware
>>>>> or tools, so I recommend we work on a name that
>> clearly
>>>> identifies
>>>>> that this repo is related to tools.
>>>>
>>>> Good idea.
>>>>
>>>>> For the pip dependencies, is the concern that a
>>>> platform that depends
>>>>> on these tools will not be buildable without running
>> a
>>>> "pip install"
>>>>> command that pulls content from the network?
>>>>
>>>> Almost. Not exactly.
>>>>
>>>> First, the concern is not specific to any given
>> platform.
>>>> My
>>>> understanding is that the extraction would target, in
>> the
>>>> longer term,
>>>> general BaseTools functionality. That would impact all
>>>> platforms that
>>>> consume & build against edk2.
>>>>
>>>> Second, it's not the network access that's concerning
>> per
>>>> se. It's the
>>>> compatibility / interoperation with any given Linux
>>>> distribution's own
>>>> (native) package management system. Such a package
>>>> management system is
>>>> generally not used on Windows, therefore Windows users
>>>> are accustomed to
>>>> installing software packages from a boatload of
>> vendors.
>>>> This is not the
>>>> case on Linux distros -- your distro vendor offers a
>>>> curated set of
>>>> packages, such that everything interoperates with
>>>> everything (ideally),
>>>> there are no file conflicts, version dependencies are
>>>> handled
>>>> automatically (e.g. if you install a high-level
>> package,
>>>> its
>>>> dependencies are pulled in automatically), and so on.
>>>>
>>>> Clearly the distro vendor does not *author* all this
>>>> software (although
>>>> they are strongly encouraged to contribute to the
>>>> upstream software
>>>> projects they package and ship). The distro vendor is
>>>> responsible for
>>>> the integration of all these packages into a
>> consistent
>>>> OS, into
>>>> consistent feature sets, and so on.
>>>>
>>>> "pip" and similar tools are generally unfit for this
>>>> approach, because
>>>> they implement a parallel package management system,
>> to
>>>> my
>>>> understanding. If they are carefully used in a user's
>>>> home directory,
>>>> they might work. If packages were installed with "pip"
>>>> system-wide, they
>>>> would almost certainly break (conflict with) the
>> distro-
>>>> provided
>>>> packages.
>>>>
>>>> Therefore, when users would like to get a new piece of
>>>> software
>>>> packaged, or an existent package refreshed (to a more
>>>> recent upstream
>>>> version), they file a feature requst with their distro
>>>> vendor (unless
>>>> the distro vendor already tracks the upstream project
>>>> closely and
>>>> performs periodic rebases anyway). Then the distro
>> vendor
>>>> ships a new
>>>> (or refreshed) package, again nicely integrated with
>> the
>>>> rest of the
>>>> system.
>>>>
>>>>> We already have to pull content to get the sources
>> and
>>>> potentially
>>>>> other dependent tools (NASM, iASL, openssl).
>>>>
>>>> Yes, these are good examples to demonstrate the
>>>> difference. Consider
>>>> NASM. If a Windows user would like to install NASM,
>> they
>>>> google "NASM
>>>> for Windows", then go to:
>>>>
>>>>
>> https://www.nasm.us/pub/nasm/releasebuilds/2.14.02/win64/
>>>>
>>>> download the ZIP file or EXE file, and install it.
>>>>
>>>> In comparison, a Fedora user runs
>>>>
>>>> dnf install nasm
>>>>
>>>> and they get the package from the Fedora package
>>>> repository. A Debian
>>>> user would run
>>>>
>>>> apt-get install nasm
>>>>
>>>> and they would get the package (which would be
>> entirely
>>>> independent of
>>>> Fedora's) from the Debian package repository.
>>>>
>>>> The various *BSDs would run a variant of
>>>>
>>>> pkg_add nasm
>>>>
>>>> I believe.
>>>>
>>>> The same applies to iASL.
>>>>
>>>>
>>>> OpenSSL is a special case. It is different from NASM
>> and
>>>> iASL. NASM and
>>>> iASL are native (hosted) applications, so any Linux
>>>> distro can easily
>>>> build them (for itself), and the binaries can be
>> shipped
>>>> to users, ready
>>>> to run. But for the purposes of edk2, OpenSSL is not
>>>> needed as a native
>>>> (hosted) utility, like NASM and iASL -- it is needed
>> as a
>>>> *cross-built*
>>>> static library, targeting the firmware architecture
>> (not
>>>> the host
>>>> architecture). Unfortunately, OpenSSL (the upstream
>>>> project) does not
>>>> fully support this use case yet, therefore Linux
>> distros
>>>> cannot just
>>>> cross-compile OpenSSL for edk2, and offer static
>> library
>>>> packages. This
>>>> is why upstream edk2 consumes OpenSSL through a git
>>>> submodule, in source
>>>> form.
>>>>
>>>> *However*: conceptually, there is no difference. In
>>>> Fedora and RHEL, we
>>>> don't build OVMF against upstream OpenSSL. We build it
>>>> against the
>>>> downstream OpenSSL source code. So ultimately there is
>> no
>>>> difference in
>>>> the distro vendor's approach -- the sources and the
>>>> binaries come from
>>>> the distro vendor.
>>>>
>>>>
>>>> So what follows from this? It means that, for using
>>>> BaseTools on a Linux
>>>> distro, users will have to install some extra Python
>>>> packages, in
>>>> addition to NASM and iASL, from their vendor's
>>>> repository. In other
>>>> words, the "edk2-library" (or "edk2-tools") project
>>>> should do upstream
>>>> releases, so that Linux distros can package them in
>> their
>>>> native package
>>>> systems. And some collaboration between upstream and
>>>> downstreams is
>>>> expected for this. (Exactly as it occurs with the iASL
>>>> and NASM upstream
>>>> projects.)
>>>>
>>>>> Can we limit the initial scope to tools that layer on
>>>> top of edk2, and
>>>>> a different future RFC would be required if there is
>> a
>>>> proposal for
>>>>> the edk2 repo to depend on another repo?
>>>>
>>>> Yes, that would be very nice.
>>>>
>>>>> If we accept this limited scope, are there still
>>>> concerns about
>>>>> initially using squash commits for changes to these
>>>> tools.
>>>>
>>>> Yes, I still have the same concern -- although it's a
>>>> milder concern,
>>>> dependent on how long squash merges are tolerated.
>>>>
>>>> Assume that a second, separate RFC gets posted down
>> the
>>>> road, let's say
>>>> a year in, which proposes to replace the in-line
>>>> component FooBar of
>>>> BaseTools, with the external project "edk2-library"
>> (or
>>>> "edk2-tools").
>>>> At that point, the git history accrued thus far, in
>> edk2-
>>>> tools, would
>>>> suddenly be inherited by all BaseTools users. And that
>>>> history wouldn't
>>>> be really helpful.
>>>>
>>>> For an example, we need not look past the edk2
>> repository
>>>> itself. Let's
>>>> say I'd like to learn about the implementation history
>> of
>>>> the
>>>> HandleProtocol() boot service, in edk2. So I run:
>>>>
>>>> $ git blame master --
>> MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>>
>>>> and I look up the CoreHandleProtocol() function, and
>> find
>>>> this:
>>>>
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 931)
>>>> EFI_STATUS
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 932)
>> EFIAPI
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 933)
>>>> CoreHandleProtocol (
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 934) IN
>>>> EFI_HANDLE UserHandle,
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 935) IN
>>>> EFI_GUID *Protocol,
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 936)
>> OUT
>>>> VOID **Interface
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 937) )
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 938) {
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 939)
>>>> return CoreOpenProtocol (
>>>>> 022c6d45ef786 MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>> (qhuang8 2008-07-24 02:54:45 +0000 940)
>>>> UserHandle,
>>>>> 022c6d45ef786 MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>> (qhuang8 2008-07-24 02:54:45 +0000 941)
>>>> Protocol,
>>>>> 022c6d45ef786 MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>> (qhuang8 2008-07-24 02:54:45 +0000 942)
>>>> Interface,
>>>>> 022c6d45ef786 MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>> (qhuang8 2008-07-24 02:54:45 +0000 943)
>>>> gDxeCoreImageHandle,
>>>>> 022c6d45ef786 MdeModulePkg/Core/Dxe/Hand/Handle.c
>>>> (qhuang8 2008-07-24 02:54:45 +0000 944)
>>>> NULL,
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 945)
>>>> EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 946)
>>>> );
>>>>> 28a00297189c3 MdeModulePkg/Core/Dxe/Hand/handle.c
>>>> (yshang1 2007-07-04 10:51:54 +0000 947) }
>>>>
>>>> Interesting, it seems like the function was originally
>>>> added in commit
>>>> 28a00297189c3, and then the argument list was modified
>> in
>>>> commit
>>>> 022c6d45ef786, a year later. So let's check out the
>>>> rationale for that
>>>> argument list update:
>>>>
>>>> $ git show --color 022c6d45ef786
>>>>
>>>> Well... The commit message says, "Code Scrub for Dxe
>>>> Core". That's all.
>>>> We also learn it comes from SVN revision 5560. And the
>>>> diffstat for the
>>>> patch is:
>>>>
>>>>> 38 files changed, 2499 insertions(+), 2504
>> deletions(-
>>>> )
>>>>
>>>> It is not overly helpful.
>>>>
>>>> Clearly, CoreHandleProtocol() is a super-robust,
>> super-
>>>> mature function,
>>>> so we don't really care for its development history
>>>> today.
>>>>
>>>> I believe the same would not apply to the "edk2-
>> library"
>>>> (or
>>>> "edk2-tools") project. When it replaced the in-line
>>>> component FooBar of
>>>> BaseTools, it's plausible it would be suddenly exposed
>> to
>>>> a larger
>>>> userbase. New issues would be encountered, and people
>>>> might want to look
>>>> at the git history -- if for nothing else, then
>> rationale
>>>> (commit
>>>> messages) on small parts of the code.
>>>>
>>>>
>>>> I can also name more recent examples. I'm not a
>> frequent
>>>> BaseTools
>>>> contributor, but it has happened occasionally. (I've
>> got
>>>> 41 patches in
>>>> the edk2 git history that touched BaseTools/, as of
>>>> commit
>>>> 0a506fc7ab8b.)
>>>>
>>>> * For
>>>> <https://bugzilla.redhat.com/show_bug.cgi?id=1540244>,
>> I
>>>> pushed
>>>> 5+1 patches:
>>>>
>>>> 67983484a443 BaseTools/footer.makefile: expand
>>>> BUILD_CFLAGS last for C files too
>>>> 03252ae287c4 BaseTools/header.makefile: remove "-
>> c"
>>>> from BUILD_CFLAGS
>>>> b8a661702643 BaseTools/Source/C: split "-O2" to
>>>> BUILD_OPTFLAGS
>>>> b0ca5dae78ff BaseTools/Source/C: take
>> EXTRA_OPTFLAGS
>>>> from the caller
>>>> 81502cee20ac BaseTools/Source/C: take
>> EXTRA_LDFLAGS
>>>> from the caller
>>>> +
>>>> aa4e0df1f0c7 BaseTools/VfrCompile: honor
>>>> EXTRA_LDFLAGS
>>>>
>>>> * For
>>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=1377>,
>> I
>>>> pushed 26
>>>> patches:
>>>>
>>>> 8ff122119941 EmulatorPkg: require GCC48 or later
>>>> 8d7cdfae8cb8 OvmfPkg: require GCC48 or later
>>>> fd158437dcce Vlv2TbltDevicePkg: assume GCC48 or
>> later
>>>> 7a9dbf2c94d1 BaseTools/Conf/tools_def.template:
>> drop
>>>> ARM/AARCH support from GCC46/GCC47
>>>> 48e64498c961 BaseTools/tools_def.template: fix up
>> LF-
>>>> only line terminator
>>>> 7381a6627a66 BaseTools/tools_def.template: strip
>>>> trailing whitespace
>>>> 9bbf156faaad BaseTools/tools_def.template: remove
>>>> GCC48_IA32_X64_DLINK_COMMON dead-end
>>>> 3c5613c5932c BaseTools/tools_def.template: remove
>>>> GCC47 leaf definitions
>>>> fc87b8d7f411 BaseTools/tools_def.template:
>> propagate
>>>> loss of GCC47 references
>>>> 91a67e0f111e BaseTools/tools_def.template: remove
>>>> GCC47 documentation
>>>> 0f234fb8a662 BaseTools/tools_def.template: remove
>>>> GCC46 leaf definitions
>>>> 83a8f313884a BaseTools/tools_def.template:
>> propagate
>>>> loss of GCC46 references
>>>> be359fa7ceec BaseTools/tools_def.template: remove
>>>> GCC46 documentation
>>>> 1458af0cbce0 BaseTools/tools_def.template: remove
>>>> GCC45 leaf definitions
>>>> 024576896d42 BaseTools/tools_def.template:
>> propagate
>>>> loss of GCC45 references
>>>> 3e77d20f5cb3 BaseTools/tools_def.template: remove
>>>> GCC45 documentation
>>>> e046dc60fb89 BaseTools/tools_def.template: remove
>>>> GCC44 leaf definitions
>>>> 38c570efede0 BaseTools/tools_def.template:
>> propagate
>>>> loss of GCC44 references
>>>> 383d29096846 BaseTools/tools_def.template: rename
>>>> GCC44_ALL_CC_FLAGS to GCC48_ALL_CC_FLAGS
>>>> 0db91daf5228 BaseTools/tools_def.template:
>> eliminate
>>>> GCC44_IA32_X64_DLINK_FLAGS
>>>> 84d21abf4e36 BaseTools/tools_def.template: rename
>>>> GCC44_IA32_X64_DLINK_COMMON to
>>>> GCC48_IA32_X64_DLINK_COMMON
>>>> 5c6ccd53244b BaseTools/tools_def.template: remove
>>>> comment about GCC44 + LzmaF86Compress
>>>> 3bc65326d6ed BaseTools/tools_def.template: remove
>>>> GCC44 documentation
>>>> f7282023e758 ArmPkg/ArmSoftFloatLib: drop build
>> flags
>>>> specific to GCC46/GCC47
>>>> 300b8c5f150c CryptoPkg/BaseCryptLib: drop build
>> flags
>>>> specific to GCC44
>>>> 7423ba9d499b Revert "MdePkg: avoid
>>>> __builtin_unreachable() on GCC v4.4"
>>>>
>>>> (Note, from this list, 7a9dbf2c94d1 was authored by
>>>> Ard, I just
>>>> included it at the right spot.)
>>>>
>>>> All of these patch series were carefully structured,
>> and
>>>> carefully
>>>> documented (in the commit messages). I would have been
>>>> devastated, had
>>>> they been squashed.
>>>>
>>>> (And it would have been questionable to squash Ard's
>>>> patch with patches
>>>> authored by me.)
>>>>
>>>>
>>>>> Is there a way for GitHub to support both squash
>>>> commits for some PRs
>>>>> and preserve a patch series for other PRs?
>>>>
>>>> I don't know.
>>>>
>>>> But honestly, what is the purpose of squash merges?
>> Don't
>>>> we expect
>>>> *all* contributions in well structured patch series?
>>>>
>>>> If review discovers an issue with the structure of the
>>>> series (for
>>>> example, not buildable in the middle, or the steps are
>>>> not logical in
>>>> that order, or some patches do too many things at once
>>>> and should be
>>>> split up, or a patch in the middle has a typo), the
>> fixes
>>>> shouldn't be
>>>> just heaped on top. The submitter should restructure
>>>> (rebase) the
>>>> series, and submit the reworked series on a new topic
>>>> branch / pull
>>>> request.
>>>>
>>>> Isn't this the contribution pattern we'd like to see
>> in
>>>> all
>>>> sub-projects, where we (the TianoCore community) have
>>>> reviewer /
>>>> maintainer responsibilities?
>>>>
>>>> Thanks,
>>>> Laszlo
>>>>
>>>>> Thanks,
>>>>>
>>>>> Mike
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: devel at edk2.groups.io
>>>> [mailto:devel at edk2.groups.io]
>>>>>> On Behalf Of Laszlo Ersek
>>>>>> Sent: Wednesday, May 8, 2019 2:56 AM
>>>>>> To: devel at edk2.groups.io; sean.brogan at microsoft.com
>>>>>> Subject: Re: [edk2-devel] RFC for Edk2-Library
>>>>>>
>>>>>> On 05/07/19 23:35, Sean via Groups.Io wrote:
>>>>>>> RFC Edk2-Library creation
>>>>>>>
>>>>>>> Create a new tianocore owned repository to host a
>>>>>> python library
>>>>>>> package in support of UEFI development. This
>> package
>>>>>> will allow easy
>>>>>>> sharing of python library code to facilitate reuse.
>>>>>> Inclusion of this
>>>>>>> package and dependency management should be managed
>>>>>> using Pip/Pypi. To
>>>>>>> start this is a supplemental package and is not
>>>>>> required to be used
>>>>>>> for edk2 builds.
>>>>>>
>>>>>> [1]
>>>>>>
>>>>>>> Examples of content here
>>>>>>>
>>>>>>> * Edk2 file type parsing
>>>>>>> * UEFI structure encode/decode in python
>>>>>>> * Packaging tools (Capsules generation, signing,
>> INF
>>>>>> gen, Cat gen)
>>>>>>> * TPM support code
>>>>>>> * Potentially move content from
>>>>>> basetools/source/python/common/*
>>>>>>
>>>>>> [2]
>>>>>>
>>>>>>> * No command line tools/interface
>>>>>>>
>>>>>>> Maintainers
>>>>>>>
>>>>>>> * Sean Brogan
>>>>>>> * Bret Barkelew
>>>>>>> * Placeholder for existing maintainer from the
>>>>>> basetools
>>>>>>>
>>>>>>> License
>>>>>>>
>>>>>>> * BSD + Patent (edk2 aligned)
>>>>>>>
>>>>>>> Contribution process and issue tracking
>>>>>>>
>>>>>>> * Follow Github PR process for contributions and
>>>> issue
>>>>>> tracking
>>>>>>> * Contributor forks repo in github
>>>>>>> * Contributor creates branch for work
>>>>>>> * Contributor updates release notes to indicate
>>>> change
>>>>>> (if necessary)
>>>>>>> * Contributor submits PR to master branch of
>>>>>> tianocore/Edk2-Library
>>>>>>> repo
>>>>>>> * Review feedback is given in PR
>>>>>>> * Python Tests are run on the repo (new
>> contributions
>>>>>> need unit tests)
>>>>>>> * Python Style (flake8) must pass
>>>>>>> * All review feedback must be completed,
>> maintainers
>>>>>> approved, and
>>>>>>> tests run successfully before PR is *squash
>> merged*
>>>>>> into master
>>>>>>
>>>>>> The sentences
>>>>>>
>>>>>> [1] "To start this is a supplemental package and is
>>>> not
>>>>>> required to be
>>>>>> used for edk2 builds."
>>>>>>
>>>>>> [2] "Potentially move content from
>>>>>> basetools/source/python/common/*"
>>>>>>
>>>>>> foreshadow that such a code movement might happen
>> down
>>>>>> the road, and the
>>>>>> external package could become a requirement for
>>>> building
>>>>>> edk2.
>>>>>>
>>>>>> That step would mean the following:
>>>>>>
>>>>>> (a) Edk2 would not remain buildable from a single
>>>>>> command
>>>>>>
>>>>>> git clone --recurse-submodules
>>>>>>
>>>>>> Building edk2 would require GNU/Linux users to
>>>> start
>>>>>> tracking
>>>>>> packages with "pip", which is independent of any
>>>>>> given distro's own
>>>>>> package management, and may cause conflicts if
>> not
>>>>>> used carefully:
>>>>>>
>>>>>>
>>>>>>
>>>>
>> https://developer.fedoraproject.org/tech/languages/pytho
>>>>>> n/pypi-installation.html
>>>>>>
>>>>>> This requirement on "pip" would only go away
>> once
>>>>>> the external
>>>>>> python dependencies were packaged for at least
>> the
>>>>>> larger GNU/Linux
>>>>>> distros.
>>>>>>
>>>>>> (b) Edk2 users running into build problems related
>> to
>>>>>> the external
>>>>>> python dependencies would have to contribute
>>>> through
>>>>>> a github-only
>>>>>> workflow. That's not a deal-breaker per se -- if
>>>> we
>>>>>> want to
>>>>>> contribute to other edk2 dependencies, such as
>>>>>> OpenSSL or nasm, we
>>>>>> also have to conform to their specific
>> development
>>>>>> models, clearly.
>>>>>>
>>>>>> However, "squash merge" is a catastrophically
>> bad
>>>>>> development model,
>>>>>> and I'd object to introducing a new edk2 build
>>>>>> dependency that
>>>>>> followed that model.
>>>>>>
>>>>>> (There are other issues with the GitHub.com
>>>>>> development workflow, as
>>>>>> discussed elsewhere, but "squash merge" takes
>> the
>>>>>> cake.)
>>>>>>
>>>>>> Problem (a) would be solvable in the long term
>>>> (through
>>>>>> collaboration
>>>>>> with distro maintainers, i.e. downstream packaging),
>>>> but
>>>>>> problem (b)
>>>>>> would not be. Thus I'm fine with the proposal, in
>> its
>>>>>> current form, only
>>>>>> if the new package is 100% an addition on top of
>> edk2,
>>>>>> even in the long
>>>>>> term, and not in any part intended as a future
>>>>>> replacement for current
>>>>>> edk2 functionality.
>>>>>>
>>>>>> Thanks,
>>>>>> Laszlo
>>>>>>
>>>>>>
>>>>>
>>>
>>
>>
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40538): https://edk2.groups.io/g/devel/message/40538
Mute This Topic: https://groups.io/mt/31536886/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list