Skip to content

Fix recent git commands to work back on 2.7.4. #1638

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

Merged
merged 1 commit into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/base/files.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ namespace vcpkg

virtual Path absolute(const Path& target, std::error_code& ec) const = 0;
Path absolute(const Path& target, LineInfo li) const;
Optional<Path> absolute(DiagnosticContext& context, const Path& target) const;

virtual std::vector<Path> find_from_PATH(View<StringView> stems) const = 0;
std::vector<Path> find_from_PATH(StringView stem) const;
Expand Down
10 changes: 9 additions & 1 deletion include/vcpkg/base/git.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@ namespace vcpkg

Optional<std::string> git_prefix(DiagnosticContext& context, const Path& git_exe, const Path& target);

Optional<Path> git_index_file(DiagnosticContext& context, const Path& git_exe, GitRepoLocator locator);
Optional<Path> git_index_file(DiagnosticContext& context,
const Filesystem& fs,
const Path& git_exe,
GitRepoLocator locator);

Optional<Path> git_absolute_git_dir(DiagnosticContext& context,
const Filesystem& fs,
const Path& git_exe,
GitRepoLocator locator);

bool git_add_with_index(DiagnosticContext& context,
const Path& git_exe,
Expand Down
29 changes: 21 additions & 8 deletions src/vcpkg/base/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1893,6 +1893,19 @@ namespace vcpkg
return result;
}

Optional<Path> ReadOnlyFilesystem::absolute(DiagnosticContext& context, const Path& target) const
{
std::error_code ec;
Optional<Path> result{this->absolute(target, ec)};
if (ec)
{
context.report_error(format_filesystem_call_error(ec, __func__, {target}));
result.clear();
}

return result;
}

std::vector<Path> ReadOnlyFilesystem::find_from_PATH(StringView stem) const
{
return this->find_from_PATH(View<StringView>{&stem, 1});
Expand Down Expand Up @@ -2052,11 +2065,11 @@ namespace vcpkg
const Path& new_path) const
{
std::error_code ec;
bool result = this->rename_or_delete(old_path, new_path, ec);
Optional<bool> result{this->rename_or_delete(old_path, new_path, ec)};
if (ec)
{
context.report_error(format_filesystem_call_error(ec, __func__, {old_path, new_path}));
return nullopt;
result.clear();
}

return result;
Expand Down Expand Up @@ -2089,11 +2102,11 @@ namespace vcpkg
Optional<bool> Filesystem::create_directory(DiagnosticContext& context, const Path& new_directory) const
{
std::error_code ec;
bool result = this->create_directory(new_directory, ec);
Optional<bool> result{this->create_directory(new_directory, ec)};
if (ec)
{
context.report_error(format_filesystem_call_error(ec, __func__, {new_directory}));
return nullopt;
result.clear();
}

return result;
Expand All @@ -2114,11 +2127,11 @@ namespace vcpkg
Optional<bool> Filesystem::create_directories(DiagnosticContext& context, const Path& new_directory) const
{
std::error_code ec;
bool result = this->create_directories(new_directory, ec);
Optional<bool> result{this->create_directories(new_directory, ec)};
if (ec)
{
context.report_error(format_filesystem_call_error(ec, __func__, {new_directory}));
return nullopt;
result.clear();
}

return result;
Expand Down Expand Up @@ -2213,11 +2226,11 @@ namespace vcpkg
CopyOptions options) const
{
std::error_code ec;
const bool result = this->copy_file(source, destination, options, ec);
Optional<bool> result{this->copy_file(source, destination, options, ec)};
if (ec)
{
context.report_error(format_filesystem_call_error(ec, __func__, {source, destination}));
return nullopt;
result.clear();
}

return result;
Expand Down
61 changes: 53 additions & 8 deletions src/vcpkg/base/git.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@

#include <algorithm>

// When making changes to this file, check that the git command lines intended do what is expected on
// vcpkg's current minimum supported git version (2.7.4). You can get a version of git that old with docker:
//
// docker run -it --rm ubuntu:16.04
// apt update
// apt install git
// git --version # check that this is 2.7.4

namespace
{
using namespace vcpkg;
Expand Down Expand Up @@ -114,6 +122,8 @@ namespace vcpkg

Optional<bool> is_shallow_clone(DiagnosticContext& context, const Path& git_exe, GitRepoLocator locator)
{
// --is-shallow-repository is not present in git 2.7.4, but in that case git just prints
// "--is-shallow-repository", which is not "true", so we safely report "false"
static constexpr StringView args[] = {StringLiteral{"rev-parse"}, StringLiteral{"--is-shallow-repository"}};
return run_cmd_trim(context, make_git_command(git_exe, locator, args)).map([](std::string&& output) {
return output == "true";
Expand All @@ -127,14 +137,48 @@ namespace vcpkg
context, make_git_command(git_exe, GitRepoLocator{GitRepoLocatorKind::CurrentDirectory, target}, args));
}

Optional<Path> git_index_file(DiagnosticContext& context, const Path& git_exe, GitRepoLocator locator)
Optional<Path> git_index_file(DiagnosticContext& context,
const Filesystem& fs,
const Path& git_exe,
GitRepoLocator locator)
{
// We can't use --path-format unconditionally as that is unavailable in git 2.7.4.
// However, the path format always appears to be absolute if we use --git-dir with an absolute path
static constexpr StringView args[] = {
StringLiteral{"rev-parse"}, StringLiteral{"--git-path"}, StringLiteral{"index"}};
return git_absolute_git_dir(context, fs, git_exe, locator).then([&](Path&& absolute_git_dir) {
auto git_path_index_cmd =
make_git_command(git_exe, GitRepoLocator{GitRepoLocatorKind::DotGitDir, absolute_git_dir}, args);
return run_cmd_trim(context, git_path_index_cmd).then([&](std::string&& proto_path) -> Optional<Path> {
Optional<Path> result;
auto& result_path = result.emplace(std::move(proto_path));
if (!result_path.is_absolute() || !fs.exists(result_path, IgnoreErrors{}))
{
context.report_error_with_log(result_path.native(),
msgGitUnexpectedCommandOutputCmd,
msg::command_line = git_path_index_cmd.command_line());
result.clear();
}

return result;
});
});
}

Optional<Path> git_absolute_git_dir(DiagnosticContext& context,
const Filesystem& fs,
const Path& git_exe,
GitRepoLocator locator)
{
static constexpr StringView args[] = {StringLiteral{"rev-parse"},
StringLiteral{"--path-format=absolute"},
StringLiteral{"--git-path"},
StringLiteral{"index"}};
return run_cmd_trim(context, make_git_command(git_exe, locator, args))
.map([](std::string&& proto_path) -> Path { return std::move(proto_path); });
static constexpr StringView args[] = {StringLiteral{"rev-parse"}, StringLiteral{"--git-dir"}};
switch (locator.kind)
{
case GitRepoLocatorKind::CurrentDirectory:
return run_cmd_trim(context, make_git_command(git_exe, locator, args))
.then([&](std::string&& proto_path) { return fs.absolute(context, locator.path / proto_path); });
case GitRepoLocatorKind::DotGitDir: return fs.absolute(context, locator.path);
default: Checks::unreachable(VCPKG_LINE_INFO);
}
}

bool git_add_with_index(DiagnosticContext& context, const Path& git_exe, const Path& target, const Path& index_file)
Expand Down Expand Up @@ -197,6 +241,7 @@ namespace vcpkg
RedirectedProcessLaunchSettings launch_settings;
launch_settings.encoding = Encoding::Utf8WithNulls;
Optional<std::vector<GitLSTreeEntry>> result;

StringView args[] = {StringLiteral{"ls-tree"}, treeish, StringLiteral{"--full-tree"}, StringLiteral{"-z"}};
auto cmd = make_git_command(git_exe, locator, args);
auto maybe_ls_tree_result = run_cmd_trim(context, cmd, launch_settings);
Expand Down Expand Up @@ -242,12 +287,12 @@ namespace vcpkg
auto read_tree_cmd = make_git_command(git_exe, locator, read_tree_args);
if (run_cmd_git_with_index(context, read_tree_cmd, git_tree_index).has_value())
{
// No --ignore-skip-worktree-bits because that was added in recent-ish git versions
auto prefix_arg = fmt::format("--prefix={}/", git_tree_temp);
StringView checkout_index_args[] = {StringLiteral{"--work-tree"},
git_tree_temp,
StringLiteral{"checkout-index"},
StringLiteral{"-af"},
StringLiteral{"--ignore-skip-worktree-bits"},
prefix_arg};
auto checkout_index_cmd = make_git_command(git_exe, locator, checkout_index_args);
bool succeeded = run_cmd_git_with_index(context, checkout_index_cmd, git_tree_index).has_value();
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.test-features.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ namespace
git_prefix(console_diagnostic_context, git_exe, builtin_ports).value_or_quiet_exit(VCPKG_LINE_INFO);
const auto locator = GitRepoLocator{GitRepoLocatorKind::CurrentDirectory, builtin_ports};
auto index_file =
git_index_file(console_diagnostic_context, git_exe, locator).value_or_quiet_exit(VCPKG_LINE_INFO);
git_index_file(console_diagnostic_context, fs, git_exe, locator).value_or_quiet_exit(VCPKG_LINE_INFO);
TempFileDeleter temp_index_file{fs, fmt::format("{}_vcpkg_{}.tmp", index_file.native(), get_process_id())};
if (!fs.copy_file(
console_diagnostic_context, index_file, temp_index_file.path, CopyOptions::overwrite_existing) ||
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/vcpkgpaths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ namespace vcpkg
if (auto prefix = maybe_prefix.get())
{
const auto locator = GitRepoLocator{GitRepoLocatorKind::CurrentDirectory, builtin_ports};
const auto maybe_index_file = git_index_file(context, git_exe, locator);
const auto maybe_index_file = git_index_file(context, fs, git_exe, locator);
if (const auto index_file = maybe_index_file.get())
{
TempFileDeleter temp_index_file{fs,
Expand Down