You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/10/18 04:17:19 UTC
[GitHub] [buildstream] gtristan opened a new pull request, #1777: setup.py: Stop including generated Cython code in source distributions.
gtristan opened a new pull request, #1777:
URL: https://github.com/apache/buildstream/pull/1777
Ensures that code will always be generated when building for a given target environment, this is because Cython generates code which accesses CPython internal symbols and we cannot guarantee that the C code we generated will be valid for the target CPython interpretor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] nanonyme commented on pull request #1777: setup.py: Stop including generated Cython code in source distributions.
Posted by GitBox <gi...@apache.org>.
nanonyme commented on PR #1777:
URL: https://github.com/apache/buildstream/pull/1777#issuecomment-1282631432
pip will handle installing cython for building by default. It should be fine to delete the C code and cythonize always. In wheel you *definitely* do not want the C code as they don't ever see a C compiler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan merged pull request #1777: setup.py: Stop including generated Cython code in source distributions.
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan merged PR #1777:
URL: https://github.com/apache/buildstream/pull/1777
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1777: setup.py: Stop including generated Cython code in source distributions.
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1777:
URL: https://github.com/apache/buildstream/pull/1777#issuecomment-1281952487
I've landed #1778 in place of this, although it still makes sense to stop distributing the generated C code, given that it is almost inconceivable that the distributed C code would be used unless `./setup.py` is invoked directly.
I'll leave this open for the time being, I just don't want to land this right before a release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1777: setup.py: Stop including generated Cython code in source distributions.
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1777:
URL: https://github.com/apache/buildstream/pull/1777#issuecomment-1281850816
Investigating further, it appears that this should not be required.
The issue we ran into in particular was with `_PyGen_Send` missing in 3.10, which is handled by cython in: https://github.com/cython/cython/commit/782a87336d88496b022bf0f2791c818221f4743a
If I'm reading this correctly, then the generated code should support `cpython <= 3.10` after this fix.
Were we to follow the upstream intentions (as I think I understand them now), then we would have set a minimal bound build time dependency on Cython when adding support for python 3.10 in BuildStream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [buildstream] gtristan commented on pull request #1777: setup.py: Stop including generated Cython code in source distributions.
Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1777:
URL: https://github.com/apache/buildstream/pull/1777#issuecomment-1281956434
> I'd rather go for this rather than #1778 (or more correctly in addition to #1778). I think the most common way to install python packages at this point is using pip, which handles dependencies nicely.
Agreed, the generated C code in source distributions is essentially mostly unused - but the patch had some weird side effects in `tox.ini`, and I'd rather not land this unnecessary change right before a release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org