search

crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.47k stars 763 forks source link

Fix building _nvx_utf8validator extension on non-x86 systems #1596

Closed yan12125 closed 1 year ago

yan12125 commented 1 year ago

x86intrin.h does not exist on such systems

oberstet commented 1 year ago

thanks! looks good .. for now .. this NVX stuff isn't finished yet in any case

yan12125 commented 1 year ago

Hmm, I've enabled it in the Arch Linux package for some time. Please tell me if it is better not to do so.

oberstet commented 1 year ago

Please tell me if it is better not to do so.

It's good to include the pieces that NVX already provides, because even though those pieces are not yet "complete" (wrt my original plan), the pieces which are there, e.g. the UTF8 validator, have pretty good test coverage https://github.com/crossbario/autobahn-python/blob/master/autobahn/nvx/test/test_nvx_utf8validator.py

IOW: the NVX UTF8 validator can indeed be used in production code (in my eyes / to my knowledge)

what is missing (I do have code .. somewhere ..) is: websocket XOR masking using intrinsics. anyways, I think your Arch package is good as it is! that is, from a quick glance.

however, looking at all the dependencies you have on the Arch package pointing to other Python modules also packaged as Arch packages: looks pretty good! I'm saying this even though I'm usually using Python packages in a venv, directly from PyPI.

One thing I am wondering though: do you provide autobahn[xbr] ? this sadly has some I guess "annoying" dependencies https://github.com/crossbario/autobahn-python/blob/201eeb2bcf9808a705b26ef35c028015f378661e/setup.py#L115

The excuse is, this is pretty cutting edge stuff with Ethereum dependencies, and this is a fast, unsettled universe ..;) Anyways, just mentioning/wondering ..

yan12125 commented 1 year ago

It's good to include the pieces that NVX already provides, because even though those pieces are not yet "complete" (wrt my original plan), the pieces which are there, e.g. the UTF8 validator, have pretty good test coverage https://github.com/crossbario/autobahn-python/blob/master/autobahn/nvx/test/test_nvx_utf8validator.py

IOW: the NVX UTF8 validator can indeed be used in production code (in my eyes / to my knowledge)

Thanks a lot for detailed comments! I will keep nvx enabled in that package.

however, looking at all the dependencies you have on the Arch package pointing to other Python modules also packaged as Arch packages: looks pretty good! I'm saying this even though I'm usually using Python packages in a venv, directly from PyPI.

Thanks! I often use venv as well. IMO either has pros and cons :)

One thing I am wondering though: do you provide autobahn[xbr] ? this sadly has some I guess "annoying" dependencies

The excuse is, this is pretty cutting edge stuff with Ethereum dependencies, and this is a fast, unsettled universe ..;) Anyways, just mentioning/wondering ..

Nope, dependencies appear only in autobahn[xbr] are not included. The reason is indeed that keeping track with so many Ethereum-related packages is difficult. Hopefully I can get time to find a way for managing so many dependencies efficiently soon :)