public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>,
	Andrew Fish <afish@apple.com>,
	Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [Patch v4 03/22] requirements.txt: Add python pip requirements file
Date: Thu, 7 Nov 2019 17:44:25 +0000	[thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9E00BC8@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <019f8e7f-388e-839b-d723-3d78027752fb@redhat.com>

Hi Laszlo,

requirements.txt is not only for CI.

If a developer wants to run the same tests that CI runs
locally that also need to install using pip and need 
this file.

Sean responded to this feedback earlier and pointed to 
some features that may depend on this specific filename
or a specific filename pattern.

https://edk2.groups.io/g/devel/message/49620

I agree that the pip command supports using a different
filename.  I considered several options:

1) Keep current requirements.txt in root
2) Change to pip_requirements.txt in root
3) Change to requirements.txt in the .pytool directory

I set (3) aside because the use of the python extensions
installed using pip are not limited to content in the
.pytool directory.  There is new content in BaseTools as
well that depends on the pip install components.  The 
root directory is the only common parent directory.

Given the feedback that there may be some services that
look for requirements.txt, I thought it would be better
to leave the filename alone and add the file header
comment block with a clear description of the file.

With this additional context, if there is still feedback
that the filename must be changed, then I would recommend
a filename change that also follows camel case in the root.  

    PipRequirements.txt

