search

Zolko-123 / FreeCAD_Assembly4

Assembly 4 workbench for FreeCAD
GNU Lesser General Public License v2.1
288 stars 76 forks source link

Fix state serialization on FreeCAD >= 0.22 #460

Closed andir closed 9 months ago

andir commented 10 months ago

As of https://github.com/FreeCAD/FreeCAD/commit/83d4080fe8 FreeCAD (for compatibility with Python 3.11) has renamed the methods to loads and dumps. Using this workbench with the latest FreeCAD main branch surfaced this issue as I wasn't able to save my documents again.

I found the change https://github.com/fra589/CurvesWB/commit/3374dbb83ea2a09787819b1e153a5697f5e19ea7 in the CurvesWB and adopted it for this WB.

Zolko-123 commented 10 months ago

thank-you for the contribution, but I don't understand what it does. Can you please explain ?

andir commented 10 months ago

Since Py3.11 the methods names setstate and getstate conflict with the method names added to the object class. Thus rename them to 'loads' and 'dumps'

(from the FreeCAD commit message).

This means that the old __setstate__ and __getstate__ methods aren't working with newer version of FreeCAD anymore. FreeCAD decided to rename them to dumps and loads instead. What my changes are doing is, depending on the FreeCAD version the WB is running setting either the new or the old functions.

Zolko-123 commented 10 months ago

FreeCAD decided to rename them to dumps and loads instead

no, that was not a FreeCAD decision, that's a decision by the pickle Python module. That commit and code are so wrong on any scale that I cannot accept that.

Also, when looking at the pickle documentation:

Warning : The pickle module is not secure. Only unpickle data you trust. It is possible to construct malicious pickle data which will execute arbitrary code during unpickling

Also, v0.22 is still in development, hopefully Werner will wake-up and come with a serious solution instead of this half-baked incompatible horror show

Zolko-123 commented 10 months ago

according to PR https://github.com/FreeCAD/FreeCAD/pull/10581 this is not necessary :

App: still support getstate/setstate for add-ons for < Py3.11

// support add-ons that use the old method names

Syres916 commented 9 months ago

@Zolko-123 it is necessary for Python 3.11 and greater as setstate is now a reserved attribute brought in by the Python developers. I'm trying in my spare time to get all the Addon authors to have this fix implemented asap because of the number of Linux distros having Python 3.11 as default and no doubt 3.12 is not far from being the new standard. It's up to you but all you're going to do is get more complaints from users trying to save their files and receiving a sea of red in the Report View.

Syres916 commented 9 months ago

@andir just some feedback on your implementation, there is no need to have the FreeCAD version check especially as version 0.21.2 has the dumps/loads in it and 0.21.3 will have the complete json code fix. You can just duplicate the code as per https://github.com/jaheyns/CfdOF/commit/6be2891a38ed67969216f7e7b6449f7c9684a8c9

Zolko-123 commented 9 months ago

I'm trying in my spare time ...

Do you think I'm paid for the time I spend on Assembly4 ?

Zolko-123 commented 9 months ago

1) This problem doesn't depend on the FreeCAD version but on the Python version. This PR doesn't solve the problem

2) realthunder proposes a solution in FreeCAD core that solves the problem where it should be solved : in the core, making FreeCAD backward compatible without this ugly hack.

See also https://github.com/FreeCAD/FreeCAD/issues/10460 . Therefore I must refuse.