DPP icon indicating copy to clipboard operation
DPP copied to clipboard

Package manager support

Open ReallySnazzy opened this issue 3 years ago β€’ 48 comments
trafficstars

Is your feature request related to a problem? Please describe. Vcpkg or conan support help alleviate the setup for multiple libraries in a project. They help to get started quickly with a framework while still allowing other libraries to be added quickly.

Describe the solution you'd like Rather than premade build setups, I think that having vcpkg or conan support would enable more ambitious discord bots to be created faster.

Describe alternatives you've considered

Additional context

Tasks added by @braindigitalis

  • [x] vcpkg
  • [x] xmake
  • [x] AUR
  • [x] SW Network
  • [x] Libhunt
  • [x] gentoo emerge/GURU
  • [x] void linux XBPS
  • [x] FreeBSD Ports/Packages
  • [ ] apt, ppa
  • [x] conan
  • [x] brew

ReallySnazzy avatar Mar 25 '22 17:03 ReallySnazzy

I added DPP to xmake repository (see https://github.com/xmake-io/xmake-repo/pull/1222), with my changes it compiles fine on Windows, MinGW, Linux, macOS in both dynamic and static linking.

Since xmake is able to handle dependencies (will find libraries on the machine like cmake but is able to download and compile them if it doesn't find them), I managed to build the lib using a very short xmake.lua.

Usually, xmake packages use CMake to build CMake libraries, but the way dependencies are handled on DPP doesn't help much (mostly because they are embedded). Which is why I patched nlohmann/fmt out of the lib.

I also fixed the export.h header which made static linking impossible on Windows, here's the diff:

diff --git a/include/dpp/export.h b/include/dpp/export.h
index b7b35b5..838f79d 100644
--- a/include/dpp/export.h
+++ b/include/dpp/export.h
@@ -24,25 +24,29 @@
 // Investigate: MSVC doesn't like this
 //static_assert(__cplusplus >= 201703L, "D++ Requires a C++17 compatible compiler. Please ensure that you have enabled C++17 in your compiler flags.");
 
-#ifdef DPP_BUILD
+#ifndef DPP_STATIC
 
-	#ifdef _WIN32
-		#include <dpp/win32_safe_warnings.h>
-	#endif
+	#ifdef DPP_BUILD
+
+		#ifdef _WIN32
+			#include <dpp/win32_safe_warnings.h>
+		#endif
 
-	#ifdef _WIN32
-		#define DPP_EXPORT __declspec(dllexport)
+		#ifdef _WIN32
+			#define DPP_EXPORT __declspec(dllexport)
+		#else
+			#define DPP_EXPORT
+		#endif
 	#else
-		#define DPP_EXPORT
+		#ifdef _WIN32
+			#define DPP_EXPORT __declspec(dllimport)
+		#else
+			#define DPP_EXPORT
+		#endif
 	#endif
+
 #else
-	#ifdef _WIN32
-		#define DPP_EXPORT __declspec(dllimport)
-		/* This is required otherwise fmt::format requires additional file linkage to your project */
-		#define FMT_HEADER_ONLY
-	#else
-		#define DPP_EXPORT
-	#endif
+	#define DPP_EXPORT
 #endif
 
 #ifndef _WIN32

SirLynix avatar May 18 '22 10:05 SirLynix

nlohmann and fmt are header only. it would be nicer if they weren't patched out, because we test with specific versions and not just whatever the latest is which could cause support issues

braindigitalis avatar May 18 '22 11:05 braindigitalis

xmake can be instructed to use a specific version of nlohmann/fmt. The main issue here is that by embedding a specific version of those libraries (and not putting them in a namespace), you conflict with the user nlohmann/fmt libs (obligating them to use the very same version you're using for API/ABI compatibility).

Since these libraries are very well tested, I think it would be better to not embed them to prevent such issues.

SirLynix avatar May 18 '22 11:05 SirLynix

I added DPP to xmake repository (see xmake-io/xmake-repo#1222), with my changes it compiles fine on Windows, MinGW, Linux, macOS in both dynamic and static linking.

Since xmake is able to handle dependencies (will find libraries on the machine like cmake but is able to download and compile them if it doesn't find them), I managed to build the lib using a very short xmake.lua.

Usually, xmake packages use CMake to build CMake libraries, but the way dependencies are handled on DPP doesn't help much (mostly because they are embedded). Which is why I patched nlohmann/fmt out of the lib.

I also fixed the export.h header which made static linking impossible on Windows, here's the diff:

diff --git a/include/dpp/export.h b/include/dpp/export.h
index b7b35b5..838f79d 100644
--- a/include/dpp/export.h
+++ b/include/dpp/export.h
@@ -24,25 +24,29 @@
 // Investigate: MSVC doesn't like this
 //static_assert(__cplusplus >= 201703L, "D++ Requires a C++17 compatible compiler. Please ensure that you have enabled C++17 in your compiler flags.");
 
-#ifdef DPP_BUILD
+#ifndef DPP_STATIC
 
-	#ifdef _WIN32
-		#include <dpp/win32_safe_warnings.h>
-	#endif
+	#ifdef DPP_BUILD
+
+		#ifdef _WIN32
+			#include <dpp/win32_safe_warnings.h>
+		#endif
 
-	#ifdef _WIN32
-		#define DPP_EXPORT __declspec(dllexport)
+		#ifdef _WIN32
+			#define DPP_EXPORT __declspec(dllexport)
+		#else
+			#define DPP_EXPORT
+		#endif
 	#else
-		#define DPP_EXPORT
+		#ifdef _WIN32
+			#define DPP_EXPORT __declspec(dllimport)
+		#else
+			#define DPP_EXPORT
+		#endif
 	#endif
+
 #else
-	#ifdef _WIN32
-		#define DPP_EXPORT __declspec(dllimport)
-		/* This is required otherwise fmt::format requires additional file linkage to your project */
-		#define FMT_HEADER_ONLY
-	#else
-		#define DPP_EXPORT
-	#endif
+	#define DPP_EXPORT
 #endif
 
 #ifndef _WIN32

with this diff applied does dpp still build ok outside of xmake? the FMT_HEADER_ONLY was there for a reason and otherwise it would build but fail at runtime...

braindigitalis avatar May 18 '22 11:05 braindigitalis

with this diff applied does dpp still build ok outside of xmake?

Yes, but DPP_STATIC should be handled in the cmakelists.

the FMT_HEADER_ONLY was there for a reason and otherwise it would build but fail at runtime...

It makes little sense to build the lib without FMT_HEADER_ONLY and use it with user code, the reason it was failing was because fmt has to be linked to the final executable without that flag, something a proper package manager guarantees.

SirLynix avatar May 18 '22 11:05 SirLynix

xmake can be instructed to use a specific version of nlohmann/fmt. The main issue here is that by embedding a specific version of those libraries (and not putting them in a namespace), you conflict with the user nlohmann/fmt libs (obligating them to use the very same version you're using for API/ABI compatibility).

Since these libraries are very well tested, I think it would be better to not embed them to prevent such issues.

I see, I will consider moving dpp's copies to a namespace to prevent conflicts.

braindigitalis avatar May 18 '22 11:05 braindigitalis

with this diff applied does dpp still build ok outside of xmake?

Yes, but DPP_STATIC should be handled in the cmakelists.

the FMT_HEADER_ONLY was there for a reason and otherwise it would build but fail at runtime...

It makes little sense to build the lib without FMT_HEADER_ONLY and use it with user code, the reason it was failing was because fmt has to be linked to the final executable without that flag, something a proper package manager guarantees.

thanks! can you please submit a pr of this diff against dev branch and I'll merge it in...

braindigitalis avatar May 18 '22 11:05 braindigitalis

I see, I will consider moving dpp's copies to a namespace to prevent conflicts.

Unfortunately this won't be enough, because of preprocessor defines (libs header guards), there's no proper way to embed fmt/nlohmann in a project headers.

thanks! can you please submit a pr of this diff against dev branch and I'll merge it in...

Someone else is working on this already, all my changes will only be applied when installing dpp through xmake.

If you wish to keep using CMake, xrepo-cmake may help you to handle dependencies in a better way.

SirLynix avatar May 18 '22 11:05 SirLynix

Vcpkg support would be quite nice, is it coming anytime soon?

M-U-X avatar Jun 23 '22 18:06 M-U-X

there's an open pr over there, ive not had time to work on it though...

braindigitalis avatar Jun 23 '22 21:06 braindigitalis

Vcpkg support would be quite nice, is it coming anytime soon?

there is now a finalized pr in vcpkg thanks to @RealTimeChris - we should be in vcpkg very soon!

braindigitalis avatar Jul 24 '22 21:07 braindigitalis

fyi Added to SW https://software-network.org/org.sw.demo.brainboxdotcc.dpp-10.0.15

egorpugin avatar Aug 03 '22 07:08 egorpugin

Nice, thanks! does this auto update from github releases?

braindigitalis avatar Aug 03 '22 08:08 braindigitalis

No, not yet. Such feature is on todo list, but won't be here soon enough.

egorpugin avatar Aug 03 '22 09:08 egorpugin

Since I have already an ebuild for Gentoo, I might as well move it to ::guru (the AUR equivalent for Gentoo). DPP is probably not big enough (in terms of consumers) for the main repo, but ::guru should suffice.

However, since I had to patch the installation for DPP to work on my Gentoo machines (mainly to correct some paths and file names), I might submit a PR making some changes to the installation procedure first.

NexAdn avatar Aug 29 '22 10:08 NexAdn

Since I have already an ebuild for Gentoo, I might as well move it to ::guru (the AUR equivalent for Gentoo). DPP is probably not big enough (in terms of consumers) for the main repo, but ::guru should suffice.

However, since I had to patch the installation for DPP to work on my Gentoo machines (mainly to correct some paths and file names), I might submit a PR making some changes to the installation procedure first.

thanks, i'd like to see this 😊

braindigitalis avatar Aug 29 '22 18:08 braindigitalis

Hey @NexAdn are you still going to move your ebuild for Gentoo to the GURU? If not, would you mind if I do that?

SirObby avatar Dec 03 '22 18:12 SirObby

bruh

thenameisluk avatar Dec 03 '22 18:12 thenameisluk

@SirObby oh boy, I totally forgot that :sweat: Feel free to move your ebuild. If you want, we can co-maintain it (you can use [email protected] as my email for metadata.xml).

NexAdn avatar Dec 04 '22 10:12 NexAdn

The Gentoo package is now uploaded to GURU and should be available for the public soon. it is maintained by @SirObby and me

NexAdn avatar Jan 04 '23 13:01 NexAdn

add openssl to vcpkg on linux its not present

RatTrap1337 avatar Jan 25 '23 17:01 RatTrap1337

add openssl to vcpkg on linux its not present

... who uses vcpkg on linux πŸ˜‚ nah, PR welcome. vcpkg is broken right now, and needs redoing.

braindigitalis avatar Jan 25 '23 18:01 braindigitalis

... who uses vcpkg on linux joy nah, PR welcome. vcpkg is broken right now, and needs redoing.

Regarding vcpkg, looking at the CI logs it just looks to me like the patch in vcpkg doesn't apply. So we'd β€œjust” have to rebase the patch onto the current master and the vcpkg build should work again.

NexAdn avatar Jan 25 '23 23:01 NexAdn

Work's fine you just have to add OpenSSL package as dependency :o

RatTrap1337 avatar Jan 26 '23 05:01 RatTrap1337

Working on Void Linux support right now

OoLunar avatar Feb 26 '23 22:02 OoLunar

is voice broken on vcpkg builds?

MuseXE avatar Mar 14 '23 06:03 MuseXE

Void Linux has officially accepted D++ as a package. See https://github.com/void-linux/void-packages/commit/e322975b871c953446fae330a9869902c1b5dee4

OoLunar avatar Jun 27 '23 18:06 OoLunar

Is there interest in having conan support?

MikeRavenelle avatar Sep 24 '23 12:09 MikeRavenelle

Is there interest in having conan support?

of course, is this something you have experience with?

braindigitalis avatar Sep 24 '23 15:09 braindigitalis

Yes I do. I can look into writing a conan recipe.

MikeRavenelle avatar Sep 24 '23 16:09 MikeRavenelle