[PATCH next 1/2] Makefile: add PYTHON_PATH to GIT-BUILD-OPTIONS

[PATCH next 1/2] Makefile: add PYTHON_PATH to GIT-BUILD-OPTIONS

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

The PYTHON_PATH environment variable is not set when running test scripts
manually i.e. when not using 'make test'.  Scripts which attempt to use
this variable will fail.  So add it to the list of variables written to
the GIT-BUILD-OPTIONS file so that the test suite will import it when
running the test scripts.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 34b7dd5..b529340 100644
--- a/Makefile
+++ b/Makefile
@@ -1949,6 +1949,7 @@ GIT-CFLAGS: FORCE
 GIT-BUILD-OPTIONS: FORCE
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
 	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@
+	@echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' >>$@
 	@echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' >>$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
 	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
-- 
1.6.6.2

[PATCH next 2/2] Remove python 2.5'isms

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

The following python 2.5 features were worked around:

    * the hashlib module was replaced with the sha module
    * the 'any' built-in method was replaced with a 'for' loop
    * a conditional expression was replaced with an 'if' statement
    * the subprocess.check_call method was replaced by a call to
      subprocess.Popen followed by a call to subprocess.wait with a
      check of its return status

These changes allow the python infrastructure to be used with python 2.4
which is distributed with RedHat's RHEL 5, for example.

t5800 was updated to check for python >= 2.4 to reflect these changes.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 git-remote-testgit.py               |    9 ++++++---
 git_remote_helpers/git/exporter.py  |    4 +++-
 git_remote_helpers/git/importer.py  |    4 +++-
 git_remote_helpers/git/non_local.py |   16 ++++++++++++----
 git_remote_helpers/git/repo.py      |    9 +++++++--
 t/t5800-remote-helpers.sh           |    4 ++--
 6 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 9253922..6356d48 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 
-import hashlib
+import sha
 import sys
 import os
 sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
@@ -19,7 +19,7 @@ def get_repo(alias, url):
     repo.get_revs()
     repo.get_head()
 
-    hasher = hashlib.sha1()
+    hasher = sha.new()
     hasher.update(repo.path)
     repo.hash = hasher.hexdigest()
 
@@ -133,7 +133,10 @@ def do_export(repo, args):
 
     path = os.path.join(dirname, 'testgit.marks')
     print path
-    print path if os.path.exists(path) else ""
+    if os.path.exists(path):
+        print path
+    else:
+        print ""
     sys.stdout.flush()
 
     update_local_repo(repo)
diff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py
index dfaab00..f40f9d6 100644
--- a/git_remote_helpers/git/exporter.py
+++ b/git_remote_helpers/git/exporter.py
@@ -48,4 +48,6 @@ class GitExporter(object):
 
         args = ["sed", "s_refs/heads/_" + self.repo.prefix + "_g"]
 
-        subprocess.check_call(args, stdin=p1.stdout)
+        child = subprocess.Popen(args, stdin=p1.stdout)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index af2919d..70a7127 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -35,4 +35,6 @@ class GitImporter(object):
         if os.path.exists(path):
             args.append("--import-marks=" + path)
 
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py
index d75ef8f..f27389b 100644
--- a/git_remote_helpers/git/non_local.py
+++ b/git_remote_helpers/git/non_local.py
@@ -29,7 +29,9 @@ class NonLocalGit(object):
         os.makedirs(path)
         args = ["git", "clone", "--bare", "--quiet", self.repo.gitpath, path]
 
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
         return path
 
@@ -43,10 +45,14 @@ class NonLocalGit(object):
             die("could not find repo at %s", path)
 
         args = ["git", "--git-dir=" + path, "fetch", "--quiet", self.repo.gitpath]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
         args = ["git", "--git-dir=" + path, "update-ref", "refs/heads/master", "FETCH_HEAD"]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
     def push(self, base):
         """Pushes from the non-local repo to base.
@@ -58,4 +64,6 @@ class NonLocalGit(object):
             die("could not find repo at %s", path)
 
         args = ["git", "--git-dir=" + path, "push", "--quiet", self.repo.gitpath]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/repo.py b/git_remote_helpers/git/repo.py
index 82d5f78..58e1cdb 100644
--- a/git_remote_helpers/git/repo.py
+++ b/git_remote_helpers/git/repo.py
@@ -19,7 +19,10 @@ def is_remote(url):
 
     prefixes = ["http", "file", "git"]
 
-    return any(url.startswith(i) for i in prefixes)
+    for prefix in prefixes:
+        if url.startswith(prefix):
+            return True
+    return False
 
 class GitRepo(object):
     """Repo object representing a repo.
@@ -50,7 +53,9 @@ class GitRepo(object):
         path = ".cached_revs"
         ofile = open(path, "w")
 
-        subprocess.check_call(args, stdout=ofile)
+        child = subprocess.Popen(args, stdout=ofile)
+        if child.wait() != 0:
+            raise CalledProcessError
         output = open(path).readlines()
         self.revmap = dict(sanitize(i) for i in output)
         if "HEAD" in self.revmap:
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index 22c7df4..4ee7b65 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -9,13 +9,13 @@ test_description='Test remote-helper import and export commands'
 
 if test_have_prereq PYTHON && "$PYTHON_PATH" -c '
 import sys
-if sys.hexversion < 0x02050000:
+if sys.hexversion < 0x02040000:
     sys.exit(1)
 '
 then
 	:
 else
-	say 'skipping git remote-testgit tests: requires Python 2.5 or newer'
+	say 'skipping git remote-testgit tests: requires Python 2.4 or newer'
 	test_done
 fi
 
-- 
1.6.6.2

Re: [PATCH next 2/2] Remove python 2.5'isms

[Sverre Rabbelier]

Heya,

On Wed, Jun 9, 2010 at 23:24, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> The following python 2.5 features were worked around:

Thanks for fixing this, although it makes me sad to see the code
uglify the way it has :P. I guess that's the pain of backwards
compatibility.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH next 2/2] Remove python 2.5'isms

[Johan Herland]

On Wednesday 09 June 2010, Sverre Rabbelier wrote:
> On Wed, Jun 9, 2010 at 23:24, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> > The following python 2.5 features were worked around:
>
> Thanks for fixing this, although it makes me sad to see the code
> uglify the way it has :P. I guess that's the pain of backwards
> compatibility.

I agree with Sverre's painful acceptance, but I believe something must be 
done with the s/hashlib/sha/ changes.

On my Python v2.6.5. I get the following:

>>> import sha
__main__:1: DeprecationWarning: the sha module is deprecated; use the 
hashlib module instead

This is added to the output of every python program that imports sha. I 
believe we should really try to use the hashlib module when available, and 
only fall back to sha when necessary. Please use something like this 
instead:

  # hashlib is only available in python >= 2.5 
  try: 
      import hashlib 
      _digest = hashlib.sha1 
  except ImportError: 
      import sha
      _digest = sha.sha 

so that we don't get the hideous DeprecationWarning.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH next 2/2] Remove python 2.5'isms

[Sverre Rabbelier]

Heya,

On Thu, Jun 10, 2010 at 00:40, Johan Herland <johan@herland.net> wrote:
>  # hashlib is only available in python >= 2.5
>  try:
>      import hashlib
>      _digest = hashlib.sha1
>  except ImportError:
>      import sha
>      _digest = sha.sha
>
> so that we don't get the hideous DeprecationWarning.

This is exactly what I meant to suggest earlier, but I didn't know the
appropriate routine in python 2.4, thanks Johan.

-- 
Cheers,

Sverre Rabbelier

[PATCH next 2/2 v2] Remove python 2.5'isms

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

The following python 2.5 features were worked around:

    * the sha module is used as a fallback when the hashlib module is
      not available
    * the 'any' built-in method was replaced with a 'for' loop
    * a conditional expression was replaced with an 'if' statement
    * the subprocess.check_call method was replaced by a call to
      subprocess.Popen followed by a call to subprocess.wait with a
      check of its return status

These changes allow the python infrastructure to be used with python 2.4
which is distributed with RedHat's RHEL 5, for example.

t5800 was updated to check for python >= 2.4 to reflect these changes.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


On 06/09/2010 05:40 PM, Johan Herland wrote:

> On my Python v2.6.5. I get the following:
> 
>>>> import sha
> __main__:1: DeprecationWarning: the sha module is deprecated; use the 
> hashlib module instead
> 
> This is added to the output of every python program that imports sha. I 
> believe we should really try to use the hashlib module when available, and 
> only fall back to sha when necessary. Please use something like this 
> instead:
> 
>   # hashlib is only available in python >= 2.5 
>   try: 
>       import hashlib 
>       _digest = hashlib.sha1 
>   except ImportError: 
>       import sha
>       _digest = sha.sha 
> 
> so that we don't get the hideous DeprecationWarning.

Like this? ...


 git-remote-testgit.py               |   15 ++++++++++++---
 git_remote_helpers/git/exporter.py  |    4 +++-
 git_remote_helpers/git/importer.py  |    4 +++-
 git_remote_helpers/git/non_local.py |   16 ++++++++++++----
 git_remote_helpers/git/repo.py      |    9 +++++++--
 t/t5800-remote-helpers.sh           |    4 ++--
 6 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 9253922..df9d512 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -1,6 +1,12 @@
 #!/usr/bin/env python
 
-import hashlib
+# hashlib is only available in python >= 2.5
+try:
+    import hashlib
+    _digest = hashlib.sha1
+except ImportError:
+    import sha
+    _digest = sha.new
 import sys
 import os
 sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
@@ -19,7 +25,7 @@ def get_repo(alias, url):
     repo.get_revs()
     repo.get_head()
 
-    hasher = hashlib.sha1()
+    hasher = _digest()
     hasher.update(repo.path)
     repo.hash = hasher.hexdigest()
 
@@ -133,7 +139,10 @@ def do_export(repo, args):
 
     path = os.path.join(dirname, 'testgit.marks')
     print path
-    print path if os.path.exists(path) else ""
+    if os.path.exists(path):
+        print path
+    else:
+        print ""
     sys.stdout.flush()
 
     update_local_repo(repo)
diff --git a/git_remote_helpers/git/exporter.py b/git_remote_helpers/git/exporter.py
index dfaab00..f40f9d6 100644
--- a/git_remote_helpers/git/exporter.py
+++ b/git_remote_helpers/git/exporter.py
@@ -48,4 +48,6 @@ class GitExporter(object):
 
         args = ["sed", "s_refs/heads/_" + self.repo.prefix + "_g"]
 
-        subprocess.check_call(args, stdin=p1.stdout)
+        child = subprocess.Popen(args, stdin=p1.stdout)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/importer.py b/git_remote_helpers/git/importer.py
index af2919d..70a7127 100644
--- a/git_remote_helpers/git/importer.py
+++ b/git_remote_helpers/git/importer.py
@@ -35,4 +35,6 @@ class GitImporter(object):
         if os.path.exists(path):
             args.append("--import-marks=" + path)
 
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/non_local.py b/git_remote_helpers/git/non_local.py
index d75ef8f..f27389b 100644
--- a/git_remote_helpers/git/non_local.py
+++ b/git_remote_helpers/git/non_local.py
@@ -29,7 +29,9 @@ class NonLocalGit(object):
         os.makedirs(path)
         args = ["git", "clone", "--bare", "--quiet", self.repo.gitpath, path]
 
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
         return path
 
@@ -43,10 +45,14 @@ class NonLocalGit(object):
             die("could not find repo at %s", path)
 
         args = ["git", "--git-dir=" + path, "fetch", "--quiet", self.repo.gitpath]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
         args = ["git", "--git-dir=" + path, "update-ref", "refs/heads/master", "FETCH_HEAD"]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
 
     def push(self, base):
         """Pushes from the non-local repo to base.
@@ -58,4 +64,6 @@ class NonLocalGit(object):
             die("could not find repo at %s", path)
 
         args = ["git", "--git-dir=" + path, "push", "--quiet", self.repo.gitpath]
-        subprocess.check_call(args)
+        child = subprocess.Popen(args)
+        if child.wait() != 0:
+            raise CalledProcessError
diff --git a/git_remote_helpers/git/repo.py b/git_remote_helpers/git/repo.py
index 82d5f78..58e1cdb 100644
--- a/git_remote_helpers/git/repo.py
+++ b/git_remote_helpers/git/repo.py
@@ -19,7 +19,10 @@ def is_remote(url):
 
     prefixes = ["http", "file", "git"]
 
-    return any(url.startswith(i) for i in prefixes)
+    for prefix in prefixes:
+        if url.startswith(prefix):
+            return True
+    return False
 
 class GitRepo(object):
     """Repo object representing a repo.
@@ -50,7 +53,9 @@ class GitRepo(object):
         path = ".cached_revs"
         ofile = open(path, "w")
 
-        subprocess.check_call(args, stdout=ofile)
+        child = subprocess.Popen(args, stdout=ofile)
+        if child.wait() != 0:
+            raise CalledProcessError
         output = open(path).readlines()
         self.revmap = dict(sanitize(i) for i in output)
         if "HEAD" in self.revmap:
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
index 22c7df4..4ee7b65 100755
--- a/t/t5800-remote-helpers.sh
+++ b/t/t5800-remote-helpers.sh
@@ -9,13 +9,13 @@ test_description='Test remote-helper import and export commands'
 
 if test_have_prereq PYTHON && "$PYTHON_PATH" -c '
 import sys
-if sys.hexversion < 0x02050000:
+if sys.hexversion < 0x02040000:
     sys.exit(1)
 '
 then
 	:
 else
-	say 'skipping git remote-testgit tests: requires Python 2.5 or newer'
+	say 'skipping git remote-testgit tests: requires Python 2.4 or newer'
 	test_done
 fi
 
-- 
1.6.6.2

Re: [PATCH next 2/2 v2] Remove python 2.5'isms

[Sverre Rabbelier]

Heya,

On Thu, Jun 10, 2010 at 02:24, Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Like this? ...

Assuming it does the right thing (can't test right now), yes :).

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH next 2/2] Remove python 2.5'isms

[Michael J Gruber]

Sverre Rabbelier venit, vidit, dixit 09.06.2010 23:28:
> Heya,
> 
> On Wed, Jun 9, 2010 at 23:24, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> The following python 2.5 features were worked around:
> 
> Thanks for fixing this, although it makes me sad to see the code
> uglify the way it has :P. I guess that's the pain of backwards
> compatibility.
> 

I doesn't need to be ugly. Why not put the Popen thing in a def
somewhere? Is there any remaining uglyness besides this:

-    return any(url.startswith(i) for i in prefixes)
+    for prefix in prefixes:
+        if url.startswith(prefix):
+            return True
+    return False

Pythonish version:
-    return any(url.startswith(i) for i in prefixes)
+    return True in (url.startswith(i) for i in prefixes)

Note that due to the use of an iterator (which P2.4 has) rather than a
list or set, the expression is just as efficient, i.e. calls
startswith() only as often as necessary.

Michael

Re: [PATCH next 2/2] Remove python 2.5'isms

[Sverre Rabbelier]

Heya,

On Thu, Jun 10, 2010 at 09:31, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Note that due to the use of an iterator (which P2.4 has) rather than a
> list or set, the expression is just as efficient, i.e. calls
> startswith() only as often as necessary.

I like that version a lot better, thanks. And I agree, the check_call
replacement code should just be put into it's own function in a
similar way, and we should use subprocess if it is available.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH next 2/2 v2] Remove python 2.5'isms

[Michael J Gruber]

Brandon Casey venit, vidit, dixit 10.06.2010 02:24:
> From: Brandon Casey <drafnel@gmail.com>
> 
> The following python 2.5 features were worked around:
> 
>     * the sha module is used as a fallback when the hashlib module is
>       not available
>     * the 'any' built-in method was replaced with a 'for' loop
>     * a conditional expression was replaced with an 'if' statement
>     * the subprocess.check_call method was replaced by a call to
>       subprocess.Popen followed by a call to subprocess.wait with a
>       check of its return status
> 
> These changes allow the python infrastructure to be used with python 2.4
> which is distributed with RedHat's RHEL 5, for example.
> 
> t5800 was updated to check for python >= 2.4 to reflect these changes.
> 
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> 
> 
> On 06/09/2010 05:40 PM, Johan Herland wrote:
> 
>> On my Python v2.6.5. I get the following:
>>
>>>>> import sha
>> __main__:1: DeprecationWarning: the sha module is deprecated; use the 
>> hashlib module instead
>>
>> This is added to the output of every python program that imports sha. I 
>> believe we should really try to use the hashlib module when available, and 
>> only fall back to sha when necessary. Please use something like this 
>> instead:
>>
>>   # hashlib is only available in python >= 2.5 
>>   try: 
>>       import hashlib 
>>       _digest = hashlib.sha1 
>>   except ImportError: 
>>       import sha
>>       _digest = sha.sha 
>>
>> so that we don't get the hideous DeprecationWarning.
> 
> Like this? ...
> 
> 
>  git-remote-testgit.py               |   15 ++++++++++++---
>  git_remote_helpers/git/exporter.py  |    4 +++-
>  git_remote_helpers/git/importer.py  |    4 +++-
>  git_remote_helpers/git/non_local.py |   16 ++++++++++++----
>  git_remote_helpers/git/repo.py      |    9 +++++++--
>  t/t5800-remote-helpers.sh           |    4 ++--
>  6 files changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/git-remote-testgit.py b/git-remote-testgit.py
> index 9253922..df9d512 100644
> --- a/git-remote-testgit.py
> +++ b/git-remote-testgit.py
> @@ -1,6 +1,12 @@
>  #!/usr/bin/env python
>  
> -import hashlib
> +# hashlib is only available in python >= 2.5
> +try:
> +    import hashlib
> +    _digest = hashlib.sha1
> +except ImportError:
> +    import sha
> +    _digest = sha.new
>  import sys
>  import os
>  sys.path.insert(0, os.getenv("GITPYTHONLIB","."))

Doesn't P2.4 have

	from hashlib import sha1 as _digest

etc.? I haven't checked for "as".

Michael

Re: [PATCH next 2/2 v2] Remove python 2.5'isms

[David Ripton]

On 2010.06.10 11:28:15 +0200, Michael J Gruber wrote:
> Doesn't P2.4 have
> 
> 	from hashlib import sha1 as _digest
> 
> etc.? I haven't checked for "as".

Hashlib was new in Python 2.5.  See the note at the top of:
http://docs.python.org/library/hashlib.html

You may have hashlib in your Python 2.4, but if so it's a third-party
module, not part of the standard library.  You shouldn't rely on
everyone having it, when it's so easy to fallback to the sha module.

"import foo as bar" was added in Python 2.0.  "as" became a reserved
word in Python 2.6.

-- 
David Ripton    dripton@ripton.net

Re: [PATCH next 2/2 v2] Remove python 2.5'isms

[Michael J Gruber]

David Ripton venit, vidit, dixit 10.06.2010 17:41:
> On 2010.06.10 11:28:15 +0200, Michael J Gruber wrote:
>> Doesn't P2.4 have
>>
>> 	from hashlib import sha1 as _digest
>>
>> etc.? I haven't checked for "as".
> 
> Hashlib was new in Python 2.5.  See the note at the top of:
> http://docs.python.org/library/hashlib.html
> 
> You may have hashlib in your Python 2.4, but if so it's a third-party
> module, not part of the standard library.  You shouldn't rely on
> everyone having it, when it's so easy to fallback to the sha module.

Well sure, that's why we need the try/except's.
> 
> "import foo as bar" was added in Python 2.0.  "as" became a reserved
> word in Python 2.6.

Thanks!. Then we can use the clean version as suggested above (inside
try/except).

Michael