public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] Fixing edk2-basetools CI
@ 2024-02-12  4:14 Rebecca Cran
  2024-02-12  6:03 ` Michael D Kinney
  0 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2024-02-12  4:14 UTC (permalink / raw)
  To: Sean, Michael D Kinney, Michael Kubacki, Joey Vagedes
  Cc: devel@edk2.groups.io

Hi everyone,


I've created a couple of PRs in the edk2-basetools project which I'm 
hoping will go towards fixing the CI and allowing releases to be 
published again.

I'd appreciate any reviews.


Disable flake8: https://github.com/tianocore/edk2-basetools/pull/115

Add pyproject.toml and update setup.py: 
https://github.com/tianocore/edk2-basetools/pull/116


Once CI is working again, I'll start working through the backlog of open 
PRs.


-- 
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115347): https://edk2.groups.io/g/devel/message/115347
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2024-02-12  6:03 UTC (permalink / raw)
  To: Rebecca Cran, Sean, Michael Kubacki, Joey Vagedes
  Cc: devel@edk2.groups.io, Kinney, Michael D

Hi Rebecca,

Thank you for working on this.

If pyproject.toml is added, then can setup.py be removed completely?

Also, the CLI commands for pyproject.toml are listed as edk2_build and
edk2_ecc.  Can't we use this feature of pyproject.toml to add build
and ecc to the command set and remove the need for the command 
wrappers? Perhaps an additional change after we have CI working?

Thanks,

Mike

> -----Original Message-----
> From: Rebecca Cran <rebecca@bsdio.com>
> Sent: Sunday, February 11, 2024 8:15 PM
> To: Sean <spbrogan@outlook.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>; Joey Vagedes
> <joeyvagedes@microsoft.com>
> Cc: devel@edk2.groups.io
> Subject: Fixing edk2-basetools CI
> 
> Hi everyone,
> 
> 
> I've created a couple of PRs in the edk2-basetools project which I'm
> hoping will go towards fixing the CI and allowing releases to be
> published again.
> 
> I'd appreciate any reviews.
> 
> 
> Disable flake8: https://github.com/tianocore/edk2-basetools/pull/115
> 
> Add pyproject.toml and update setup.py:
> https://github.com/tianocore/edk2-basetools/pull/116
> 
> 
> Once CI is working again, I'll start working through the backlog of
> open
> PRs.
> 
> 
> --
> Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115348): https://edk2.groups.io/g/devel/message/115348
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2024-02-12 11:26 UTC (permalink / raw)
  To: Kinney, Michael D, Sean, Michael Kubacki, Joey Vagedes
  Cc: devel@edk2.groups.io

No, setup.py or some other build backend configuration (e.g. Hatchling, 
Flit or PDM) is still needed to build the distribution package.

I don't know if it could be simplified though: I'll look into that later.


Yes, I suspect we could use the CLI commands feature and remove the 
wrappers. I'll work on that later, probably after I've gone through the PRs.


-- 

Rebecca Cran


On 2/11/24 23:03, Kinney, Michael D wrote:
> Hi Rebecca,
>
> Thank you for working on this.
>
> If pyproject.toml is added, then can setup.py be removed completely?
>
> Also, the CLI commands for pyproject.toml are listed as edk2_build and
> edk2_ecc.  Can't we use this feature of pyproject.toml to add build
> and ecc to the command set and remove the need for the command
> wrappers? Perhaps an additional change after we have CI working?
>
> Thanks,
>
> Mike
>
>> -----Original Message-----
>> From: Rebecca Cran <rebecca@bsdio.com>
>> Sent: Sunday, February 11, 2024 8:15 PM
>> To: Sean <spbrogan@outlook.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Michael Kubacki
>> <mikuback@linux.microsoft.com>; Joey Vagedes
>> <joeyvagedes@microsoft.com>
>> Cc: devel@edk2.groups.io
>> Subject: Fixing edk2-basetools CI
>>
>> Hi everyone,
>>
>>
>> I've created a couple of PRs in the edk2-basetools project which I'm
>> hoping will go towards fixing the CI and allowing releases to be
>> published again.
>>
>> I'd appreciate any reviews.
>>
>>
>> Disable flake8: https://github.com/tianocore/edk2-basetools/pull/115
>>
>> Add pyproject.toml and update setup.py:
>> https://github.com/tianocore/edk2-basetools/pull/116
>>
>>
>> Once CI is working again, I'll start working through the backlog of
>> open
>> PRs.
>>
>>
>> --
>> Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115349): https://edk2.groups.io/g/devel/message/115349
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Joey Vagedes via groups.io @ 2024-02-12 16:08 UTC (permalink / raw)
  To: Rebecca Cran, Kinney, Michael D, Sean, Michael Kubacki
  Cc: devel@edk2.groups.io

Hello. It can be simplified - You can view the pyproject.toml for edk2-pytool-library and edk2-pytool-extensions:

https://github.com/tianocore/edk2-pytool-library/blob/master/pyproject.toml

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,
Joey
-----Original Message-----
From: Rebecca Cran <rebecca@bsdio.com> 
Sent: Monday, February 12, 2024 3:27 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Sean <spbrogan@outlook.com>; Michael Kubacki <mikuback@linux.microsoft.com>; Joey Vagedes <joeyvagedes@microsoft.com>
Cc: devel@edk2.groups.io
Subject: [EXTERNAL] Re: Fixing edk2-basetools CI

[You don't often get email from rebecca@bsdio.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

No, setup.py or some other build backend configuration (e.g. Hatchling, Flit or PDM) is still needed to build the distribution package.

I don't know if it could be simplified though: I'll look into that later.


Yes, I suspect we could use the CLI commands feature and remove the wrappers. I'll work on that later, probably after I've gone through the PRs.


--

Rebecca Cran


On 2/11/24 23:03, Kinney, Michael D wrote:
> Hi Rebecca,
>
> Thank you for working on this.
>
> If pyproject.toml is added, then can setup.py be removed completely?
>
> Also, the CLI commands for pyproject.toml are listed as edk2_build and 
> edk2_ecc.  Can't we use this feature of pyproject.toml to add build 
> and ecc to the command set and remove the need for the command 
> wrappers? Perhaps an additional change after we have CI working?
>
> Thanks,
>
> Mike
>
>> -----Original Message-----
>> From: Rebecca Cran <rebecca@bsdio.com>
>> Sent: Sunday, February 11, 2024 8:15 PM
>> To: Sean <spbrogan@outlook.com>; Kinney, Michael D 
>> <michael.d.kinney@intel.com>; Michael Kubacki 
>> <mikuback@linux.microsoft.com>; Joey Vagedes 
>> <joeyvagedes@microsoft.com>
>> Cc: devel@edk2.groups.io
>> Subject: Fixing edk2-basetools CI
>>
>> Hi everyone,
>>
>>
>> I've created a couple of PRs in the edk2-basetools project which I'm 
>> hoping will go towards fixing the CI and allowing releases to be 
>> published again.
>>
>> I'd appreciate any reviews.
>>
>>
>> Disable flake8: 
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>> hub.com%2Ftianocore%2Fedk2-basetools%2Fpull%2F115&data=05%7C02%7Cjoey
>> vagedes%40microsoft.com%7C3010ff4bb5724033a86708dc2bbd8121%7C72f988bf
>> 86f141af91ab2d7cd011db47%7C1%7C0%7C638433340138909394%7CUnknown%7CTWF
>> pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn0%3D%7C0%7C%7C%7C&sdata=PFGp81sc5%2FhjgpuOE1QVukPni8TxWUAMk8FpqiHQp
>> 6E%3D&reserved=0
>>
>> Add pyproject.toml and update setup.py:
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>> hub.com%2Ftianocore%2Fedk2-basetools%2Fpull%2F116&data=05%7C02%7Cjoey
>> vagedes%40microsoft.com%7C3010ff4bb5724033a86708dc2bbd8121%7C72f988bf
>> 86f141af91ab2d7cd011db47%7C1%7C0%7C638433340138916993%7CUnknown%7CTWF
>> pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
>> Mn0%3D%7C0%7C%7C%7C&sdata=bVr7iJr1jyXOaO7OLY4FMm4PS5VqJgXuRflUV1id%2B
>> n0%3D&reserved=0
>>
>>
>> Once CI is working again, I'll start working through the backlog of 
>> open PRs.
>>
>>
>> --
>> Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115353): https://edk2.groups.io/g/devel/message/115353
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran via groups.io @ 2024-02-12 16:42 UTC (permalink / raw)
  To: devel, joeyvagedes, Rebecca Cran, Kinney, Michael D, Sean,
	Michael Kubacki

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://github.com/tianocore/edk2-pytool-library/blob/master/pyproject.toml
> 
> 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 (#115360): https://edk2.groups.io/g/devel/message/115360
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Joey Vagedes via groups.io @ 2024-02-12 17:06 UTC (permalink / raw)
  To: Rebecca Cran, devel@edk2.groups.io, Rebecca Cran,
	Kinney, Michael D, Sean, Michael Kubacki

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Ftianocore%2Fedk2-pytool-library%2Fblob%2Fmaster%2Fpyproject.t
> oml&data=05%7C02%7Cjoeyvagedes%40microsoft.com%7Ca37eecc569d54b5dbbf30
> 8dc2be996f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6384335294831
> 52477%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=8RSQNcjgH02JyPiGCKmyf2u7
> 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 (#115361): https://edk2.groups.io/g/devel/message/115361
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2024-02-13 19:27 UTC (permalink / raw)
  To: Joey Vagedes, Rebecca Cran, devel@edk2.groups.io,
	Kinney, Michael D, Sean, Michael Kubacki

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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>> ub.com%2Ftianocore%2Fedk2-pytool-library%2Fblob%2Fmaster%2Fpyproject.t
>> oml&data=05%7C02%7Cjoeyvagedes%40microsoft.com%7Ca37eecc569d54b5dbbf30
>> 8dc2be996f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6384335294831
>> 52477%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
>> TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=8RSQNcjgH02JyPiGCKmyf2u7
>> 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 (#115405): https://edk2.groups.io/g/devel/message/115405
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  2024-02-13 19:27           ` Rebecca Cran
@ 2024-02-13 19:42             ` Joey Vagedes via groups.io
  2024-02-13 19:46               ` Rebecca Cran
                                 ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Joey Vagedes via groups.io @ 2024-02-13 19:42 UTC (permalink / raw)
  To: Rebecca Cran, Rebecca Cran, devel@edk2.groups.io,
	Kinney, Michael D, Sean, Michael Kubacki

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 (#115411): https://edk2.groups.io/g/devel/message/115411
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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-27  1:49               ` Rebecca Cran
  2 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2024-02-13 19:46 UTC (permalink / raw)
  To: Joey Vagedes, Rebecca Cran, devel@edk2.groups.io,
	Kinney, Michael D, Sean, Michael Kubacki

I don't think these issues _block_ this PR from being merged, do they? I 
don't think there are any regressions from the current package - it's 
just that the current package is equally broken.

The only thing that is a breaking change is the switch from figuring out 
the next version number from PyPI to using tags. When this and the 
pyflakes8 PR get merged I'll tag it with v0.1.50.


-- 

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 (#115412): https://edk2.groups.io/g/devel/message/115412
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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-27  1:49               ` Rebecca Cran
  2 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2024-02-16  7:51 UTC (permalink / raw)
  To: Joey Vagedes, Rebecca Cran, devel@edk2.groups.io,
	Kinney, Michael D, Sean, Michael Kubacki

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 (#115538): https://edk2.groups.io/g/devel/message/115538
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  2024-02-16  7:51               ` Rebecca Cran
@ 2024-02-16 17:24                 ` Michael D Kinney
  2024-02-23  7:55                   ` Yuwei Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Michael D Kinney @ 2024-02-16 17:24 UTC (permalink / raw)
  To: Rebecca Cran, Joey Vagedes, Rebecca Cran, devel@edk2.groups.io,
	Sean, Michael Kubacki
  Cc: Kinney, Michael D

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  2024-02-16 17:24                 ` Michael D Kinney
@ 2024-02-23  7:55                   ` Yuwei Chen
  2024-02-23 14:48                     ` Rebecca Cran
       [not found]                     ` <17B6855E281DBF07.24621@groups.io>
  0 siblings, 2 replies; 15+ messages in thread
From: Yuwei Chen @ 2024-02-23  7:55 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D, Rebecca Cran,
	Joey Vagedes, Rebecca Cran, Sean, Michael Kubacki

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  2024-02-23  7:55                   ` Yuwei Chen
@ 2024-02-23 14:48                     ` Rebecca Cran
       [not found]                     ` <17B6855E281DBF07.24621@groups.io>
  1 sibling, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2024-02-23 14:48 UTC (permalink / raw)
  To: Chen, Christine, devel@edk2.groups.io, Kinney, Michael D,
	Joey Vagedes, Rebecca Cran, Sean, Michael Kubacki

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
       [not found]                     ` <17B6855E281DBF07.24621@groups.io>
@ 2024-02-23 22:40                       ` Rebecca Cran
  0 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2024-02-23 22:40 UTC (permalink / raw)
  To: Chen, Christine, devel@edk2.groups.io, Kinney, Michael D,
	Joey Vagedes, Rebecca Cran, Sean, Michael Kubacki

Thanks. I've merged that PR, but there's another problem - the 'build' 
package isn't available.

I've created another PR to fix it: 
https://github.com/tianocore/edk2-basetools/pull/118


-- 
Rebecca Cran


On 2/23/24 07:48, Rebecca Cran wrote:
> 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.
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115896): https://edk2.groups.io/g/devel/message/115896
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] Fixing edk2-basetools CI
  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-27  1:49               ` Rebecca Cran
  2 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2024-02-27  1:49 UTC (permalink / raw)
  To: Joey Vagedes, Rebecca Cran, devel@edk2.groups.io,
	Kinney, Michael D, Sean, Michael Kubacki

Is anyone familiar with how publishing to PyPi works? The "Publish to 
PyPI" step is failing:

Starting: Publish to PyPI
==============================================================================
Task         : Command line
Description  : Run a command line script using Bash on Linux and macOS 
and cmd.exe on Windows
Version      : 2.231.1
Author       : Microsoft Corporation
Help         : 
https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line
==============================================================================
Generating script.
Script contents: shell
twine upload -r Pypi-edk2-basetools --config-file 
D:\a\_temp\twineAuthenticate\HtPdQh\.pypirc dist/*
========================== Starting Command Output 
===========================
"C:\Windows\system32\cmd.exe" /D /E:ON /V:OFF /S /C "CALL 
"D:\a\_temp\53734b8d-4c1a-4bd2-99b3-15f4d79433fc.cmd""
Uploading distributions to https://upload.pypi.org/legacy/
Uploading edk2_basetools-0.1.50-py3-none-any.whl

   0% ---------------------------------------- 0.0/16.9 kB � --:-- � ?
   0% ---------------------------------------- 0.0/16.9 kB � --:-- � ?
100% ---------------------------------------- 16.9/16.9 kB � 00:00 � ?
100% ---------------------------------------- 16.9/16.9 kB � 00:00 � ?
WARNING  Error during upload. Retry with the --verbose option for more 
details.
ERROR    HTTPError: 403 Forbidden from https://upload.pypi.org/legacy/
          Invalid or non-existent authentication information. See
          https://pypi.org/help/#invalid-auth for more information.
##[error]Cmd.exe exited with code '1'.
Finishing: Publish to PyPI

-- 
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 (#115996): https://edk2.groups.io/g/devel/message/115996
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-02-27  1:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
     [not found]                     ` <17B6855E281DBF07.24621@groups.io>
2024-02-23 22:40                       ` Rebecca Cran
2024-02-27  1:49               ` Rebecca Cran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox