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

["Yuwei Chen" <yuwei.chen@intel.com>]

Thanks a lot Rebecca~~

Thanks,
Christine

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> D Kinney
> Sent: Saturday, February 17, 2024 1:24 AM
> To: Rebecca Cran <rebecca@bsdio.com>; Joey Vagedes
> <joeyvagedes@microsoft.com>; Rebecca Cran
> <rebecca@os.amperecomputing.com>; devel@edk2.groups.io; Sean
> <spbrogan@outlook.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] Fixing edk2-basetools CI
> 
> 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%7Cf64b5d2ea8624
> 5c44e5
> > >>>
> >
> 208dc2cc9e93d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6384
> 344929
> > >>>
> >
> 36969249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMzIi
> > >>>
> >
> LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ORN2iHQhOfz
> kN9F462p
> > >>> 2H6mb8Hl9ubIIeX%2BDBorzmC0%3D&reserved=0
> > >>> ub.com%2Ftianocore%2Fedk2-pytool-
> > library%2Fblob%2Fmaster%2Fpyproject.
> > >>> t
> > >>>
> >
> oml&data=05%7C02%7Cjoeyvagedes%40microsoft.com%7Ca37eecc569d54b
> 5dbbf3
> > >>> 0
> > >>>
> >
> 8dc2be996f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638433
> 529483
> > >>> 1
> > >>>
> >
> 52477%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJ
> > >>> B
> > >>>
> >
> TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=8RSQNcjgH02JyPiG
> CKmyf2u
> > >>> 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 (#115869): https://edk2.groups.io/g/devel/message/115869
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]
-=-=-=-=-=-=-=-=-=-=-=-