Page MenuHomePhabricator
Create Task
Maniphest T268468

captcha.py needs to be ported to Python 3
Closed, ResolvedPublic
Actions

Description

When updating the Puppet manifests for MediaWiki, I noticed that captcha.py is based on Python 2. That's not an immediate issue (the OS release we're migrating to (Debian Buster) still supports Python 2, but it will become an issue with the next OS release (which will no longer support Python 2).

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
mediawiki::packages: Drop python-piloperations/puppetproduction+2 -3
Make GenerateFancyCaptchas.php execute captcha(-old)?.py via python3mediawiki/extensions/ConfirmEditmaster+1 -1
Update captch(-old)?.py shebang to python 3mediawiki/extensions/ConfirmEditmaster+2 -2
Extend PIL Python package with Python 3 counterpartsoperations/puppetproduction+2 -2
Customize query in gerrit

Related Objects

Event Timeline

MoritzMuehlenhoff created this task.Nov 23 2020, 1:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 23 2020, 1:01 PM
Reedy subscribed.Nov 23 2020, 4:50 PM

What needs porting about it?

Between rECOEeea24979eda6: Make captcha python scripts python3 compatible from T157888: Make captcha(-old)?.py python3 compatible, rECOE69512c80465d: Use floor division (//) when calculating chunks in python and rECOE520e1ab0369a: Make captcha.py work under python 3 from T263223: captcha.py issues due to change in `/` behaviour in python 3... This should all be done now

Certainly after the last two patches it seems to work fine

Dzahn triaged this task as Medium priority.Nov 23 2020, 11:01 PM
Reedy added a comment.Nov 23 2020, 11:24 PM
$ 2to3 captcha.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Refactored captcha.py
--- captcha.py	(original)
+++ captcha.py	(refactored)
@@ -157,27 +157,27 @@
 			word = word + chr(97 + random.randint(0,25))
 
 	if verbose:
-		print("word is %s" % word)
+		print(("word is %s" % word))
 
 	if len(word) < min_length:
 		if verbose:
-			print("skipping word pair '%s' because it has fewer than %d characters" % (word, min_length))
+			print(("skipping word pair '%s' because it has fewer than %d characters" % (word, min_length)))
 		return None
 
 	if max_length > 0 and len(word) > max_length:
 		if verbose:
-			print("skipping word pair '%s' because it has more than %d characters" % (word, max_length))
+			print(("skipping word pair '%s' because it has more than %d characters" % (word, max_length)))
 		return None
 
 	if nonalpha.search(word):
 		if verbose:
-			print("skipping word pair '%s' because it contains non-alphabetic characters" % word)
+			print(("skipping word pair '%s' because it contains non-alphabetic characters" % word))
 		return None
 
 	for naughty in blacklist:
 		if naughty in word:
 			if verbose:
-				print("skipping word pair '%s' because it contains blacklisted word '%s'" % (word, naughty))
+				print(("skipping word pair '%s' because it contains blacklisted word '%s'" % (word, naughty)))
 			return None
 	return word
 
@@ -291,7 +291,7 @@
 
 	p = multiprocessing.Pool(threads);
 	data = []
-	print("Generating %s CAPTCHA images separated in %s image(s) per chunk run by %s threads..." % (count, chunks, threads))
+	print(("Generating %s CAPTCHA images separated in %s image(s) per chunk run by %s threads..." % (count, chunks, threads)))
 	for i in range(0, threads):
 		data.append([chunks, words, blacklist, opts, font, fontsize])
 
RefactoringTool: Files that need to be modified:
RefactoringTool: captcha.py
$ 2to3 captcha-old.py 
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Refactored captcha-old.py
--- captcha-old.py	(original)
+++ captcha-old.py	(refactored)
@@ -140,27 +140,27 @@
 			word = word + chr(97 + random.randint(0,25))
 
 	if verbose:
-		print("word is %s" % word)
+		print(("word is %s" % word))
 
 	if len(word) < min_length:
 		if verbose:
-			print("skipping word pair '%s' because it has fewer than %d characters" % (word, min_length))
+			print(("skipping word pair '%s' because it has fewer than %d characters" % (word, min_length)))
 		return None
 
 	if max_length > 0 and len(word) > max_length:
 		if verbose:
-			print("skipping word pair '%s' because it has more than %d characters" % (word, max_length))
+			print(("skipping word pair '%s' because it has more than %d characters" % (word, max_length)))
 		return None
 
 	if nonalpha.search(word):
 		if verbose:
-			print("skipping word pair '%s' because it contains non-alphabetic characters" % word)
+			print(("skipping word pair '%s' because it contains non-alphabetic characters" % word))
 		return None
 
 	for naughty in blacklist:
 		if naughty in word:
 			if verbose:
-				print("skipping word pair '%s' because it contains blacklisted word '%s'" % (word, naughty))
+				print(("skipping word pair '%s' because it contains blacklisted word '%s'" % (word, naughty)))
 			return None
 	return word
 
@@ -274,7 +274,7 @@
 
 	p = multiprocessing.Pool(threads);
 	data = []
-	print("Generating %s CAPTCHA images separated in %s image(s) per chunk run by %s threads..." % (count, chunks, threads))
+	print(("Generating %s CAPTCHA images separated in %s image(s) per chunk run by %s threads..." % (count, chunks, threads)))
 	for i in range(0, threads):
 		data.append([chunks, words, blacklist, opts, font, fontsize])
 
RefactoringTool: Files that need to be modified:
RefactoringTool: captcha-old.py
Reedy changed the task status from Open to Stalled.Dec 1 2020, 2:07 PM

Marking stalled as it's unclear what needs fixing (if anything)

MoritzMuehlenhoff changed the task status from Stalled to Open.Dec 1 2020, 2:11 PM

Well, at minimum the shebang needs to be switched to #!/usr/bin/python3. If that's all that's needed, even better.

gerritbot added a comment.Dec 1 2020, 2:17 PM

Change 644528 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/ConfirmEdit@master] Update captch(-old)?.py shebang to python 3

https://gerrit.wikimedia.org/r/644528

gerritbot added a project: Patch-For-Review.Dec 1 2020, 2:17 PM
Reedy added a comment.Dec 1 2020, 2:27 PM

I think that's possibly all that is needed at this point. 2to3 changes aren't necessary, I think?

I have tested (not extensively) on python3, and it seemed to work fine as of the last patches from earlier in the year. Always possible there's some edge cases (especially when running the larger batches) that I didn't find.

When it's merged and live in prod, I'll have another look at the logs and see

gerritbot added a comment.Dec 1 2020, 2:31 PM

Change 644532 had a related patch set uploaded (by Muehlenhoff; owner: Muehlenhoff):
[operations/puppet@production] Extend PIL Python package with Python 3 counterparts

https://gerrit.wikimedia.org/r/644532

Reedy added a comment.Dec 1 2020, 6:15 PM
In T268468#6659719, @MoritzMuehlenhoff wrote:

Well, at minimum the shebang needs to be switched to #!/usr/bin/python3. If that's all that's needed, even better.

Actually, while this may help if it's run standalone (which we don't, and there's no +x on the scripts anyway)... It doesn't actually help with how MediaWiki executes the script, as it uses python not python3 in the eventual command run via PHP

Currently it does

		$cmd = sprintf( "python %s --key %s --output %s --count %s --dirs %s",
			wfEscapeShellArg( dirname( __DIR__ ) . '/' . $captchaScript ),
			wfEscapeShellArg( $wgCaptchaSecret ),
			wfEscapeShellArg( $tmpDir ),
			wfEscapeShellArg( (string)$countGen ),
			wfEscapeShellArg( $wgCaptchaDirectoryLevels )
		);

So as is, it would continue running under (potentially) 2.7 anyway:

$ cat test.py
#!/bin/python3
import sys
print("Python version")
print (sys.version)
$ python --version
Python 2.7.18
$ python test.py 
Python version
2.7.18 (default, Aug  4 2020, 11:16:42) 
[GCC 9.3.0]

I'll fix the usage in the maintenance script too

gerritbot added a comment.Dec 1 2020, 6:32 PM

Change 644586 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/ConfirmEdit@master] Make GenerateFancyCaptchas.php execute captcha(-old)?.py via python3

https://gerrit.wikimedia.org/r/644586

Reedy added a comment.Dec 1 2020, 6:34 PM

I'm guessing there's no reason to actually make the python version (ie whether to use python or python3 configurable with a flag/argument to GenerateFancyCaptchas.php... Right?

MoritzMuehlenhoff added a comment.Dec 1 2020, 7:01 PM
In T268468#6660710, @Reedy wrote:

I'm guessing there's no reason to actually make the python version (ie whether to use python or python3 configurable with a flag/argument to GenerateFancyCaptchas.php... Right?

No, I can't think of any reason. Python 2 is dead since almost a year and even super-old distros have some version of Python 3.

gerritbot added a comment.Dec 1 2020, 7:11 PM

Change 644532 merged by Dzahn:
[operations/puppet@production] Extend PIL Python package with Python 3 counterparts

https://gerrit.wikimedia.org/r/644532

Dzahn subscribed.Dec 2 2020, 4:41 PM

python3-pil is now installed across the board:

https://debmonitor.wikimedia.org/packages/python3-pil

gerritbot added a comment.Dec 13 2020, 8:11 PM

Change 644528 merged by jenkins-bot:
[mediawiki/extensions/ConfirmEdit@master] Update captch(-old)?.py shebang to python 3

https://gerrit.wikimedia.org/r/644528

gerritbot added a comment.Dec 13 2020, 8:15 PM

Change 644586 merged by jenkins-bot:
[mediawiki/extensions/ConfirmEdit@master] Make GenerateFancyCaptchas.php execute captcha(-old)?.py via python3

https://gerrit.wikimedia.org/r/644586

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.22; 2020-12-15).Dec 13 2020, 9:00 PM
Maintenance_bot removed a project: Patch-For-Review.Dec 13 2020, 9:10 PM
LSobanski subscribed.Jan 10 2023, 12:22 PM

@Reedy @MoritzMuehlenhoff is there anything else left to do here or can the task be resolved?

MoritzMuehlenhoff closed this task as Resolved.Jan 10 2023, 12:27 PM
MoritzMuehlenhoff claimed this task.

This is done, the ConfirmEdit extension as deployed in production uses Python 3 and then Puppet manifests are installing python3-pil. Resolving.

gerritbot added a comment.Nov 21 2023, 12:14 PM

Change 976202 had a related patch set uploaded (by Muehlenhoff; author: Muehlenhoff):

[operations/puppet@production] mediawiki::packages: Drop python-pil

https://gerrit.wikimedia.org/r/976202

gerritbot added a project: Patch-For-Review.Nov 21 2023, 12:14 PM
gerritbot added a comment.Nov 23 2023, 7:38 AM

Change 976202 merged by Muehlenhoff:

[operations/puppet@production] mediawiki::packages: Drop python-pil

https://gerrit.wikimedia.org/r/976202

Maintenance_bot removed a project: Patch-For-Review.Nov 23 2023, 8:10 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits