#5709 closed defect (fixed)

fcollada build fix for gcc 10 (WritePhysicsRigidBodyInstance)

Reported by: pcpa Owned by: Itms
Priority: Release Blocker Milestone: Alpha 24
Component: Build & Packages Keywords:
Cc: Itms Patch: Phab:D2745

Description

Build will fail with a message in the pattern:

g++ -o "../../../binaries/system/libCollada.so" obj/Collada_Release/CommonConvert.o obj/Collada_Release/DLL.o obj/Collada_Release/Decompose.o obj/Collada_Release/GeomReindex.o obj/Collada_Release/Maths.o obj/Collada_Release/PMDConvert.o obj/Collada_Release/PSAConvert.o obj/Collada_Release/StdSkeletons.o obj/Collada
_Release/XMLFix.o obj/Collada_Release/precompiled.o    -L../../../binaries/system -L../../../libraries/source/fcollada/lib -L/usr/X11R6/lib -shared -Wl,-soname=libCollada.so -Wl,--no-undefined -Wl,--as-needed -Wl,-z,relro -Wl,-rpath,/usr/lib64/0ad -rdynamic -lFColladaSR -lxml2 -ldl
/usr/bin/ld: ../../../libraries/source/fcollada/lib/libFColladaSR.a(FAXInstanceExport.o): in function `FArchiveXML::WritePhysicsRigidBodyInstance(FCDObject*, _xmlNode*)':
/builddir/build/BUILD/0ad-0.0.23b-alpha/libraries/source/fcollada/src/FColladaPlugins/FArchiveXML/FAXInstanceExport.cpp:258: undefined reference to `_xmlNode* FArchiveXML::AddPhysicsParameter<FMVector3, 0>(_xmlNode*, char const*, FCDParameterAnimatableT<FMVector3, 0>&)'
/usr/bin/ld: /builddir/build/BUILD/0ad-0.0.23b-alpha/libraries/source/fcollada/src/FColladaPlugins/FArchiveXML/FAXInstanceExport.cpp:259: undefined reference to `_xmlNode* FArchiveXML::AddPhysicsParameter<FMVector3, 0>(_xmlNode*, char const*, FCDParameterAnimatableT<FMVector3, 0>&)'
collect2: error: ld returned 1 exit status

After bug report for gcc, and created a patch, it was eventually found that it is not a gcc fault, as reported at a similar issue at https://bugzilla.redhat.com/show_bug.cgi?id=1794127#c7

The explicit instantiation is required to make the code correct. The standard says in [temp.pre] p10:

A function template, member function of a class template, variable template, or static data member of a class template shall be defined in every translation unit in which it is implicitly instantiated unless the corresponding specialization is explicitly instantiated in some translation unit; no diagnostic is required.

The suggested patch is probably the best way to correct the issue.

Uncertain how to report this to FCollada upstream.

Attachments (3)

0ad-fcollada.patch (2.3 KB ) - added by pcpa 16 months ago.
0ad-fcollada.patch
gcc.txt (16.8 KB ) - added by elexis 15 months ago.
gcc 10 build log
clang.txt (548.2 KB ) - added by elexis 15 months ago.
clang 10 build log

Download all attachments as: .zip

Change History (12)

by pcpa, 16 months ago

Attachment: 0ad-fcollada.patch added

0ad-fcollada.patch

comment:1 by stanislas69, 16 months ago

Cc: Itms added

Hey pcpa,

Thanks for the patch. Our review process changed, please upload the patch to Phabricator (https://code.wildfiregames.com) so it can be reviewed. see wiki:Phabricator.

I'm not sure one can even build the game with GCC10 though as Spidermonkey 45 might still be not compatible.

As to reporting this to upstream FCollada, I believe we have the most updated Fcollada ever, as I ported all the patches from every github I could find to our repo (Some are not merged yet)

Thanks again.

comment:2 by pcpa, 16 months ago

Hi stanislas69,

I will consider taking all steps to use wiki:Phabricator but for the moment I just did want to report the need for the patch. I am afraid I could need to create an account, etc, just for this patch :)

The patch is required for Fedora 32, that uses gcc 10, and packages 0ad-0.0.23b-alpha.

It took me some time to figure out the patch, but it should be trivial for C++ experts.

comment:3 by stanislas69, 16 months ago

I understand. Are you a maintainer for Fedora? If so, you could maybe join our channel #0ad-dev on quakenet so you can ping us and we can ping you was the need to arise.

Last edited 16 months ago by stanislas69 (previous) (diff)

comment:4 by pcpa, 16 months ago

Yes. I am the 0ad package maintainer in Fedora.

Somewhat inactive lately. Just did need to fix the build, or the package would be retired next month.

https://src.fedoraproject.org/rpms/0ad

I maintain it for quite some time (7+ years ago added as an official Fedora package)

https://src.fedoraproject.org/rpms/0ad/commits/f16

I will check about quakenet and autojoin to lurk on #0ad-dev

comment:5 by stanislas69, 16 months ago

Thanks!

comment:6 by elexis, 15 months ago

Milestone: BacklogAlpha 24
Priority: Should HaveRelease Blocker
Summary: fcollada build fix for gcc 10fcollada build fix for gcc 10 (WritePhysicsRigidBodyInstance)

Can't compile the program on Arch Linux with gcc 10.1.0 nor clang version 10.0.0, I get the same stack.

With the patch the build succeeds, I didn't check the semantics.

Last edited 15 months ago by elexis (previous) (diff)

by elexis, 15 months ago

Attachment: gcc.txt added

gcc 10 build log

by elexis, 15 months ago

Attachment: clang.txt added

clang 10 build log

comment:7 by wraitii, 15 months ago

Based on my investigations in #5756 this is indeed the correct patch, I will commit it alongside D2749

comment:8 by stanislas69, 15 months ago

Patch: Phab:D2745

comment:9 by Itms, 13 months ago

Owner: set to Itms
Resolution: fixed
Status: newclosed

In 23794:

Fix building on GCC 10, fixes #5709, #5756.

Patch By: pcpa and wraitii
Tested By: Nescio, andy5995 and others
Differential Revision: https://code.wildfiregames.com/D2745

Note: See TracTickets for help on using tickets.