Hi Sean, Can you send an RFC V2 that makes the following changes:
1) Add use of python virtual environments 2) Update Repository name: edk2-tools-library 3) Provide a summary of the APIs/Services that this PIP module provides and the APIs/Services from the edk2 repo that this PIP module depends on. 4) Contribution process. Add recommendation that PRs be focused on changes that make sense to be squashed. Submit a different PR for a different feature/issue. Break up a complex PR into multiple PRs. 5) Remove the following bullet: "* Potentially move content from basetools/source/python/common/*" We can discuss this concept after TianoCore platforms are successfully using these new features and the overlap between the edk2 repo and edk2-tools-library repo is clearly understood. A follow on task should evaluate GitHub PR options for submitting and preserving a patch series. Thanks, Mike > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Monday, May 13, 2019 1:29 PM > To: Kinney, Michael D <michael.d.kin...@intel.com>; > devel@edk2.groups.io; sean.bro...@microsoft.com > Subject: Re: [edk2-devel] RFC for Edk2-Library > > 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@edk2.groups.io > [mailto:devel@edk2.groups.io] > >> On Behalf Of Laszlo Ersek > >> Sent: Monday, May 13, 2019 3:46 AM > >> To: Kinney, Michael D <michael.d.kin...@intel.com>; > >> devel@edk2.groups.io; sean.bro...@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:ler...@redhat.com] > >>>> Sent: Thursday, May 9, 2019 3:56 PM > >>>> To: Kinney, Michael D <michael.d.kin...@intel.com>; > >>>> devel@edk2.groups.io; sean.bro...@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@edk2.groups.io > >>>> [mailto:devel@edk2.groups.io] > >>>>>> On Behalf Of Laszlo Ersek > >>>>>> Sent: Wednesday, May 8, 2019 2:56 AM > >>>>>> To: devel@edk2.groups.io; > sean.bro...@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 (#41261): https://edk2.groups.io/g/devel/message/41261 Mute This Topic: https://groups.io/mt/31536886/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
