Bug 49060 - Migrate to Python 3
Migrate to Python 3
Status: NEW
Product: UCS
Classification: Unclassified
Component: General
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 5.0
Assigned To: UCS maintainers
UCS maintainers
https://hutten.knut.univention.de/med...
:
Depends on: 49219 49392 49665 50942 49325 49683 49704 49720 50461 50475 50483 50943 51282
Blocks: 49397
  Show dependency treegraph
 
Reported: 2019-03-22 11:45 CET by Jannik Ahlers
Modified: 2020-06-23 13:22 CEST (History)
8 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
suggested patch for UCR binary API (2.67 KB, patch)
2019-07-11 15:09 CEST, Florian Best
Details | Diff
migrate-ucs-template-python3.py (1.78 KB, text/x-python)
2019-12-06 12:26 CET, Florian Best
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jannik Ahlers univentionstaff 2019-03-22 11:45:48 CET
In UCS we are still using python 2.7.
Official support for python 2 will end on 2019-01-01¹.
Debian stretch still includes python 2.7, so the package will still get some maintenance until the LTS ends in 2022².
Samba 4.10 will be the last release that comes with full support for
Python 2³. As Samba has a ~6 month release cycle, the next UCS version has to have py3 to be able to use the newest Samba version.

It would be best to migrate before the end of 2019, but a migration for the next UCS major release would probably still be acceptable.

Another pro for migrating before a new major release is that we have to support 4.4 for up to 7 years, so eventually we would have to maintain py 2.7 ourselves.


