From 52ff77f12a85fafb7b12fc74ae1add2e05bae196 Mon Sep 17 00:00:00 2001 From: Thomas1664 <46387399+Thomas1664@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:30:25 +0000 Subject: [PATCH 1/5] Move git out of VcpkgPaths --- include/vcpkg/base/git.h | 7 ++ include/vcpkg/vcpkgpaths.h | 5 +- src/vcpkg/base/git.cpp | 67 ++++++++++++++-- src/vcpkg/commands.portsdiff.cpp | 3 +- src/vcpkg/registries.cpp | 8 +- src/vcpkg/vcpkgpaths.cpp | 134 +++++-------------------------- 6 files changed, 94 insertions(+), 130 deletions(-) diff --git a/include/vcpkg/base/git.h b/include/vcpkg/base/git.h index a3a3ba0034..d21fd43f14 100644 --- a/include/vcpkg/base/git.h +++ b/include/vcpkg/base/git.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -42,6 +43,8 @@ namespace vcpkg std::string old_path; }; + Command git_cmd_builder(const GitConfig& config); + // Try to extract a port name from a path. // The path should start with the "ports/" prefix std::string try_extract_port_name_from_path(StringView path); @@ -58,4 +61,8 @@ namespace vcpkg // Check whether a repository is a shallow clone ExpectedL is_shallow_clone(const GitConfig& config); + + ExpectedL git_head_sha(const GitConfig& config, StringView refname = "HEAD"); + // runs `git fetch {uri} {treeish}` + ExpectedL git_fetch(const Filesystem& fs, const GitConfig& config, StringView repo, StringView treeish); } diff --git a/include/vcpkg/vcpkgpaths.h b/include/vcpkg/vcpkgpaths.h index 90fd4d093c..9b26f141fa 100644 --- a/include/vcpkg/vcpkgpaths.h +++ b/include/vcpkg/vcpkgpaths.h @@ -112,7 +112,7 @@ namespace vcpkg const std::string& get_tool_version(StringView tool, MessageSink& status_messages) const; GitConfig git_builtin_config() const; - Command git_cmd_builder(const Path& dot_git_dir, const Path& work_tree) const; + Command git_cmd_builder(Path dot_git_dir, Path work_tree) const; // Git manipulation in the vcpkg directory ExpectedL get_current_git_sha() const; @@ -125,9 +125,8 @@ namespace vcpkg // Git manipulation for remote registries // runs `git fetch {uri} {treeish}`, and returns the hash of FETCH_HEAD. // Use {treeish} of "HEAD" for the default branch + GitConfig git_registries_config() const; ExpectedL git_fetch_from_remote_registry(StringView uri, StringView treeish) const; - // runs `git fetch {uri} {treeish}` - ExpectedL git_fetch(StringView uri, StringView treeish) const; ExpectedL git_show_from_remote_registry(StringView hash, const Path& relative_path_to_file) const; ExpectedL git_find_object_id_for_remote_registry_path(StringView hash, const Path& relative_path_to_file) const; diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index 9b9ccffb6d..1647d28381 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -1,4 +1,7 @@ +#include + #include +#include #include #include #include @@ -8,10 +11,8 @@ #include -namespace +namespace vcpkg { - using namespace vcpkg; - Command git_cmd_builder(const GitConfig& config) { auto cmd = Command(config.git_exe); @@ -26,14 +27,11 @@ namespace } return cmd; } -} -namespace vcpkg -{ std::string try_extract_port_name_from_path(StringView path) { static constexpr StringLiteral prefix = "ports/"; - static constexpr size_t min_path_size = sizeof("ports/*/") - 1; + static constexpr size_t min_path_size = prefix.size(); if (path.size() >= min_path_size && Strings::starts_with(path, prefix)) { auto no_prefix = path.substr(prefix.size()); @@ -199,4 +197,59 @@ namespace vcpkg Tools::GIT) .map([](std::string&& output) { return "true" == Strings::trim(std::move(output)); }); } + + ExpectedL git_head_sha(const GitConfig& config, StringView refname) + { + const auto cmd_line = git_cmd_builder(config).string_arg("rev-parse").string_arg(refname); + return flatten_out(cmd_execute_and_capture_output(cmd_line), Tools::GIT) + .map([](std::string&& output) { + Strings::inplace_trim(output); + return std::move(output); + }) + .map_error([&](LocalizedString&& err) { + return msg::format(msgGitCommandFailed, msg::command_line = cmd_line.command_line()) + .append_raw('\n') + .append(std::move(err)); + }); + } + + ExpectedL git_fetch(const Filesystem& fs, const GitConfig& config, StringView repo, StringView treeish) + { + const auto& work_tree = config.git_work_tree; + fs.create_directories(work_tree, VCPKG_LINE_INFO); + + auto lock_file = work_tree / ".vcpkg-lock"; + + auto guard = fs.take_exclusive_file_lock(lock_file, stderr_sink, IgnoreErrors{}); + + auto init_registries_git_dir = git_cmd_builder(config).string_arg("init"); + auto maybe_init_output = flatten(cmd_execute_and_capture_output(init_registries_git_dir), Tools::GIT); + if (!maybe_init_output) + { + return msg::format(msgGitFailedToInitializeLocalRepository, msg::path = work_tree) + .append_raw('\n') + .append(msgGitCommandFailed, msg::command_line = init_registries_git_dir.command_line()) + .append_raw('\n') + .append(std::move(maybe_init_output).error()); + } + + auto fetch_git_ref = git_cmd_builder(config) + .string_arg("fetch") + .string_arg("--update-shallow") + .string_arg("--") + .string_arg(repo) + .string_arg(treeish); + + auto maybe_fetch_output = flatten(cmd_execute_and_capture_output(fetch_git_ref), Tools::GIT); + if (!maybe_fetch_output) + { + return msg::format(msgGitFailedToFetch, msg::value = treeish, msg::url = repo) + .append_raw('\n') + .append(msgGitCommandFailed, msg::command_line = fetch_git_ref.command_line()) + .append_raw('\n') + .append(std::move(maybe_fetch_output).error()); + } + + return {Unit{}}; + } } diff --git a/src/vcpkg/commands.portsdiff.cpp b/src/vcpkg/commands.portsdiff.cpp index def776f9dd..654971ba65 100644 --- a/src/vcpkg/commands.portsdiff.cpp +++ b/src/vcpkg/commands.portsdiff.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -73,7 +74,7 @@ namespace { static constexpr StringLiteral VALID_COMMIT_OUTPUT = "commit\n"; Checks::msg_check_exit(VCPKG_LINE_INFO, - cmd_execute_and_capture_output(paths.git_cmd_builder(paths.root / ".git", paths.root) + cmd_execute_and_capture_output(git_cmd_builder(paths.git_builtin_config()) .string_arg("cat-file") .string_arg("-t") .string_arg(git_commit_id)) diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp index 2ab0267639..79db6225ab 100644 --- a/src/vcpkg/registries.cpp +++ b/src/vcpkg/registries.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -1012,13 +1013,12 @@ namespace if (!maybe_contents) { msg::println(msgFetchingBaselineInfo, msg::package_name = m_repo); - auto maybe_err = m_paths.git_fetch(m_repo, m_baseline_identifier); + auto maybe_err = + git_fetch(m_paths.get_filesystem(), m_paths.git_registries_config(), m_repo, m_baseline_identifier); if (!maybe_err) { get_global_metrics_collector().track_define(DefineMetric::RegistriesErrorCouldNotFindBaseline); - return msg::format_error(msgFailedToFetchRepo, msg::url = m_repo) - .append_raw('\n') - .append(maybe_err.error()); + return msg::format_error(maybe_err.error()); } maybe_contents = m_paths.git_show_from_remote_registry(m_baseline_identifier, path_to_baseline); diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index 7d48e87932..1c5b1aa153 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -888,19 +888,10 @@ namespace vcpkg return conf; } - Command VcpkgPaths::git_cmd_builder(const Path& dot_git_dir, const Path& work_tree) const + Command VcpkgPaths::git_cmd_builder(Path dot_git_dir, Path work_tree) const { - Command ret(get_tool_exe(Tools::GIT, out_sink)); - if (!dot_git_dir.empty()) - { - ret.string_arg(Strings::concat("--git-dir=", dot_git_dir)); - } - if (!work_tree.empty()) - { - ret.string_arg(Strings::concat("--work-tree=", work_tree)); - } - ret.string_arg("-c").string_arg("core.autocrlf=false"); - return ret; + const GitConfig config{get_tool_exe(Tools::GIT, out_sink), std::move(dot_git_dir), std::move(work_tree)}; + return vcpkg::git_cmd_builder(config); } ExpectedL VcpkgPaths::get_current_git_sha() const @@ -910,19 +901,12 @@ namespace vcpkg return {*sha, expected_left_tag}; } - return flatten_out( - cmd_execute_and_capture_output( - git_cmd_builder(this->root / ".git", this->root).string_arg("rev-parse").string_arg("HEAD")), - Tools::GIT) - .map([](std::string&& output) { - Strings::inplace_trim(output); - return std::move(output); - }); + return git_head_sha(git_builtin_config()); } LocalizedString VcpkgPaths::get_current_git_sha_baseline_message() const { - const auto& git_config = git_builtin_config(); + const auto git_config = git_builtin_config(); if (is_shallow_clone(git_config).value_or(false)) { return msg::format(msgShallowRepositoryDetected, msg::path = git_config.git_dir); @@ -1027,95 +1011,25 @@ namespace vcpkg .append(msgWhileGettingLocalTreeIshObjectsForPorts); } - ExpectedL VcpkgPaths::git_fetch_from_remote_registry(StringView repo, StringView treeish) const + GitConfig VcpkgPaths::git_registries_config() const { - auto& fs = get_filesystem(); - - const auto& work_tree = m_pimpl->m_registries_work_tree_dir; - fs.create_directories(work_tree, VCPKG_LINE_INFO); - const auto& dot_git_dir = m_pimpl->m_registries_dot_git_dir; - - auto init_cmd = git_cmd_builder(dot_git_dir, work_tree).string_arg("init"); - auto maybe_init_output = flatten(cmd_execute_and_capture_output(init_cmd), Tools::GIT); - if (!maybe_init_output) - { - return msg::format_error(msgGitCommandFailed, msg::command_line = init_cmd.command_line()) - .append_raw('\n') - .append(maybe_init_output.error()); - } - - auto lock_file = work_tree / ".vcpkg-lock"; - - auto guard = fs.take_exclusive_file_lock(lock_file, stderr_sink, IgnoreErrors{}); - auto fetch_git_ref = git_cmd_builder(dot_git_dir, work_tree) - .string_arg("fetch") - .string_arg("--update-shallow") - .string_arg("--") - .string_arg(repo) - .string_arg(treeish); - - auto maybe_fetch_output = flatten(cmd_execute_and_capture_output(fetch_git_ref), Tools::GIT); - if (!maybe_fetch_output) - { - return msg::format_error(msgGitFailedToFetch, msg::value = treeish, msg::url = repo) - .append_raw('\n') - .append(msgGitCommandFailed, msg::command_line = fetch_git_ref.command_line()) - .append_raw('\n') - .append(std::move(maybe_fetch_output).error()); - } - - auto get_fetch_head = git_cmd_builder(dot_git_dir, work_tree).string_arg("rev-parse").string_arg("FETCH_HEAD"); - return flatten_out(cmd_execute_and_capture_output(get_fetch_head), Tools::GIT) - .map([](std::string&& output) { return Strings::trim(output).to_string(); }) - .map_error([&](LocalizedString&& err) { - return msg::format_error(msgGitCommandFailed, msg::command_line = get_fetch_head.command_line()) - .append_raw('\n') - .append(std::move(err)); - }); + GitConfig conf; + conf.git_exe = get_tool_exe(Tools::GIT, out_sink); + conf.git_dir = m_pimpl->m_registries_dot_git_dir; + conf.git_work_tree = m_pimpl->m_registries_work_tree_dir; + return conf; } - ExpectedL VcpkgPaths::git_fetch(StringView repo, StringView treeish) const + ExpectedL VcpkgPaths::git_fetch_from_remote_registry(StringView repo, StringView treeish) const { - auto& fs = get_filesystem(); - - const auto& work_tree = m_pimpl->m_registries_work_tree_dir; - fs.create_directories(work_tree, VCPKG_LINE_INFO); - - auto lock_file = work_tree / ".vcpkg-lock"; + auto git_config = git_registries_config(); + auto maybe_fetch_result = vcpkg::git_fetch(get_filesystem(), git_config, repo, treeish); - auto guard = fs.take_exclusive_file_lock(lock_file, stderr_sink, IgnoreErrors{}); - - const auto& dot_git_dir = m_pimpl->m_registries_dot_git_dir; - - auto init_registries_git_dir = git_cmd_builder(dot_git_dir, work_tree).string_arg("init"); - auto maybe_init_output = flatten(cmd_execute_and_capture_output(init_registries_git_dir), Tools::GIT); - if (!maybe_init_output) - { - return msg::format_error(msgGitFailedToInitializeLocalRepository, msg::path = work_tree) - .append_raw('\n') - .append(msgGitCommandFailed, msg::command_line = init_registries_git_dir.command_line()) - .append_raw('\n') - .append(std::move(maybe_init_output).error()); - } - - auto fetch_git_ref = git_cmd_builder(dot_git_dir, work_tree) - .string_arg("fetch") - .string_arg("--update-shallow") - .string_arg("--") - .string_arg(repo) - .string_arg(treeish); - - auto maybe_fetch_output = flatten(cmd_execute_and_capture_output(fetch_git_ref), Tools::GIT); - if (!maybe_fetch_output) + if (!maybe_fetch_result) { - return msg::format_error(msgGitFailedToFetch, msg::value = treeish, msg::url = repo) - .append_raw('\n') - .append(msgGitCommandFailed, msg::command_line = fetch_git_ref.command_line()) - .append_raw('\n') - .append(std::move(maybe_fetch_output).error()); + return std::move(maybe_fetch_result).error(); } - - return {Unit{}}; + return git_head_sha(git_config, "FETCH_HEAD"); } // returns an error if there was an unexpected error; returns nullopt if the file doesn't exist at the specified @@ -1124,24 +1038,14 @@ namespace vcpkg { auto revision = fmt::format("{}:{}", hash, relative_path.generic_u8string()); return flatten_out(cmd_execute_and_capture_output( - git_cmd_builder(m_pimpl->m_registries_dot_git_dir, m_pimpl->m_registries_work_tree_dir) - .string_arg("show") - .string_arg(revision)), + vcpkg::git_cmd_builder(git_registries_config()).string_arg("show").string_arg(revision)), Tools::GIT); } ExpectedL VcpkgPaths::git_find_object_id_for_remote_registry_path(StringView hash, const Path& relative_path) const { auto revision = fmt::format("{}:{}", hash, relative_path.generic_u8string()); - return flatten_out(cmd_execute_and_capture_output( - git_cmd_builder(m_pimpl->m_registries_dot_git_dir, m_pimpl->m_registries_work_tree_dir) - .string_arg("rev-parse") - .string_arg(revision)), - Tools::GIT) - .map([](std::string&& output) { - Strings::inplace_trim(output); - return std::move(output); - }); + return git_head_sha(git_registries_config(), revision); } ExpectedL VcpkgPaths::git_read_tree(const Path& destination, StringView tree, const Path& dot_git_dir) const From 702a14886d02414782e5aa9cb8634921be83cca9 Mon Sep 17 00:00:00 2001 From: Thomas1664 <46387399+Thomas1664@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:47:15 +0000 Subject: [PATCH 2/5] fix error format --- src/vcpkg/vcpkgpaths.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index 1c5b1aa153..dd65dc71fb 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -841,8 +841,8 @@ namespace vcpkg if (m_pimpl->m_bundle.read_only) { Strings::append(ret, " vcpkg-readonly: true\n"); - const auto sha = get_current_git_sha(); - Strings::append(ret, " vcpkg-scripts version: ", sha ? StringView(*sha.get()) : "unknown", "\n"); + const auto sha = get_current_git_sha().value_or("unknown"); + Strings::append(ret, " vcpkg-scripts version: ", StringView{sha}, "\n"); } else { @@ -1027,7 +1027,7 @@ namespace vcpkg if (!maybe_fetch_result) { - return std::move(maybe_fetch_result).error(); + return msg::format_error(maybe_fetch_result.error()); } return git_head_sha(git_config, "FETCH_HEAD"); } From 984109dfcda37acefbc29fd4f23daf09b3637b40 Mon Sep 17 00:00:00 2001 From: Thomas1664 <46387399+Thomas1664@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:50:36 +0000 Subject: [PATCH 3/5] remove redundant #include --- src/vcpkg/base/git.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index 1647d28381..6fb75cca21 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -1,13 +1,10 @@ #include -#include #include #include #include #include #include -#include -#include #include From 2d38f5ac6a4c0f80e540d999acbde7a281f064f7 Mon Sep 17 00:00:00 2001 From: Thomas1664 <46387399+Thomas1664@users.noreply.github.com> Date: Mon, 8 Apr 2024 07:55:53 +0000 Subject: [PATCH 4/5] restore min_path_size --- src/vcpkg/base/git.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index 6fb75cca21..bb90e8a701 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -28,7 +28,7 @@ namespace vcpkg std::string try_extract_port_name_from_path(StringView path) { static constexpr StringLiteral prefix = "ports/"; - static constexpr size_t min_path_size = prefix.size(); + static constexpr size_t min_path_size = sizeof("ports/*/") - 1; if (path.size() >= min_path_size && Strings::starts_with(path, prefix)) { auto no_prefix = path.substr(prefix.size()); From 7414ff60d6604bd21aad4b4f8e05421ae10e5d3b Mon Sep 17 00:00:00 2001 From: Thomas1664 <46387399+Thomas1664@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:38:50 +0200 Subject: [PATCH 5/5] partially address cr comments --- include/vcpkg/base/git.h | 3 ++- include/vcpkg/vcpkgpaths.h | 2 +- src/vcpkg/base/git.cpp | 2 +- src/vcpkg/vcpkgpaths.cpp | 10 +++++----- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/vcpkg/base/git.h b/include/vcpkg/base/git.h index d21fd43f14..824bca94ca 100644 --- a/include/vcpkg/base/git.h +++ b/include/vcpkg/base/git.h @@ -62,7 +62,8 @@ namespace vcpkg // Check whether a repository is a shallow clone ExpectedL is_shallow_clone(const GitConfig& config); - ExpectedL git_head_sha(const GitConfig& config, StringView refname = "HEAD"); + // runs git ref-parse for a given refname, e.g. HEAD + ExpectedL git_ref_sha(const GitConfig& config, StringView refname = "HEAD"); // runs `git fetch {uri} {treeish}` ExpectedL git_fetch(const Filesystem& fs, const GitConfig& config, StringView repo, StringView treeish); } diff --git a/include/vcpkg/vcpkgpaths.h b/include/vcpkg/vcpkgpaths.h index 9b26f141fa..6f7ae88855 100644 --- a/include/vcpkg/vcpkgpaths.h +++ b/include/vcpkg/vcpkgpaths.h @@ -112,7 +112,7 @@ namespace vcpkg const std::string& get_tool_version(StringView tool, MessageSink& status_messages) const; GitConfig git_builtin_config() const; - Command git_cmd_builder(Path dot_git_dir, Path work_tree) const; + Command git_cmd_builder(const Path& dot_git_dir, const Path& work_tree) const; // Git manipulation in the vcpkg directory ExpectedL get_current_git_sha() const; diff --git a/src/vcpkg/base/git.cpp b/src/vcpkg/base/git.cpp index bb90e8a701..e562d85b2f 100644 --- a/src/vcpkg/base/git.cpp +++ b/src/vcpkg/base/git.cpp @@ -195,7 +195,7 @@ namespace vcpkg .map([](std::string&& output) { return "true" == Strings::trim(std::move(output)); }); } - ExpectedL git_head_sha(const GitConfig& config, StringView refname) + ExpectedL git_ref_sha(const GitConfig& config, StringView refname) { const auto cmd_line = git_cmd_builder(config).string_arg("rev-parse").string_arg(refname); return flatten_out(cmd_execute_and_capture_output(cmd_line), Tools::GIT) diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index dd65dc71fb..d4eca49aa0 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -888,9 +888,9 @@ namespace vcpkg return conf; } - Command VcpkgPaths::git_cmd_builder(Path dot_git_dir, Path work_tree) const + Command VcpkgPaths::git_cmd_builder(const Path& dot_git_dir, const Path& work_tree) const { - const GitConfig config{get_tool_exe(Tools::GIT, out_sink), std::move(dot_git_dir), std::move(work_tree)}; + const GitConfig config{get_tool_exe(Tools::GIT, out_sink), dot_git_dir, work_tree}; return vcpkg::git_cmd_builder(config); } @@ -901,7 +901,7 @@ namespace vcpkg return {*sha, expected_left_tag}; } - return git_head_sha(git_builtin_config()); + return git_ref_sha(git_builtin_config()); } LocalizedString VcpkgPaths::get_current_git_sha_baseline_message() const @@ -1029,7 +1029,7 @@ namespace vcpkg { return msg::format_error(maybe_fetch_result.error()); } - return git_head_sha(git_config, "FETCH_HEAD"); + return git_ref_sha(git_config, "FETCH_HEAD"); } // returns an error if there was an unexpected error; returns nullopt if the file doesn't exist at the specified @@ -1045,7 +1045,7 @@ namespace vcpkg const Path& relative_path) const { auto revision = fmt::format("{}:{}", hash, relative_path.generic_u8string()); - return git_head_sha(git_registries_config(), revision); + return git_ref_sha(git_registries_config(), revision); } ExpectedL VcpkgPaths::git_read_tree(const Path& destination, StringView tree, const Path& dot_git_dir) const