[PATCH] Python scripts audited for minimum compatible version and checks added.

Signed-off-by: Eric S. Raymond <redacted>
---
 contrib/ciabot/ciabot.py           | 5 +++++
 contrib/fast-import/import-zips.py | 5 +++++
 contrib/hg-to-git/hg-to-git.py     | 5 +++++
 contrib/p4import/git-p4import.py   | 5 +++++
 contrib/svn-fe/svnrdump_sim.py     | 4 ++++
 git-p4.py                          | 5 +++++
 git-remote-testgit.py              | 5 +++++
 git_remote_helpers/git/__init__.py | 4 ++++
 8 files changed, 38 insertions(+)

diff --git a/contrib/ciabot/ciabot.py b/contrib/ciabot/ciabot.py
index bd24395..b55648f 100755
--- a/contrib/ciabot/ciabot.py
+++ b/contrib/ciabot/ciabot.py

@@ -50,6 +50,11 @@
 import os, sys, commands, socket, urllib
 from xml.sax.saxutils import escape
 
+if sys.hexversion < 0x02000000:
+	# The limiter is the xml.sax module
+        sys.stderr.write("import-zips.py: requires Python 2.0.0 or later.")
+        sys.exit(1)
+
 # Changeset URL prefix for your repo: when the commit ID is appended
 # to this, it should point at a CGI that will display the commit
 # through gitweb or something similar. The defaults will probably

diff --git a/contrib/fast-import/import-zips.py b/contrib/fast-import/import-zips.py
index 82f5ed3..d9ad71d 100755
--- a/contrib/fast-import/import-zips.py
+++ b/contrib/fast-import/import-zips.py

@@ -13,6 +13,11 @@ from sys import argv, exit
 from time import mktime
 from zipfile import ZipFile
 
+if sys.hexversion < 0x01060000:
+	# The limiter is the zipfile module
+        sys.stderr.write("import-zips.py: requires Python 1.6.0 or later.")
+        sys.exit(1)
+
 if len(argv) < 2:
 	print 'Usage:', argv[0], '<zipfile>...'
 	exit(1)

diff --git a/contrib/hg-to-git/hg-to-git.py b/contrib/hg-to-git/hg-to-git.py
index 046cb2b..9f39ce5 100755
--- a/contrib/hg-to-git/hg-to-git.py
+++ b/contrib/hg-to-git/hg-to-git.py

@@ -23,6 +23,11 @@ import os, os.path, sys
 import tempfile, pickle, getopt
 import re
 
+if sys.hexversion < 0x02030000:
+   # The behavior of the pickle module changed significantly in 2.3
+   sys.stderr.write("hg-to-git.py: requires Python 2.3 or later.")
+   sys.exit(1)
+
 # Maps hg version -> git version
 hgvers = {}
 # List of children for each hg revision

diff --git a/contrib/p4import/git-p4import.py b/contrib/p4import/git-p4import.py
index b6e534b..fb48e2a 100644
--- a/contrib/p4import/git-p4import.py
+++ b/contrib/p4import/git-p4import.py

@@ -14,6 +14,11 @@ import sys
 import time
 import getopt
 
+if sys.hexversion < 0x02020000:
+   # The behavior of the marshal module changed significantly in 2.2
+   sys.stderr.write("git-p4import.py: requires Python 2.2 or later.")
+   sys.exit(1)
+
 from signal import signal, \
    SIGPIPE, SIGINT, SIG_DFL, \
    default_int_handler

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..ed43dbb 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py

