Python help needed

[Bruno Postle]

Hi all, Hugin has a python API which is currently a little bit broken and neglected.

So can we have a volunteer to fix it up? The alternative is to remove it altogether. It isn't a lot of work, these are the problems I'm aware of:

1. The python scripts all call #!/usr/bin/env python, which may randomly call either python2 or python3, which is silly because the Hugin library will only ever be built for one or the other.

2. At this point in time, supporting python2 is excessive, the cmake build needs fixing so that only a python3 API can be built (which will also help fix the above problem).

3. The scripts only run when an api-max test succeeds. Since nobody remembers to check the scripts and increment this number for every Hugin release, it guarantees that the python scripts are always broken. This test should be removed altogether.

Any volunteers?

--
Bruno

[Dr Rainer Woitok]

Bruno,

On Friday, 2019-05-17 14:12:59 +0100, you wrote:

> ...

> 1. The python scripts all call #!/usr/bin/env python, which may randomly call either python2 or python3, which is silly because the Hugin library will only ever be built for one or the other.

If I didn't completely missunderstand the problem, this seems easy: just
use

#! /usr/bin/env/python2

assuming your scripts originally ran under Python 2.7 and nobody has yet
converted them to Python 3.5, and assuming the Hugin library is current-
ly being build for Python 2.7. This will buy you one and a half years
until Python 2.7 support will end.

Sincerely,
Rainer

[Harry van der Wolf]

Hi,

Op vr 17 mei 2019 om 17:27 schreef Dr Rainer Woitok <rainer...@gmail.com>:

If I didn't completely missunderstand the problem, this seems easy: just
use

   #! /usr/bin/env/python2

If using this work-around, the shebang actually should not be "#! /usr/bin/env/python2". 

It should be "#!/usr/bin/env python2". No space between "#! /" and a space between the env command and python2. 

And it will not work if hugin and the hsi is compiled against python 3 (like compiled from  /usr/include/python3.5)

But I agree with Bruno: Some volunteer should fix it, or it should be removed.

Harry

[Bruno Postle]

On 17 May 2019 22:14:09 BST, Harry van der Wolf wrote:
>
>But I agree with Bruno: Some volunteer should fix it, or it should be
>removed.

Thanks, I'm sure I could fix it too, but if nobody actually wants the python interface, there is no point maintaining it. All we need is somebody who values it to step up and demonstrate that it is valued.

--
Bruno

[kfj]

Sorry, I'm a bit late noticing this thread.

