Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Port Python extensions to Python 3 #145

Closed
wants to merge 163 commits into from

Conversation

EduardDopler
Copy link
Contributor

As Python 2 will be discontinued and I don't want to see nemo-extensions depend on that gray version after end-of-support, I ported all Python scripts to the new version. For the source package of nemo-python the minimum version is 3.3 now which is supported since around Ubuntu Quantal/Raring, Debian Jessie and in all supported Fedora versions. The extensions are satisfied with Python 3.0 or 3.1.

Many extensions were in bad shape concerning programming standards, conventions and documentation. As we know, this makes it hard to maintain software and introduce new features or fix bugs without introducing new bugs e.g. due to unknown side effects. Moreover, by cleaning up the code and simplifying it where possible, performance issues were addressed as the old code often made unnecessary checks, loops or system calls.

Details on the changes can be found in the corresponding commit messages. Here are the most prominent ones:

nemo-compare: f66d179 bea81f0

  • total cleanup, new preference tool, less error-prone

nemo-dropbox: 744da22

  • almost no changes (2500 lines of dirty mystery code)

nemo-emblems: 603c8d0

  • total cleanup, better performance
  • fixes bug which passed changes of multiple emblem property pages only to the most recently opened one

nemo-folder-color-switcher: e7f20ae e921dd9

  • included in this repository (see below)
  • total cleanup, better performance

nemo-metadata-columns: 722f809 cf6d1f8 d3308bb 25a3d9f

  • new extension (replaces nemo-media-columns)
  • more tags, simpler lookups, translated tag values, logically combined
    values

nemo-metadata-tab: 60fe0e2 fd7d7c7 cf6d1f8 d3308bb 25a3d9f

  • new extension (replaces nemo-audio-tab)
  • more file types (audio, video, image, PDF)
  • all printable tags are read, the most important ones are displayed by default, the other ones are behind an expander
  • translated tag values, cover art for audio files (ID3 and FLAC)
  • image evaluation can be toggled in-code (as nemo has parts of that built-in)

nemo-pastebin: bd3f90f

  • less error-prone, new preference tool, more options, watches config file automatically
  • notifications are now essential, providing feedback throughout the process
  • added actions to the notifications ("Copy URL to clipboard", "Open in browser")

nemo-terminal: 4dfe557

  • minor cleanup
  • introduced translations

nemo-rabbitvcs:

  • removed due to major incompatibilities (both frontend and backend cannot be ported to Python 3, plus, it seems to produce many issues anyway. RIP.)
  • I have a nemo-git extention on my personal TODO list.
  • Initial work for visual git feedback is now (Jan 8 2016) introduced in daf3705.

As mentioned, (nemo-)folder-color-switcher was imported into this repository since it seems to belong here. All caja parts were removed (there is a caja-extensions repository for that).

In addition to the changes listed above all build files received updates and follow a mostly consistent style now. E.g. all Python extensions use pybuild, all C extensions provide the same autoconf layout in their configure.ac files. Besides, some deprecation warnings during the build process were addressed; some translation files have updated line references. Henceforth all refactored extensions present proper README files. Important changes for each extension are mentioned in */debian/changelog.

All extensions are tested with Linux Mint 17.2 and 17.3 and work flawlessly for several weeks.

This commit closes or resolves the following GitHub issues (or makes them obsolete):

clefebvre and others added 30 commits August 26, 2014 15:13
Replace os.system with subprocess.call
nemo-compare.pot now reflects the actual translation strings.
Existing translations were updated as well. Note that the copyright
information from po files were incompatible with the GPL and hence
removed.
Also, reference information in po files are now relative to the top
directory of the application as recommended[1][2] by GNU gettext. In
particular, references now start with "src/".

[1] https://www.gnu.org/software/gettext/manual/gettext.html#C-Sources-Context
[2] https://www.gnu.org/software/gettext/manual/gettext.html#po_002fPOTFILES_002ein
The %-style substitutions allow fast combining of a fixed string with
variable content. So
    "Compare to: " + other
changed to
    "Compare to: %s" % other
Unlike concatenating, substituations account for flexible insertions and
can thus be helpful for more natural descriptions in some languages.
Moreover, RTL translations become possible.
@leigh123linux
Copy link
Contributor

leigh123linux commented Sep 23, 2016

@clem rabbitvcs provides the nemo extension

https://github.com/rabbitvcs/rabbitvcs/commits/master

rabbitvcs upstream seems pretty dead and bug riddled.

P.S I wouldn't be bothered by it's complete removal.

@EduardDopler
Copy link
Contributor Author

@clefebvre The problem is that the old Python 2 scripts won't work with a nemo-python built against Python 3. So one would have to edit all old extensions in order to make them work semantically identical (this is not limited to only changing print to a function…). After that tedious work all bugs and shortcomings I found while refactoring or rewriting the extensions would still be existent. But as the files changed, the changes found in this PR could not be applied anymore; one would have to address all merge conflicts again.
If this is something* someone wants to do, he or she is very free to do so. But I won't.

*something= go back to current Python 2 state, apply changes only to nemo-python, then fix all 2to3-related bugs, test if all extensions work as expected, then fix build scripts (I updated a lot of them). At that state everything should work like it does now besides running on Py3 instead of Py2. Note though that I rewrote some extensions from scratch in a (hopefully) clean, documented, understandable and notably more efficient/faster manner. If you want that changes too, you'd have to extract my changes from this PR too and fix all merge conflicts coming from that new state.

@clefebvre
Copy link
Member

I fully understand that. After you put so much (way too much I would say) efforts into this, and mixed this huge variety of changes into a PR which I cannot merge, I am not asking you to produce more work. I'm thankful for what you've done, I'm sorry you did it the way you did and I'm not expecting you to split this into workable WIPs.

If some of these changes go through (and some should, whether we move to python 3 or not, that in itself really illustrates the need for modularity and isolation here), extra work will be required and I'm not suggesting that falls onto you.

@EduardDopler
Copy link
Contributor Author

@clefebvre And I fully understand your point as a the LM maintainer. Maybe I wouldn't merge such a PR either. But—just to explain my decision back then—I felt that splitting the work into several PRs didn't make sense because, as I said, all depends on nemo-python and its Python version chosen there, i.e. if I ported nemo-python first without the extensions, the PR would have broken every extension. Same goes for the extensions themselves: Porting them without having an already ported nemo-python makes no sense. Hence I put everything together because that's what my perception of a Pull Request is in the end: It introduces something without breaking anything existent.
Anyway, I think we made our points.

@clefebvre
Copy link
Member

We did.

And I kind of agree with you on putting extensions and extension-system into the same bag and thus the same PR. We sometimes split dependent PRs (and sometimes we have to, for instance when they affect distinct projects, that's often the case in Cinnamon for instance... a WIP affecting the settings daemon + some lib + the settings + the screensaver or whatever), and it's ok, as long as we get it all done during the cycle. It's not really an issue one way or another. Both approaches have their pros and cons.

What's more problematic is the merge of the folder switcher, the renaming/refactoring, the bug fixes, basically all the work you've put into that after/during the python 3 migration. I can't really have the ability to discuss them in isolation and reviewing them is much more complicated than reviewing the migration itself.

Other than the work you've done here, your comments and the situation we got ourselves into... all of this is also helping us. I do many different things within Linux Mint. The hardest part of my job is to turn down help or to refuse work that has already been done. It is not easy to listen to criticism but it is valuable.

You don't know this but your comments were discussed by many developers and this led to a very long discussion about style and best practices. Many disagreed about various things but we eventually decided to set up developer guidelines and put down on paper what felt important to many of us.

Today, I drafted a new section to list recommendations on pull requests. This document is very new and it can certainly get a lot better, but it's a start:

https://www.gitbook.com/book/linuxmint/developer-guidelines/details

So there you have it. It wasn't very pleasant, but I just wanted to show you. Some of your work might help implementing a potential move towards GTK3, but it's not just that. Your negative experience on this PR is helping us as well.

@EduardDopler
Copy link
Contributor Author

Okay, thank you for your elaboration. I'm kind of glad I could help—even though in a different way I intended 🍻

@muzena
Copy link
Contributor

muzena commented Sep 23, 2016

And what now, does it would be merged in some future time?
@EduardDopler
I install them, but they dosen't work well, maybe is missing some dependency.
nemo-metadata-tab on some multimedia files png jpg mkv avi mp3. there is no more tabs in properties,
nemo-metadata-columns eg. on video files Resolution FPS, aspect ratio... all is mostly unknown

@brmmm3
Copy link
Contributor

brmmm3 commented Sep 24, 2016

And what if nemo-python would support both python versions? nemo-python could be
extended so that it detects via the first line in the python file if it is for
Python 2 or Python 3. In this case it is not mandatory to transform all
extensions at once.
I had a look into the source code and I think it is relatively easy to implement
this.

We did.
And I kind of agree with you on putting extensions and extension-system into
the same bag and thus the same PR. We sometimes split dependent PRs (and
sometimes we have to, for instance when they affect distinct projects, that's
often the case in Cinnamon for instance... a WIP affecting the settings daemon

  • some lib + the settings + the screensaver or whatever), and it's ok, as long
    as we get it all done during the cycle. It's not really an issue one way or
    another. Both approaches have their pros and cons.
    What's more problematic is the merge of the folder switcher, the
    renaming/refactoring, the bug fixes, basically all the work you've put into
    that after/during the python 3 migration. I can't really have the ability to
    discuss them in isolation and reviewing them is much more complicated than
    reviewing the migration itself.
    Other than the work you've done here, your comments and the situation we got
    ourselves into... all of this is also helping us. I do many different things
    within Linux Mint. The hardest part of my job is to turn down help or to
    refuse work that has already been done. It is not easy to listen to criticism
    but it is valuable.
    You don't know this but your comments were discussed by many developers and
    this led to a very long discussion about style and best practices. Many
    disagreed about various things but we eventually decided to set up developer
    guidelines and put down on paper what felt important to many of us.
    Today, I drafted a new section to list recommendations on pull requests. This
    document is very new and it can certainly get a lot better, but it's a start:
    https://www.gitbook.com/book/linuxmint/developer-guidelines/details
    So there you have it. It wasn't very pleasant, but I just wanted to show you.
    Some of your work might help implementing a potential move towards GTK3, but
    it's not just that. Your negative experience on this PR is helping us as well.

    You are receiving this because you commented.
    Reply to this email directly, view it on GitHub, or mute the thread.

@clefebvre
Copy link
Member

@brmmm3 that's interesting. I never really thought about this. This would be great.

@brmmm3
Copy link
Contributor

brmmm3 commented Sep 26, 2016

I've already modified the code. It was easy. A bit more tricky now is the
modification of the build system. I'm not aware of autoconf and I have no
time to learn it. Would be great if someone else could do this. I can
pastebin the code when I am back home.

Regards

Martin

Am 26.09.2016 11:49 Vorm. schrieb "Clement Lefebvre" <
notifications@github.com>:

@brmmm3 https://github.com/brmmm3 that's interesting. I never really
thought about this. This would be great.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#145 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AURgARzjE6wxMTtGNMiIuUcvqSrgm72aks5qt5U3gaJpZM4G4tVS
.

@brmmm3
Copy link
Contributor

brmmm3 commented Sep 26, 2016

The easiest and IMHO cleanest way to support both is to modify Doplers nemo-
python plugin so that all occurrance of "nemo-python" are renamed to "nemo-
python3" so that installs as nemo-python3 and the extension search path
is /usr/share/nemo-python3/extensions.
I've done the changes and installed the plugin. But now nemo is crashing.
Setting NEMO_PYTHON_DEBUG to misc shows the following output:
nemo_module_initialize: entered
nemo_python_load_dir: entered dirname=/usr/share/nemo-python/extensions
nemo-python:g_module_open /usr/lib/x86_64-linux-gnu/libpython2.7.so.1.0
nemo-python:Py_Initialize
nemo-python:PySys_SetArgv
nemo-python:Sanitize the python search path
nemo-python:init_pygobject
nemo-python:import nemo
sys:1: PyGIWarning: Nemo was imported without specifying a version first. Use
gi.require_version('Nemo', '3.0') before import to ensure that the right version
gets loaded.
nemo_python_load_file: entered filename=nemo-media-columns
nemo_python_object_get_type: entered type=ColumnExtension
nemo-python:Loaded python modules
nemo_python_load_file: entered filename=nemo-compare
nemo_python_object_get_type: entered type=NemoCompareExtension
nemo-python:Loaded python modules
nemo_python_load_file: entered filename=nemo_terminal
nemo_python_object_get_type: entered type=NemoTerminalProvider
nemo-python:Loaded python modules
nemo_python_load_file: entered filename=nemo-folder-color-switcher
nemo_python_object_get_type: entered type=ChangeColorFolder
nemo-python:Loaded python modules
nemo_python_load_file: entered filename=nemo-emblems
nemo_python_object_get_type: entered type=EmblemPropertyPage
nemo-python:Loaded python modules
nemo_python_load_dir: entered dirname=/home/martin/.local/share/nemo-
python/extensions
nemo_module_list_types: entered
nemo_python_object_class_init: entered
nemo_python_object_instance_init: entered
nemo_python_object_class_init: entered
nemo_python_object_instance_init: entered
nemo_python_object_class_init: entered
nemo_python_object_instance_init: entered
[Nemo Terminal] I: Initializing the Nemo extension
nemo_python_object_class_init: entered
nemo_python_object_instance_init: entered
Initializing folder-color-switcher extension...
nemo_python_object_class_init: entered
nemo_python_object_instance_init: entered
Initializing nemo-image-converter extension
nemo-python3:nemo_module_initialize
nemo_module_initialize: entered
nemo-python3:nemo_python_load_dir
nemo_python_load_dir: entered Dirname=/usr/share/nemo-python3/extensions
nemo_python_load_dir: entered INIT=0
nemo_python_load_dir: entered NAME0=submenu.py
nemo_python_load_dir: entered NAME1=submenu.py
nemo_python_load_dir: entered MODULE=submenu
nemo-python3:INIT PYTHON
nemo-python3:g_module_open /usr/lib/x86_64-linux-gnu/libpython3.5m.so.1.0
nemo-python3:PY3: Py_Initialize
nemo-python3:PySys_SetArgv
nemo-python3:Sanitize the python search path
nemo-python3:init_pygobject
nemo-python3:import nemo
nemo-python3:nemo_python_load_file
nemo_python_load_file: entered Filename=submenu
Speicherzugriffsfehler
It seems to be crashing at this line:
main_module = PyImport_AddModule("main");
I also needed to replace Py_IsInitialized() by the "initialized" status
variable, because the function returns true, although Python3 is not
initialized.
It seems that Python2 functions are called. So currently I'm a little bit
confused what is going wrong here.

@brmmm3 that's interesting. I never really thought about this. This would be
great.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@clefebvre
Copy link
Member

@brmmm3 were you successful in the end, did you manage to make this work?

@brmmm3
Copy link
Contributor

brmmm3 commented Oct 26, 2016

Hi,

I've merged the two existing versions of the nemo-python plugin into one to
support both, Python 2 and Python 3. I could create an so-file. But when the
modules for Python 2 and Python 3 (libpython) are loaded nemo-python fails. The
problem is that both libraries are using the same function names. So when the
Python 2 library is loaded, then the symbols for the Python 3 part are
initialized too. Executing a Python 3 script will then cause a segfault.
A solution would be maybe to link statically to both python libraries, but then
the nemo-python plugin would become very big and the solution would not be
really nice. You can find the sources in the attachment.
I think the best solution is to convert all python scripts at once to Python 3.

Best regards,

Martin

@brmmm3 were you successful in the end, did you manage to make this work?

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@brmmm3
Copy link
Contributor

brmmm3 commented Oct 28, 2016 via email

@clefebvre
Copy link
Member

Ok, well it was worth trying anyhow. Thanks @brmmm3.

I guess we're back to square one. We can try planning that for 3.4.

We're just a few days from 3.2 right now.

@clefebvre
Copy link
Member

Hi everyone,

Thanks for your patience with this. We'll do the following:

  • We'll leave things as they are in Nemo 3.2.
  • We'll tackle this migration in preparation for Nemo 3.4.
  • We'll warn the rabbitvcs team about our migration to python 3.

That gives rabbitvcs 6 months to anticipate a change. I'm not sure it's enough, assuming it's possible. But it's better than just breaking compatibility overnight.

@clefebvre clefebvre changed the title Port Python extensions to Python 3 [WIP] Port Python extensions to Python 3 Nov 7, 2016
@clefebvre
Copy link
Member

No response from rabbitvcs.

I'm happy to discontinue support for it and to move to python3.

@clefebvre
Copy link
Member

Follow up on this PR. This didn't make it in 3.6. There's no hurdles left for this to happen, it's just that we had to focus on other things so this was left aside. We'll try to migrate to python 3 for 3.8. There's no real hurry, but most of the work is done in this PR already so it's just a matter of re-applying changes and testing things to make sure it all works. We'll get it done eventually, thank you for your patience.

@adamplumb
Copy link

Hi guys, I'm the maintainer of RabbitVCS. I made a new RabbitVCS release in June that fixed a bunch of bugs and prepared the way for a future Python3 release, using the six module. The nemo extension that we have has also been updated, so it should run in python3. The RabbitVCS UI/backend code still requires python2, for now, but I'm hoping to get some time to work on it.

@clefebvre
Copy link
Member

Hi @adamplumb,

Shall we delete the rabbitvcs extension we have in this repository for the time being? We can rely on yours in the meantime and decide late whether to bring it back here or if you want to develop it yourself?

We can proceed to migrate to python3 then also.

@adamplumb
Copy link

adamplumb commented Dec 31, 2017 via email

@mtwebster
Copy link
Member

no longer needed

@mtwebster mtwebster closed this Jul 21, 2018
@mrbesen
Copy link

mrbesen commented Jun 28, 2020

Is it planned to merge nemo-metadata-tab and the other changes for those plugins?
I would love to be able to see the audio metadata in the properties of a File.

@mtwebster
Copy link
Member

We don't maintain that plugin - all the ones that we maintain have been ported to python3.

You'll need to contact whoever maintains that plugin. (I see some PPAs for it but I'm not sure where the actual source is maintained).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment