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.

Sign up for GitHub

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

DPP: De-const have_voice option and add coroutines support. #4824

Merged
merged 11 commits into from
Aug 5, 2024
+53 −10
Merged

DPP: De-const have_voice option and add coroutines support. #4824

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1e33e4a
fix(dpp): Remove repeated have_voice condition
Nk125 Aug 2, 2024
baec75d
feat(dpp): Make have_voice option modifiable
Nk125 Aug 2, 2024
4db9dfe
feat(dpp): Add coroutine support
Nk125 Aug 2, 2024
84b45d9
fix(dpp)!: Change have_voice to voice inside package
Nk125 Aug 4, 2024
ee9454b
fix(dpp): Add C++ minimum version for test
Nk125 Aug 4, 2024
ef43fab
fix(dpp): Set C++ language for library dinamically
Nk125 Aug 4, 2024
371cb43
style(dpp): Remove requested empty lines
Nk125 Aug 4, 2024
11d688d
Merge branch 'xmake-io:dev' into dpp-feats-coro-voice
Nk125 Aug 4, 2024
5ef2f4b
Merge branch 'xmake-io:dev' into dev
Nk125 Aug 4, 2024
25017c3
Merge branch 'dpp-feats-coro-voice' of https://github.com/Nk125/xmake…
Nk125 Aug 4, 2024
8936697
feat(dpp): Re-add have_voice flag with deprecated warning
Nk125 Aug 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions packages/d/dpp/port/xmake.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,32 @@
add_rules("mode.debug", "mode.release")

add_requires("fmt", "nlohmann_json", "libsodium", "libopus", "openssl", "zlib")
add_requires("fmt", "nlohmann_json", "openssl", "zlib")

option("coro", {default = false})

Nk125 marked this conversation as resolved.
Show resolved Hide resolved
option("have_voice", {default = true})

if has_config("have_voice") then
add_requires("libopus", "libsodium")
end

target("dpp")
set_kind("$(kind)")
set_languages("c++17")
add_includedirs("include", "include/dpp")
add_headerfiles("include/(dpp/**.h)")
add_files("src/dpp/**.cpp")
add_packages("fmt", "nlohmann_json", "libsodium", "libopus", "openssl", "zlib")
add_packages("fmt", "nlohmann_json", "openssl", "zlib")

if has_config("have_voice") then
add_packages("libopus", "libsodium")
add_defines("HAVE_VOICE")
end

if has_config("coro") then
set_languages("c++20")
add_defines("DPP_CORO")
end

add_defines("DPP_BUILD", "DPP_USE_EXTERNAL_JSON")

Expand Down
15 changes: 10 additions & 5 deletions packages/d/dpp/xmake.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ package("dpp")

add_deps("nlohmann_json", "openssl", "zlib")

add_configs("have_voice", { description = "Enable voice support for the library.", default = true, type = "boolean" , readonly = true})
add_configs("have_voice", { description = "Enable voice support for the library.", default = true, type = "boolean" , readonly = false})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename have_voice to voice, we need not any have_ with_ enable_ preifx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was changed but the main issue is that this change will break the current user code bases, the flag was left unchanged since 2022 (introduced here), the most reliable solution I was considering is showing a deprecation warning to anyone is using have_voice flag and then in later DPP versions erase it from the next versions.

Another solution I propose is doing a temporal alias from have_voice to voice internally in the script, and doing the same as before, show a deprecation warning and then delete the flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can reserve have_voice and add some deprecated warnings.

and add new voice config , then tell user to use voice instead of have_voice

Copy link
Contributor Author

@Nk125 Nk125 Aug 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in the last commit, please review it for other advices, thanks for the support!


add_configs("coro", { description = "Enable experimental coroutines support for the library.", default = false, type = "boolean" , readonly = false})

if is_plat("linux", "macosx") then
add_syslinks("pthread")
Expand All @@ -68,9 +70,8 @@ package("dpp")
package:add("deps", "libsodium", "libopus")
end

if package:config("have_voice") then
package:add("defines", "HAVE_VOICE")
package:add("deps", "libsodium", "libopus")
if package:config("coro") then
package:add("defines", "DPP_CORO")
end

if package:version():le("v10.0.13") then
Expand Down Expand Up @@ -103,7 +104,11 @@ package("dpp")
io.replace("include/dpp/restrequest.h", "#include <nlohmann/json_fwd.hpp>", "#include <nlohmann/json.hpp>", {plain = true})
os.rmdir("include/dpp/nlohmann")

local configs = {}
local configs = {
have_voice = package:config("have_voice"),
coro = package:config("coro")
}

if package:version():ge("v10.0.29") and package:is_plat("windows") then
configs.cxflags = "/bigobj /Gy"
end
Expand Down
Loading