Okay, here I am standing up for the python interface. I regularly use the woa script. But I got frustrated by the disappearance of the python interface with every new release because nobody thought of adapting the api-max values in the scripts. These api-min and api-max values were Yuv's idea, and have consistently led to making the python interface unusable with every new release; for some time I sent bug notifications but grew tired of it - instead I just adapted my local repo clone. Since the last update to the api-max values seems to have happened in 2017, I suppose noone apart from me misses the python interface, but there's no need to deliberately cripple it by wrong api-max values. The python interface adapts automatically to new hugin versions (because SWIG looks at hugin's source code), and the worst thing that can happen is that some old python script doesn't work anymore because it tries to use an old API - as far as I remember this has happened only once since I wrote the python interface, and ll other failures were due to wrong api-max values.

So I propose to remove the API check, I think it's useless.

Kay

--
Bruno

[Bruno Postle]

Great, can you do a patch to remove the api-max check, and force python3 throughout?

--
Bruno

[kfj]

On Thursday, July 18, 2019 at 9:57:35 PM UTC+2, Bruno Postle wrote:

Great, can you do a patch to remove the api-max check, and force python3 throughout?

I don't know (yet) how to remove the api checking code. But that's not a problem: the api-check entries in the plugins are not mandatory. If they are simply removed from the plugins, the plugins will try to run with whatever API hugin offers, and simply fail to function if the API is not right. So the quickest way to disable the API check is simply to remove these two lines from all plugins:

# @api-min  2014.0
# @api-max  1029.0

If, on the hugin side, somone feels that the code to do the API check is in their way, they would be free to remove it. That's an area of the code I am not familiar with - I may look into it if I find the time. But then, if the plugin thing should ever become popular, maybe having the *option* to limit plugins to a certain API subset is nice to have. I'd say leave the code in, just remove the checks from the plugins.

I also propose to remove the plugin src/hugin_script_interface/plugins/crop_cp.py: the CP tool in the openGL viewer is better, and the plugin tries to use wxPython, which produces added complications. Best to get rid of it.

I think those changes are simple enough to do by someone who has repo write access, but if you'd like a patch I can probably put one together.

When it comes to picking only python3, I'm not sure how to do that, but I'll try and look into the matter eventually (busy with other stuff now). Can we postpone this to some later time, or can someone else do that bit?

Hope that helps

Kay

[Terry Duell]

On Fri, 19 Jul 2019 18:40:22 +1000, 'kfj' via hugin and other free
panoramic software <hugi...@googlegroups.com> wrote:

[snip]

> When it comes to picking only python3, I'm not sure how to do that, but
> I'll try and look into the matter eventually (busy with other stuff now).
> Can we postpone this to some later time, or can someone else do that bit?

There is a bug report related to this issue, (
https://bugs.launchpad.net/bugs/1829309) in which I have provided a
patch, which works here for me, but for some reason I was unable to push
to the repo. I had hoped another with admin privileges would push it but
that doesn't seem to have happened.
Have a look at that patch, and the source, and see if you think it is an
adequate fix.

Cheers,
--
Regards,
Terry Duell

[kfj]

On Saturday, July 20, 2019 at 1:13:09 AM UTC+2, Tduell wrote:

On Fri, 19 Jul 2019 18:40:22 +1000, 'kfj' via hugin and other free  
panoramic software <hugi...@googlegroups.com> wrote:

[snip]

> When it comes to picking only python3, I'm not sure how to do that, but
> I'll try and look into the matter eventually (busy with other stuff now).
> Can we postpone this to some later time, or can someone else do that bit?

There is a bug report related to this issue, (  
https://bugs.launchpad.net/bugs/1829309)  in which I have provided a  
patch, which works here for me, but for some reason I was unable to push  
to the repo. I had hoped another with admin privileges would push it but  
that doesn't seem to have happened.

I don't have admin privileges either, and won't ever have because I won't do sourceforge.

Have a look at that patch, and the source, and see if you think it is an  
adequate fix.

It seems to do what it's supposed to do - it builds and works okay on my system (ubuntu 16.04 LTS), but I have both python2 and python3 on this machine. How about older systems which only have python2? Wouldn't trying to force python3 on such machines fail, because they simply don't have it?

Kay

[Terry Duell]

On Sat, 20 Jul 2019 17:39:51 +1000, 'kfj' via hugin and other free

panoramic software <hugi...@googlegroups.com> wrote:

[snip]

> I don't have admin privileges either, and won't ever have because I won't
> do sourceforge.

OK, then I suspect you may be able to manage this by testing your changes
locally, then attach a patch to a bug report asking for an admin to commit
and push the changes.

>
> Have a look at that patch, and the source, and see if you think it is an
>> adequate fix.
>>
>
> It seems to do what it's supposed to do - it builds and works okay on my
> system (ubuntu 16.04 LTS), but I have both python2 and python3 on this
> machine. How about older systems which only have python2? Wouldn't trying
> to force python3 on such machines fail, because they simply don't have
> it?

I wouldn't have a clue if there are distros still using Python2.
If it's an issue is there a way you can test which version of Python and
then use appropriate code?
I'm sure others here will have some ideas about this.

[kfj]

On Saturday, July 20, 2019 at 1:13:09 AM UTC+2, Tduell wrote:

I checked some more, and came to the conclusion that the 'shebang line' ,' #! /usr/bin/env python' in the plugins is irrelevant when they are launched as plugins by hugin. Hugin, with the python interface linked in, *is* a python interpreter, and it's either python2 or python3, depending on what cmake figures out for it. The shebang, is only relevant when the plugin is executed as a stand-alone script: Some of the plugins (like woa.py) are programs in their own right, and the #! /usr/bin/env python tells the shell to launch 'python' to interpret the script. So no matter what the shebang is, when used as plugins from hugin, the shebang has no effect. There is no need to make any change to the plugin code.

I think forcing the linking-in of a python3 interpreter should be handled by cmake. On my system, for example (I mistyped, I'm on ubuntu 18.04 LTS), when I build hugin as per instructions in the wiki, it comes out a python2 interpreter. There's docu I found here

https://cmake.org/cmake/help/git-stage/module/FindPython3.html

Does that help?

Kay

[Terry Duell]

Hello kfj,

On Sun, 21 Jul 2019 03:10:13 +1000, 'kfj' via hugin and other free

panoramic software <hugi...@googlegroups.com> wrote:

[snip]

>

> I checked some more, and came to the conclusion that the 'shebang line'
> ,'
> #! /usr/bin/env python' in the plugins is irrelevant when they are
> launched as plugins by hugin. Hugin, with the python interface linked
> in, *is* a
> python interpreter, and it's either python2 or python3, depending on what
> cmake figures out for it. The shebang, is only relevant when the plugin
> is executed as a stand-alone script: Some of the plugins (like woa.py)
> are
> programs in their own right, and the #! /usr/bin/env python tells the
> shell to launch 'python' to interpret the script. So no matter what the
> shebang
> is, when used as plugins from hugin, the shebang has no effect. There is
> no need to make any change to the plugin code.
>
> I think forcing the linking-in of a python3 interpreter should be handled
> by cmake. On my system, for example (I mistyped, I'm on ubuntu 18.04
> LTS), when I build hugin as per instructions in the wiki, it comes out a
> python2 interpreter. There's docu I found here
>
> https://cmake.org/cmake/help/git-stage/module/FindPython3.html
>
> Does that help?
>

I don't think I can make any useful comment on the above.
I needed to overcome a build a problem to enable testing of other code
changes, and simply implemented a suggestion that was provided and then
provided the resulting patch.
I would only add that you should probably just get on and test your ideas
and when satisfied propose a patch.
Re your earlier comment that you don't use Sourceforge, are you not
intending to maintain a local hg repository?

[Harry van der Wolf]

Op za 20 jul. 2019 om 19:10 schreef 'kfj' via hugin and other free panoramic software <hugi...@googlegroups.com>:

On Saturday, July 20, 2019 at 1:13:09 AM UTC+2, Tduell wrote:

On Fri, 19 Jul 2019 18:40:22 +1000, 'kfj' via hugin and other free   

I think forcing the linking-in of a python3 interpreter should be handled by cmake. On my system, for example (I mistyped, I'm on ubuntu 18.04 LTS), when I build hugin as per instructions in the wiki, it comes out a python2 interpreter. There's docu I found here

https://cmake.org/cmake/help/git-stage/module/FindPython3.html

Does that help?

Kay

I think we can simply say that python 3 should be mandatory. python 3.0 was released in December 2008. And python 3.0 is long out of support.

Any OS of around that date is already out of support: patches, virus, etc.  Even the long LTS Ubuntu versions and relatively long supported Debian versions are out of support.

Windows 7 was released in 2009 and is out of service January 2020. Windows 8 out of support. Windows 8.1 still quite some years to go. 

Python.org really promotes to use 3.x. Why would any "new" user try to download 2.7? Most Windows users want to have the "latest and greatest" anyway (sorry, my prejudice).

MacOS updates are only bound by older hardware, otherwise upgrades are free and easy to install. MacOS is always running (way) behind with python but that can easily be upgraded.

If you want an old (unsupported) OS with python 2.7, you can simply take an older Hugin version. All Hugin versions since 2009~2011 function very nice. Use a "classic" Hugin on a "classic" OS.

But that's my personal view on supporting "old stuff".

Harry

[Kay F. Jahnke]

On 21.07.19 10:16, Harry van der Wolf wrote:
>
> I think we can simply say that python 3 should be mandatory.

Okay, fine, I have no problem with python3. Make it mandatory. All the
python code in hugin should work with python3.

Kay

[Kay F. Jahnke]

What I mean is that the shebang is irrelevant for plugins. I think it's
only the build system complaining: it sees a python script, looks into
it and notices that the shebang is ambiguous. But since the python
interpreter linked into hugin processes the script, this is a non-issue.
Your patch satisfies the fedora build system, but it makes no difference
apart from that.

> I would only add that you should probably just get on and test your
> ideas and when satisfied propose a patch.

I attach a patch removing the shebangs and the api-min and api-max. I
did the patch with hg diff --git, hope that's all right. This patch
should fix both the api-check issue and the shebang problem, and removes
crop_cp.py which I think is no longer useful. It does not address the
enforcement of python3-only.

> Re your earlier comment that you don't use Sourceforge, are you not
> intending to maintain a local hg repository?

Of course I do have a local repo, but I don't have a sourceforge
account. I'd prefer if the code were on github or bitbucket.

Kay

[Terry Duell]

On Mon, 22 Jul 2019 01:40:57 +1000, 'Kay F. Jahnke' via hugin and other

free panoramic software <hugi...@googlegroups.com> wrote:


[snip]

> I attach a patch removing the shebangs and the api-min and api-max. I
> did the patch with hg diff --git, hope that's all right. This patch
> should fix both the api-check issue and the shebang problem, and removes
> crop_cp.py which I think is no longer useful. It does not address the
> enforcement of python3-only.

If you have tested the patch and are satisfied that the patch is OK, then
I'd suggest adding it to the bug report, that will ensure that it is seen
by other 'bughunters' and developers, and hopefully be tested on other
systems.
I haven't tested it on Fedora 30 builds as yet.

[Terry Duell]

Hello Kay,

On Mon, 22 Jul 2019 01:40:57 +1000, 'Kay F. Jahnke' via hugin and other

free panoramic software <hugi...@googlegroups.com> wrote:

[snip]

> What I mean is that the shebang is irrelevant for plugins. I think it's
> only the build system complaining: it sees a python script, looks into
> it and notices that the shebang is ambiguous. But since the python
> interpreter linked into hugin processes the script, this is a non-issue.
> Your patch satisfies the fedora build system, but it makes no difference
> apart from that.

> I attach a patch removing the shebangs and the api-min and api-max. I
> did the patch with hg diff --git, hope that's all right. This patch
> should fix both the api-check issue and the shebang problem, and removes
> crop_cp.py which I think is no longer useful. It does not address the
> enforcement of python3-only.

I've removed all my local changes from my repo and applied your patch and
attempted to rpmbuild in Fedora 30, with the following message when the
build failed...

Bytecompiling .py files below
/home/terry/rpmbuild/BUILDROOT/hugin-2019.1.0-0.2.20190723hg.fc30.x86_64/usr/lib/debug/usr/lib64/python2.7
using /usr/bin/python2.7
Bytecompiling .py files below
/home/terry/rpmbuild/BUILDROOT/hugin-2019.1.0-0.2.20190723hg.fc30.x86_64/usr/lib64/python2.7
using /usr/bin/python2.7
+ /usr/lib/rpm/brp-python-hardlink
+ /usr/lib/rpm/redhat/brp-mangle-shebangs
*** WARNING: ./usr/share/hugin/data/plugins/shooting_pattern.py is
executable but has empty or no shebang, removing executable bit
*** WARNING: ./usr/share/hugin/data/plugins/top_five.py is executable but
has empty or no shebang, removing executable bit
*** WARNING: ./usr/share/hugin/data/plugins/woa.py is executable but has
empty or no shebang, removing executable bit
*** WARNING: ./usr/share/hugin/data/plugins-templates/plugin_skeleton.py
is executable but has empty or no shebang, removing executable bit
*** WARNING: ./usr/share/hugin/data/plugins-templates/dual_use.py is
executable but has empty or no shebang, removing executable bit
*** ERROR: ambiguous python shebang in
/usr/lib64/python2.7/site-packages/hpi.py: #!/usr/bin/env python. Change
it to python3 (or python2) explicitly.
error: Bad exit status from /var/tmp/rpm-tmp.wFPWQa (%install)

Looks like that patch isn't all that is needed for a build in Fedora 30,
assuming I applied the patch correctly.
Not sure what 'removing executable bit' in the above warnings means.
Hope this helps.
I don't think I know enough about this business to do much more than test
like this.

[kfj]

The warnings are okay if the scripts are executable, but you can get rid of the warnings by clearing the execurable flag (chmod -x)

*** ERROR: ambiguous python shebang in  
/usr/lib64/python2.7/site-packages/hpi.py: #!/usr/bin/env python. Change  
it to python3 (or python2) explicitly.

I did not remove the shebang from this one, sorry, may mistake. Please just remove the line with

#! /usr/bin/env python

It's just the same as with the other python scripts. Also remove the executable flaf with chmod.

error: Bad exit status from /var/tmp/rpm-tmp.wFPWQa (%install)

Looks like that patch isn't all that is needed for a build in Fedora 30,  
assuming I applied the patch correctly.

You applied it correctly, but there were bits missing . I did the changes I mentioned above here and made a new patch - not sure if the removal of the excutable flag shows in the diff, if not, just do chmod a-x on offending files. I'm off on a lengthy holiday now, so I hope this is enough for now.

with regards

Kay

 

 

[kfj]

On Tuesday, July 23, 2019 at 7:08:01 AM UTC+2, kfj wrote:

 

I did the changes I mentioned above here and made a new patch - not sure if the removal of the excutable flag shows in the diff, if not, just do chmod a-x on offending files.

Sorry, one of my local changes leaked into the previous patch. here's the proper one. And yes, the mode changes did make it into the patch.

with regards

Kay

 

 

[Terry Duell]

On Tue, 23 Jul 2019 15:12:03 +1000, 'kfj' via hugin and other free

panoramic software <hugi...@googlegroups.com> wrote:

[snip]

>

> Sorry, one of my local changes leaked into the previous patch. here's the
> proper one. And yes, the mode changes did make it into the patch.
>

I tried to apply that patch, but hg didn't like it much.
Not sure what happened but my local repo seems to be a bit buggered up,
and not being an expert on how to fix all this I've scrapped it and made a
new one.
I think I'll leave any further testing of patches/changes on this until
they are committed to the repo.

[T. Modes]

Am Dienstag, 23. Juli 2019 07:12:03 UTC+2 schrieb kfj:

Sorry, one of my local changes leaked into the previous patch. here's the proper one. And yes, the mode changes did make it into the patch.

There are still more changes in the patch. Also all files are contained twice.

Furthermore, I don't think that simply removing the shebang is the correct solution. This is only an ugly hack.

[Kay F. Jahnke]

Now I'm back online again, sorry for the delay.

On 19.08.19 17:34, T. Modes wrote:

> snip

> There are still more changes in the patch.

What additional changes do you mean? I intended the patch to contain

- the removal of the executable flag in the file's permissions
- the removal of the shebangs
- the removal of crop_cp.py

> Also all files are contained twice.

I don't know what you mean by that, can you explain?

> Furthermore, I don't think that simply removing the shebang is the
> correct solution. This is only an ugly hack.

I don't agree. We're talking about python code which is used as a plugin
for hugin. In this constellation, hugin (having been 'extended' to
contain a python interpreter) performs reading, interpretation and
execution of this code. Neither the shell nor any python interpreters
installed on the system are involved or needed. The 'shebang' is a hint
to a shell how to process code which is launched by the shell; it's not
part of python syntax (seen from the python side, it's merely a comment,
like the offending api-min and api-max tags). The shebangs which I
propose to remove from the python plugins are only useful if these
plugins are written to function on the command line of a shell as well,
so that one can simply launch them from a command line by writing

woa.py ...

instead of having to issue the more involved

python woa.py ...

or

python3 woa.py ...

So the 'shebang' is, like the executable permission, simply a matter of
convenience for executing these scripts from a shell environment. For
hugin plugins, this is not needed. You might use any bit of text with
any (or no) file extension and any permissions sufficient to let hugin
read the file, as long as it contains valid python code compatible with
the hugin script interface.

I'd like to add that the changes I propose (apart from removing
crop_cp.py) have little effect of hugin as such - my intention was to
help with the difficulties Terry (sorry, Terry, for talking about you in
the third person here) encountered trying to build a fedora package,
because the fedora package building tool choked on the content as-is.
Once Terry gets his repo situation sorted and manages to apply my patch,
he can say whether it solves his packaging problem, and if he can't
apply the patch we can discuss what is going wrong. My experience with
packaging for debian may not be applicable, but debian packages
typically live in their dedicated repo with any debian-specific patches
local to that repo, independent of upstream. Since I am not involved in
fedora packaging, I don't know how their procedure is, but I presume
it's not much different. So even if upstream (the hugin repo) does opt
not to remove shebangs and change permissions, Terry, as the fedora
package builder, may opt to do so in the content being packaged. This
might help him build a valid fedora package without having to wait for
upstream to make a decision concerning the matter.

I tried to avoid sending patches by explaining what I think needs to be
done, which is simply

- remove the shebangs
- remove execute permission from the python files

additionally I recommend to

- remove crop_cp.py

... but I was asked to provide a patch. Seems like my patch did not go
down well, sorry. I'll happily try again.

> This is only an ugly hack.

I think that this comment is opinionated and dismissive. 'Only' implies
that it is missing something important, and 'ugly' is hardly a
technically neutral attribute. I have previously, and now again, tried
to explain why I think that my proposed changes are technically sound,
and if you'd like to discuss the matter on this basis, please let's do so.

Kay

>
> --
> A list of frequently asked questions is available at:
> http://wiki.panotools.org/Hugin_FAQ
> ---
> You received this message because you are subscribed to a topic in the
> Google Groups "hugin and other free panoramic software" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/hugin-ptx/oSmt89pqPCg/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> hugin-ptx+...@googlegroups.com
> <mailto:hugin-ptx+...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/hugin-ptx/abec238f-8614-4834-96dd-aeee28cc2135%40googlegroups.com
> <https://groups.google.com/d/msgid/hugin-ptx/abec238f-8614-4834-96dd-aeee28cc2135%40googlegroups.com?utm_medium=email&utm_source=footer>.

[T. Modes]

Am Samstag, 24. August 2019 15:30:43 UTC+2 schrieb kfj:

Now I'm back online again, sorry for the delay.

On 19.08.19 17:34, T. Modes wrote:

> snip

> There are still more changes in the patch.

What additional changes do you mean? I intended the patch to contain

From you python_plugins.diff

diff --git a/src/hugin_base/panodata/PanoramaOptions.h b/src/hugin_base/panodata/PanoramaOptions.h
--- a/src/hugin_base/panodata/PanoramaOptions.h
+++ b/src/hugin_base/panodata/PanoramaOptions.h
@@ -240,7 +240,7 @@

 I don't know what changes do PanoramaOptions.h have to do with the plugins.

> Also all files are contained twice.

I don't know what you mean by that, can you explain?

line 1-7 are

diff --git a/src/hugin_script_interface/plugins-dev/dual_use.py b/src/hugin_script_interface/plugins-dev/dual_use.py
--- a/src/hugin_script_interface/plugins-dev/dual_use.py
+++ b/src/hugin_script_interface/plugins-dev/dual_use.py
@@ -1,5 +1,3 @@
-#!/usr/bin/env python
-
then the same lines again at line 456, 929 and 1390 (so the same changes are 4 times in the diff)

This apply is several files, I just mention the first one.

The shebangs which I
propose to remove from the python plugins are only useful if these
plugins are written to function on the command line of a shell as well,
so that one can simply launch them from a command line by writing

woa.py ...

instead of having to issue the more involved

python woa.py ...

or

python3 woa.py ...

So you remove this convenience function as a shortcut for you...

And so you have to know if Hugin is compiled with Python 2 or Python 3. A correct shebang would solve this issue for the user.

Thomas

[Rogier Wolff]

On Sat, Aug 24, 2019 at 03:30:35PM +0200, 'Kay F. Jahnke' via hugin and other free panoramic software wrote:

> The 'shebang' is a hint to a shell how to process code which is
> launched by the shell; it's not part of python syntax (seen from the
> python side, it's merely a comment, like the offending api-min and
> api-max tags).

If I remember correctly in the very old days when the shell was asked
to execute say "/bin/which" (which is a #!/bin/sh shell script on my
current system), the kernel would return "not an executable". The
shell would indeed then open the file, check for #! as the first two
characters and then launch the interpreter with the proper argument.

This was in the days that we ran Unix on a PDP-11 with 256k RAM. (I
don't remember if we ran BSD or sysV on that thing.....)

Anyway, on modern systems it is the kernel and not the shell that does
this.

I would vote for keeping the #! as the first two characters.
What comes after that could either be /usr/bin/python, or
/usr/bin/hugin .

In both cases, this should be made to "do the right thing".

If possible, this would mean starting the script within hugin. (I
don't know what kinds of scripts we're talking about, but a script
that asks the user to browse to say a specific type of pictures and
then causes some preprocessing to happen before triggering an
optimization and output step would make this possible. On the other
hand if a script can only be run when you already have images loaded
inside hugin, then this option does not apply. )

The other option for "right thing" would be to print an error message:
"This can only be run from inside hugin". (*)

When executing scripts hugin should try to enforce this first line
to be correct.

Take care that when it makes sense to be able to start scripts on
their own, you should NOT check the PATH part of the
"#!/...path.../program\n" . Just check for the #!/ first three letters
and the right program name at the end of the line.

Roger.

(*)
So the easiest way would be to have:
#!/usr/bin/hugin -s
as the first line of the script. Then the manual page for
hugin would say:

-s <filename>
print "This script must be started from inside hugin" and exit.


--
+-- Rogier Wolff -- www.harddisk-recovery.nl -- 0800 220 20 20 --
- Datarecovery Services Nederland B.V. Delft. KVK: 30160549 -
| Files foetsie, bestanden kwijt, alle data weg?!
| Blijf kalm en neem contact op met Harddisk-recovery.nl!

[Kay F. Jahnke]

On 24.08.19 15:56, T. Modes wrote:

>
> From you python_plugins.diff
> diff --git a/src/hugin_base/panodata/PanoramaOptions.h
> b/src/hugin_base/panodata/PanoramaOptions.h
> --- a/src/hugin_base/panodata/PanoramaOptions.h
> +++ b/src/hugin_base/panodata/PanoramaOptions.h
> @@ -240,7 +240,7 @@

> ??I don't know what changes do PanoramaOptions.h have to do with the
> plugins
Nothing at all, that was a private change which accidentally leaked into
the first patch I sent

>
>
> > Also all files are contained twice.
>
> I don't know what you mean by that, can you explain?
>
>
> line 1-7 are
> diff --git a/src/hugin_script_interface/plugins-dev/dual_use.py
> b/src/hugin_script_interface/plugins-dev/dual_use.py
> --- a/src/hugin_script_interface/plugins-dev/dual_use.py
> +++ b/src/hugin_script_interface/plugins-dev/dual_use.py
> @@ -1,5 +1,3 @@
> -#!/usr/bin/env python
> -
> then the same lines again at line 456, 929 and 1390 (so the same changes
> are 4 times in the diff)
> This apply is several files, I just mention the first one.

I see. You are right. Inspecting my history I see that I used '>>'
instead of '>' to write to the patch. I was in a hurry. My mistake.
Corrected patch attached. This new patch is against a pull as of
2019-08-24 17:10 CEST and has:

- removal of the api limits in the plugin scripts
- removal of the shebangs (I still think that's a good idea)
- change of permissions removing execute permission

And it was done with

hg diff --git

because, as far as I understand it, this transports the mode changes as
well, which plain hg diff does not. I hope that works now.

I hope that Terry can apply this patch and that it allows the fedora
package to build. Then we can discuss if we should use shebangs at all,
and if so, which it should be. Roger has posted concerning this issue,
I'll answer that separately.

Kay

[Kay F. Jahnke]

On 24.08.19 16:19, Rogier Wolff wrote:
> On Sat, Aug 24, 2019 at 03:30:35PM +0200, 'Kay F. Jahnke' via hugin and other free panoramic software wrote:
>
>> The 'shebang' is a hint to a shell how to process code which is
>> launched by the shell; it's not part of python syntax (seen from the
>> python side, it's merely a comment, like the offending api-min and
>> api-max tags).
>
> If I remember correctly in the very old days when the shell was asked
> to execute say "/bin/which" (which is a #!/bin/sh shell script on my
> current system), the kernel would return "not an executable". The
> shell would indeed then open the file, check for #! as the first two
> characters and then launch the interpreter with the proper argument.
>
> This was in the days that we ran Unix on a PDP-11 with 256k RAM. (I
> don't remember if we ran BSD or sysV on that thing.....)
>
> Anyway, on modern systems it is the kernel and not the shell that does
> this.
>
> I would vote for keeping the #! as the first two characters.
> What comes after that could either be /usr/bin/python, or
> /usr/bin/hugin .
>
> In both cases, this should be made to "do the right thing".
>
> If possible, this would mean starting the script within hugin. (I
> don't know what kinds of scripts we're talking about,

We are talking about the scripts in

src/hugin_script_interface/plugins/

and

src/hugin_script_interface/plugins_dev/

> but a script
> that asks the user to browse to say a specific type of pictures and
> then causes some preprocessing to happen before triggering an
> optimization and output step would make this possible. On the other
> hand if a script can only be run when you already have images loaded
> inside hugin, then this option does not apply. )

When the python plugins are invoked by hugin they all take a panorama
object as their sole argument - so there has to be a panorama in hugin.

> The other option for "right thing" would be to print an error message:
> "This can only be run from inside hugin". (*)

It depends on the python file: you can write it so that it only runs as
a hugin plugin, or you can write it so that it can run as a plugin and
stand-alone as well. But maybe that's too much artistry; I thought this
would make it simpler to maintain because both modes of operation are in
a single file. Maybe it's better to stick to plugins only and remove the
stand-alone capability.

> When executing scripts hugin should try to enforce this first line
> to be correct.

So you mean hugin should look for a "#! /usr/bin/hugin" shebang and
refuse to run the plugin if that is missing?

> Take care that when it makes sense to be able to start scripts on
> their own, you should NOT check the PATH part of the
> "#!/...path.../program\n" . Just check for the #!/ first three letters
> and the right program name at the end of the line.
>
> Roger.
>
> (*)
> So the easiest way would be to have:
> #!/usr/bin/hugin -s
> as the first line of the script. Then the manual page for
> hugin would say:
>
> -s <filename>
> print "This script must be started from inside hugin" and exit.
>

I'm not sure if I follow. The plugins are not launched by invocation of
'hugin' on the command line. They are executed by clicking on an entry
in the 'actions' menu (which is only present if hugin was compiled with
plugin capability and if there are plugins passing the api-min and
api-max test). This is the only way users get access to them, unless
they explicitly invoke the python files in the relevant subdirectory of
their hugin installation.

Kay

[Bruno Postle]

On Sat 24-Aug-2019 at 18:05 +0200, Hugin ptx wrote:
>
>It depends on the python file: you can write it so that it only runs
>as a hugin plugin, or you can write it so that it can run as a plugin
>and stand-alone as well. But maybe that's too much artistry; I thought
>this would make it simpler to maintain because both modes of operation
>are in a single file. Maybe it's better to stick to plugins only and
>remove the stand-alone capability.

Only python code that can be run on the command-line _needs_ to have a
shebang, this should always be #!/usr/bin/python3 since we want to
drop python2 support.

It does no harm having a shebang in other python files since it
helps text editors with syntax highlighting etc...

--
Bruno

[Kay F. Jahnke]

Looks like my propositions regarding the shebang did not go down well;
nevertheless I find that upstream seems to have taken in my last patch.
It may be ugly, but I think it works. I accept that a proper shebang
would be nice, and so I did some work towards moving to python3:

I tried to build hugin with hsi/hpi and python3 support. I worked on a
ubuntu 19.04 system (to have a recent cmake). I had to install the usual
suspects and provide a hand-compiled vigra, because the one provided by
the package management missed openEXR. Compiling with hsi/hpi support
(cmake with -DBUILD_HSI:BOOL=ON) for python2 worked as expected. Next I
modified the cmake code to use python3 and swig3, and managed to get the
build all the way up to hugin_stitch_project, where it terminated due to
linker errors.

That the build proceeded so far means that I was able to build the hugin
python module (I checked, it imports into python3), but there is a
problem when linking with the hugin python interface (which embeds
python3 into hugin): a lot of python-specific symbols are not found. I
tried to explicitly add the python3 libs to the set of dependencies in
the cmake code (and tried all other tricks I could think of...), but to
no avail. So I'm stuck.

Would anyone care to pick up my attempts so far and look into the linker
errors? Prefereably someone who's good at cmake (which I'm not)?

I attach a patch against a pull from today noon-ish, which has the
modifications to the cmake code.

Kay

[Kay F. Jahnke]

I figured out what I did wrong: I changed the dependency only. When I
instead added the python libs to 'common_libs' in src/CMakeLists.txt the
remainder of the build worked as well. I attach a patch which has:

- builds hugin hsi/hpi with embedded python3
- plugins have explicit python3 shebangs and exec permission
- api-min and api-max tags removed from plugins
- change to woa.py to convert tiffdump output from bytestring to string

Note that the detection of the python3 dev environment as is done in my
CMake code requires a reasonably recent CMake (I used what comes with
ubuntu 19.04).

Please check it out and let me know what you think.

Kay