After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 650763 - gdbus-codegen is broken with python 2.7
gdbus-codegen is broken with python 2.7
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gdbus
2.29.x
Other Windows
: Normal normal
: ---
Assigned To: David Zeuthen (not reading bugmail)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-05-21 23:03 UTC by Sam Thursfield
Modified: 2012-03-21 11:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch, fix by renaming parser.py (27.16 KB, patch)
2011-05-21 23:03 UTC, Sam Thursfield
needs-work Details | Review
First option, simply rename gdbus-codegen directory to gdbus_codegen. With $libdir on PYTHONPATH dirtyness... (462.07 KB, patch)
2011-06-29 17:37 UTC, Dieter Verfaillie
none Details | Review
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness... (462.66 KB, patch)
2011-06-29 17:38 UTC, Dieter Verfaillie
none Details | Review
190945: Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness... (462.47 KB, application/octet-stream)
2011-06-29 19:05 UTC, Dieter Verfaillie
  Details
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness... (462.69 KB, patch)
2011-06-30 09:02 UTC, Dieter Verfaillie
none Details | Review
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness... (465.77 KB, patch)
2011-06-30 16:44 UTC, Dieter Verfaillie
none Details | Review
Second option, rename gdbus-codegen directory to gdbus/codegen. (476.19 KB, patch)
2011-07-25 08:50 UTC, Dieter Verfaillie
reviewed Details | Review
Avoid using - (hyphen) in gdbus-codegen directory name because it's an invalid character in Python module names. (473.50 KB, patch)
2011-07-26 10:32 UTC, Dieter Verfaillie
none Details | Review
Use relative imports for the gdbus/codegen package, except for the config module. (3.07 KB, patch)
2011-07-26 10:33 UTC, Dieter Verfaillie
none Details | Review
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable which makes it possible to also use relative imports for gdbus-codegen's config module. (6.54 KB, patch)
2011-07-26 10:33 UTC, Dieter Verfaillie
none Details | Review
gdbus-codegen: fix shebang (622 bytes, patch)
2011-07-26 10:34 UTC, Dieter Verfaillie
none Details | Review
Make gdbus-codegen 'relocatable' at runtime on Windows. (1.04 KB, patch)
2011-07-26 10:34 UTC, Dieter Verfaillie
none Details | Review
Avoid using - (hyphen) in gdbus-codegen directory name because it's an invalid character in Python module names. (473.71 KB, patch)
2011-07-26 18:32 UTC, Dieter Verfaillie
reviewed Details | Review
Use relative imports for the gdbus/codegen package, except for the config module. (3.17 KB, patch)
2011-07-26 18:32 UTC, Dieter Verfaillie
reviewed Details | Review
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable which makes it possible to also use relative imports for gdbus-codegen's config module. (6.55 KB, patch)
2011-07-26 18:33 UTC, Dieter Verfaillie
reviewed Details | Review
gdbus-codegen: fix shebang (642 bytes, patch)
2011-07-26 18:33 UTC, Dieter Verfaillie
reviewed Details | Review
Make gdbus-codegen 'relocatable' at runtime on Windows. (1.08 KB, patch)
2011-07-26 18:34 UTC, Dieter Verfaillie
accepted-commit_now Details | Review
Add gdbus-codegen to glib-zip.in (1.08 KB, patch)
2011-07-29 07:26 UTC, Dieter Verfaillie
accepted-commit_now Details | Review
Avoid using - (hyphen) in gdbus-codegen directory name (473.80 KB, patch)
2011-08-24 07:42 UTC, Dieter Verfaillie
none Details | Review
Use relative imports for the gdbus/codegen package (2.88 KB, patch)
2011-08-24 07:42 UTC, Dieter Verfaillie
none Details | Review
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable (6.62 KB, patch)
2011-08-24 07:44 UTC, Dieter Verfaillie
none Details | Review
Introduce the UNINSTALLED_GLIB_SRCDIR environment variable (2.78 KB, patch)
2011-08-24 07:45 UTC, Dieter Verfaillie
none Details | Review
gdbus-codegen: fix shebang (692 bytes, patch)
2011-08-24 07:46 UTC, Dieter Verfaillie
none Details | Review
Make gdbus-codegen 'relocatable' at runtime on Windows. (1002 bytes, patch)
2011-08-24 07:47 UTC, Dieter Verfaillie
none Details | Review
Add gdbus-codegen to glib-zip.in (1.13 KB, patch)
2011-08-24 07:49 UTC, Dieter Verfaillie
none Details | Review
gdbus-codegen: fix shebang (1.24 KB, patch)
2011-08-25 10:23 UTC, Dieter Verfaillie
none Details | Review
Avoid using - (hyphen) in gdbus-codegen directory name (493.48 KB, patch)
2011-08-25 19:58 UTC, Dieter Verfaillie
committed Details | Review
Use relative imports for the gdbus/codegen package (2.88 KB, patch)
2011-08-25 19:59 UTC, Dieter Verfaillie
committed Details | Review
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable (6.62 KB, patch)
2011-08-25 19:59 UTC, Dieter Verfaillie
committed Details | Review
Introduce the UNINSTALLED_GLIB_SRCDIR environment variable (2.78 KB, patch)
2011-08-25 20:00 UTC, Dieter Verfaillie
committed Details | Review
gdbus-codegen: fix shebang (1.23 KB, patch)
2011-08-25 20:01 UTC, Dieter Verfaillie
committed Details | Review
Make gdbus-codegen 'relocatable' at runtime on Windows. (1002 bytes, patch)
2011-08-25 20:01 UTC, Dieter Verfaillie
committed Details | Review
Add gdbus-codegen to glib-zip.in (1.13 KB, patch)
2011-08-25 20:02 UTC, Dieter Verfaillie
committed Details | Review

Description Sam Thursfield 2011-05-21 23:03:36 UTC
Created attachment 188319 [details] [review]
patch, fix by renaming parser.py

gdbus-codegen has a submodule called 'parser'; there is also a standard Python module called 'parser'. With Python 2.7.1 on Windows, the standard module is imported rather than the local one.

I imagine this has something to do with the 'absolute and relative imports' business that began in Python 2.5: http://docs.python.org/whatsnew/2.5.html#pep-328.

Attached is a patch to rename 'parser.py' to 'dbusxmlparser.py', in case you think that's a good solution. I didn't seem to be able to convice Python to import the local module for whatever reason.
Comment 1 Colin Walters 2011-05-22 16:16:52 UTC
Review of attachment 188319 [details] [review]:

::: gio/gdbus-codegen/codegen_main.py
@@ +28,3 @@
 import utils
 import dbustypes
+import dbusxmlparser

These should all change to:

from . import dbustypes
from . import parser

etc.  See:
http://docs.python.org/release/2.5.2/ref/import.html
"Relative import"
Comment 2 Dieter Verfaillie 2011-06-20 08:29:21 UTC
The proposed patch only hides the symptoms of the problem,
but does not really solve it. The error here is the hyphen
in the "gdbus-codegen" directory name. Because of that,
Python does not treat gdbus-codegen as a package, even with
the presence of the "__init__.py" file. Hence Python never
even tries to load gdbus-codegen/parser.py on the "import parser"
statement.

The correct solution would be to either:
- rename the gdbus-codegen directory to gdbus_codegen (underscores
  are allowed, but discouraged by PEP8)
- if the above is deemed too ugly to create a proper package hierarchy:
   /gio/gdbus/
   /gio/gdbus/__init__.py
   /gio/gdbus/codegen
   /gio/gdbus/codegen/__init__.py

Which of these would be preferred?
Comment 3 Colin Walters 2011-06-20 15:18:48 UTC
(In reply to comment #2)
> 
> - rename the gdbus-codegen directory to gdbus_codegen (underscores
>   are allowed, but discouraged by PEP8)

Let's go with this.

> - if the above is deemed too ugly to create a proper package hierarchy:

Note that we don't actually want gdbus_codegen to be a public module, which is why it's installed in $libdir/gdbus-codegen.
Comment 4 Dieter Verfaillie 2011-06-29 17:36:37 UTC
>> - rename the gdbus-codegen directory to gdbus_codegen (underscores
>>   are allowed, but discouraged by PEP8)

>Let's go with this.

This does have the nasty side-effect of having to put $libdir
(for example /usr/lib) on PYTHONPATH when gdbus-codegen is executed
from it's installed location (ie not from $srcdir/$builddir when
building glib). If $libdir contains thousands of files/directories
import times could be slow on some systems (and besides, thinking
more about this, it just "feels dirty").

>> - if the above is deemed too ugly to create a proper package hierarchy:

>Note that we don't actually want gdbus_codegen to be a public module, which is
>why it's installed in $libdir/gdbus-codegen.

Yes, I'm aware of that. What I meant with the second option was to
create a $libdir/gdbus/codegen (or even better: $libdir/gdbus-2.0/codegen)
directory and put $libdir/gdbus-2.0 on PYTHONPATH. "Feels" less dirty.

I'll attach 2 patches, each of them implementing one of the
proposed solutions. Feel free to choose :) Also, I'll be happy
to cut the chosen patch in pieces (for easier review/cleaner
scm history)

One remark about both patches: imports have been changed to relative
imports as per Comment 1 except for the config module. That's because
during glib build-time config.py is generated in $builddir/gio/... from
the config.py.in template and does not live next to the other .py files
that live in $srcdir/gio/... Thus, building with $builddir != $srcdir
leads to import errors if we where to state "from . import config".
Comment 5 Dieter Verfaillie 2011-06-29 17:37:32 UTC
Created attachment 190944 [details] [review]
First option, simply rename gdbus-codegen directory to gdbus_codegen. With $libdir on PYTHONPATH dirtyness...
Comment 6 Dieter Verfaillie 2011-06-29 17:38:20 UTC
Created attachment 190945 [details] [review]
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness...
Comment 7 Dieter Verfaillie 2011-06-29 19:05:59 UTC
Created attachment 190962 [details]
190945: Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness...