¹ https://www.python.org/dev/peps/pep-0373/
² https://en.wikipedia.org/wiki/Debian_version_history#Release_table
³ https://www.samba.org/samba/history/samba-4.10.0.html
Comment 2 Daniel Tröder univentionstaff 2019-03-23 08:05:55 CET
For our Debian packages to support Python 3 we have to move from python-support to dh-python.
Work done in a hackathon in 4.3-3 branch "juern/dh-python" was rebased to 4.4-0 in branch "dtroeder/dh-python-4.4-0".
Comment 3 Florian Best univentionstaff 2019-03-23 10:49:47 CET
(In reply to Daniel Tröder from comment #2)
> For our Debian packages to support Python 3 we have to move from
> python-support to dh-python.
> Work done in a hackathon in 4.3-3 branch "juern/dh-python" was rebased to
> 4.4-0 in branch "dtroeder/dh-python-4.4-0".

Maybe change the default import order to python3 modules?
Or use six.moves for all those imports?
univention-python-legacy still needs python_support? (why name it legacy? it it the old univention-python?)
Comment 4 Daniel Tröder univentionstaff 2019-03-24 21:51:06 CET
(In reply to Florian Best from comment #3)
> (In reply to Daniel Tröder from comment #2)
> > For our Debian packages to support Python 3 we have to move from
> > python-support to dh-python.
> > Work done in a hackathon in 4.3-3 branch "juern/dh-python" was rebased to
> > 4.4-0 in branch "dtroeder/dh-python-4.4-0".
> 
> Maybe change the default import order to python3 modules?
> Or use six.moves for all those imports?
Very good - I wasn't aware of this.
> univention-python-legacy still needs python_support? (why name it legacy? it
> it the old univention-python?)
No, it's a module to help find univention.* below both /usr/share/pyshared (installed by pysupport) and /usr/lib/python2.7/dist-packages (installed by dh_python2).
Without this, depending on the import order, a lot of imports will fail.
The reason is the gradual migration to dh_python2. If the Python interpreter imported, lets say univention.ucr first from dist-packages, then it will fail when it tries to import univention.admin, as it is not yet located there, but is still in pyshared.
The module provided by univention-python-legacy is not about Python 2, but about the transition from pysupport to dh_python2.
Comment 5 Florian Best univentionstaff 2019-03-25 07:45:37 CET
(In reply to Daniel Tröder from comment #4)
> The module provided by univention-python-legacy is not about Python 2, but
> about the transition from pysupport to dh_python2.
Ok.

An idea to at least make this easier in the future would be to use namespacing packages:
https://www.python.org/dev/peps/pep-0420/
Namespacing allows having sub-packages in different locations.  But I don't know if we need this?
This would require that we don't have a __init__.py for at least univention/ and univention/management/console/modules/ (and every other package under univention/ which we need to extend from different pathes).
Comment 6 Arvid Requate univentionstaff 2019-03-25 13:42:46 CET
Maybe univention-pysupport-transition is a better name?
Comment 7 Florian Best univentionstaff 2019-04-02 12:12:52 CEST
As we discussed last week:
The "except ImportError: sys.exit(0)" in the PAM scripts is problematic.
The script can be set to optional in the PAM stack via setting homedir/mount/required=false.
univention-user-quota and univention-skel are already marked as optional.

So please revert that part.

(In reply to Arvid Requate from comment #6)
> Maybe univention-pysupport-transition is a better name?
@Daniel, what do you think about this name? I find it good.

A question I have is:
Do we need a python3-* package for every python package we have? I think you said something like this.
Comment 8 Daniel Tröder univentionstaff 2019-04-02 14:05:52 CEST
(In reply to Florian Best from comment #7)
> As we discussed last week:
> The "except ImportError: sys.exit(0)" in the PAM scripts is problematic.
> The script can be set to optional in the PAM stack via setting
> homedir/mount/required=false.
> univention-user-quota and univention-skel are already marked as optional.
> 
> So please revert that part.
[dtroeder/dh-python-4.4-0] 60b0ae3ebf Revert "Bug #49060: make sure system can be logged into (PAM isn't blocked by an ImportError)"

This reverts commit c44724a4
The problem will be dealt with by Bug #49219.

> (In reply to Arvid Requate from comment #6)
> > Maybe univention-pysupport-transition is a better name?
> @Daniel, what do you think about this name? I find it good.
"univention-pysupport-transition" is an excellent name.

> A question I have is:
> Do we need a python3-* package for every python package we have? I think you
> said something like this.
Yes - Debian expects us to use that naming scheme: https://wiki.debian.org/Python/LibraryStyleGuide

For library code we must provide Python2 and Python3 packages.
Standalone software like daemons, scripts etc. should simply be ported to Python3 without keeping a useless Python2 version around.
Comment 9 Florian Best univentionstaff 2019-04-02 21:11:02 CEST
(In reply to Daniel Tröder from comment #8)
> > A question I have is:
> > Do we need a python3-* package for every python package we have? I think you
> > said something like this.
> Yes - Debian expects us to use that naming scheme:
> https://wiki.debian.org/Python/LibraryStyleGuide
> 
> For library code we must provide Python2 and Python3 packages.
> Standalone software like daemons, scripts etc. should simply be ported to
> Python3 without keeping a useless Python2 version around.

Where is the python code installed then? And does /usr/share/pyshared/univention/ still exists?
Comment 10 Philipp Hahn univentionstaff 2019-04-03 09:48:21 CEST
(In reply to Florian Best from comment #9)
> Where is the python code installed then? And does
> /usr/share/pyshared/univention/ still exists?

No, not not assume it is used. python2* and python3* packages use different paths as they most often do NOT share identical files. Using /u/s/pyshared/ directly was never right.
Comment 11 Florian Best univentionstaff 2019-04-04 15:15:36 CEST
(In reply to Philipp Hahn from comment #10)
> (In reply to Florian Best from comment #9)
> > Where is the python code installed then? And does
> > /usr/share/pyshared/univention/ still exists?
> 
> No, not not assume it is used. python2* and python3* packages use different
> paths as they most often do NOT share identical files. Using /u/s/pyshared/
> directly was never right.
Okay, then the first step - even before migrating to dh_python2 would be to fix all the current /usr/share/pyshared *cross-package* references.

Agree?

services/univention-support-info/univention-support-info:»   if os.path.isfile('/usr/share/pyshared/univention/config_registry.py'):
services/univention-support-info/univention-support-info:»   »   config_registry = imp.load_source('config_registry', '/usr/share/pyshared/univention/config_registry.py')
services/univention-support-info/univention-support-info:»   »   sys.path.append('/usr/share/pyshared')
→ must be done prior to UCR

services/univention-printserver/cups-printers.py:»   »   »   subprocess.call(['python', '/usr/share/pyshared/univention/lib/share_restrictions.py'])
→ must be done prior to univention-lib

services/univention-nfs/81univention-nfs-server.inst:if [ -f /usr/share/pyshared/univention/admin/handlers/policies/thinclient.py ] ; then
→ must be done prior to UDM

management/univention-ldap/univention-backup2master:if [ -f /usr/share/pyshared/univention/admin/handlers/policies/thinclient.py ] ; then
→ must be done prior to UDM

ucs-school-umc-computerroom/debian/ucs-school-umc-computerroom.install:umc/python/computerroom/plugins usr/share/pyshared/univention/management/console/modules/computerroom
→ must be compatible to customer extensions
ucs-school-umc-computerroom/italc2-ctrl.py:sys.path.insert(0, '/usr/share/pyshared/univention/management/console/modules/computerroom')
→ must be done prior to UMC

management/univention-directory-manager-modules/listener/udm_extension.py:PYSHARED_DIR = '/usr/share/pyshared/'
→ TBD

base/univention-config-registry/python/univention-config-registry:»   sys.path.append('/usr/share/pyshared')
→ TBD

base/univention-lib/python/admember.py:if sys.path[0] == '/usr/share/pyshared/univention/s4connector/s4':
→ must be done prior to S4 connector

The following are okay, as they reference only files in their own packages:
services/univention-cloud-init/debian/univention-cloud-init.links:/usr/share/pyshared/cloudinit/config/cc_ucs_setup.py /usr/lib/python2.6/dist-packages/cloudinit/config/cc_ucs_setup.py
services/univention-cloud-init/debian/univention-cloud-init.links:/usr/share/pyshared/cloudinit/config/cc_ucs_setup.py /usr/lib/python2.7/dist-packages/cloudinit/config/cc_ucs_setup.py
services/univention-admin-diary/scripts/make_udm_events.py:# python $0 2> errors >> /usr/share/pyshared/univention/admindiary/events.py
services/univention-ad-connector/debian/univention-ad-connector.univention-service:programs=/usr/share/pyshared/univention/connector/ad/main.py
services/univention-ad-connector/scripts/prepare-new-instance:    sed -i "s|/usr/share/pyshared/univention/connector/ad/main.py.*|/usr/share/pyshared/univention/connector/ad/main.py --configbase \"$CONFIGBASENAME\"|" /usr/sbin/univention-ad-"$CONFIGBASENAME"
services/univention-ad-connector/univention-ad-connector:/usr/bin/python2.7 -W ignore /usr/share/pyshared/univention/connector/ad/main.py
base/univention-lib/shell/license.sh:»   python /usr/share/pyshared/univention/lib/license_tools.py
base/univention-system-activation/conffiles/etc/apache2/sites-available/univention-system-activation.conf:WSGIScriptAlias /license /usr/share/pyshared/univention/system_activation/wsgi.py
base/univention-updater/umc/python/updater/__init__.py:HOOK_DIRECTORY = '/usr/share/pyshared/univention/management/console/modules/updater/hooks'
Comment 12 Florian Best univentionstaff 2019-04-04 15:45:48 CEST
A good way to replace the pyshared references is to use e.g. *python -m univention.foo* instead of calling *python /usr/share/pythonX.X/univention/foo/__init__.py*.
Not possible everywhere and the package must support either a __main__.py or a if __name__ == '__main__' block.
Otherwise, it seems, one must use a hardcoded python version reference, which would be bad.
Comment 13 Florian Best univentionstaff 2019-04-05 13:28:15 CEST
pyshared is also used in the univention-support-info script (http://updates.software-univention.de/download/scripts/univention-support-info):

   619 try:
   620 »   from univention import config_registry
   621 except ImportError:
   622 »   if os.path.isfile('/usr/share/pyshared/univention/config_registry.py'):
   623 »   »   import imp
   624 »   »   config_registry = imp.load_source('config_registry', '/usr/share/pyshared/univention/config_registry.py')
   625 »   else: 
   626 »   »   sys.path.append('/usr/share/pyshared')
   627 »   »   from univention import config_registry
Comment 14 Florian Best univentionstaff 2019-04-05 13:32:06 CEST
We should remove the sitecustomize.py completely from the python3 package!
This only sets the default encoding from ascii to utf-8, which is the default for python3 (See also Bug #49009).
Comment 16 Daniel Tröder univentionstaff 2019-04-10 17:11:53 CEST
(In reply to Florian Best from comment #15)
> https://packaging.python.org/guides/packaging-namespace-packages/
That (pkgutil-style) is exactly what univention-pysupport-transition does.
Comment 17 Philipp Hahn univentionstaff 2019-04-11 14:48:04 CEST
(In reply to Daniel Tröder from comment #16)
> (In reply to Florian Best from comment #15)
> > https://packaging.python.org/guides/packaging-namespace-packages/
> That (pkgutil-style) is exactly what univention-pysupport-transition does.

The same "hack" is needed for "univention.management.console.modules" and "univention.admin.handlers.**" (and other) directories, where packages created by third-parties are installed: Either our new modules in "/usr/lib/python2.7/dist-packages/univention/**" would get used or the third-party-modules from "/usr/lib/pymodules/python2.7/univention/**".

management/univention-management-console/dev/dh-umc-module-install uses the hard-corded path "usr/share/pyshared/univention/management/console/modules/" to install the modules. This assumes that "pysupport" is used. As soon the the source package calling "dh-umc-module-install" is changed to use dh_python2, the module will be installed in the wrong location, will no longer get linked to "pymodules/" and thus will no longer be found by UMC.
Comment 18 Daniel Tröder univentionstaff 2019-04-11 17:23:02 CEST
(In reply to Philipp Hahn from comment #17)
> management/univention-management-console/dev/dh-umc-module-install uses the
> hard-corded path "usr/share/pyshared/univention/management/console/modules/"
> to install the modules. This assumes that "pysupport" is used. As soon the
> the source package calling "dh-umc-module-install" is changed to use
> dh_python2, the module will be installed in the wrong location, will no
> longer get linked to "pymodules/" and thus will no longer be found by UMC.
Yep - look what we had to do in univention-management-console-module-appcenter.postinst:
https://git.knut.univention.de/univention/ucs/commit/1f07ca77366c3f323018540b599628e0cb314f0f
Comment 19 Florian Best univentionstaff 2019-04-15 19:01:56 CEST
There is a (at least theoretical) problem during upgrade of univention-python to dh_python2:

All running python processes which started with the old univention.* package will be unable to (re)import any new univention.*-packages/modules, as their contents possibly moved.

Example:
# python (started before upgrading)
>>> import univention
>>> univention
<module 'univention' from '/usr/lib/pymodules/python2.7/univention/__init__.pyc'>
>>> # now upgrade and install univention-python-transition (in another terminal)
>>> import univention.fstab                                                                                                                                                                                                                                                      
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named fstab
>>> import univention.admin
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/pymodules/python2.7/univention/admin/__init__.py", line 625, in <module>
    from univention.admin import modules, objects, syntax, hook, mapping  # noqa
  File "/usr/lib/pymodules/python2.7/univention/admin/modules.py", line 44, in <module>
    import univention.admin.uldap
  File "/usr/lib/pymodules/python2.7/univention/admin/uldap.py", line 38, in <module>
    import univention.uldap
ImportError: No module named uldap

This can make problems for all services which are importing things dynamically during runtime instead of import time, or if files are reload()ed.
I think UMC, UDM and maybe the listener does this.

A univention-python-transition package doesn't help in this case! Because
1. it was not installed *before* starting any python services
→ so the namespacing __init__ hasn't been loaded yet
2. even if there would be a namespaing __init__.py in "pyshared", there is none in "dist-packages". So the imports are still failing.
→ Chicken-Egg problem; We can't add a "dist-packages" namespace __init__.py in a different debian package because package-owns-file-conflict.
A upgrade-safe working way to be to create these files without debian-packaging:
# mkdir /usr/lib/python2.7/dist-packages/univention/
# echo 'from pkgutil import extend_path; __path__ = extend_path(__path__, __name__)' > /usr/lib/python2.7/dist-packages/univention/__init__.py
(and other __init__.py files in other locations)
But this enforces nevertheless a server reboot/service restart before upgrading any package to dh_python2.

Opinions?
* Enforce server reboot
→ not an option
* restart services with known problems
→ not an option
* release dist-packages __init__.py's for all relevant packages and wait until a minor release with the rest of the dh_python2 migration?
→ imho not an option
* ignore problematic situations and risk some tracebacks?
* create symlinks for all old files, so they can be imported in the interim-time (aka. simulate python-support)?
* etc?

Additionally:
I think we don't need a univention-python-transition package.
It is enough to simply add the file to debian/python-univention.install:
+modules/__init__.py usr/share/pyshared/univention/
+modules/__init__.py usr/lib/pymodules/python2.7/univention/
Comment 20 Daniel Tröder univentionstaff 2019-04-15 23:20:54 CEST
(In reply to Florian Best from comment #19)
> Opinions?
> * restart services with known problems
> → not an option
Why is that not an option. I think it's the best one.
It targets the problem directly and with minimal side effects.

It's pretty easy to find all running Python processes:
$ fuser /usr/lib/python2.7/lib-dynload/*

To get only Python processes that use univention.debug:
$ fuser /usr/lib/pyshared/python2.7/univention/_debug.so
Comment 21 Florian Best univentionstaff 2019-04-16 07:59:13 CEST
There might be third party components (e.g. UCS@school, OX, etc?) which are affected.
If you restart UMC-Webserver all sessions, UMC-Server all running module processes are lost - including the session which are doing the package update.
Killing UDM-CLI should be okay.
Comment 22 Daniel Tröder univentionstaff 2019-04-16 10:32:49 CEST
IMHO that's not such a big problem, if done as a trigger at the end of the update. Wouldn't be beautiful, but would be a one-time-thing.

Another possibility would be to show the user an interactive popup with a prompt for confirmation, that if she starts the update, at the end some services (list of processes/services) will be restartet to ensure system stability. IMHO that would be the best option. BTW: Debian Buster does that already with both a dialog on the cmdline and a notification on the desktop.

Another possibility might be to release univention-python-transition now. Then with and after the next UCS release (which will restart all(?) Python processes) start releasing dh_python2 packages.
Comment 23 Philipp Hahn univentionstaff 2019-04-16 11:12:29 CEST
Debian by default stops services in prerm and re-starts them in postinst for good reason. Keeping services running in the background while performing an update is always tricky.
<man:dh_installinit(1)> -> --restart-after-upgrade | --no-restart-after-upgrade | --no-stop-on-upgrade | --no-restart-on-upgrade

(In reply to Daniel Tröder from comment #22)
> Another possibility might be to release univention-python-transition now.
> Then with and after the next UCS release (which will restart all(?) Python
> processes) start releasing dh_python2 packages.

This should be released as soon as possible so we can start the transition. My telling since nearly 7 years (Bug #28497)
Comment 24 Florian Best univentionstaff 2019-04-16 12:04:38 CEST
For the package "univention-python" the migration seems to be stable with our components.
Affected modules are univention.{dns|fstab|hooks|ipv4|misc|password|uldap|utf8}.

I tested a upgrade with UMC and all UMC modules opened: Seems not problematic.

univention-directory-listener "force-reload" does not have problems:
It reloads the modules via C PyMarshal_ReadLastObjectFromFile / PyNode_Compile.

Heimdal uses PyImport_ImportModule. I didn't test it yet.
Comment 25 Florian Best univentionstaff 2019-05-02 16:57:24 CEST
We have to specify which of our API's are unicode-API's and which are bytes-API's.
In python3 mixing bytes with unicode is not possible anymore. So we have to make it explicit:

python3 -c 'import univention.admin.uldap; lo, po = univention.admin.uldap.getMachineConnection(decode_ignorelist=["krb5Key"]); print(lo.get("uid=Administrator,cn=users,"));'

{'uidNumber': [b'2002'], 'krb5KeyVersionNumber': [b'1'], 'cn': [b'Administrator\xc3\xa4 Administrator'], 'ownCloudEnabled': [b'1'], 'givenName': [b'Administrator\xc3\xa4'], 'krb5MaxRenew': [b'604800'], 'sambaPasswordHistory': [b'50A44762150916AF3CA9DEFB8DDF907E31D3CCCA6D6782011158648DF398E0EB'], 'univentionObjectType': [b'users/user'], 'oxDisplayName': [b'Administrator\xc3\xa4 Administrator'], 'sambaPrimaryGroupSID': [b'S-1-5-21-1189137934-891802616-1255459322-512'], 'uid': [b'Administrator'], 'sn': [b'Administrator'], 'objectClass': [b'krb5KDCEntry', b'univentionPerson', b'univentionPolicyReference', b'oxUserObject', b'organizationalPerson', b'automount', b'nextcloudUser', b'inetOrgPerson', b'krb5Principal', b'person', b'univentionPWHistory', b'univentionMail', b'univentionObject', b'ownCloudUser', b'shadowAccount', b'sambaSamAccount', b'top', b'posixAccount'], 'sambaNTPassword': [b'CAA1239D44DA7EDF926BCE39F5C65D0F'], 'nextcloudEnabled': [b'1'], 'oxTimeZone': [b'Europe/Berlin'], 'krb5MaxLife': [b'86400'], 'mailPrimaryAddress': [b'administrator@dev.local'], 'oxAccess': [b'premium'], 'homeDirectory': [b'/home/Administrator'], 'sambaBadPasswordTime': [b'0'], 'gecos': [b'Administratorae Administrator'], 'loginShell': [b'/bin/bash'], 'sambaBadPasswordCount': [b'0'], 'krb5PrincipalName': [b'Administrator@DEV.LOCAL'], 'sambaPwdLastSet': [b'1552568530'], 'oxTelephoneAssistant': [b'Administrator'], 'univentionMailUserQuota': [b'0'], 'sambaAcctFlags': [b'[U          ]'], 'isOxUser': [b'OK'], 'univentionPolicyReference': [b'cn=default-admins,cn=admin-settings,cn=users,cn=policies,dc=dev,dc=local'], 'displayName': [b'Administrator\xc3\xa4 Administrator'], 'description': [b'Built-in account for administering the computer/domain'], 'oxLanguage': [b'de_DE'], 'shadowLastChange': [b'17969'], 'krb5Key': [b'0@\xa1\x1b0\x19\xa0\x03\x02\x01\x11\xa1\x12\x04\x10\x1bg\xa5\x1ey\xb0\xcd\x1c\xa4PP\xafB\xe3\xa1\n\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'0H\xa1#0!\xa0\x03\x02\x01\x10\xa1\x1a\x04\x18\xb9\xda\xc7\xc8\x07m\xd5b\xc78\xb9\xcbC\x01\xcd\x8f\x1a\xd3\x85p\x1c\x07C=\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'0P\xa1+0)\xa0\x03\x02\x01\x12\xa1"\x04 \x95\xba\xd3xh-.Z\x19\x99\xfa\xf4\x17\x16g\x838\xfb&\xfc\x8a\xc0E\xdb\x0cz6\xc9n~\x17\x06\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'08\xa1\x130\x11\xa0\x03\x02\x01\x02\xa1\n\x04\x08n\xe3\xc2\x16\x02W\x86\xc7\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'0@\xa1\x1b0\x19\xa0\x03\x02\x01\x17\xa1\x12\x04\x10\xca\xa1#\x9dD\xda~\xdf\x92k\xce9\xf5\xc6]\x0f\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'08\xa1\x130\x11\xa0\x03\x02\x01\x03\xa1\n\x04\x08n\xe3\xc2\x16\x02W\x86\xc7\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator', b'08\xa1\x130\x11\xa0\x03\x02\x01\x01\xa1\n\x04\x08n\xe3\xc2\x16\x02W\x86\xc7\xa2!0\x1f\xa0\x03\x02\x01\x03\xa1\x18\x04\x16DEV.LOCALAdministrator'], 'univentionUMCProperty': [b'udmUserGridView=default', b'favorites=udm:users/user,udm:groups/group,udm:computers/computer,appcenter:appcenter,updater'], 'sambaSID': [b'S-1-5-21-1189137934-891802616-1255459322-500'], 'pwhistory': [b'$6$p0ejkeMFO9q0y9tH$uW9tpObAvvnFX4zn5N1lOgHdf7vSuxI6mur/3e9P4ufBBT/BQcs69nFhQzNgsJVXVUpor2dX5ByQAMPQW4Uo8/'], 'gidNumber': [b'5000'], 'krb5KDCFlags': [b'126'], 'userPassword': [b'{crypt}$6$1FIYEG3cdDfXBN8Y$Vh7iekOGUrjHwQPEh4XUiYhD03xtf8w1Ka/eb5JEgyCg1DDz9RNMm9LOg/ZtgGPfSQeemJUliENhXgOChopQO/']}

→ univention.uldap.access() and python-ldap is currently a bytes-API.

>>> lo.lo.lo.modify_s(univention.admin.uldap.getMachineConnection()[0].whoami(), [(ldap.MOD_REPLACE, 'displayName', ['master20'])])
Traceback (most recent call last):
TypeError: ('Tuple_to_LDAPMod(): expected a byte string in the list', 'master20')
>>> lo.lo.lo.modify_s(univention.admin.uldap.getMachineConnection()[0].whoami(), [(ldap.MOD_REPLACE, u'displayName', [b'master20'])])

→ python-ldap only allows bytes. I think it also has a unicode-mode.

univention.uldap supports setting attributes as bytes or unicode:
$ python3
>>> lo,po = univention.admin.uldap.getAdminConnection()
>>> lo.modify(univention.admin.uldap.getMachineConnection()[0].whoami(), [('displayName', 'master20', 'master21')])
'cn=master20,cn=dc,cn=computers,dc=dev,dc=local'
>>> lo.modify(univention.admin.uldap.getMachineConnection()[0].whoami(), [('displayName', b'master21', b'master20')])
'cn=master20,cn=dc,cn=computers,dc=dev,dc=local'

DN's must be unicode:
>>> lo.get(b'cn=master20,cn=dc,cn=computers,dc=dev,dc=local')
Traceback (most recent call last):
…
TypeError: search_ext() argument 1 must be str, not bytes

For Univention Directory Manager:
We probably want to decode bytes ldap attributes to unicode string udm properties.
self.oldattr = {u'attr': [b'value']}
self.info = {u'property': u'value'}

For most things we can just unmap/decode as UTF-8. Some properties probably want to preserve the pure bytes, so we should decode as ISO8859-1 (instead of keeping it as bytes).
Maybe univention.admin.property() or univention.admin.mapping() should get an encoding argument.

For Univention Config Registry:
→ should probably be a unicode API

$ python3
>>> ucr['ldap/base']
'dc=dev,dc=local'
>>> ucr[b'ldap/base']
(handler_set not working atm)

Supporting both, unicode and bytes is discouraged:
"""(it is *highly* recommended you don’t design APIs that can take both due to the difficulty of keeping the code working; as stated earlier it is difficult to do well)"""
https://docs.python.org/3.5/howto/pyporting.html#text-versus-binary-data

I think best would be to expose unicode API's wherever it's possible. External API's where bytes is required (ldap, database access, file access) we should have a layer arround this which does correct handling of en/de-coding.
Comment 26 Florian Best univentionstaff 2019-05-13 15:16:38 CEST
We need a UCS 4.5 or 5.0 for switching to the python3 applications due to external written:
* UDM hooks
* UDM syntax classes
* UMC modules
* listener modules
* UCR modules/handlers
* UCR templates

and maybe:
* updater hooks
* diagnostic plugins
* UCS@school computerroom module plugins
* S4/AD connector mapping
* ucslint plugins
Comment 27 Florian Best univentionstaff 2019-05-16 09:57:52 CEST
In python3 configparser is a change of behavior: % is not allowed anymore in the content of the items because it is evaluated as %s syntax. So one can use e.g. %(default)s.

This causes error in UCR currently:

Traceback (most recent call last):                                                                       
  File "/usr/sbin/ucr", line 64, in <module>
    ub.main(convertToUtf8(sys.argv[1:]))                                                               
  File "/usr/lib/python3/dist-packages/univention/config_registry/frontend.py", line 687, in main
    handler_func(args, cmd_opts)                               
  File "/usr/lib/python3/dist-packages/univention/config_registry/frontend.py", line 305, in handler_search
    info = cri.ConfigRegistryInfo(install_mode=False)               
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 104, in __init__
    self.__load_variables(registered_only, load_customized)                
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 274, in __load_variables                                                                                                                                                                       
    self.read_variables(cfgfile)                                    
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 255, in read_variables                                                                                                                                                                          
    for name, value in cfg.items(sec):                                                                   
  File "/usr/lib/python3.5/configparser.py", line 857, in items
    return [(option, value_getter(option)) for option in d.keys()]                                                                     
  File "/usr/lib/python3.5/configparser.py", line 857, in <listcomp>                      
    return [(option, value_getter(option)) for option in d.keys()]                                                                                                                                                                                                               
  File "/usr/lib/python3.5/configparser.py", line 854, in <lambda>                          
    section, option, d[option], d)                                                                                                                                                                                                                                     
  File "/usr/lib/python3.5/configparser.py", line 393, in before_get                                                                                                                                                                                                             
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/usr/lib/python3.5/configparser.py", line 445, in _interpolate_some                                                                                                                                                                                                      
    "found: %r" % (rest,))                                                                       
configparser.InterpolationSyntaxError: '%' must be followed by '%' or '(', found: "%U' wird zur Laufzeit auf den aktuellen Benutzer expandiert."

Unfortionately, we are using this % syntax sometimes escaped, sometimes not.

So, patching it ala:
Replace cfg.read(filename) with:
>                with open(filename) as fd:
>                        cfg.read_string(fd.read().replace('%', '%%'))
does not work.

Affected files are at least:
/etc/univention/registry.info/variables/ucs-school-netlogon-user-logonscripts.cfg
/etc/univention/registry.info/variables/ucs-school-umc-exam-master.cfg
/etc/univention/registry.info/variables/univention-base-files.cfg
/etc/univention/registry.info/variables/univention-ldap-server.cfg
/etc/univention/registry.info/variables/univention-printserver-pdf.cfg
/etc/univention/registry.info/variables/univention-samba4.cfg

After fixing this, there are more strictness checks:
Traceback (most recent call last):
  File "/usr/sbin/ucr", line 64, in <module>
    ub.main(convertToUtf8(sys.argv[1:]))
  File "/usr/lib/python3/dist-packages/univention/config_registry/frontend.py", line 687, in main
    handler_func(args, cmd_opts)
  File "/usr/lib/python3/dist-packages/univention/config_registry/frontend.py", line 305, in handler_search
    info = cri.ConfigRegistryInfo(install_mode=False)
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 104, in __init__
    self.__load_variables(registered_only, load_customized)
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 275, in __load_variables
    self.read_variables(cfgfile)
  File "/usr/lib/python3/dist-packages/univention/config_registry_info.py", line 246, in read_variables
    cfg.read(filename)
  File "/usr/lib/python3.5/configparser.py", line 696, in read
    self._read(fp, filename)
  File "/usr/lib/python3.5/configparser.py", line 1089, in _read
    fpname, lineno)
configparser.DuplicateOptionError: While reading from '/etc/univention/registry.info/variables/ucs-school-netlogon-user-logonscripts.cfg' [line 51]: option 'description[de]' in section 'ucsschool/userlogon/disabled_share_links/*' already exists

Maybe ucslint should detect these cases?
Comment 28 Philipp Hahn univentionstaff 2019-06-17 12:14:09 CEST
(In reply to Florian Best from comment #27)
> In python3 configparser is a change of behavior: % is not allowed anymore in
> the content of the items because it is evaluated as %s syntax. So one can
> use e.g. %(default)s.
...
> configparser.InterpolationSyntaxError: '%' must be followed by '%' or '(',
> found: "%U' wird zur Laufzeit auf den aktuellen Benutzer expandiert."
...
> configparser.DuplicateOptionError: While reading from
> '/etc/univention/registry.info/variables/ucs-school-netlogon-user-
> logonscripts.cfg' [line 51]: option 'description[de]' in section
> 'ucsschool/userlogon/disabled_share_links/*' already exists

Fixed by using
  configparser.ConfigParser.__init__(self, strict=False, interpolation=None)

> Maybe ucslint should detect these cases?

yes
Comment 29 Florian Best univentionstaff 2019-06-19 21:00:40 CEST
We should add to the UCR templates markers @!@ before the actual python code a "from __future__ import print_function" (maybe more) and try to evaluate it with that.
If that succeeds we are happy, otherwise we can print a warning and execute it without it.
Comment 30 Florian Best univentionstaff 2019-07-11 15:03:41 CEST
We can introduce a @!3@ marker which would evaluate the code in UCR templates as python3.
Or we can add a Python3 flag to the File:/Subfile: definition.

The templates are written via a subprocess call (handler.py:run_filter() subprocess.Popen(sys.execuatable)), so they don't depend on how e.g. "ucr set" is called.
Comment 31 Florian Best univentionstaff 2019-07-11 15:09:55 CEST
Created attachment 10116 [details]
suggested patch for UCR binary API

See also Bug #49775. We migrated via Bug #49129 the UCR template filter API to a unicode/utf-8 API but there was acutally a template which produces binary data (e.g. a mo file). I am suggesting the following patch for UCR if we want to keep backwards binary complatibility.
Comment 32 Florian Best univentionstaff 2019-10-28 10:11:41 CET
(In reply to Florian Best from comment #30)
> We can introduce a @!3@ marker which would evaluate the code in UCR
> templates as python3.
> Or we can add a Python3 flag to the File:/Subfile: definition.
> 
> The templates are written via a subprocess call (handler.py:run_filter()
> subprocess.Popen(sys.execuatable)), so they don't depend on how e.g. "ucr
> set" is called.

Sönke suggested the following:

Regarding UCR templates and Python3: couldn't you include a special marker (@%@PYTHON3READY@%@) in the Python3 UCR templates, so that UCR knows that these templates can be evaluated natively with Py3 and everything that doesn't have the marker yet is processed with an extra Python2 process? Then Py2 templates are slower in processing, but still work. With UCR modules this could be done similarly (if the effort is worth it at all). What do you think?
Comment 33 Florian Best univentionstaff 2019-11-06 17:49:35 CET
Meanwhile we have a lot of python3-univention-* packages. To install them, there are currently dependency problems because of not yet available packages.
There is a way to create an empty debian package:

fakepackage() {
  equivs-control "$1" && \
  sed "s/^Package: .*/Package: $1/g; s/^# Version:.*/Version: ${2:1.0}/g" -i "$1" && \
  equivs-build "$1" && \
  dpkg -i "$1"_*.deb;
}

fakepackage() { equivs-control "$1" && sed "s/^Package: .*/Package: $1/g; s/^# Version:.*/Version: ${2:1.0}/g" -i "$1" && equivs-build "$1" && dpkg -i "$1"_*.deb; }

fakepackage python3-ldb
fakepackage python3-samba
fakepackage python3-trml2pdf
fakepackage python3-univention-heimdal 4.2
fakepackage python3-m2crypto
fakepackage python3-smbpasswd
#fakepackage python3-concurrent.futures

This makes it possible to install the current packages :-)
Comment 34 Florian Best univentionstaff 2019-11-06 17:56:15 CET
I created a git branch "python3/4.4-2" for the development.
I create a build scope "python3" in the 4.4-0 release:

echo -e "deb [trusted=yes] http://omar.knut.univention.de/build2/ ucs_4.4-0-python3/all/" >> /etc/apt/sources.list
echo -e "deb [trusted=yes] http://omar.knut.univention.de/build2/ ucs_4.4-0-python3/\$(ARCH)/" >> /etc/apt/sources.list

I created a Jenkins Job which installs all upgrades from the build scope:
https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-2/view/Product%20Tests/job/product-test-python3-ucs-tests/
(the url might change in the future, my plan is to create a python2-runner and a python3-runner)

All changes in the git branch should be done by appending a "-python3" suffix to the debian/changelog version number.

I cherry-picked python-ldap and pyasn1 from debian buster into the python3 scope and build it there (by downpatching the debhelper version build dependency).
I cherry-picked python-pam from ubuntu.
Comment 35 Florian Best univentionstaff 2019-12-05 23:58:52 CET
It's worth reading https://www.python-ldap.org/en/python-ldap-3.2.0/bytes_mode.html
Comment 36 Florian Best univentionstaff 2019-12-06 12:26:19 CET
Created attachment 10256 [details]
migrate-ucs-template-python3.py

I wrote a script which replaces all UCR template files to Python 3 syntax using futurize.
Comment 37 Florian Best univentionstaff 2019-12-20 15:35:51 CET
The branch git:fbest/49060-migrate-print-statements-in-ucr-conffiles converts all print statements in the UCR conffiles - leaving out the one, which would only be supported by python 3.