Hi Rebecca, This approach makes a lot of sense. Thank you for the updated. I have approved.
Can we open some new Issues to re-enable flake8/ruf and codecov so those tasks are not forgotten. Thanks, Mike > -----Original Message----- > From: Rebecca Cran <rebe...@bsdio.com> > Sent: Thursday, February 15, 2024 11:51 PM > To: Joey Vagedes <joeyvage...@microsoft.com>; Rebecca Cran > <rebe...@os.amperecomputing.com>; devel@edk2.groups.io; Kinney, Michael > D <michael.d.kin...@intel.com>; Sean <spbro...@outlook.com>; Michael > Kubacki <mikub...@linux.microsoft.com> > Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI > > I've updated the PR > https://github.com/tianocore/edk2-basetools/pull/116/ to include > commits > from the pyflake8 disable PR and a couple more commits that cause CI to > start passing. I realize that disabling both flake8 and codecov isn't > good, but my thinking is to get builds working again then we can work > on > fixing the issues in the background. The edk2-basetools project needs > lots of work. > > > Add pyproject.toml and fix setup.py deprecation warnings > > Disable running Pyflakes in CI since it's blocking releases > > Fix comments in build-publish-whl-steps.yml > > Update basic-setup-steps.yml to require Python 3.10 > > Disable codecov to fix CI and due to very low coverage > > > -- > Rebecca Cran > > > On 2/13/24 12:42, Joey Vagedes wrote: > > I agree - there are multiple blocking issues that will require some > fixes - One additional thing I noted is that multiple areas still > perform relative path imports - i.e. the import starts with `from .`. > > > > These will all need to be updated to import from edk2basetools as > some point soon as importing via relative paths can result in it > importing the wrong file if the pypath environment variable is > influenced. This open issue actually stems from that: > https://github.com/tianocore/edk2-basetools/issues/110 as the end > result is that edk2-basetools pip module can unexpectedly end up using > the BaseTools/* source code rather than the pip module source code. > > > > Thanks, > > Joey > > > > -----Original Message----- > > From: Rebecca Cran <rebe...@bsdio.com> > > Sent: Tuesday, February 13, 2024 11:28 AM > > To: Joey Vagedes <joeyvage...@microsoft.com>; Rebecca Cran > <rebe...@os.amperecomputing.com>; devel@edk2.groups.io; Kinney, Michael > D <michael.d.kin...@intel.com>; Sean <spbro...@outlook.com>; Michael > Kubacki <mikub...@linux.microsoft.com> > > Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI > > > > [You don't often get email from rebe...@bsdio.com. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > Thanks, I've updated > > https://github.com/tianocore/edk2-basetools/pull/116 with the changes > you suggested. > > > > Unfortunately several of the tools don't work with the > projects.scripts section because they don't have Main etc. It turns out > even Ecc is currently broken in the edk2-basetools PyPI package because > PYTHONPATH doesn't contain the parent directory. I'll plan to fix the > issues in the near future, once I've worked through the other PRs. > > > > > > -- > > > > Rebecca Cran > > > > > > On 2/12/24 10:06, Joey Vagedes wrote: > >> Hello! Regarding flake8, I see. That will take some time to work > through - I completely understand. > >> > >> When ready to work through that, I suggest switching to ruff. It can > resolve some errors automatically for you and allows you to customize > it more than flake8 (it is also faster). > >> > >> I just ran ruff on the project, showing about 11k errors. One nice > thing is that a large majority of it is regarding importing using *. By > fixing those, you can reduce the errors by almost 70% (to 3400 errors), > so it is not *quite* as bad as it seems (though 3400 is still A LOT). > >> > >> -------------------------------------------------------------------- > -- > >> ----------------------------------------------------- > >> > >> Regarding the setup tools question, the build-backend part of pytoml > (L3) is the entry point that is invoked - so we are still invoking > setuptools. However, it pulls the necessary metadata from > pyproject.toml rather than setup.py's setup() function. The main thing > that is different is the "get_version" functionality, which we > replicate in the pyproject.toml with setuptools-scm[toml] which simply > uses the latest tag to set the version. > >> > >> -------------------------------------------------------------------- > -- > >> ----------------------------------------------------- > >> > >> Additionally, I wanted to comment on the wrappers that you > mentioned. > >> > >> Using the [projects.scripts] section (i.e. CLI Commands feature) is > in my opinion the correct way to move forward as users will be able to > invoke commands themselves easily (i.e. just call Bin2Pcd, and it will > work). However, the wrappers will continue to be necessary until all > python source is removed from Basetools, so that users can toggle > between pip or local. However, once source is removed form BaseTools, > removing the wrappers is the next step forward. > >> > >> In the meantime, you will be able to update the binpipwrappers to > something akin to the following (untested, so I don't garuntee it will > be exactly this: > >> > >> @setlocal > >> Ecc %* > >> > >> Rather than > >> > >> @setlocal > >> @set ToolName=%~n0% > >> @%PYTHON_COMMAND% -m edk2basetools.%ToolName%.EccMain %* > >> > >> -------------------------------------------------------------------- > -- > >> ----------------------------------------------------- > >> > >> One thing to keep in mind once Python is completely removed from > BaseTools, is that consumers will need to get in the habit of verifying > the correct version of basetools is installed for the commit they are > on (or automate this checking) particularly for anyone that regularly > switches between stable branches. > >> > >> Thanks, > >> Joey > >> > >> -----Original Message----- > >> From: Rebecca Cran <rebe...@os.amperecomputing.com> > >> Sent: Monday, February 12, 2024 8:42 AM > >> To: devel@edk2.groups.io; Joey Vagedes <joeyvage...@microsoft.com>; > >> Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D > >> <michael.d.kin...@intel.com>; Sean <spbro...@outlook.com>; Michael > >> Kubacki <mikub...@linux.microsoft.com> > >> Subject: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI > >> > >> [You don't often get email from rebe...@os.amperecomputing.com. > Learn > >> why this is important at > https://aka.ms/LearnAboutSenderIdentification > >> ] > >> > >> On 2/12/2024 9:08 AM, Joey Vagedes via groups.io wrote: > >>> Hello. It can be simplified - You can view the pyproject.toml for > edk2-pytool-library and edk2-pytool-extensions: > >>> > >>> https://git/ > >>> > h%2F&data=05%7C02%7Cjoeyvagedes%40microsoft.com%7Cf64b5d2ea86245c44e5 > >>> > 208dc2cc9e93d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6384344929 > >>> > 36969249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi > >>> > LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ORN2iHQhOfzkN9F462p > >>> 2H6mb8Hl9ubIIeX%2BDBorzmC0%3D&reserved=0 > >>> ub.com%2Ftianocore%2Fedk2-pytool- > library%2Fblob%2Fmaster%2Fpyproject. > >>> t > >>> > oml&data=05%7C02%7Cjoeyvagedes%40microsoft.com%7Ca37eecc569d54b5dbbf3 > >>> 0 > >>> > 8dc2be996f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638433529483 > >>> 1 > >>> > 52477%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ > >>> B > >>> > TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=8RSQNcjgH02JyPiGCKmyf2u > >>> 7 > >>> Ep6BC9ldwiND0ULj9Zs%3D&reserved=0 > >>> > >>> Also, Why disable flake8 rather than fix the changes? I also > suggest switching to ruff, as it does the same thing but better, and > with way more rules / configuration options. > >> Thanks. I'm not sure that looks so much simpler than my > pyproject.toml but I'll update it to follow the same layout. Also, > pyproject.toml says it's using the setuptools build backend but you > don't have a setup.py: > >> how does that work? > >> > >> I decided to disable flake8 for now because the BaseTools Python > code is very non-compliant: the log file has over 41,000 lines. In most > places it looks like people have been trying to write it as C code. > >> > >> A few of the lines: > >> > >> .\edk2basetools\Common\TargetTxtClassObject.py:44:80: E203 > whitespace before ':' > >> .\edk2basetools\Common\TargetTxtClassObject.py:61:14: E111 > indentation > >> is not a multiple of 4 > >> .\edk2basetools\Common\TargetTxtClassObject.py:61:14: E117 > >> over-indented > >> .\edk2basetools\Common\TargetTxtClassObject.py:63:50: F405 > 'FILE_NOT_FOUND' may be undefined, or defined from star imports: > >> .BuildToolError > >> .\edk2basetools\Common\TargetTxtClassObject.py:73:121: E501 line too > >> long (123 > 120 characters) > >> .\edk2basetools\Common\TargetTxtClassObject.py:84:38: F405 > 'FILE_OPEN_FAILURE' may be undefined, or defined from star imports: > >> .BuildToolError > >> .\edk2basetools\Common\TargetTxtClassObject.py:100:108: E502 the > >> backslash is redundant between brackets > >> .\edk2basetools\Common\TargetTxtClassObject.py:108:121: E501 line > too > >> long (139 > 120 characters) > >> .\edk2basetools\Common\TargetTxtClassObject.py:118:121: E501 line > too > >> long (140 > 120 characters) > >> .\edk2basetools\Common\TargetTxtClassObject.py:123:97: E502 the > >> backslash is redundant between brackets > >> .\edk2basetools\Common\TargetTxtClassObject.py:128:21: F841 local > >> variable 'V' is assigned to but never used > >> > >> > >> -- > >> Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115544): https://edk2.groups.io/g/devel/message/115544 Mute This Topic: https://groups.io/mt/104306226/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
