Commit 4fa33dfc authored by Arnaud Fontaine's avatar Arnaud Fontaine

erp5: py3: `func_{code,defaults}` was replaced in Python3 by `__{code,defaults}__`.

Python2 already provides __code__ and __defaults__ so this just does the same on
PythonScript.
parent db6993c2
--- Zope2-2.13.30/src/Shared/DC/Scripts/Signature.py 2022-04-25 08:05:09.312966168 +0000
+++ Zope2-2.13.30/src/Shared/DC/Scripts/Signature.py 2022-04-25 08:06:20.120743425 +0000
@@ -35,7 +35,7 @@ def _setFuncSignature(self, defaults=None, varnames=(), argcount=-1):
argcount = len(varnames)
# Generate a change only if we have to.
if self.func_defaults != defaults:
- self.func_defaults = defaults
+ self.func_defaults = self.__defaults__ = defaults
code = FuncCode(varnames, argcount)
if self.func_code != code:
- self.func_code = code
+ self.func_code = self.__code__ = code
...@@ -591,6 +591,8 @@ Acquisition-patches = ${:_profile_base_location_}/../../component/egg-patch/Acqu ...@@ -591,6 +591,8 @@ Acquisition-patches = ${:_profile_base_location_}/../../component/egg-patch/Acqu
Acquisition-patch-options = -p1 Acquisition-patch-options = -p1
python-magic-patches = ${:_profile_base_location_}/../../component/egg-patch/python_magic/magic.patch#de0839bffac17801e39b60873a6c2068 python-magic-patches = ${:_profile_base_location_}/../../component/egg-patch/python_magic/magic.patch#de0839bffac17801e39b60873a6c2068
python-magic-patch-options = -p1 python-magic-patch-options = -p1
Zope2-patches = ${:_profile_base_location_}/../../component/egg-patch/Zope/PythonScript-2.13.patch#124c0d37394dd5020c6fd241ad75cc29
Zope2-patch-options = -p1
[eggs-all-scripts] [eggs-all-scripts]
recipe = zc.recipe.egg recipe = zc.recipe.egg
...@@ -631,6 +633,7 @@ pysvn = 1.9.15+SlapOSPatched001 ...@@ -631,6 +633,7 @@ pysvn = 1.9.15+SlapOSPatched001
python-ldap = 2.4.32+SlapOSPatched001 python-ldap = 2.4.32+SlapOSPatched001
python-magic = 0.4.12+SlapOSPatched001 python-magic = 0.4.12+SlapOSPatched001
PyPDF2 = 1.26.0+SlapOSPatched001 PyPDF2 = 1.26.0+SlapOSPatched001
Zope2 = 2.13.30+SlapOSPatched001
## https://lab.nexedi.com/nexedi/slapos/merge_requests/648 ## https://lab.nexedi.com/nexedi/slapos/merge_requests/648
pylint = 1.4.4 pylint = 1.4.4
......
  • Please no Zope-related pure-python patch inside slapos.git. They're harder to maintain and introduce useless dependency between ERP5 & SlapOS.

  • I suppose that here, _setFuncSignature is imported by other modules before we have the opportunity to monkey-patch. Then a way is to patch the function's __code__.

  • I don't think they are harder to maintain, especially with my very painful experience to update monkey-patches to Python3 and finding out that a lot of them are based on very old code. If it would have been normal Zope patches, it would have been updated when we update Zope version and we can verify that they are still useful or not (see product/PloneHotfix20121106 which is still there)...

    Edited by Arnaud Fontaine
  • Yes, there are:

    • Patch files in repository are annoying to modify.
    • You're not autonomous when working on erp5.git: you have to do a lot of stupid slapos.git stuff, like temporarily change test suites to follow master instead 1.0 and not forget to revert (won't you ?)

    If it would have been normal Zope patches, it would have been updated when we update Zope version and we can verify that they are still useful or not

    I rather think that we modified our patches more often than we needed to adapt them. Your work is exceptional. What can be really better is to fork repositories.

    Zope2 was not patched in slapos.git, let's continue this way.

    product/PloneHotfix20121106

    It's a security fix and it may have been easier to deploy. But I agree it's a bad place to maintain this particular patch (backported upstream changes).

  • We should probably have a forum discussion about the pro dans cons of all approaches and decide a direction to follow, but patching with an actual patch is good because the definition of "how to patch a dependency" is in the same repository as the definition of "which version of a dependency to use", so if we update a dependency we see directly that there is a patch that we may want to update. Also sometimes the patch won't apply, which tells us that this needs attention (unlike a monkey patch which keep using the old version)

    With an actual patch, it's also easier to run original software test (at least in theory, we don't do this at the moment, at least not automatically).

    Also, if it makes harder to patch external softwares, it's a feature :)

  • unlike a monkey patch which keep using the old version

    Monkey patches can be even safer, like erp5@77f9030e (assert) or https://lab.nexedi.com/nexedi/neoppod/blob/master/neo/client/__init__.py

  • mentioned in merge request erp5!1622 (closed)

    Toggle commit list
  • For this specific patch, since we are already patching Python Scripts class a lot, maybe we could apply this as a monkey patch as well ?

    /cc @romain

  • Isn't this line in buildout.cfg incompatible with this line in zope-versions.cfg ?

  • It depends on the meaning of "incompatible" :) but zope-versions.cfg is a copy of zope's profile ( see cf99b892 (comment 137750) or eb5c6047 (comment 154172) ). If I understand correctly our buildout patch which started with slapos.buildout@ac51c1d5 , Zope2 = 2.13.30+SlapOSPatched001 and Zope2 = 2.13.30 are the same version

  • Let say, confusing. Anyway, this is not the source of my current issue, so, please forget what I said

  • mentioned in merge request erp5!1603 (merged)

    Toggle commit list
  • ah https://github.com/zopefoundation/Products.PythonScripts/commit/590125ae7c20994055c12be9ba03ae0b43b102ec was missing from the patch !

    Applying this patch will cause business template format to change so we'll have to also do erp5!1595 (merged) and re-export all business templates .

    /cc @xiaowu.zhang

  • That also explains why after some time this error disappear (because scripts are recompiled)

    Edited by Jérome Perrin
  • mentioned in commit e7f9fa99

    Toggle commit list
  • mentioned in commit ee9ece6a

    Toggle commit list
  • mentioned in merge request !1178 (closed)

    Toggle commit list
  • mentioned in commit 3e654be3

    Toggle commit list
  • mentioned in commit 414e7dd3

    Toggle commit list
  • mentioned in merge request erp5!1634 (merged)

    Toggle commit list
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment