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

Use CMAKE_SYSTEM_PROCESSOR instead of CMAKE_HOST_SYSTEM_PROCESSOR #902

Merged
merged 1 commit into from
Jul 23, 2023

Conversation

josch
Copy link
Sponsor Contributor

@josch josch commented Jul 23, 2023

The former is exactly the same as the latter if compiling natively, which is the output of uname -m. But this is the wrong architecture when cross compiling where we do not want the architecture we are compiling on (the build architecture in GNU terminology) but the architecture we are compiling for (the host architecture in GNU terminology). The CMAKE_SYSTEM_PROCESSOR variable is set to the correct host architecture value when cross compiling.

With this change, it is possible to cross compile a arm64 executable of box64 on an amd64 machine. Without this change, the "install" target will be omitted, because CMakeLists.txt wrongly uses the build architecture to decide whether the install target should be added or not. Instead it should use the host architecture.

The former is exactly the same as the latter if compiling natively,
which is the output of `uname -m`. But this is the wrong architecture
when cross compiling where we do not want the architecture we are
compiling on (the build architecture in GNU terminology) but the
architecture we are compiling for (the host architecture in GNU
terminology). The CMAKE_SYSTEM_PROCESSOR variable is set to the correct
host architecture value when cross compiling.

With this change, it is possible to cross compile a arm64 executable of
box64 on an amd64 machine. Without this change, the "install" target
will be omitted, because CMakeLists.txt wrongly uses the build
architecture to decide whether the install target should be added or
not. Instead it should use the host architecture.
@ptitSeb ptitSeb merged commit da098cc into ptitSeb:main Jul 23, 2023
22 checks passed
@theofficialgman
Copy link
Contributor

oh nice. I was patching cmakelists so I could cross comile install (used with checkinstall when building debs)

  # allow installation even on x86_64 (needed for checkinstall)
  sed -i "s/NOT _x86 AND NOT _x86_64/true/g" ../CMakeLists.txt

@theofficialgman
Copy link
Contributor

theofficialgman commented Jul 23, 2023

@josch this does not work
the install target is still omitted for me
https://github.com/Pi-Apps-Coders/box64-debs/actions/runs/5637162904/job/15269971660#step:7:710

what are your cross compilation commands?

@josch
Copy link
Sponsor Contributor Author

josch commented Jul 23, 2023

Hi, I see you are using https://github.com/Pi-Apps-Coders/box64-debs/blob/master/create-deb.sh to build this? I do not have time for a full review but that script does not look good. You seem to try and re-invent the wheel to do things other tools will do for you automatically. Using checkinstall is a bad way to create packages for Debian. It's meant for a quick-and-dirty solution but not appropriate for something that other people than yourself are supposed to use.

Instead of trying to run everything manually, you can take advantage of existing tools like apt, dpkg and debhelper to do the whole process of installing build dependencies, cross-building the package, running the tests and creating the *.deb package for you. You then get the correct cmake command for free.

I think the best way forward for you would be to either:

  • copy the ./debian directory from box64 in Debian https://salsa.debian.org/debian/box64/ and run dpkg-buildpackage to cross-build your *.deb packages or
  • use the existing packaging to figure out the exact cmake invocation that will cross build box64 successfully

Here is the last successful cross-build log as proof that the packaging of box64 I did for Debian indeed works as expected:
https://salsa.debian.org/debian/box64/-/jobs/4462302

@theofficialgman
Copy link
Contributor

I am well aware of the alternative tools. I inherited the buildscripts from someone else.
since you didn't answer the statement I will rephrase it.

you are using debhelper which sets the CMAKE_SYSTEM_PROCESSOR variable. This isn't set automatically by cmake.

see: https://github.com/Debian/debhelper/blob/5d1bb29841043d8e47ebbdd043e6cd086cad508e/lib/Debian/Debhelper/Buildsystem/cmake.pm#L114-L119

this change helps when cross compiling with debhelper but should be documented. if you are using any other tool (or manually running the commands) you will have to pass the appropriate CMAKE_SYSTEM_PROCESSOR argument to cmake

@josch
Copy link
Sponsor Contributor Author

josch commented Jul 23, 2023

you are using debhelper which sets the CMAKE_SYSTEM_PROCESSOR variable. This isn't set automatically by cmake.

It is. For reference see the CMake documentation:

https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_PROCESSOR.html

this change helps when cross compiling with debhelper but should be documented.

It is. By the CMake docs.

if you are using any other tool (or manually running the commands) you will have to pass the appropriate CMAKE_SYSTEM_PROCESSOR argument to cmake

Usually that is done as part of a toolchain file. But it is not the job of box64 to document how CMake works.

@theofficialgman
Copy link
Contributor

sigh... read the docs again that you linked. I won't bother explaining it

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.

None yet

3 participants