Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cross-compiling #10

Closed
wants to merge 1 commit into from
Closed

Conversation

eigendude
Copy link

@eigendude eigendude commented May 19, 2019

Description

When packaging crc32c for OpenEmbedded, I hit a build error. Cross-compiling fails because setup.py assumes it's running on the build machine. However, setup.py treats the command line as an opaque string, so we can't pass build system parameters normally.

Fortunately, setuptools uses a file called setup.cfg to set parameters. I added support for machine configuration in a new build system section.

How has this been tested?

I created an OpenEmbedded recipe for crc32c:

DESCRIPTION = "A python package exposing the Intel SSE4.2 CRC32C instruction."
HOMEPAGE = "https://github.com/ICRAR/crc32c"
SECTION = "devel/python"
LICENSE = "LGPLv2"
LIC_FILES_CHKSUM = "file://setup.py;beginline=81;endline=81;md5=9e2e837c46174eef7a5db8b5c82dd34f"

SRC_URI[md5sum] = "9d799f4c836c003eb7fe23545c78e9f6"
SRC_URI[sha256sum] = "bdcd28f26b62838919480d465a0d166207a36c4f104102a0b6edf5b498544d36"

inherit pypi setuptools

Without the patch, I get this error:

| arm-sundstrom-linux-gnueabi-gcc: error: unrecognized command line option '-msse4.2'
| arm-sundstrom-linux-gnueabi-gcc: error: unrecognized command line option '-mpclmul'

Then I apply the patch, and append this to my recipe:

distutils_do_configure_prepend() {
    sed -i 's/#machine =.*/machine = ${MACHINE_ARCH}/' ${S}/setup.cfg
}

After the patch, crc32c can be used in the completed build.

@eigendude eigendude force-pushed the fix-cross-compile branch 2 times, most recently from 3e47ec1 to 77f51e8 Compare May 19, 2019 05:24
This fix uses the 'setup.cfg' file used by setuptools to define parameters
for cross-compiling.

The reason for using this config file is that command-line parameters can't
be passed to setup.py, as the command-line is ideally opaque to the
installer. 'setup.cfg' was added to setuptools as a useful middle-ground
between modifying 'setup.py' and the command line. Now, build systems can
modify a config file instead of the source directly.
@eigendude eigendude force-pushed the fix-cross-compile branch from 77f51e8 to 3de44d8 Compare May 19, 2019 16:15
rtobar added a commit that referenced this pull request May 20, 2019
This new option should allow users to build the module in a
cross-compilation scenario, where the host's platform is not the
target's.

This is a different way of implementing the idea shown in #10.

Signed-off-by: Rodrigo Tobar <[email protected]>
@rtobar
Copy link
Contributor

rtobar commented May 20, 2019

Hi,

Thanks for showing interest in the package, and for highlighting the problem of cross-compilation. In principle I think the idea is good, but I'm not convinced this is the best approach. First, it requires the setup.cfg file to be always present, but secondly it bypasses the normal distutils mechanisms to handle extra options. Given that our setup.py already provides its own build_ext command it would be easy to add an option there to specify the target platform, and have it default to the host platform.

I've pushed a commit in a new platform_option showing what I mean. If you agree I'd prefer to go in that direction, which integrates better with the rest of distutils, it doesn't require new modules to be added to our codebase, works correctly in the absence of a setup.cfg file, and also allows users to specify the option in the command line (i.e., python setup.py build_ext --platform=arm). In your recipe you can then change the sed command to something like echo -e "[build_ext]\nplatform = ${MACHINE_ARCH}" > setup.cfg.

@eigendude
Copy link
Author

I was unaware of how build_ext fit into the picture. This is much nicer! I'll try your patch now.

@eigendude
Copy link
Author

Using your patch and a modified bitbake recipe, this builds for arm successfully.

When I do python3 -c "import crc32c" I get:

ImportError: 

SSE4.2 extensions providing a crc32c hardware instruction are not available in
your processor. If you still need to use the crc32c checksum algorithm this
package comes with a software implementation that can be loaded instead. For
that set the CRC32C_SW_MODE environment variable before loading the package to
one of the following values:

 * 'auto' to use software implementation if no CPU hardware support is found.
 * 'force' to use software implementation regardless of CPU hardware support.
 * '1' means 'force', but will be eventually discontinued.

Which goes away when I set CRC32C_SW_MODE.

I've only tried importing on arm. Let me know if there's a python command I should try running.

@rtobar
Copy link
Contributor

rtobar commented May 21, 2019

Thanks for letting me know that it works with the alternative solution. I take it you don't have a problem with me using that instead of yours then.

Regarding the ImportError, as the error says this is fully intended as things currently stand. The initial package implementation only exposed the hardware-accelerated crc32c instructions of Intel processors, and has thus always failed to import if such instructions were not present. A software implementation was added later, but the default importing behavior stayed to avoid giving the correct impression to users, who may think they are getting hardware acceleration in situations they aren't.

I think you would agree with me that the discussion about this importing behavior is separate from this pull request though, so I'll be closing this now.

@rtobar rtobar closed this May 21, 2019
@eigendude eigendude deleted the fix-cross-compile branch May 21, 2019 21:40
@eigendude
Copy link
Author

to avoid giving the correct impression to users, who may think they are getting hardware acceleration in situations they aren't

This is exactly how it should be.

Thanks for helping me get cross compiling working!

kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 4, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 4, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 5, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 5, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 5, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 6, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 6, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 6, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 6, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 6, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 7, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 7, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 7, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 7, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 7, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 8, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 8, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
kraj added a commit to YoeDistro/meta-openembedded that referenced this pull request Mar 9, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
halstead pushed a commit to openembedded/meta-openembedded that referenced this pull request Mar 9, 2022
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
Signed-off-by: Trevor Gamblin <[email protected]>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
Signed-off-by: Trevor Gamblin <[email protected]>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Do not poke at build system for finding platform
platform is target specific and when cross compiling it should be
detected differently, in this case lets pass it via environment so that
it can be set in recipe

Looks like we're not the first to need to specify the target platform
[1]
when building this package.  According to upstream, we can just update
setup.cfg instead of patching setup.py.

[1] ICRAR/crc32c#10

Signed-off-by: Justin Bronder <[email protected]>
Signed-off-by: Khem Raj <[email protected]>
Signed-off-by: Trevor Gamblin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants