threads / patch / 32223

patchsvnrdump_sim: start the script with /usr/bin/env python

Subject: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

## tl;dr

8 messages between Nov 28, 2012 and Nov 28, 2012. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Christian Couder· Nov 28, 2012, 02:57 UTC · lore

All the python scripts except contrib/svn-fe/svnrdump_sim.py start with "#!/usr/bin/env python".

This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 contrib/svn-fe/svnrdump_sim.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to contrib/svn-fe/svnrdump_sim.py +1 −1

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..d219180 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 """
 Simulates svnrdump by replaying an existing dump from a file, taking care
 of the specified revision range.

-- 
1.7.11.rc3.17.g55b3f8c

Junio C Hamano· Nov 28, 2012, 07:36 UTC · re: Christian Couder · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

Christian Couder <chriscool@tuxfamily.org> writes:

> All the python scripts except contrib/svn-fe/svnrdump_sim.py
> start with "#!/usr/bin/env python".
>
> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.

I suspect you need a bit more than that.

    $ make git-p4
    $ diff -u git-p4.py git-p4

shows you how we tell the scripts how to find their interpreters (that way, there is no need to rely on the existence of /usr/bin/env).

Show 16 quoted lines

>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  contrib/svn-fe/svnrdump_sim.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
> index 1cfac4a..d219180 100755
> --- a/contrib/svn-fe/svnrdump_sim.py
> +++ b/contrib/svn-fe/svnrdump_sim.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/env python
>  """
>  Simulates svnrdump by replaying an existing dump from a file, taking care
>  of the specified revision range.

Felipe Contreras· Nov 28, 2012, 08:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> All the python scripts except contrib/svn-fe/svnrdump_sim.py
>> start with "#!/usr/bin/env python".
>>
>> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>
> I suspect you need a bit more than that.
>
>     $ make git-p4
>     $ diff -u git-p4.py git-p4
>
> shows you how we tell the scripts how to find their interpreters
> (that way, there is no need to rely on the existence of
> /usr/bin/env).

That works if somebody managed to export PYTHON_PATH, which very very often is not the case for me.

./git-p4 zsh: ./git-p4: bad interpreter: /usr/bin/python: no such file or directory

In this case git-p4.py is correct, and git-p4 is not.

Either way, this is for contrib, and we don't have a standard procedure for python scripts there. /usr/bin/env is better than nothing, and in the vast majority of cases, more than enough.

Cheers.

-- 
Felipe Contreras

Christian Couder· Nov 28, 2012, 08:55 UTC · re: Felipe Contreras · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 19 quoted lines

> On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Christian Couder <chriscool@tuxfamily.org> writes:
>>
>>> All the python scripts except contrib/svn-fe/svnrdump_sim.py
>>> start with "#!/usr/bin/env python".
>>>
>>> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>>
>> I suspect you need a bit more than that.
>>
>>     $ make git-p4
>>     $ diff -u git-p4.py git-p4
>>
>> shows you how we tell the scripts how to find their interpreters
>> (that way, there is no need to rely on the existence of
>> /usr/bin/env).
>
> That works if somebody managed to export PYTHON_PATH, which very very
> often is not the case for me.

Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh, svnrdump.py is used as is. So if your python is not /usr/bin/python, you cannot just add something to $PATH to pass the test.

Best regards, Christian.

Junio C Hamano· Nov 28, 2012, 16:57 UTC · re: Christian Couder · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

Christian Couder <christian.couder@gmail.com> writes:

Show 24 quoted lines

> On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Christian Couder <chriscool@tuxfamily.org> writes:
>>>
>>>> All the python scripts except contrib/svn-fe/svnrdump_sim.py
>>>> start with "#!/usr/bin/env python".
>>>>
>>>> This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.
>>>
>>> I suspect you need a bit more than that.
>>>
>>>     $ make git-p4
>>>     $ diff -u git-p4.py git-p4
>>>
>>> shows you how we tell the scripts how to find their interpreters
>>> (that way, there is no need to rely on the existence of
>>> /usr/bin/env).
>>
>> That works if somebody managed to export PYTHON_PATH, which very very
>> often is not the case for me.
>
> Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
> svnrdump.py is used as is.

You need a fix for that; didn't I already say "you need a bit more than that"?

Felipe Contreras· Nov 28, 2012, 19:19 UTC · re: Junio C Hamano · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:

Show 8 quoted lines

>>> That works if somebody managed to export PYTHON_PATH, which very very
>>> often is not the case for me.
>>
>> Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
>> svnrdump.py is used as is.
>
> You need a fix for that; didn't I already say "you need a bit more
> than that"?

I disagree. Most of the contrib scripts are expected to be used as they are. There's no step in the Makefile that will convert them, and it's up to each distribution to decide what to do with them. This is what Arch Linux does:

  # more contrib stuff
  cp -a ./contrib/* $pkgdir/usr/share/git/
  # scripts are for python 2.x
  sed -i 's|#![ ]*/usr/bin/env python|#!/usr/bin/env python2|' \
    $(find "$pkgdir" -name '*.py') \
    "$pkgdir"/usr/lib/git-core/git-p4 \
    "$pkgdir"/usr/share/git/gitview/gitview

At some point we might decide to change this, but at the moment contrib scripts are pretty much stand-alone.

Cheers.

-- 
Felipe Contreras

Junio C Hamano· Nov 28, 2012, 19:33 UTC · re: Felipe Contreras · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 7 quoted lines

> On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> You need a fix for that; didn't I already say "you need a bit more
>> than that"?
>
> I disagree. Most of the contrib scripts are expected to be used as
> they are.

You are only looking at one of the uses for this script, when there are two.

You are correct that distros may install with whatever tweaks of their own, and to help their tweak process (like the one that specifically notices "/usr/bin/env python" as you wrote), changing the "#!/usr/bin/python" to match others would be a good change.

But that change alone is not sufficient for this one, which is used from t/ script. You cannot treat this one like import-zips and hg-to-git that we do not use in-tree. Somewhere before t9020 uses it, it needs the treatment similar to the rewriting that is done for git-p4.py to git-p4.

Felipe Contreras· Nov 28, 2012, 21:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] svnrdump_sim: start the script with /usr/bin/env python

On Wed, Nov 28, 2012 at 8:33 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 23 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, Nov 28, 2012 at 5:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> You need a fix for that; didn't I already say "you need a bit more
>>> than that"?
>>
>> I disagree. Most of the contrib scripts are expected to be used as
>> they are.
>
> You are only looking at one of the uses for this script, when there
> are two.
>
> You are correct that distros may install with whatever tweaks of
> their own, and to help their tweak process (like the one that
> specifically notices "/usr/bin/env python" as you wrote), changing
> the "#!/usr/bin/python" to match others would be a good change.
>
> But that change alone is not sufficient for this one, which is used
> from t/ script.  You cannot treat this one like import-zips and
> hg-to-git that we do not use in-tree.  Somewhere before t9020 uses
> it, it needs the treatment similar to the rewriting that is done for
> git-p4.py to git-p4.

Unless the tests are moved to contrib, which I think is a good practice: should anything in contrib break 'make test'? I don't think so.

Cheers.

-- 
Felipe Contreras