public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Rebecca Cran" <rebecca@bsdio.com>
To: "Chen, Christine" <yuwei.chen@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	Joey Vagedes <joeyvagedes@microsoft.com>,
	Rebecca Cran <rebecca@os.amperecomputing.com>,
	Sean <spbrogan@outlook.com>,
	Michael Kubacki <mikuback@linux.microsoft.com>
Subject: Re: [edk2-devel] Fixing edk2-basetools CI
Date: Fri, 23 Feb 2024 07:48:48 -0700	[thread overview]
Message-ID: <0bdae95e-7284-4a95-b1ac-7e729d399d99@bsdio.com> (raw)
In-Reply-To: <PH7PR11MB59153A57021928FE3E29308596552@PH7PR11MB5915.namprd11.prod.outlook.com>

Could someone review Fix build-publish-whl-steps.yml: we no longer use 
setup.py by bcran · Pull Request #117 · tianocore/edk2-basetools · 
GitHub <https://github.com/tianocore/edk2-basetools/pull/117> please?
It should be the last change needed to fix publishing releases.

-- 
Rebecca Cran

On 2/23/2024 12:55 AM, Chen, Christine wrote:
> 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 (#115886): https://edk2.groups.io/g/devel/message/115886
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]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2024-02-23 14:49 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-12  4:14 [edk2-devel] Fixing edk2-basetools CI Rebecca Cran
2024-02-12  6:03 ` Michael D Kinney
2024-02-12 11:26   ` Rebecca Cran
2024-02-12 16:08     ` Joey Vagedes via groups.io
2024-02-12 16:42       ` Rebecca Cran via groups.io
2024-02-12 17:06         ` Joey Vagedes via groups.io
2024-02-13 19:27           ` Rebecca Cran
2024-02-13 19:42             ` Joey Vagedes via groups.io
2024-02-13 19:46               ` Rebecca Cran
2024-02-16  7:51               ` Rebecca Cran
2024-02-16 17:24                 ` Michael D Kinney
2024-02-23  7:55                   ` Yuwei Chen
2024-02-23 14:48                     ` Rebecca Cran [this message]
     [not found]                     ` <17B6855E281DBF07.24621@groups.io>
2024-02-23 22:40                       ` Rebecca Cran
2024-02-27  1:49               ` Rebecca Cran

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=0bdae95e-7284-4a95-b1ac-7e729d399d99@bsdio.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