public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Joey Vagedes via groups.io" <joeyvagedes=microsoft.com@groups.io>
To: Rebecca Cran <rebecca@os.amperecomputing.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	Rebecca Cran <rebecca@bsdio.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	Sean <spbrogan@outlook.com>,
	Michael Kubacki <mikuback@linux.microsoft.com>
Subject: Re: [edk2-devel] Fixing edk2-basetools CI
Date: Mon, 12 Feb 2024 17:06:28 +0000	[thread overview]
Message-ID: <BY1PR21MB3943B2B592DFFD88A601DA5FBF482@BY1PR21MB3943.namprd21.prod.outlook.com> (raw)
In-Reply-To: <2c52fbac-dbe1-423a-a9d7-0bbe473cd9e9@os.amperecomputing.com>

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



  reply	other threads:[~2024-02-12 17:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-12  4:14 [edk2-devel] Fixing edk2-basetools CI Rebecca Cran
2024-02-12  6:03 ` Michael D Kinney
2024-02-12 11:26   ` Rebecca Cran
2024-02-12 16:08     ` Joey Vagedes via groups.io
2024-02-12 16:42       ` Rebecca Cran via groups.io
2024-02-12 17:06         ` Joey Vagedes via groups.io [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BY1PR21MB3943B2B592DFFD88A601DA5FBF482@BY1PR21MB3943.namprd21.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox