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

Does not build on i586 (GTAV plugin fails) #5849

Closed
susnux opened this issue Sep 1, 2022 · 11 comments · Fixed by #5850
Closed

Does not build on i586 (GTAV plugin fails) #5849

susnux opened this issue Sep 1, 2022 · 11 comments · Fixed by #5850
Labels
build Everything related to compiling/building the code client

Comments

@susnux
Copy link
Contributor

susnux commented Sep 1, 2022

Description

When building for i586 the GTAV Plugin fails (which is ok as the game only works on amd64), but rather then failing the i586 build should default to disable amd64 only plugins.

Steps to reproduce

build on i586

Mumble version

1.4.274

Mumble component

Client

OS

Linux

Reproducible?

Yes

Additional information

Caused by memory alignment, which is different on 32 bit and 64 bit machines:

[ 1615s] In file included from /home/abuild/rpmbuild/BUILD/mumble-src/plugins/gtav/Game.h:9,
[ 1615s]                  from /home/abuild/rpmbuild/BUILD/mumble-src/plugins/gtav/gtav.cpp:6:
[ 1615s] /home/abuild/rpmbuild/BUILD/mumble-src/plugins/gtav/structs.h:218:37: error: static assertion failed
[ 1615s]   218 | static_assert(sizeof(CCameraAngles) == 0x408, "");
[ 1615s]       |               ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
[ 1615s] /home/abuild/rpmbuild/BUILD/mumble-src/plugins/gtav/structs.h:218:37: note: the comparison reduces to '(1028 == 1032)'

Relevant log output

No response

Screenshots

No response

@susnux susnux added bug A bug (error) in the software triage This issue is waiting to be triaged by one of the project members labels Sep 1, 2022
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Sep 1, 2022

i586

What even is that? Isn't this like archaically old?

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Sep 1, 2022

You'll have to build without plugins using -Dplugins=OFF when invoking cmake

@Krzmbrzl Krzmbrzl added support and removed bug A bug (error) in the software triage This issue is waiting to be triaged by one of the project members labels Sep 1, 2022
@susnux
Copy link
Contributor Author

susnux commented Sep 1, 2022

What even is that? Isn't this like archaically old?

32bit x86 processor.
There are still people running 32bit OS on their 64bit CPU for reasons.

You'll have to build without plugins using -Dplugins=OFF when invoking cmake

That will disable all plugins, while only one plugin is causing the build failure.

Currently I use this patch for building the latest release:

diff -Nur mumble-src/plugins/CMakeLists.txt new/plugins/CMakeLists.txt
--- mumble-src/plugins/CMakeLists.txt	2022-08-21 18:52:23.000000000 +0200
+++ new/plugins/CMakeLists.txt	2022-09-01 12:24:10.507433751 +0200
@@ -24,6 +24,9 @@
 
 foreach(ITEM ${ITEMS})
 	if(IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${ITEM}")
+		if (ITEM STREQUAL "gtav" AND CMAKE_SIZEOF_VOID_P LESS 8)
+			continue()
+		endif() 
 		set(PLUGIN_RETRACTED OFF)
 
 		# If the plugin is retracted the corresponding CMakeLists.txt is supposed to set the

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Sep 1, 2022

@davidebeatrici do you think this is something we can fix in-code?

@davidebeatrici
Copy link
Member

davidebeatrici commented Sep 1, 2022

Yes, absolutely, we just have to force alignment.

When building for i586 the GTAV Plugin fails (which is ok as the game only works on amd64), but rather then failing the i586 build should default to disable amd64 only plugins.

Back when we introduced the very first x86_64 (x64, amd64) plugins, we excluded them from x86 (i386, i486, i586, i686) builds as at the time our code did not support targeting processes with a different pointer size: #2607

The limitation was fixed in #3262 and we build 64 bit plugins for 32 bit targets since #4252.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Sep 1, 2022

Alright - will you create a PR?

@davidebeatrici
Copy link
Member

@susnux Could you verify that #5850 fixes the issue for you, please?

@susnux
Copy link
Contributor Author

susnux commented Sep 1, 2022

@davidebeatrici

Could you verify that #5850 fixes the issue for you, please?

Still getting this error
log.txt

@davidebeatrici
Copy link
Member

Fixed, sorry. Could you test again?

@susnux
Copy link
Contributor Author

susnux commented Sep 2, 2022

@davidebeatrici

Fixed, sorry. Could you test again?

Now it works!

@davidebeatrici
Copy link
Member

Excellent, thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Everything related to compiling/building the code client
Projects
None yet
3 participants