Oops, had some uncommitted changes in my option2 branch. So this is the
correct, working patch. Sorry about the noise!
Comment 8 Dieter Verfaillie 2011-06-30 09:02:39 UTC
Created attachment 190997 [details] [review]
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness...

Using relative imports as per Comment 1 implies raising the minimum Python
version to 2.5. Updated patch to take care of this.

See http://docs.python.org/whatsnew/2.5.html#pep-328-absolute-and-relative-imports for more info.
Comment 9 Colin Walters 2011-06-30 13:24:00 UTC
(In reply to comment #4)

 
> One remark about both patches: imports have been changed to relative
> imports as per Comment 1 except for the config module. That's because
> during glib build-time config.py is generated in $builddir/gio/... from
> the config.py.in template and does not live next to the other .py files
> that live in $srcdir/gio/... Thus, building with $builddir != $srcdir
> leads to import errors if we where to state "from . import config".

Note you can use __path__ to fix this.  See:

http://git.gnome.org/browse/gobject-introspection/commit/?id=158db28b46eb1e9b24705bf8e9c3e6ca69ae54b4
Comment 10 Dieter Verfaillie 2011-06-30 16:44:39 UTC
Created attachment 191041 [details] [review]
Second option, rename gdbus-codegen directory to gdbus-2.0/codegen. Without $libdir on PYTHONPATH dirtyness...

(In reply to comment #9)
> Note you can use __path__ to fix this.  See:
> 
> http://git.gnome.org/browse/gobject-introspection/commit/?id=158db28b46eb1e9b24705bf8e9c3e6ca69ae54b4

Nice, thanks!

Updated patch uses an UNINSTALLED_GLIB_BUILDDIR environment variable
which is set to $(top_builddir), gio/gdbus/codegen/__init__.py takes
care of the rest.
Comment 11 Dieter Verfaillie 2011-07-25 08:50:03 UTC
Created attachment 192600 [details] [review]
Second option, rename gdbus-codegen directory to gdbus/codegen.

Rebased patch against current HEAD.
Comment 12 Colin Walters 2011-07-25 15:44:17 UTC
Review of attachment 192600 [details] [review]:

The commit message could be better here; something like:

"Avoid using - in gdbus-codegen directory name because it's an invalid character in Python module names."

It's hard to review this patch due to the mass of renames, but it looks roughly correct.  Did you:

1) Test a build with srcdir != builddir?
2) Test a build with no glib installed to be sure we're not picking up the one from jhbuild?
Comment 13 Dieter Verfaillie 2011-07-25 20:12:05 UTC
(In reply to comment #12)
> Review of attachment 192600 [details] [review]:
> 
> The commit message could be better here; something like:
> 
> "Avoid using - in gdbus-codegen directory name because it's an invalid
> character in Python module names."
> 
> It's hard to review this patch due to the mass of renames, but it looks
> roughly correct.  

Allow me a day to break the patch up into manageable chunks and improve
the commit messages.

> Did you:
> 1) Test a build with srcdir != builddir?

Yes, my personal MinGW/MSYS build scripts are configured to do that
as a matter of course, but that does depend on GLib bug #653167

> 2) Test a build with no glib installed to be sure we're not picking up the one
> from jhbuild?

Yes/no:

No: GLib's configure script needs pkg-config and the Windows port of
pkg-config depends on glib being on PATH (0.26 even dropped support for
the embedded glib version so I guess this is going to be the case on
other platform too).

Yes: glib's lib/pkgconfig is not put on PKG_CONFIG_PATH in my build
script.
Comment 14 Dieter Verfaillie 2011-07-26 10:32:24 UTC
Created attachment 192655 [details] [review]
Avoid using - (hyphen) in gdbus-codegen directory name because it's an invalid character in Python module names.
Comment 15 Dieter Verfaillie 2011-07-26 10:33:04 UTC
Created attachment 192656 [details] [review]
Use relative imports for the gdbus/codegen package, except for the config module.
Comment 16 Dieter Verfaillie 2011-07-26 10:33:46 UTC
Created attachment 192657 [details] [review]
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable which makes it possible to also use relative imports for gdbus-codegen's config module.
Comment 17 Dieter Verfaillie 2011-07-26 10:34:15 UTC
Created attachment 192658 [details] [review]
gdbus-codegen: fix shebang
Comment 18 Dieter Verfaillie 2011-07-26 10:34:45 UTC
Created attachment 192659 [details] [review]
Make gdbus-codegen 'relocatable' at runtime on Windows.
Comment 19 Dieter Verfaillie 2011-07-26 16:28:20 UTC
Tested with jhbuild on a Linux box and well, this series doesn't work as
there's a conflict between the gdbus binary and the gdbus directory
(doesn't happen on windows because it's called gdbus.exe there).

Sigh, this should have been obvious but oh well, another valuable
lesson learned...

New patches coming shortly when builds on windows, jhbuild on my Gentoo
machine and a fedora vm finish successfully.
Comment 20 Dieter Verfaillie 2011-07-26 18:32:09 UTC
Created attachment 192692 [details] [review]
Avoid using - (hyphen) in gdbus-codegen directory name because it's an invalid character in Python module names.
Comment 21 Dieter Verfaillie 2011-07-26 18:32:53 UTC
Created attachment 192693 [details] [review]
Use relative imports for the gdbus/codegen package, except for the config module.
Comment 22 Dieter Verfaillie 2011-07-26 18:33:22 UTC
Created attachment 192694 [details] [review]
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable which makes it possible to also use relative imports for gdbus-codegen's config module.
Comment 23 Dieter Verfaillie 2011-07-26 18:33:45 UTC
Created attachment 192695 [details] [review]
gdbus-codegen: fix shebang
Comment 24 Dieter Verfaillie 2011-07-26 18:34:23 UTC
Created attachment 192696 [details] [review]
Make gdbus-codegen 'relocatable' at runtime on Windows.
Comment 25 Dieter Verfaillie 2011-07-29 07:26:12 UTC
Created attachment 192853 [details] [review]
Add gdbus-codegen to glib-zip.in

Another patch for the series, this time adding gdbus-codegen related bits
to glib-zip.in.

If you're more comfortable looking at these patches in a branch I'm
maintaining them here for now: https://github.com/dieterv/glib/tree/windows
(but expect other commits unrelated to this bug report and non-fast-forward
updates on that branch though...)
Comment 26 Colin Walters 2011-08-22 18:03:37 UTC
Review of attachment 192692 [details] [review]:

The commit title is too long, and you have an empty body.  See https://live.gnome.org/GnomeLove/SubmittingPatches for guidelines on commit messages.  Something like this would be better:

Avoid using - (hyphen) in gdbus-codegen directory name

It's an invalid character in Python module names, and this prevents us from being able to import it.

https://bugzilla.gnome.org/show_bug.cgi?id=650763
Comment 27 Colin Walters 2011-08-22 18:05:04 UTC
Review of attachment 192693 [details] [review]:

::: configure.ac
@@ +367,2 @@
 # Need suitable python path for greport
+AM_PATH_PYTHON(2.5,,PYTHON="/usr/bin/env python2.5")

This change shouldn't be in this patch (I already did it actually in 2512341fa6b1842adc5b403ce9eb22b9f3478ee3 )
Comment 28 Colin Walters 2011-08-22 18:07:20 UTC
Review of attachment 192694 [details] [review]:

You might consider also adding UNINSTALLED_GLIB_SRCDIR too, and this would avoid setting PYTHONPATH in the Makefile.am.

Also, just like the first commit, the commit message needs to be split up (and add a link to this bug):

Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable

This makes it possible to also use relative imports for gdbus-codegen's config module.

https://bugzilla.gnome.org/show_bug.cgi?id=650763
Comment 29 Colin Walters 2011-08-22 18:12:35 UTC
Review of attachment 192695 [details] [review]:

Shouldn't we actually be substituting the value of $(PYTHON)?
Comment 30 Colin Walters 2011-08-22 18:13:36 UTC
Review of attachment 192696 [details] [review]:

Hmmm.  I don't know enough about this to review it intelligently; but I don't see anything obviously wrong.
Comment 31 Colin Walters 2011-08-22 18:14:19 UTC
Review of attachment 192853 [details] [review]:

Don't know anything about glib-zip, looks OK to me.
Comment 32 Dieter Verfaillie 2011-08-24 07:42:12 UTC
Created attachment 194551 [details] [review]
Avoid using - (hyphen) in gdbus-codegen directory name

(In reply to comment #26)
> Review of attachment 192692 [details] [review]:
> 
> The commit title is too long, and you have an empty body.

Fixed.
Comment 33 Dieter Verfaillie 2011-08-24 07:42:58 UTC
Created attachment 194552 [details] [review]
Use relative imports for the gdbus/codegen package

(In reply to comment #27)
> Review of attachment 192693 [details] [review]:
> 
> This change shouldn't be in this patch (I already did it actually in
> 2512341fa6b1842adc5b403ce9eb22b9f3478ee3 )

Fixed.
Comment 34 Dieter Verfaillie 2011-08-24 07:44:36 UTC
Created attachment 194553 [details] [review]
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable

(In reply to comment #28)
> Also, just like the first commit, the commit message needs to be split up

Fixed.
Comment 35 Dieter Verfaillie 2011-08-24 07:45:09 UTC
Created attachment 194554 [details] [review]
Introduce the UNINSTALLED_GLIB_SRCDIR environment variable

(In reply to comment #28)
> Review of attachment 192694 [details] [review]:
> 
> You might consider also adding UNINSTALLED_GLIB_SRCDIR too, and this would
> avoid setting PYTHONPATH in the Makefile.am.

Yeah, keeps thing consistent with how it's done in gobject-introspection I
guess... Anyway, done.
Comment 36 Dieter Verfaillie 2011-08-24 07:46:13 UTC
Created attachment 194555 [details] [review]
gdbus-codegen: fix shebang

Fixed commit message as suggested in comment #26.

(In reply to comment #29)
> Review of attachment 192695 [details] [review]:
> 
> Shouldn't we actually be substituting the value of $(PYTHON)?

At build time `/usr/bin/env python` should be the same as $(PYTHON), no?
But at runtime, funny things can (and will) happen when a value is hardcoded
in the shebang line that is potentially only valid on the system used to
build glib. Thus simply using /usr/bin/env solves this elegantly.
Comment 37 Dieter Verfaillie 2011-08-24 07:47:17 UTC
Created attachment 194556 [details] [review]
Make gdbus-codegen 'relocatable' at runtime on Windows.

Fixed commit message as suggested in comment #26

(In reply to comment #30)
> Review of attachment 192696 [details] [review]:
> 
> Hmmm.  I don't know enough about this to review it intelligently; but I don't
> see anything obviously wrong.

Ok.
Comment 38 Dieter Verfaillie 2011-08-24 07:49:10 UTC
Created attachment 194557 [details] [review]
Add gdbus-codegen to glib-zip.in

Fixed commit message as suggested in comment #26

(In reply to comment #31)
> Review of attachment 192853 [details] [review]:
> 
> Don't know anything about glib-zip, looks OK to me.

Ok. I'm only aware of Tor Lillqvist's build scripts using this to create
the zip archives distributed on www.gtk.org/ftp.gnome.org. I'm using
this in my own scripts for the moment but have not yet decided if I'll
continue to do so (if I ever find the time to get these set up in a
mingw-get repository then I'll need .tar.lzma archives cut up even finer
than simply a "bin" and "dev" archive).

Thanks again for the review,
mvg,
Dieter
Comment 39 David Zeuthen (not reading bugmail) 2011-08-24 20:15:20 UTC
*** Bug 657274 has been marked as a duplicate of this bug. ***
Comment 40 Ionut Biru 2011-08-24 21:49:30 UTC
Review of attachment 194555 [details] [review]:

this change doesn't allow using PYTHON binary detected at ./configure. 
patch from Bug 657274 should fix this problem
Comment 41 Dieter Verfaillie 2011-08-25 07:28:07 UTC
(In reply to comment #40)
> Review of attachment 194555 [details] [review]:
> 
> this change doesn't allow using PYTHON binary detected at ./configure. 
> patch from Bug 657274 should fix this problem

Relevant Makefiles already execute gdbus-codegen like this:
  ...
  $(PYTHON) $(top_builddir)/gio/gdbus-2.0/codegen/gdbus-codegen
  ...
So the Python binary detected at ./configure is used when building glib.
See tests/gdbus-object-manager-example/Makefile.am for an example.


At runtime (when glib has been installed into some $prefix), funny things
can (and will) happen when a value is hardcoded in the shebang line that
is potentially only valid on the system used to build glib. Think OBS,
multi-prefixed windows binaries like on ftp.gnome.org, ...


So I really don't see why you'd want to force the shebang line to
whatever-is-valid-on-the-build-system (where the shebang is not even
used in the first place), to a value that has a good chance of not even
existing when it actually starts to matter?
Comment 42 Ionut Biru 2011-08-25 07:41:48 UTC
(In reply to comment #41)
> 
> So I really don't see why you'd want to force the shebang line to
> whatever-is-valid-on-the-build-system (where the shebang is not even
> used in the first place), to a value that has a good chance of not even
> existing when it actually starts to matter?

some distros don't use python as python2 and at runtime, projects that use gdbus-codegen fail at compilation. think wider in the future when more and more distros will keep up with the trend and start using python3 as python.
Comment 43 Dieter Verfaillie 2011-08-25 09:04:06 UTC
(In reply to comment #42)
> some distros don't use python as python2

I know.

> and at runtime, projects that use gdbus-codegen fail at compilation.

Either:
1) those projects should call AM_PATH_PYTHON in their configure.ac
   and use $(PYTHON) in their Makefiles to call gdbus-codegen or anything
   else that needs Python; or
2) glib installs a Makefile.gdbuscodegen that does "the right thing" and
   can be included in the Makefiles those projects use (analogous to
   what gobject-introspection does with Makefile.introspection); or
3) whatever you use to build packages simply sets the PYTHON
   environment variable to something relevant for the package it's
   building (in this case /path/to/python2.X) instead of relying on
   a system default that can be anything.

Don't know what's best but IMHO 1) might be expecting too much of
people, 2) looks like overkill and 3) is something already in use
to solve the Python 2/3 incompatibility mess.

In your case (looking at your email address), I suspect you can simply
add export PYTHON="/usr/bin/python2" into the build() function of
the PKGBUILD files for the packages that fail. It's already done like
that here for example: http://projects.archlinux.org/svntogit/packages.git/tree/subversion/trunk/PKGBUILD

> think wider in the future when more and more distros will keep up
> with the trend and start using python3 as python.

If comment #41 did not convince you yet, here's another case where
hard-coding the shebang fails: Let's fast forward to the future a bit,
where gdbus-codegen has become a Python 3 program. Your distro has installed
Python 3.4 (remember, we're in the future here) and that version was used
to build glib. Thus your distro installed glib with a gdbus-codegen script
that has  #!/usr/bin/python3.4 as it's shebang line. All of the sudden,
your distro upgrades to Python 3.6. And then you'll start having
"bad interpreter: No such file or directory" errors. Stuff like that has
happened before, for example bug #571784.

Even worse, as it's a hard-coded shebang so there's no way to override it
like you can with #!/usr/bin/env python, where you simply set the PYTHON
environment variable to something that's know to be there and get on with
life...

> think wider in the future

No need for hidden personal attacks like that, let's keep things
fun and friendly.

mvg,
Dieter
Comment 44 Ionut Biru 2011-08-25 09:33:06 UTC
Your example with 3.4 and 3.6 is legit I only had in mind the case when only /usr/bin/python and /usr/bin/python2 or /usr/bin/python3 are detected

If this behavior is not accepted, it is fine to drop the @PYTHON\@ sed from Makefile.am?

My logic of using the build time shebang was that sed line and I was thinking that whoever added that feature, intended to use it
Comment 45 Dieter Verfaillie 2011-08-25 10:23:40 UTC
Created attachment 194689 [details] [review]
gdbus-codegen: fix shebang

(In reply to comment #44)
> If this behavior is not accepted, it is fine to drop the @PYTHON\@ sed from
> Makefile.am?

Ah yes, nice catch. I've updated the patch to do so. Thanks!
Comment 46 Colin Walters 2011-08-25 17:44:52 UTC
(In reply to comment #43)
>
> If comment #41 did not convince you yet, here's another case where
> hard-coding the shebang fails: Let's fast forward to the future a bit,
> where gdbus-codegen has become a Python 3 program. Your distro has installed
> Python 3.4 (remember, we're in the future here) and that version was used
> to build glib. Thus your distro installed glib with a gdbus-codegen script
> that has  #!/usr/bin/python3.4 as it's shebang line. All of the sudden,
> your distro upgrades to Python 3.6. And then you'll start having
> "bad interpreter: No such file or directory" errors. Stuff like that has
> happened before, for example bug #571784.

Hmm, why wouldn't we have /usr/bin/python3 ?

> Even worse, as it's a hard-coded shebang so there's no way to override it

Well, anyone making packages can pretty trivially sed things, and AFAIK in many cases they do.

However, the "right thing" here is definitely up in the air.
Comment 47 Colin Walters 2011-08-25 18:32:35 UTC
Note I did end up committing the patch from bug 657274
Comment 48 Dieter Verfaillie 2011-08-25 18:37:09 UTC
(In reply to comment #46)
> Well, anyone making packages can pretty trivially sed things, and AFAIK in many
> cases they do.
> 
> However, the "right thing" here is definitely up in the air.

Fair enough. I'm only trying to defend what seems to me to be the most
flexible, less surprise-inducing option.

If this shebang thing becomes an issue, maybe it should be separated in it's
own bug report as other scripts all over GNOME components already use the
/usr/bin/env logic.

Hope this doesn't block the other patches in this series...
Comment 49 Colin Walters 2011-08-25 18:47:29 UTC
(In reply to comment #48)
>
> Fair enough. I'm only trying to defend what seems to me to be the most
> flexible, less surprise-inducing option.

I agree that /usr/bin/env was the best choice *before* Arch Linux screwed over the whole ecosystem by putting a different incompatible language as "python".

> If this shebang thing becomes an issue, maybe it should be separated in it's
> own bug report as other scripts all over GNOME components already use the
> /usr/bin/env logic.
> 
> Hope this doesn't block the other patches in this series...

No, it shouldn't.  However I can't apply your patches to git master - are they based on a different tree?
Comment 50 Dieter Verfaillie 2011-08-25 19:55:51 UTC
(In reply to comment #49)
> I agree that /usr/bin/env was the best choice *before* Arch Linux screwed over
> the whole ecosystem by putting a different incompatible language as "python".

<tongue-in-cheek>
Hi,

I'm part of the whole ecosystem by using and testing this stuff on common
Linux distros and Windows (MSYS/MinGW) and I don't particularly like to be
screwed over.

Personally, I couldn't care less about the idiosyncrasies of a single distro.
On the other hand portability and thus flexibility are my first priorities,
so I have already pointed out a solution Arch themselves already seem to use
in comment #43 to make their build system work with "/usr/bin/env python"
shebang lines and packages one way or the other depending on Python 2.

Even if they don't like that, maybe they can sed things to suit their
particular needs (like you suggested in comment #46) and let the rest of
the whole ecosystem enjoy what was formerly known to be the best choice.
</tongue-in-cheek>

That said, I've always wondered about the lack of "/usr/bin/env python2"
and "/usr/bin/env python3" (and similar for other script interpreters).

I'll let this discussion rest now, it is becoming off-topic.

> > Hope this doesn't block the other patches in this series...
> 
> No, it shouldn't.  However I can't apply your patches to git master - are they
> based on a different tree?

Oh, looks like there have been changes to gdbus-codegen again. I'll attach
rebased patches in a moment...

... with the shebang patch still included. Do with it as you like ;)

mvg,
Dieter
Comment 51 Dieter Verfaillie 2011-08-25 19:58:18 UTC
Created attachment 194741 [details] [review]
Avoid using - (hyphen) in gdbus-codegen directory name
Comment 52 Dieter Verfaillie 2011-08-25 19:59:07 UTC
Created attachment 194742 [details] [review]
Use relative imports for the gdbus/codegen package
Comment 53 Dieter Verfaillie 2011-08-25 19:59:46 UTC
Created attachment 194743 [details] [review]
Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable
Comment 54 Dieter Verfaillie 2011-08-25 20:00:36 UTC
Created attachment 194744 [details] [review]
Introduce the UNINSTALLED_GLIB_SRCDIR environment variable
Comment 55 Dieter Verfaillie 2011-08-25 20:01:04 UTC
Created attachment 194745 [details] [review]
gdbus-codegen: fix shebang
Comment 56 Dieter Verfaillie 2011-08-25 20:01:40 UTC
Created attachment 194746 [details] [review]
Make gdbus-codegen 'relocatable' at runtime on Windows.
Comment 57 Dieter Verfaillie 2011-08-25 20:02:10 UTC
Created attachment 194747 [details] [review]
Add gdbus-codegen to glib-zip.in
Comment 58 John Stowers 2011-08-25 20:31:30 UTC
Wow, just saw this.

Can we just let archlinux fix the mess they created and go back to using the established /usr/bin/env/python (as in the patches by Dieter)
Comment 59 Ionut Biru 2011-08-25 20:36:54 UTC
If that change is reverted then I propose to use the same hack applied to gtester-report by using install-exec-hook
Comment 60 Colin Walters 2011-08-25 21:19:06 UTC
(In reply to comment #50)
>
> On the other hand portability and thus flexibility are my first priorities,
> so I have already pointed out a solution Arch themselves already seem to use
> in comment #43 to make their build system work with "/usr/bin/env python"
> shebang lines and packages one way or the other depending on Python 2.

I don't understand how in your proposal Arch can make it work without patching glib - since your patches use "/usr/bin/env python" unconditionally.  Or am I missing something?

Related to this there is a PEP proposal:
http://www.python.org/dev/peps/pep-0394/
 
> Even if they don't like that, maybe they can sed things to suit their
> particular needs (like you suggested in comment #46) and let the rest of
> the whole ecosystem enjoy what was formerly known to be the best choice.

Right.  Though I wouldn't be opposed to a configure option or something...

Alright.  I think I'm pretty sure if anyone's going to lose here, it's going to be Arch.   I'd still like to figure out some way to work around it upstream though.

> That said, I've always wondered about the lack of "/usr/bin/env python2"
> and "/usr/bin/env python3" (and similar for other script interpreters).

So the word on "python2" is that upstream Python won't ship it since Python2 is in maintenance mode, and while it exists in e.g. RHEL, it doesn't in Debian.
Comment 61 Colin Walters 2011-08-25 21:27:52 UTC
(In reply to comment #60)
>
> Right.  Though I wouldn't be opposed to a configure option or something...

I think if Arch wanted to supply a patch which detected the mess in configure.ac - default to /usr/bin/env python, but if "/usr/bin/env python" is Python 3, try "/usr/bin/env python2".  Then substitute it in all the scripts as @PYTHON@ (override Automake's determination here).

That's a separate bug though.
Comment 62 Colin Walters 2011-08-25 21:31:02 UTC
Attachment 194741 [details] pushed as 0eaec4e - Avoid using - (hyphen) in gdbus-codegen directory name
Attachment 194742 [details] pushed as 5dc3c2e - Use relative imports for the gdbus/codegen package
Attachment 194743 [details] pushed as 5391aae - Introduce the UNINSTALLED_GLIB_BUILDDIR environment variable
Attachment 194744 [details] pushed as cd0cd95 - Introduce the UNINSTALLED_GLIB_SRCDIR environment variable
Attachment 194746 [details] pushed as a2614ef - Make gdbus-codegen 'relocatable' at runtime on Windows.
Attachment 194747 [details] pushed as 7d67971 - Add gdbus-codegen to glib-zip.in
Comment 63 Colin Walters 2011-08-25 21:40:52 UTC
I've pushed this all; thanks for the great work Dieter!
Comment 64 Dieter Verfaillie 2011-08-26 07:07:41 UTC
(In reply to comment #60)
> I don't understand how in your proposal Arch can make it work without patching
> glib - since your patches use "/usr/bin/env python" unconditionally.  Or am I
> missing something?

$ export PYTHON=/path/to/python2
$ /usr/bin/env | grep PYTHON
PYTHON=/path/to/python2
$ /usr/bin/env python
Python 2.7.2(...)
>>> ^D

$ export PYTHON=/path/to/python3
$ /usr/bin/env | grep PYTHON
PYTHON=/path/to/python3
$ /usr/bin/env python
Python 3.2(...)
>>> ^D

$ export PYTHON=/home/dieterv/checkout/experimental/Python32/bin/python
$ /usr/bin/env | grep PYTHON
PYTHON=/home/dieterv/checkout/experimental/Python32/bin/python
$ /usr/bin/env python
Python 3.2(...)
>>> ^D

Their (and other) build systems already export the PYTHON envvar to
point to /usr/bin/python2 for some packages before ./configure && make.
With Arch, it looks like it is done manually in the PKGBUILD files. Some
other distros do so automatically when Python2 is detected as a dependency
(and Python2 is a dependency of glib if you want gdbus-codegen and other
stuff to function).

I think this gets picked up by AM_PATH_PYTHON through the first thing it
does: AC_ARG_VAR([PYTHON], [the Python interpreter])

See here for more info:
http://git.savannah.gnu.org/cgit/automake.git/tree/m4/python.m4

> Related to this there is a PEP proposal:
> http://www.python.org/dev/peps/pep-0394/

Yeah, that's for the installed binaries (or symlinks) on Unix systems, so
we can have /usr/bin/python2, /usr/bin/python3 and /usr/bin/python which
can be anything. This is not portable, Windows is excluded from this PEP 
and rightly so, it's already treated as a "mutli-prefix" system:
C:\Python26
C:\Python27
C:\Python32
...

Some talk about PYTHONPATH envvars, but sadly nothing about PYTHON2 or
PYTHON3 envvars to be used with /usr/bin/env shebangs. So we're left
shoehorning multiple incompatible script interpreters into a single
PYTHON envvar.
 
> Alright.  I think I'm pretty sure if anyone's going to lose here, it's going to
> be Arch.   I'd still like to figure out some way to work around it upstream
> though.

Nobody loses anything except maybe a little bit of time to make sure the
PYTHON envvar points to /usr/bin/python2 where it matters. But everybody
is already doing this anyway...
 
> > That said, I've always wondered about the lack of "/usr/bin/env python2"
> > and "/usr/bin/env python3" (and similar for other script interpreters).
> 
> So the word on "python2" is that upstream Python won't ship it since Python2 is
> in maintenance mode, and while it exists in e.g. RHEL, it doesn't in Debian.

See above, it's about the envvars :)

(In reply to comment #61)
> I think if Arch wanted to supply a patch which detected the mess in
> configure.ac - default to /usr/bin/env python, but if "/usr/bin/env python" is
> Python 3, try "/usr/bin/env python2".  Then substitute it in all the scripts as
> @PYTHON@ (override Automake's determination here).

A proper solution would probably be best implemented in automake's python.m4.
Glib could ship an updated version of python.m4 until a newer automake version
then becomes required. I'm already working on some python.m4 improvement
to be used with gobject-introspection, pygobject, glade, etc related to
the windows port but things evolve slowly. As you said, in time, that will be
a separate bug report :)

(In reply to comment #63)
> I've pushed this all; thanks for the great work Dieter!

Many thanks for continuing to listen to my ramblings :)

mvg,
Dieter
Comment 65 Dieter Verfaillie 2011-08-26 07:24:06 UTC
(In reply to comment #59)
> If that change is reverted then I propose to use the same hack applied to
> gtester-report by using install-exec-hook

I suppose you mean executing as $(PYTHON) gtester-report from Makefiles (it
already has a #!/usr/bin/env python shebang)? I'll look into this in the
days to come. If you meant something else, please don't hesitate to mail me ;)

Thanks,
Dieter
Comment 66 Ionut Biru 2011-08-30 16:06:52 UTC
You reverted that commit without understanding my point, even that i replied via email.

My point is that usr/bin/gtester-report uses the detected PYTHON and gdbus-codegen doesn't. It is an inconsistency across the project.

Clearly is a bug in either gstester-report or in gdbus-codegen. In which one is it?
Comment 67 Dieter Verfaillie 2011-08-30 19:25:27 UTC
(In reply to comment #66)
> You reverted that commit without understanding my point, even that i replied
> via email.
> 
> My point is that usr/bin/gtester-report uses the detected PYTHON and
> gdbus-codegen doesn't. It is an inconsistency across the project.
> 
> Clearly is a bug in either gstester-report or in gdbus-codegen. In which one is
> it?

gtester-report. Haven't had much (well, any) hacking time in the last days
and don't expect to be able to do much until monday, unfortunately. But do
expect a new bug report for gtester-report (I'll cc you) the moment I can
find a spare 30 minutes or so...
Comment 68 Dieter Verfaillie 2011-09-05 09:38:01 UTC
(In reply to comment #67)
> gtester-report. Haven't had much (well, any) hacking time in the last days
> and don't expect to be able to do much until monday, unfortunately. But do
> expect a new bug report for gtester-report (I'll cc you) the moment I can
> find a spare 30 minutes or so...

Done as bug #658237.