Python 3 support by monnerat · Pull Request #34 · mate-desktop/python-caja

monnerat · 2019-01-19T00:44:37Z

This PR introduces Python 3 compatibility while retaining Python 2 compatibility.
It also features build failures when a symbol cannot be resolved at link time, and allows to make dist without building the documentation.
Runs nice here on Fedora 29 x86_64.

Ref: #30

Thanks for considering it.

This also requires to link with the gmodule library.

The updated sources are still compatible with Python 2.

Python 3 appends the ABI flags to the version in the shared library file name: since this library is loaded as a gmodule, the correct file name must be provided.

raveit65 · 2019-01-24T12:55:37Z

Which program do you use to test this in f29?
rabitcvs-caja doesnt shows me any icon in menus and as root i got this warnings.

[root@mother rave]# sudo caja
Traceback (most recent call last):
  File "/usr/share/caja-python/extensions/RabbitVCS.py", line 75, in <module>
    import rabbitvcs.vcs.status
  File "/usr/lib/python3.7/site-packages/rabbitvcs/vcs/status.py", line 403, in <module>
    class TestStatusObjects(unittest.TestCase):
  File "/usr/lib/python3.7/site-packages/rabbitvcs/vcs/status.py", line 408, in TestStatusObjects
    os.path.join(base, chr(x)) for x in range(97,123) 
  File "/usr/lib/python3.7/site-packages/rabbitvcs/vcs/status.py", line 408, in <listcomp>
    os.path.join(base, chr(x)) for x in range(97,123) 
NameError: name 'base' is not defined
Initializing caja-open-terminal extension
Initializing caja-image-converter extension
Initializing caja-dropbox 1.21.0
Initializing caja-xattr-tags extension

Beside from that it builds fine with f29 and python3
Didn't test to build it for f28 and python2.

raveit65 · 2019-01-24T13:05:56Z

And building for f28 with python2 gives me that build error.

caja-python-object.c:58:26: warning: implicit declaration of function 'PyInt' [-Wimplicit-function-declaration]
 #define INT_ASLONG(obj)  PyInt(obj)
                          ^~~~~
caja-python-object.c:449:8: note: in expansion of macro 'INT_ASLONG'
  ret = INT_ASLONG(py_ret);
        ^~~~~~~~~~
/bin/sh ../libtool  --tag=CC   --mode=link gcc -I/usr/include/python2.7  -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -module -avoid-version -Wl,-z,defs -Wl,-z,relro  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -o libcaja-python.la -rpath /usr/lib64/caja/extensions-2.0 libcaja_python_la-caja-python.lo libcaja_python_la-caja-python-object.lo -lpython2.7  -lgmodule-2.0 -pthread -lglib-2.0 -lcaja-extension -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0  
libtool: link: gcc -shared  -fPIC -DPIC  .libs/libcaja_python_la-caja-python.o .libs/libcaja_python_la-caja-python-object.o   -lpython2.7 -lgmodule-2.0 -lcaja-extension -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -O2 -g -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -Wl,-z -Wl,defs -Wl,-z -Wl,relro -Wl,-z -Wl,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -pthread   -pthread -Wl,-soname -Wl,libcaja-python.so -o .libs/libcaja-python.so
.libs/libcaja_python_la-caja-python-object.o: In function `caja_python_object_update_file_info':
/builddir/build/BUILD/python-caja-1.21.0/src/caja-python-object.c:449: undefined reference to `PyInt'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:482: libcaja-python.la] Error 1
make[2]: Leaving directory '/builddir/build/BUILD/python-caja-1.21.0/src'
make[1]: *** [Makefile:553: all-recursive] Error 1
make[1]: Leaving directory '/builddir/build/BUILD/python-caja-1.21.0'
make: *** [Makefile:437: all] Error 2

Ref: #34 (comment)

monnerat · 2019-01-24T13:40:27Z

Which program do you use to test this in f29?
rabitcvs-caja doesnt shows me any icon in menus and as root i got this warnings.

Yes, I know: rabbitvcs is not yet ready for Python 3. However I'm working on the conversion (not yet ready), and I already have menus and icons (they are often wrong and need manual refresh, but that's rabbitvcs business!).
I did not test with another extension.

And building for f28 with python2 gives me that build error.

Oops, my bad: this is a cut and paste error that should be fixed now by commit 2f08aba

raveit65 · 2019-01-24T15:56:05Z

Ok, python2 build works again with latest commit and i could test it with a rebuild of obsolete caja-terminal rpm,
https://koji.fedoraproject.org/koji/taskinfo?taskID=32228076
Which works fine.
In f28/29 we have only a few packages which requires python-caja.

[root@mother rave]# dnf repoquery --whatrequires python2-caja
caja-terminal-0:0.10-3.fc28.x86_64
nextcloud-client-caja-0:2.3.3-2.fc28.x86_64
nitroshare-extension-caja-0:0.3.4-6.fc28.x86_64
owncloud-client-caja-0:2.4.0-2.fc28.x86_64
python-caja-devel-1:1.20.0-1.fc28.x86_64
python-caja-devel-1:1.20.1-1.fc28.x86_64
python-caja-devel-1:1.20.2-1.fc28.x86_64
python-caja-devel-1:1.21.0-1.fc28.x86_64
python-caja-devel-1:1.21.0-2.fc28.x86_64
rabbitvcs-caja-0:0.17.1-4.fc28.noarch