Best regards,

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, November 7, 2019 2:40 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Andrew Fish
> <afish@apple.com>; Leif Lindholm
> <leif.lindholm@linaro.org>
> Subject: Re: [Patch v4 03/22] requirements.txt: Add
> python pip requirements file
> 
> On 11/07/19 02:13, Michael D Kinney wrote:
> > From: Sean Brogan <sean.brogan@microsoft.com>
> >
> > Add pip requirements file that is used to install the
> python pip
> > modules build from the edk2-pytool-library and edk2-
> pytool-extensions
> > repositories.
> >
> > These python modules provide the extensions required
> to perform EDK II
> > Continuous Integration(CI) builds.
> >
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> >  requirements.txt | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >  create mode 100644 requirements.txt
> >
> > diff --git a/requirements.txt b/requirements.txt new
> file mode 100644
> > index 0000000000..4ad72cfc98
> > --- /dev/null
> > +++ b/requirements.txt
> > @@ -0,0 +1,17 @@
> > +## @file
> > +# EDK II Python PIP requirements file # # This file
> provides the list
> > +of python components to install using PIP.
> > +#
> > +# Copyright (c) Microsoft Corporation. All rights
> reserved.<BR> # #
> > +SPDX-License-Identifier: BSD-2-Clause-Patent # #
> > +https://pypi.org/project/pip/ #
> >
> +https://pip.pypa.io/en/stable/user_guide/#requirements-
> files
> > +#
> >
> +https://pip.pypa.io/en/stable/reference/pip_install/#re
> quirements-fil
> > +e-format
> > +#
> > +##
> > +
> > +edk2-pytool-library==0.10.*
> > +edk2-pytool-extensions==0.12.*
> >
> 
> This is better, but I still find the plain
> "requirements.txt" filename in the root directory of the
> project very confusing. What component
> *exactly* insists on this file path and file name?
> 
> I've checked
> 
> 
> https://pip.pypa.io/en/stable/user_guide/#requirements-
> files
> 
> and it seems like "pip" can take any pathname as an
> argument to option "-r".
> 
> There must be a component in the CI environment that
> invokes "pip". Can we file a feature request for that
> component, that it try "pip-requirements.txt" first?
> 
> Hmmm... I just googled "github pip requirements.txt",
> assuming that "github" was the component calling "pip".
> In the result list, I've found:
> 
> https://github.com/ClearingHouse/clearinghoused/blob/mas
> ter/pip-requirements.txt
> 
> Is it possible that github already knows to look for
> "pip-requirements.txt"? (Honestly I only suggested "pip-
> requirements.txt" above because it seemed sensible.) If
> that's the case, we should use it.
> 
> 
> Furthermore, my understanding is that "the list of
> python components to install using PIP" is only there
> for CI purposes. Can we please state that explicitly in
> the comment block? (The commit message already does
> that, and that's great.)
> 
> Basically when someone clones edk2 and runs "ls -l" for
> the first time, there's a good chance "requirements.txt"
> will be among the few files they open right after.
> 
> Thanks!
> Laszlo


  parent reply	other threads:[~2019-11-07 17:44 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-07  1:13 [Patch v4 00/22] Enable Phase 1 of EDK II CI Michael D Kinney
2019-11-07  1:13 ` [Patch v4 01/22] Maintainers.txt: Add continuous integration(CI) directories Michael D Kinney
2019-11-07  1:13 ` [Patch v4 02/22] .gitignore: Ignore python compiled files, extdeps, and vscode Michael D Kinney
2019-11-07 10:26   ` Laszlo Ersek
2019-11-07  1:13 ` [Patch v4 03/22] requirements.txt: Add python pip requirements file Michael D Kinney
2019-11-07 10:39   ` Laszlo Ersek
2019-11-07 15:43     ` Leif Lindholm
2019-11-07 17:44     ` Michael D Kinney [this message]
2019-11-08 13:12       ` Laszlo Ersek
2019-11-08 16:58         ` Michael D Kinney
2019-11-07 10:49   ` Laszlo Ersek
2019-11-07  1:13 ` [Patch v4 04/22] BaseTools: Add RC_PATH define for VS2017/2019 Michael D Kinney
2019-11-07  1:13 ` [Patch v4 05/22] BaseTools: Add YAML files with path env and tool extdeps Michael D Kinney
2019-11-07  1:13 ` [Patch v4 06/22] BaseTools: Add BaseTools plugins to support CI Michael D Kinney
2019-11-07  1:13 ` [Patch v4 07/22] .pytool/Plugin: Add CI plugins Michael D Kinney
2019-11-07  6:58   ` Liming Gao
2019-11-07  1:13 ` [Patch v4 08/22] CryptoPkg: Add YAML file for CI builds Michael D Kinney
2019-11-07  5:06   ` Wang, Jian J
2019-11-07  1:13 ` [Patch v4 09/22] FatPkg: " Michael D Kinney
2019-11-07  2:12   ` Ni, Ray
2019-11-07  1:13 ` [Patch v4 10/22] FmpDevicePkg: " Michael D Kinney
2019-11-07  1:13 ` [Patch v4 11/22] MdeModulePkg: " Michael D Kinney
2019-11-07  3:03   ` Wu, Hao A
2019-11-07 20:02     ` Michael D Kinney
2019-11-07  1:13 ` [Patch v4 12/22] MdePkg: " Michael D Kinney
2019-11-07  1:13 ` [Patch v4 13/22] NetworkPkg: " Michael D Kinney
2019-11-07  1:13 ` [Patch v4 14/22] PcAtChipsetPkg: Add YAML files " Michael D Kinney
2019-11-07  2:12   ` Ni, Ray
2019-11-07  1:13 ` [Patch v4 15/22] SecurityPkg: " Michael D Kinney
2019-11-07  5:08   ` Wang, Jian J
2019-11-07  1:13 ` [Patch v4 16/22] ShellPkg: Add YAML file " Michael D Kinney
2019-11-07  2:12   ` Ni, Ray
2019-11-07  1:13 ` [Patch v4 17/22] UefiCpuPkg: " Michael D Kinney
2019-11-07  2:12   ` Ni, Ray
2019-11-07 10:42   ` Laszlo Ersek
2019-11-07 10:48     ` Laszlo Ersek
2019-11-07 19:23       ` [edk2-devel] " Michael D Kinney
2019-11-07 19:33         ` Sean
2019-11-08 14:43           ` Laszlo Ersek
2019-11-07  1:13 ` [Patch v4 18/22] SignedCapsulePkg: Use BaseCryptLibNull to reduce package CI time Michael D Kinney
2019-11-07  1:13 ` [Patch v4 19/22] .pytool: Add CISettings.py and Readme.md Michael D Kinney
2019-11-07 16:16   ` [edk2-devel] " rebecca
2019-11-07  1:13 ` [Patch v4 20/22] .azurepipelines: Add Azure Pipelines YML configuration files Michael D Kinney
2019-11-07  1:13 ` [Patch v4 21/22] .mergify: Add Mergify YML pull request rules configuration file Michael D Kinney
2019-11-07  1:13 ` [Patch v4 22/22] Readme.md: Add CI build status badges Michael D Kinney
2019-11-07 10:44   ` Laszlo Ersek
2019-11-07 16:00   ` Leif Lindholm
2019-11-07 19:42     ` Michael D Kinney
2019-11-07 23:16       ` Leif Lindholm
2019-11-08  9:24       ` Leif Lindholm
2019-11-07 15:35 ` [Patch v4 00/22] Enable Phase 1 of EDK II CI Liming Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E92EE9817A31E24EB0585FDF735412F5B9E00BC8@ORSMSX113.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox