ElDavoo / wa-crypt-tools

Manage WhatsApp .crypt12, .crypt14 and .crypt15 files.
GNU General Public License v3.0
636 stars 80 forks source link

Relative import for proto package does not work #63

Closed ElDavoo closed 1 year ago

ElDavoo commented 1 year ago

See Readme.
Why do we have to replace relative import of proto files with absolute import?
E.g. why do we have to replace: import C14_cipher_version_pb2 as C14__cipher__version__pb2
with import wa_crypt_tools.C14_cipher_version_pb2 as C14__cipher__version__pb2
??
This makes more difficult to automate proto class generation.

Mikel12455 commented 1 year ago

To be completely honest, the only thing I knew a couple of days ago is that those imports just didn't work.

Though I just tested something and they can be replaced with something like:

from . import C14_cipher_version_pb2 as C14__cipher__version__pb2

I don't know if this fixes the issue, so let me know!

ElDavoo commented 1 year ago

The issue is "find a way to generate proto classes from protoc that work". Fixing imports after generating the proto classes is already what i'm doing (see readme), but it's only a workaround. The problem is: if you use protoc to generate the python files, then importing will fail.

Mikel12455 commented 1 year ago

The problem is: if you use protoc to generate the python files, then importing will fail.

Oh okay, the problem is with protoc, not with its generated Python files. These might be useful then:

Protocol Buffer issue

Tool to fix imports

The tool can be installed with pip, so it may become a dev dependency too if actually useful. Gonna test it right now.

Mikel12455 commented 1 year ago

So I moved all of the proto folder inside of the package and ran

protol --in-place --python-out . protoc --protoc-path=[path/to/protoc] --proto-path=. *.proto (protoc-path is only needed because I don't have protoc setup in PATH)

It actually changed all of the imports to relative imports like:

from . import C14_cipher_pb2 as C14__cipher__pb2

Though I still haven't run any tests with an encrypted db. I'll update tomorrow.

Mikel12455 commented 1 year ago

Run a couple of tests and it seems to work

ElDavoo commented 1 year ago

Ok, I will try to test when I have some time

markdoerr commented 1 year ago

Hi @ElDavoo, using 'protoletariat' is a very ugly solution, why is the "relative import problem" not solved in the first place by simply generating the correct import syntax:

from . import my_pb2 as ... instead of: import my_pb2 as ... ?

Very simple, no other tools required ...

ElDavoo commented 1 year ago

why is the "relative import problem" not solved in the first place by simply generating the correct import synta

Because I'm a bad developer =)

Seriously, there was a reason, but i completely forgot it. maybe it was a stupid reason.

Mikel12455 commented 1 year ago

Hi @ElDavoo, using 'protoletariat' is a very ugly solution, why is the "relative import problem" not solved in the first place by simply generating the correct import syntax:

from . import my_pb2 as ... instead of: import my_pb2 as ... ?

Very simple, no other tools required ...

How do you generate them like that? protoc always generates them like import my_pb2 as ..., so you have to manually fix the imports. protoletariat just does this automatically.

If having a new dependency seems overkill though, we can just revert to the manual fix and change the README.md a bit.

ElDavoo commented 1 year ago

Maybe they're talking about the imports in the main script

markdoerr commented 1 year ago

Hi @ElDavoo, using 'protoletariat' is a very ugly solution, why is the "relative import problem" not solved in the first place by simply generating the correct import syntax: from . import my_pb2 as ... instead of: import my_pb2 as ... ? Very simple, no other tools required ...

How do you generate them like that? protoc always generates them like import my_pb2 as ..., so you have to manually fix the imports. protoletariat just does this automatically.

Who would be able to directly fix the import issue in the protoc compiler ?

I think all three of us (@ElDavoo , @Mikel12455 and myself agree on the same solution) - do you know, whom we could address to fix the import generation in protoc ?

If having a new dependency seems overkill though, we can just revert to the manual fix and change the README.md a bit.

Yes, indeed, @Mikel12455 - the problem should be fixed in the protoc compiler, not by an extra tool that is just made to fix a bug that should be easily solvable.

markdoerr commented 1 year ago

I raised the issue in the grpc / protoc community:

https://github.com/grpc/grpc/issues/29459

Please feel free to support it there to increase attention for the issue. Thanks.

ElDavoo commented 1 year ago

I think the revelant discussion is the root issue that has already been linked by Mikel:

Protocol Buffer issue

I reopened the issue because I thought you had a better solution for this problem, but we can't make any further action in this repository now, so I will close it again.

We all know that proletariat is a hacky workaround, and before Mikel's commits I manually patched the files, but it's all we can do for now, unless the linked protobuf issue gets progress.