I have no idea to test python3 build with nextcloud-client-caja, nitroshare-extension-caja or owncloud-client-caja extensions.
@mate-desktop/core-team @mate-desktop/contributors
Maybe there are more packages in debian/ubuntu or other distros?
And someone use one :)

monnerat · 2019-01-24T16:18:35Z

I have no idea to test python3 build with nextcloud-client-caja, nitroshare-extension-caja or owncloud-client-caja extensions.

Me neither :-(
Maybe we should include a test/example extension in this project? Something very simple that can be manually installed in /usr/share/caja-python/extensions for testing purposes.
If you agree, I can write one and submit it in another pull request: this will take some time however.

raveit65 · 2019-01-24T16:28:54Z

Sure, adding an test example make sense.

monnerat · 2019-01-26T14:55:00Z

Maybe we should include a test/example extension in this project?

Sure, adding an test example make sense.

I did not see there were already some example extensions in subdir examples :-|

Please check PR #35

You can then use the new mixed.py example for (almost) full-featured testing.

raveit65

I tested python3 port with examples from #35
Compiled with python2 is already tested in previous comment by me.
Looks good to me.

vkareh

This works well, thanks!

The example extension scripts need to be in sync with new versions of referenced foreign packages. This commit also makes them compatible with Python version 3 (retaining Python 2 compatibility). An additional example extension "mixed" is added: it implements all caja extension features and can also be used as a new extension pattern. See source header comment for a description. Ref: #34 (comment)

monnerat · 2019-01-31T13:19:05Z

Thanks for merge!

raveit65 · 2019-02-01T12:31:36Z

@monnerat
I just added your commits to fedora builds
https://bodhi.fedoraproject.org/updates/FEDORA-2019-4e0728ee21
https://bodhi.fedoraproject.org/updates/FEDORA-2019-305db4d8db

monnerat · 2019-02-01T12:35:06Z

I just added your commits to fedora builds

Thanks: I will check and leave karma :-)

monsta · 2019-02-04T15:09:00Z

Should we keep AM_PATH_PYTHON([2.7]) as is for detection?
I had to pass PYTHON=python3 env var to ./configure to make it build against Python 3 library on a system where /usr/bin/python still points to Python 2...

monnerat · 2019-02-04T15:18:19Z

Should we keep AM_PATH_PYTHON([2.7]) as is for detection?

Well, I've left it unchanged as a minimum version requirement.
But if that is problematic, we may introduce a --with-python-version=xxx configure argument with a default value if not provided.

monsta · 2019-02-05T13:10:44Z

Might be useful, yes. At least it would be an explicit option for configure, not some undocumented env var... 🙂

monnerat · 2019-02-05T16:29:56Z

I had to pass PYTHON=python3 env var to ./configure to make it build against Python 3 library on a system where /usr/bin/python still points to Python 2...

I've submitted PR #37 to fix this problem.

we may introduce a --with-python-version=xxx configure argument with a default value if not provided.

I have implemented it as an internal setting of PYTHON envvar rather than the proposed solution, because AM_PATH_PYTHON has no maximum version requirement :-(

At least it would be an explicit option for configure, not some undocumented env var.

Sure!

monnerat added 5 commits January 19, 2019 01:10

Fail to build if some referenced symbol cannot be resolved at link time.

f0ef25f

This also requires to link with the gmodule library.

Allow to "make dist" without building the docs.

f7dc3fe

Implement Python 3 C API compatibility using conditional and macros.

e200f04

The updated sources are still compatible with Python 2.

Make Python statements compatible with version 2 and 3.

da078d9

Include Python 3 ABI flags in shared library name.

010bd84

Python 3 appends the ABI flags to the version in the shared library file name: since this library is loaded as a gmodule, the correct file name must be provided.

raveit65 requested review from a team January 19, 2019 08:26

Fix Python 2 INT_ASLONG function.

2f08aba

Ref: #34 (comment)

monnerat mentioned this pull request Jan 26, 2019

Modernize examples #35

Merged

raveit65 approved these changes Jan 26, 2019

View reviewed changes

raveit65 requested a review from a team January 26, 2019 19:48

vkareh approved these changes Jan 30, 2019

View reviewed changes

vkareh merged commit d349766 into mate-desktop:master Jan 30, 2019

monnerat deleted the pm-python3 branch February 5, 2019 14:37

monnerat mentioned this pull request Feb 5, 2019

Add an explicit --with-python= configure option #37

Closed

monsta mentioned this pull request Feb 6, 2019

Python 3 compatibility #30

Closed

Uh oh!

Conversation

monnerat commented Jan 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raveit65 commented Jan 24, 2019

Uh oh!

raveit65 commented Jan 24, 2019

Uh oh!

monnerat commented Jan 24, 2019

Uh oh!

raveit65 commented Jan 24, 2019

Uh oh!

monnerat commented Jan 24, 2019

Uh oh!

raveit65 commented Jan 24, 2019

Uh oh!

monnerat commented Jan 26, 2019

Uh oh!

raveit65 left a comment

Choose a reason for hiding this comment

Uh oh!

vkareh left a comment

Choose a reason for hiding this comment

Uh oh!

monnerat commented Jan 31, 2019

Uh oh!

raveit65 commented Feb 1, 2019

Uh oh!

monnerat commented Feb 1, 2019

Uh oh!

monsta commented Feb 4, 2019

Uh oh!

monnerat commented Feb 4, 2019

Uh oh!

monsta commented Feb 5, 2019

Uh oh!

monnerat commented Feb 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

monnerat commented Jan 19, 2019 •

edited

Loading