search

bigmlcom / sensenet

0 stars 8 forks source link

fix: pin protobuf version during build #39

Closed aclemente-bigml closed 7 months ago

aclemente-bigml commented 2 years ago

Due to a breaking change in protobuf, we need to make sure to not use a too recent version of the library when installing "older" versions of tensorflow. This was documented here

This was breaking the installation process, at least on mac m1. If someone can test it on a different arch, we can make sure this works on other archs as well.

charleslparker commented 2 years ago

FYI I just saw this message in the output of the macos-arm64 run of CIBuildWheel, which is probably relevant:

Warning: While arm64 wheels can be built on x86_64, they cannot be tested. The ability to test the arm64 wheels will be added in a future release of cibuildwheel, once Apple Silicon CI runners are widely available. To silence this warning, set `CIBW_TEST_SKIP: *-macosx_arm64`.

So we're flying a bit blind at the moment with the arm64 wheels.

aclemente-bigml commented 2 years ago

If this fixes the issue you were having, I will take your word for it that this is the right fix (I can't really test anything because I don't have access to apple silicon).

Actually what I wanted to make sure was that this did not break a non-apple silicon as well :) I just tested it in a Linux VM and it seems to work in there (just a pip install .) which is a good sign.

I tried to run the cibuildwheel locally to check the apple silicon wheel building process, but I ran into another set of issues with delocateing the resulting wheels. But that's okay for now, I don't think that's something that I introduced with this.

With that, I am moderately confident that I did not break anything extra, so I will bump the version.

charleslparker commented 2 years ago

@aclementev - Don't know if you looked into it, but the pinned version of protobuf seems incompatible with the standard tensorflow, according to the tests. If you want, you can try pinning this version just for M1, further down in the setup file:

https://github.com/bigmlcom/sensenet/blob/master/setup.py#L30

unmonoqueteclea commented 7 months ago

I don't think this is a problem any more