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

Jump to bottom

Introduce --x-cmake-debug and --x-cmake-configure-debug #1173

Merged
merged 4 commits into from
Sep 4, 2023
Merged

Introduce --x-cmake-debug and --x-cmake-configure-debug #1173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0bda566
Introduce --x-cmake-debug and --x-cmake-configure-debug
Neumann-A Aug 28, 2023
7dcd792
* Make multiple uses of the new switches an error
BillyONeal Aug 29, 2023
7189509
clang-format
BillyONeal Aug 29, 2023
cf63f08
Merge pull request #1 from BillyONeal/cmake_debugging
Neumann-A Aug 29, 2023
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
1 change: 1 addition & 0 deletions include/vcpkg/fwd/vcpkgcmdarguments.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ namespace vcpkg
struct HelpTableFormatter;
struct VcpkgCmdArguments;
struct FeatureFlagSettings;
struct PortApplicableSetting;
}
20 changes: 20 additions & 0 deletions include/vcpkg/vcpkgcmdarguments.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ namespace vcpkg
bool dependency_graph;
};

struct PortApplicableSetting
{
std::string value;

PortApplicableSetting(StringView setting);
PortApplicableSetting(const PortApplicableSetting&);
PortApplicableSetting(PortApplicableSetting&&);
PortApplicableSetting& operator=(const PortApplicableSetting&);
PortApplicableSetting& operator=(PortApplicableSetting&&);
bool is_port_affected(StringView port_name) const noexcept;

private:
std::vector<std::string> affected_ports;
};

struct VcpkgCmdArguments
{
static VcpkgCmdArguments create_from_command_line(const ILineReader& fs,
Expand Down Expand Up @@ -161,6 +176,11 @@ namespace vcpkg
constexpr static StringLiteral GITHUB_REPOSITORY_OWNER_ID = "GITHUB_REPOSITORY_OWNER_ID";
Optional<std::string> github_repository_owner_id;

constexpr static StringLiteral CMAKE_DEBUGGING_ARG = "cmake-debug";
Optional<PortApplicableSetting> cmake_debug;
constexpr static StringLiteral CMAKE_CONFIGURE_DEBUGGING_ARG = "cmake-configure-debug";
Optional<PortApplicableSetting> cmake_configure_debug;

constexpr static StringLiteral CMAKE_SCRIPT_ARG = "cmake-args";
std::vector<std::string> cmake_args;

Expand Down
21 changes: 21 additions & 0 deletions src/vcpkg-test/arguments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,24 @@ TEST_CASE ("Feature flag off", "[arguments]")
auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
CHECK(!v.versions_enabled());
}

TEST_CASE ("CMake debugger flags", "[arguments]")
{
std::vector<std::string> t = {"--x-cmake-debug",
"\\\\.\\pipe\\tespipe;zlib;bar;baz",
Neumann-A marked this conversation as resolved.
Show resolved Hide resolved
"--x-cmake-configure-debug",
"\\\\.\\pipe\\configure-pipe"};
auto v = VcpkgCmdArguments::create_from_arg_sequence(t.data(), t.data() + t.size());
auto& cmake_debug = v.cmake_debug.value_or_exit(VCPKG_LINE_INFO);
REQUIRE(cmake_debug.value == "\\\\.\\pipe\\tespipe");
REQUIRE(!cmake_debug.is_port_affected("7zip"));
REQUIRE(cmake_debug.is_port_affected("zlib"));
REQUIRE(cmake_debug.is_port_affected("bar"));
REQUIRE(cmake_debug.is_port_affected("baz"));
REQUIRE(!cmake_debug.is_port_affected("bazz"));

auto& cmake_configure_debug = v.cmake_configure_debug.value_or_exit(VCPKG_LINE_INFO);
REQUIRE(cmake_configure_debug.value == "\\\\.\\pipe\\configure-pipe");
REQUIRE(cmake_configure_debug.is_port_affected("7zip"));
REQUIRE(cmake_configure_debug.is_port_affected("zlib"));
}
Comment on lines +164 to +183
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 only tests PortApplicableSetting and not the actual cmake invocation.
I am more concerned about cmake <something> -P <script> being a valid cmake call.

Copy link
Member

Choose a reason for hiding this comment

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

That would require setting up, as you said, the whole separate processes business. I don't have good ideas on testing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then testing by direct usage? I mean it is an --x- option any way.

18 changes: 18 additions & 0 deletions src/vcpkg/commands.build.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,24 @@ namespace vcpkg
variables.emplace_back("ARIA2", paths.get_tool_exe(Tools::ARIA2, stdout_sink));
}

if (auto cmake_debug = args.cmake_debug.get())
{
if (cmake_debug->is_port_affected(scf.core_paragraph->name))
{
variables.emplace_back("--debugger");
variables.emplace_back(fmt::format("--debugger-pipe={}", cmake_debug->value));
}
}

if (auto cmake_configure_debug = args.cmake_configure_debug.get())
{
if (cmake_configure_debug->is_port_affected(scf.core_paragraph->name))
{
variables.emplace_back(fmt::format("-DVCPKG_CMAKE_CONFIGURE_OPTIONS=--debugger;--debugger-pipe={}",
cmake_configure_debug->value));
}
}

for (const auto& cmake_arg : args.cmake_args)
{
variables.emplace_back(cmake_arg);
Expand Down
40 changes: 40 additions & 0 deletions src/vcpkg/vcpkgcmdarguments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#include <vcpkg/metrics.h>
#include <vcpkg/vcpkgcmdarguments.h>

#include <algorithm>
#include <string>
#include <utility>

namespace
{
using namespace vcpkg;
Expand Down Expand Up @@ -231,6 +235,28 @@ namespace vcpkg
}
}

PortApplicableSetting::PortApplicableSetting(StringView setting)
{
auto split = Strings::split(setting, ';');
if (!split.empty())
{
value = std::move(split[0]);
split.erase(split.begin());
Util::sort(split);
affected_ports = std::move(split);
}
}

PortApplicableSetting::PortApplicableSetting(const PortApplicableSetting&) = default;
PortApplicableSetting::PortApplicableSetting(PortApplicableSetting&&) = default;
PortApplicableSetting& PortApplicableSetting::operator=(const PortApplicableSetting&) = default;
PortApplicableSetting& PortApplicableSetting::operator=(PortApplicableSetting&&) = default;

bool PortApplicableSetting::is_port_affected(StringView port_name) const noexcept
{
return affected_ports.empty() || std::binary_search(affected_ports.begin(), affected_ports.end(), port_name);
}

VcpkgCmdArguments VcpkgCmdArguments::create_from_command_line(const ILineReader& fs,
const int argc,
const CommandLineCharType* const* const argv)
Expand Down Expand Up @@ -290,6 +316,18 @@ namespace vcpkg
StabilityTag::Experimental,
args.asset_sources_template_arg,
msg::format(msgAssetSourcesArg));
{
std::string raw_cmake_debug;
if (args.parser.parse_option(CMAKE_DEBUGGING_ARG, StabilityTag::Experimental, raw_cmake_debug))
Copy link
Contributor Author

@Neumann-A Neumann-A Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if parse_option couldn't be taught to do this automatically if fulfills the concept of being StringConstructable.

Copy link
Member

Choose a reason for hiding this comment

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

It could but then parse_option would have to be a template... if it were used in more than 1 place I think that'd be worth it but at least yet I don't think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How is this:
https://learn.microsoft.com/en-us/vcpkg/commands/common-options#asset-sources
https://learn.microsoft.com/en-us/vcpkg/commands/common-options#binarysource
getting parsed?

Aren't those options using a kind of comma separation?

{
args.cmake_debug.emplace(raw_cmake_debug);
}

if (args.parser.parse_option(CMAKE_CONFIGURE_DEBUGGING_ARG, StabilityTag::Experimental, raw_cmake_debug))
{
args.cmake_configure_debug.emplace(raw_cmake_debug);
}
}

args.parser.parse_multi_option(
OVERLAY_PORTS_ARG,
Expand Down Expand Up @@ -804,5 +842,7 @@ namespace vcpkg
constexpr StringLiteral VcpkgCmdArguments::VERSIONS_FEATURE;

constexpr StringLiteral VcpkgCmdArguments::CMAKE_SCRIPT_ARG;
constexpr StringLiteral VcpkgCmdArguments::CMAKE_DEBUGGING_ARG;
constexpr StringLiteral VcpkgCmdArguments::CMAKE_CONFIGURE_DEBUGGING_ARG;
constexpr StringLiteral VcpkgCmdArguments::EXACT_ABI_TOOLS_VERSIONS_SWITCH;
}