Re: [edk2-devel] Fixing edk2-basetools CI

["Michael D Kinney" <michael.d.kinney@intel.com>]

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 <rebecca@bsdio.com>
> Sent: Thursday, February 15, 2024 11:51 PM
> To: Joey Vagedes <joeyvagedes@microsoft.com>; Rebecca Cran
> <rebecca@os.amperecomputing.com>; devel@edk2.groups.io; Kinney, Michael
> D <michael.d.kinney@intel.com>; Sean <spbrogan@outlook.com>; Michael
> Kubacki <mikuback@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 <rebecca@bsdio.com>
> > Sent: Tuesday, February 13, 2024 11:28 AM
> > To: Joey Vagedes <joeyvagedes@microsoft.com>; Rebecca Cran
> <rebecca@os.amperecomputing.com>; devel@edk2.groups.io; Kinney, Michael
> D <michael.d.kinney@intel.com>; Sean <spbrogan@outlook.com>; Michael
> Kubacki <mikuback@linux.microsoft.com>
> > Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI
> >
> > [You don't often get email from rebecca@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 <rebecca@os.amperecomputing.com>
> >> Sent: Monday, February 12, 2024 8:42 AM
> >> To: devel@edk2.groups.io; Joey Vagedes <joeyvagedes@microsoft.com>;
> >> Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Sean <spbrogan@outlook.com>; Michael
> >> Kubacki <mikuback@linux.microsoft.com>
> >> Subject: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI
> >>
> >> [You don't often get email from rebecca@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-