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

g++ complains with multiple definition of static constexpr in mqtt::message #442

Closed
frbitten opened this issue Jun 14, 2023 · 18 comments
Closed
Labels
bug Confirmed bug fix added A fix has been pushed to the repo and is being tested
Milestone

Comments

@frbitten
Copy link

frbitten commented Jun 14, 2023

I keep having the same problem that was described in issue #216.

Apparently the change to cmake compiling with c++17 has been reverted.

I'm using version 1.2.0.

[build] [ 29%] Linking CXX executable niopEngine
[build] /usr/bin/ld: deps/Install/paho.mqtt.cpp/lib/libpaho-mqttpp3.a(message.cpp.o):(.rodata+0x50): multiple definition of `mqtt::message::DFLT_QOS'; CMakeFiles/niopEngine.dir/tools/communications/mqtt/MQTT.cpp.o:(.rodata._ZN4mqtt7message8DFLT_QOSE[_ZN4mqtt7message8DFLT_QOSE]+0x0): first defined here
[build] /usr/bin/ld: deps/Install/paho.mqtt.cpp/lib/libpaho-mqttpp3.a(message.cpp.o):(.rodata+0x54): multiple definition of `mqtt::message::DFLT_RETAINED'; CMakeFiles/niopEngine.dir/tools/communications/mqtt/MQTT.cpp.o:(.rodata._ZN4mqtt7message13DFLT_RETAINEDE[_ZN4mqtt7message13DFLT_RETAINEDE]+0x0): first defined here
[build] /usr/bin/ld: /usr/lib/aarch64-linux-gnu/libuuid.a(la-gen_uuid.o): in function `uuid_generate':
[build] (.text+0xb70): multiple definition of `uuid_generate'; deps/Install/paho.mqtt.c/lib/libpaho-mqtt3a.a(WebSocket.c.o):WebSocket.c:(.text+0x610): first defined here
[build] /usr/bin/ld: /usr/lib/aarch64-linux-gnu/libuuid.a(la-unparse.o): in function `uuid_unparse':
[build] (.text+0xd8): multiple definition of `uuid_unparse'; deps/Install/paho.mqtt.c/lib/libpaho-mqtt3a.a(WebSocket.c.o):WebSocket.c:(.text+0x6a0): first defined here
[build] collect2: error: ld returned 1 exit status

I saw that this only happens if you use it as a static library

@fpagliughi
Copy link
Contributor

🤦

@frbitten
Copy link
Author

When changing to shared library the problem does not happen.

@fpagliughi
Copy link
Contributor

There have been similar issues popping up over the years as the language and compilers evolve, often without clear guidelines or improper implementations. It just gets tricky trying to implement something that works for one target that doesn't break another. Which version(s) of g++ are you seeing this in? I will try to recreate it.

@frbitten
Copy link
Author

Ubuntu 20.4 (Jetson)
aarch64
g++ 11

@fpagliughi
Copy link
Contributor

OK. Nice. I have a pile of NVidia boards right in front of me. I'll give it a try.

@PlaZm0
Copy link

PlaZm0 commented Aug 9, 2023

Any Updates on this topic? I am still having the same issue with 1.2.0

@nheirbaut
Copy link

Exactly the same issue here unfortunately. Also using 1.2.0 with Conan and CMake and g++ (Debian 10.2.1-6) 10.2.1 20210110. Is there a workaround?

@rogeriomm
Copy link

rogeriomm commented Sep 21, 2023

Same here using VCPKG, CMake and g++, Ubuntu 22.04

@PlaZm0
Copy link

PlaZm0 commented Sep 21, 2023

Exactly the same issue here unfortunately. Also using 1.2.0 with Conan and CMake and g++ (Debian 10.2.1-6) 10.2.1 20210110. Is there a workaround?

I have created my own conan package where I have fixed the issue and made it available in my local cache. You could also write your own patch for version 1.2.0.

This is the patch file i have created to fix the issue for me.
0001-c++17_build_fix-for-1-2.patch

@nheirbaut
Copy link

Exactly the same issue here unfortunately. Also using 1.2.0 with Conan and CMake and g++ (Debian 10.2.1-6) 10.2.1 20210110. Is there a workaround?

I have created my own conan package where I have fixed the issue and made it available in my local cache. You could also write your own patch for version 1.2.0.

This is the patch file i have created to fix the issue for me. 0001-c++17_build_fix-for-1-2.patch

Thank you, this helps!

@fpagliughi
Copy link
Contributor

OK. I see the problem... @xkszlti posted the underlying problem in #216 with the gcc issue that is causing this problem. I will put the full issue here, so that I will remember all this in six months.

The problem is that the library is being built to the C++11 spec with gcc and the applications are being built to the C++17 spec. It is the mixing of the standards that is causing the problem.

If both the library and app were compiled to C++11 or to C++17, then there would not be a linker issue. If the library were compiled to C++17, it looks like everything forward might be fine? I can compile the app as C++20 with the C++17 library and it works.

This blurb from the gcc bug report explains:

C++11 standard requires the definition of static constexpr data members if they are odr-used and C++17 allows it even though it's redundant.

In the short term I think @PlaZm0 's patch is the best solution... just don't use constexpr if we're building the library to C++11.

The upcoming release (v1.3) will keep with C++11, so that would work for now (I hope!)

The following release (v1.4?) will make the jump to C++17, and then we can make use of constexpr and other newer features in the library. Maybe make the target standard a CMake option, defaulted to C++17? If that doesn't over-complicate things.

@fpagliughi fpagliughi added this to the v1.3 milestone Nov 11, 2023
@fpagliughi fpagliughi added the bug Confirmed bug label Nov 11, 2023
fpagliughi added a commit that referenced this issue Nov 11, 2023
@fpagliughi fpagliughi added the fix added A fix has been pushed to the repo and is being tested label Nov 11, 2023
@fpagliughi
Copy link
Contributor

OK, I think I got all occurrences of this problem. The same pattern was present in the classes message, will_options, subscribe_options and client.

The thread_queue also has a static constexpr, but as a template class, I don't think it has the same issue.

I pushed it all the way up to master. Let me know if there are still any problems with this version.

@fpagliughi
Copy link
Contributor

I'm assuming this is fixed now. Please re-open if not.

@itfanr
Copy link

itfanr commented Dec 21, 2023

Same here using VCPKG, CMake and g++, Ubuntu 22.04

Me too.

But got no error with clang.

@fpagliughi
Copy link
Contributor

@itfanr What version of the library are you using?

@itfanr
Copy link

itfanr commented Dec 22, 2023

@itfanr What version of the library are you using?

image

image

image

I installed paho.mqtt.cpp with vcpkg.

@fpagliughi
Copy link
Contributor

It looks like vcpkg is still using v1.2 of this library?
This was fixed in v1.3.

Unfortunately, we don't maintain any of the packages distributed by the different managers. Perhaps ask them to update their package?

@itfanr
Copy link

itfanr commented Dec 22, 2023

@fpagliughi Thank you.

The later released vcpkg have updated the version of paho.mqtt.cpp .

I will try to update my vcpkg baseline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug fix added A fix has been pushed to the repo and is being tested
Projects
None yet
Development

No branches or pull requests

6 participants