@@ -7,6 +7,10 @@ to the highest revision that should be available.
 """
 import sys, os
 
+if sys.hexversion < 0x02040000:
+	# The limiter is the ValueError() calls. This may be too conservative
+        sys.stderr.write("svnrdump-sim.py: requires Python 2.4 or later.")
+        sys.exit(1)
 
 def getrevlimit():
         var = 'SVNRMAX'

diff --git a/git-p4.py b/git-p4.py
index 551aec9..ec060b4 100755
--- a/git-p4.py
+++ b/git-p4.py

@@ -12,6 +12,11 @@ import optparse, sys, os, marshal, subprocess, shelve
 import tempfile, getopt, os.path, time, platform
 import re, shutil
 
+if sys.hexversion < 0x02040000:
+    # The limiter is the subprocess module
+    sys.stderr.write("git-p4.py: requires Python 2.4 or later.")
+    sys.exit(1)
+
 verbose = False
 
 # Only labels/tags matching this will be imported/exported

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..22d2eb6 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py

@@ -31,6 +31,11 @@ from git_remote_helpers.git.exporter import GitExporter
 from git_remote_helpers.git.importer import GitImporter
 from git_remote_helpers.git.non_local import NonLocalGit
 
+if sys.hexversion < 0x01050200:
+    # os.makedirs() is the limiter
+    sys.stderr.write("git-remote-testgit.py: requires Python 1.5.2 or later.")
+    sys.exit(1)
+
 def get_repo(alias, url):
     """Returns a git repository object initialized for usage.
     """

diff --git a/git_remote_helpers/git/__init__.py b/git_remote_helpers/git/__init__.py
index e69de29..776e891 100644
--- a/git_remote_helpers/git/__init__.py
+++ b/git_remote_helpers/git/__init__.py

@@ -0,0 +1,4 @@
+if sys.hexversion < 0x02040000:
+    # The limiter is the subprocess module
+    sys.stderr.write("git_remote_helpers: requires Python 2.4 or later.")
+    sys.exit(1)

-- 
1.8.1.rc2



-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

"The state calls its own violence `law', but that of the individual `crime'"
	-- Max Stirner

On Thu, Dec 20, 2012 at 09:13:37AM -0500, Eric S. Raymond wrote:

Should the error message say ciabot.py?

-Peff

Jeff King [off-list ref]:

On Thu, Dec 20, 2012 at 09:13:37AM -0500, Eric S. Raymond wrote:

quoted

diff --git a/contrib/ciabot/ciabot.py b/contrib/ciabot/ciabot.py
index bd24395..b55648f 100755
--- a/contrib/ciabot/ciabot.py
+++ b/contrib/ciabot/ciabot.py

@@ -50,6 +50,11 @@
 import os, sys, commands, socket, urllib
 from xml.sax.saxutils import escape
 
+if sys.hexversion < 0x02000000:
+	# The limiter is the xml.sax module
+        sys.stderr.write("import-zips.py: requires Python 2.0.0 or later.")
+        sys.exit(1)

Should the error message say ciabot.py?

-Peff

Gack.  Yes.  Thaty's what I get for cut-and-pasting too quickly.
The information about xnml.sex is correct, though.

Want me to resubmit, or will you just patch it?

Note by the way that I still think the entire ciabot subtree (which is 
my code) should just be nuked.  CIA is not coming back, wishful thinking 
on Ilkotech's web page notwithstanding.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 20/12/2012 15:13, Eric S. Raymond ha scritto:

Signed-off-by: Eric S. Raymond <redacted>
---

Just my two cents.
Isn't it better to have some core Python support inside a "python/"
directory in the git source tree (e.g. e simple python/git.py), and have
*all* python code import that module?

Then you can impose a reasonable version limitation (e.g. Python >= 2.5)
inside that module (and/or inside its setup.py file).

Another advantage is that the python/git.py module can contain some very
base support for interfacing git plumbing commands, instead of having
all internal (and external) modules reinventing the wheel.

I'm writing an external command for Git, and I do plan to write such a
package, so that I don't have to reimplement all the base support in my
command source code.



Regards   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDUhzoACgkQscQJ24LbaUQFuQCfb6QgkJHdxQSEB7nLXMN8TSmI
6/IAn3svylllaIBQfZKf0lEzNBtZJQMK
=Ar20
-----END PGP SIGNATURE-----

esr@thyrsus.com wrote on Thu, 20 Dec 2012 09:13 -0500:

If 2.3 does not have the subprocess module, this script will fail
at the import, and not run your version test.

All the uses of sys.stderr.write() should probably include a
newline.  Presumably you used write instead of print to avoid
2to3 differences.

The name of this particular script, as users would type it, is
"git p4"; no dash and no ".py".

Many of your changes have these three problems; I just picked on
my favorite one.

This one, though, is a bit of a lie because git_remote_helpers
needs 2.4, and you add that version enforcement in the library.

I assume what you're trying to do here is to make the
version-related failures more explicit, rather than have users
parse an ImportError traceback, e.g.  That seems somewhat useful
for people with ancient installs.

But what about the high-end of the version range?  I'm pretty
sure most of these scripts will throw syntax errors on >= 3.0,
how should we catch that before users see it?

		-- Pete

Pete Wyckoff [off-list ref]:

esr@thyrsus.com wrote on Thu, 20 Dec 2012 09:13 -0500:

quoted

diff --git a/git-p4.py b/git-p4.py
index 551aec9..ec060b4 100755
--- a/git-p4.py
+++ b/git-p4.py

@@ -12,6 +12,11 @@ import optparse, sys, os, marshal, subprocess, shelve
 import tempfile, getopt, os.path, time, platform
 import re, shutil
 
+if sys.hexversion < 0x02040000:
+    # The limiter is the subprocess module
+    sys.stderr.write("git-p4.py: requires Python 2.4 or later.")
+    sys.exit(1)
+
 verbose = False

If 2.3 does not have the subprocess module, this script will fail
at the import, and not run your version test.

Yes, the import of subprocess should move to after the check.

All the uses of sys.stderr.write() should probably include a
newline.  Presumably you used write instead of print to avoid
2to3 differences.

That is correct.
 

The name of this particular script, as users would type it, is
"git p4"; no dash and no ".py".

Many of your changes have these three problems; I just picked on
my favorite one.

Should I resubmit, or do you intend to fix these while merging?
 

quoted

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..22d2eb6 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py

@@ -31,6 +31,11 @@ from git_remote_helpers.git.exporter import GitExporter
 from git_remote_helpers.git.importer import GitImporter
 from git_remote_helpers.git.non_local import NonLocalGit
 
+if sys.hexversion < 0x01050200:
+    # os.makedirs() is the limiter
+    sys.stderr.write("git-remote-testgit.py: requires Python 1.5.2 or later.")
+    sys.exit(1)
+

This one, though, is a bit of a lie because git_remote_helpers
needs 2.4, and you add that version enforcement in the library.

Agreed. The goal here was simply to have the depedencies of the individual
scripts be clearly documented, and establish a practice for future
submitters to emulate.

I assume what you're trying to do here is to make the
version-related failures more explicit, rather than have users
parse an ImportError traceback, e.g.

See above.  At least half the point is making our dependencies
explicit rather than implicit, so we can make better policy
decisions.

But what about the high-end of the version range?  I'm pretty
sure most of these scripts will throw syntax errors on >= 3.0,
how should we catch that before users see it?

That's a problem for another day, when 3.x is more widely deployed.
I'd be willing to run 2to3 on these scripts and check forward 
compatibility.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>