1714690 - [meta] Remove usages of "six"

Status: NEW

(Firefox Build System :: Mach Core, task, P3)

Description

[Mitchell Hentges [:mhentges] 🦀]

Mach always runs with Python 3, so we should be able to remove six usages.

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Future contributors 👋
Before jumping in to this ticket, see the list of six usages and decide on a specific section of code that you'd like to improve (e.g.: tools/tryselect). Then, leave a comment here or chat with a build member in the #build channel on Matrix, and we'll create a sub-bug that can be assigned to you. Looking forward to meeting you!

Comment 2

[Shivam Singhal [ :championshuttler ]]

this bug also can be converted into meta :)

Comment 3

[Mitchell Hentges [:mhentges] 🦀]

Nice, thanks Shivam!
Note: this might be able to be automated with pyupgrade: https://github.com/asottile/pyupgrade

Comment 4

[Sean Pedigo]

Attachment: Bug 1714690 - Remove usages of "six" from gen_features_definitions r=mhentges,Gijs

Removed six library, added str and bytes to values in Any()

Comment 5

[Sean Pedigo]

Hi, I've already written code to update toolkit/components/featuregates/gen_feature_definitions.py

This was a trivial change because I wanted to ensure that my first commit was small while I am still learning.

I'm not working right now, so while I have the time off I'd like to contribute a lot more here. Once I figure out what I need to do, or where I screwed up I think it would be pretty easy to knock most of this out pretty quickly. So please let me know where there are any mistakes, or anything that needs to be revised.

Thank you!

Comment 6

[Sean Pedigo]

Attachment: Bug 1714690 - Removed usages of six from components directory. r?mhentges

Depends on D136699

Comment 7

[Mitchell Hentges [:mhentges] 🦀]

Hey, thanks for the patches!
A couple notes:

1. Let's break up this work into separate sub-bugs, and associate patches with them accordingly. For example, for the first two patches you've provided here, I've created a bug for 'em here. You can do this by editing the commit message for your patches to refer to "Bug 1751990" instead, then re-moz-phab them to have them automatically update to point to the new bug.
   • In general, I'd recommend splitting this up by directory in python/* directory in testing/*, and otherwise by top-level directory.
2. As part of this work, heads up in advance that code in third_party/python and testing/web-platform/tests/tools/third_party are third-party, and shouldn't be modified :)
3. If you get the ERROR: Reformat python error in Phabricator, you can resolve that by running ./mach lint -l black --fix

Looking forward to working with you to have this six stuff cleaned up 😁

Comment 8

[Sean Pedigo]

Attachment: Bug 1714690 - Fixing linting issue for test_gen_feature_definitions.py r?mhentges

Depends on D136759

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::Mach Core' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 11

[Alex Hochheiden [:ahochheiden]]

Attachment: Bug 1714690 - Removed usages of six from components directory. r?#build

Depends on D136699

Differenti Revision: https://phabricator.services.mozilla.com/D136759

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/029e0b6bd2ad
Remove usages of "six" from gen_features_definitions r=mhentges,nalexander

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8af3093ff121
Removed usages of six from components directory. r=firefox-build-system-reviewers,nalexander

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/029e0b6bd2ad
https://hg.mozilla.org/mozilla-central/rev/8af3093ff121

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 16

[Richard Cheng]

Hello I am new to contributing firefox nightly, I want to take one of the file in this issue as my first attempt
Can I get the chance?
Also I want to choose fix build/RunCbindgen.py

Comment 17

[Alex Hochheiden [:ahochheiden]]

Hi Richard, go for it! I'll review any patch you submit.

Here's a link to help get you started: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-write-a-patch

Comment 18

[Richard Cheng]

(In reply to Alex Hochheiden [:ahochheiden] from comment #17)

Hi Richard, go for it! I'll review any patch you submit.

Here's a link to help get you started: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-write-a-patch

Hello I have a little question, after I get the source code and run it as the tutorial in https://firefox-source-docs.mozilla.org/setup/windows_build.html
I couldn't find the .git or .hg file in my directory
I asked people in https://chat.mozilla.org/#/room/#introduction:mozilla.org
Looks like I have to use git clone to clone the repo down
But the tutorial use wget

Do I need to git clone and re-build the whole project again?

Comment 19

[Richard Cheng]

Attachment: Bug 1714690 - Remove usages of "six" in build/RunCbindgen.py. r?#build

Hochheiden [:ahochheiden]

Comment 20

[Richard Cheng]

Attachment: Bug 1714690 - Remove usages of "six" in build/RunCbindgen.py. r?Alex

Hochheiden [:ahochheiden]

Depends on D178515

Comment 21

[Richard Cheng]

(In reply to Alex Hochheiden [:ahochheiden] from comment #17)

Hi Richard, go for it! I'll review any patch you submit.

Here's a link to help get you started: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-write-a-patch

Hello Alex, I've submit my patches and asked for your code review.
I am not sure If I did send the code review to you or not, If you didn't receive it pls let me know and I'll try to correct it.

Comment 22

[Richard Cheng]

Alex Hochheiden [:ahochheiden]
I've submit a patch and pass the auto review bot test, I would be thankful if you can review it for me.

Comment 23

[Alex Hochheiden [:ahochheiden]]

I was on vacation, but also did not get the notification. If you want to add me specifically as a reviewer you need to add r?ahochheiden, but adding r?#build is usually preferred anyway (that goes to all build system reviewers).

I agree with Glandium's review comment, take care of that and we can move forward.

Also, please update the summary stating what you're doing.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb52e6ee18c6
Remove usages of "six" in build/RunCbindgen.py. r=firefox-build-system-reviewers,glandium

Comment 25

[Atila Butkovits]

Backed out for causing bustages at complaining about charmap.

Backout link: https://hg.mozilla.org/integration/autoland/rev/68ec5d009309ef9993821a1a63175de4fa85ae0f

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fb52e6ee18c65d0d8818f001f0a49805e43c16bc&selectedTaskRun=TwTUktHtTtOdNslWMEq3Yw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=417831069&repo=autoland&lineNumber=4300

Comment 26

[Richard Cheng]

(In reply to Atila Butkovits from comment #25)

Backed out for causing bustages at complaining about charmap.

Backout link: https://hg.mozilla.org/integration/autoland/rev/68ec5d009309ef9993821a1a63175de4fa85ae0f

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fb52e6ee18c65d0d8818f001f0a49805e43c16bc&selectedTaskRun=TwTUktHtTtOdNslWMEq3Yw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=417831069&repo=autoland&lineNumber=4300

The error is caused by encoding error, should I use utf-8 to encode it?

Comment 27

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 28

[[:anutrix]]

Excluding 'third_party' directory, there's 197 usages of 'import six' in 192 files left in mozilla-unified.
I want to add some patches to remove them. Do we need create separate tickets/bugs for each directory before raising patches? Or can I just start making patches?

Comment 29

[[:anutrix]]

Glad that https://bugzilla.mozilla.org/show_bug.cgi?id=1959849 has landed.
About 35-40% files containing import six no longer use it.

Is there something similar(and automated) planned for the rest(roughly, 100-120 files to go)?

Additionally, we can also create a task ticket to replace all instances of isinstance(s, (str,)) with isinstance(s, str) maybe. There's only about 45-50 of them.

Comment 30

[Alex Hochheiden [:ahochheiden]]

(In reply to [:anutrix] from comment #29)

Is there something similar(and automated) planned for the rest(roughly, 100-120 files to go)?

There's nothing else planned as far as automatically doing migrations. I'm not aware of any pre-existing tools other than pyupgrade that have this feature, but if you find one (or want to try implementing your own) feel free to try and apply it.