From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 13 May 2019 13:28:54 -0700 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 278A48830B; Mon, 13 May 2019 20:28:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-123-237.rdu2.redhat.com [10.10.123.237]) by smtp.corp.redhat.com (Postfix) with ESMTP id D6E381001DC2; Mon, 13 May 2019 20:28:52 +0000 (UTC) Subject: Re: [edk2-devel] RFC for Edk2-Library To: "Kinney, Michael D" , "devel@edk2.groups.io" , "sean.brogan@microsoft.com" References: <24030.1557264917803034398@groups.io> <46700279-e217-0a45-8637-9c24cb5cc4eb@redhat.com> From: "Laszlo Ersek" Message-ID: <7412bfb8-ff72-5b2e-8389-f2c9cbea71d1@redhat.com> Date: Mon, 13 May 2019 22:28:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 13 May 2019 20:28:54 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 ; >> devel@edk2.groups.io; sean.brogan@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@redhat.com] >>>> Sent: Thursday, May 9, 2019 3:56 PM >>>> To: Kinney, Michael D ; >>>> devel@edk2.groups.io; sean.brogan@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 >>>> , >> 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 >>>> , >> 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.brogan@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 >>>>>> >>>>>> >>>>> >>> >> >> >> >