Skip to content

Commit

Permalink
Ensure generated GitHub issue bodies are not too long to post (#1257)
Browse files Browse the repository at this point in the history
  • Loading branch information
autoantwort authored Nov 7, 2023
1 parent 0e69395 commit c9e4be1
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 58 deletions.
6 changes: 5 additions & 1 deletion include/vcpkg/commands.build.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,16 @@ namespace vcpkg
std::vector<std::string> error_logs;
};

void append_log(const Path& path, const std::string& log, size_t max_size, std::string& out);
void append_logs(std::vector<std::pair<Path, std::string>>&& logs, size_t max_size, std::string& out);

LocalizedString create_error_message(const ExtendedBuildResult& build_result, const PackageSpec& spec);

std::string create_github_issue(const VcpkgCmdArguments& args,
const ExtendedBuildResult& build_result,
const VcpkgPaths& paths,
const InstallPlanAction& action);
const InstallPlanAction& action,
bool include_manifest);

ExtendedBuildResult build_package(const VcpkgCmdArguments& args,
const VcpkgPaths& paths,
Expand Down
3 changes: 2 additions & 1 deletion include/vcpkg/commands.install.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ namespace vcpkg
const VcpkgPaths& paths,
StatusParagraphs& status_db,
BinaryCache& binary_cache,
const IBuildLogsRecorder& build_logs_recorder);
const IBuildLogsRecorder& build_logs_recorder,
bool include_manifest_in_github_issue = false);

void command_install_and_exit(const VcpkgCmdArguments& args,
const VcpkgPaths& paths,
Expand Down
3 changes: 2 additions & 1 deletion include/vcpkg/commands.set-installed.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ namespace vcpkg
Triplet host_triplet,
const KeepGoing keep_going,
const bool only_downloads,
const PrintUsage print_cmake_usage);
const PrintUsage print_cmake_usage,
bool include_manifest_in_github_issue);
void command_set_installed_and_exit(const VcpkgCmdArguments& args,
const VcpkgPaths& paths,
Triplet default_triplet,
Expand Down
71 changes: 71 additions & 0 deletions src/vcpkg-test/issue_body.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#include <vcpkg-test/util.h>

#include <vcpkg/base/files.h>
#include <vcpkg/base/strings.h>

#include <vcpkg/commands.build.h>

namespace
{
const std::string file_content = R"(00 32 byte long line xxxxxxxxxx
01 32 byte long line xxxxxxxxxx
02 32 byte long line xxxxxxxxxx
03 32 byte long line xxxxxxxxxx
04 32 byte long line xxxxxxxxxx
05 32 byte long line xxxxxxxxxx
06 32 byte long line xxxxxxxxxx)";

const auto expected_body = R"(<details><summary>test 2</summary>
```
00 32 byte long line xxxxxxxxxx
...
Skipped 4 lines
...
05 32 byte long line xxxxxxxxxx
06 32 byte long line xxxxxxxxxx
```
</details>
)";

const auto block_prefix = "<details><summary>test</summary>\n\n```\n";
const auto block_postfix = "\n```\n</details>\n\n";
}

TEST_CASE ("Testing append_log", "[github-issue-body]")
{
using namespace vcpkg;
{
std::string out;
append_log("test", file_content, 100, out);
CHECK(out == ""); // Not enough space at all
out.clear();
append_log("test 2", file_content, file_content.size(), out);
CHECK(out == expected_body); // Not enough space
out.clear();
append_log("test", file_content, static_cast<int>(file_content.size() + 100), out);
CHECK(out == block_prefix + file_content + block_postfix); // Enough space
}
}

TEST_CASE ("Testing append_log extra_size", "[github-issue-body]")
{
using namespace vcpkg;
{
std::string out;
std::vector<std::pair<Path, std::string>> logs{
{"not_included_1", file_content}, {"test", file_content}, {"test 2", file_content}};
append_logs(std::move(logs), 500, out);
CHECK(out == block_prefix + file_content + block_postfix + expected_body);
}
{
using namespace std::string_literals;
std::string out;
std::vector<std::pair<Path, std::string>> logs{{"test", file_content}, {"test", "smal"}, {"test", "smal"}};
// Enough space to output all logs, but if we would output the largest log first, there would be not enough
// space for this log if every log get the size total_size/number_of_logs.
append_logs(std::move(logs), file_content.size() + 3 * 110, out);
CHECK(out == fmt::format("{0}smal{1}{0}smal{1}{0}{2}{1}", block_prefix, block_postfix, file_content));
}
}
141 changes: 93 additions & 48 deletions src/vcpkg/commands.build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1484,55 +1484,81 @@ namespace vcpkg
return res;
}

struct CreateLogDetails
void append_log(const Path& path, const std::string& log, size_t max_log_length, std::string& out)
{
const Filesystem& fs;
StringLiteral details_start = "<details><summary>{}</summary>\n\n```\n";
StringLiteral skipped_msg = "\n...\nSkipped {} lines\n...";
StringLiteral details_end = "\n```\n</details>\n\n";
const size_t context_size = path.native().size() + details_start.size() + details_end.size() +
skipped_msg.size() + 6 /* digits for skipped count */;
const size_t minimum_log_size = std::min(size_t{100}, log.size());
if (max_log_length < context_size + minimum_log_size)
{
return;
}
max_log_length -= context_size;
fmt::format_to(std::back_inserter(out), details_start.c_str(), path.native());

std::string operator()(const Path& path) const
const auto start_block_max_length = max_log_length * 1 / 3;
const auto end_block_max_length = max_log_length - start_block_max_length;
if (log.size() > max_log_length)
{
static constexpr auto MAX_LOG_LENGTH = 50'000;
static constexpr auto START_BLOCK_LENGTH = 3'000;
static constexpr auto START_BLOCK_MAX_LENGTH = 5'000;
static constexpr auto END_BLOCK_LENGTH = 43'000;
static constexpr auto END_BLOCK_MAX_LENGTH = 45'000;
auto log = fs.read_contents(path, VCPKG_LINE_INFO);
if (log.size() > MAX_LOG_LENGTH)
auto first_block_end = log.find_last_of('\n', start_block_max_length);
if (first_block_end == std::string::npos)
{
auto first_block_end = log.find_first_of('\n', START_BLOCK_LENGTH);
if (first_block_end == std::string::npos || first_block_end > START_BLOCK_MAX_LENGTH)
{
first_block_end = START_BLOCK_LENGTH;
}

auto last_block_end = log.find_last_of('\n', log.size() - END_BLOCK_LENGTH);
if (last_block_end == std::string::npos || last_block_end < log.size() - END_BLOCK_MAX_LENGTH)
{
last_block_end = log.size() - END_BLOCK_LENGTH;
}

auto first = log.begin() + first_block_end;
auto last = log.begin() + last_block_end;
auto skipped_lines = std::count(first, last, '\n');
log.replace(first, last, fmt::format("\n...\nSkipped {} lines\n...", skipped_lines));
first_block_end = start_block_max_length;
}

while (!log.empty() && log.back() == '\n')
auto last_block_start = log.find_first_of('\n', log.size() - end_block_max_length);
if (last_block_start == std::string::npos)
{
log.pop_back();
last_block_start = log.size() - end_block_max_length;
}

return fmt::format("<details><summary>{}</summary>\n\n```\n{}\n```\n</details>", path.native(), log);
auto first = log.begin() + first_block_end;
auto last = log.begin() + last_block_start;
auto skipped_lines = std::count(first, last, '\n');
out.append(log.begin(), first);
fmt::format_to(std::back_inserter(out), skipped_msg.c_str(), skipped_lines);
out.append(last, log.end());
}
else
{
out += log;
}
};

while (!out.empty() && out.back() == '\n')
{
out.pop_back();
}
Strings::append(out, details_end);
}

void append_logs(std::vector<std::pair<Path, std::string>>&& logs, size_t max_size, std::string& out)
{
Util::sort(logs, [](const auto& left, const auto& right) { return left.second.size() < right.second.size(); });
auto size_per_log = max_size / logs.size();
size_t maximum = out.size();
for (const auto& entry : logs)
{
maximum += size_per_log;
const auto available = maximum - out.size();
append_log(entry.first, entry.second, available, out);
}
}

std::string create_github_issue(const VcpkgCmdArguments& args,
const ExtendedBuildResult& build_result,
const VcpkgPaths& paths,
const InstallPlanAction& action)
const InstallPlanAction& action,
bool include_manifest)
{
constexpr size_t MAX_ISSUE_SIZE = 65536;
const auto& fs = paths.get_filesystem();
std::string result;
fmt::format_to(std::back_inserter(result),
std::string issue_body;
// The logs excerpts are as large as possible. So the issue body will often reach MAX_ISSUE_SIZE.
issue_body.reserve(MAX_ISSUE_SIZE);
fmt::format_to(std::back_inserter(issue_body),
"Package: {} -> {}\n\n**Host Environment**\n\n- Host: {}-{}\n",
action.displayname(),
action.source_control_file_and_location.value_or_exit(VCPKG_LINE_INFO).to_version(),
Expand All @@ -1544,43 +1570,59 @@ namespace vcpkg
if (const auto* compiler_info = abi_info->compiler_info.get())
{
fmt::format_to(
std::back_inserter(result), "- Compiler: {} {}\n", compiler_info->id, compiler_info->version);
std::back_inserter(issue_body), "- Compiler: {} {}\n", compiler_info->id, compiler_info->version);
}
}

fmt::format_to(std::back_inserter(result), "-{}\n", paths.get_toolver_diagnostics());
fmt::format_to(std::back_inserter(result),
fmt::format_to(std::back_inserter(issue_body), "-{}\n", paths.get_toolver_diagnostics());
fmt::format_to(std::back_inserter(issue_body),
"**To Reproduce**\n\n`vcpkg {} {}`\n",
args.get_command(),
Strings::join(" ", args.get_forwardable_arguments()));
fmt::format_to(std::back_inserter(result),
fmt::format_to(std::back_inserter(issue_body),
"**Failure logs**\n\n```\n{}\n```\n",
paths.get_filesystem().read_contents(build_result.stdoutlog.value_or_exit(VCPKG_LINE_INFO),
VCPKG_LINE_INFO));
result.append(Strings::join("\n", build_result.error_logs, CreateLogDetails{fs}));

std::string postfix;
const auto maybe_manifest = paths.get_manifest();
if (auto manifest = maybe_manifest.get())
{
fmt::format_to(
std::back_inserter(result),
"**Additional context**\n\n<details><summary>vcpkg.json</summary>\n\n```\n{}\n```\n</details>\n",
Json::stringify(manifest->manifest));
if (include_manifest || manifest->manifest.contains("builtin-baseline"))
{
fmt::format_to(
std::back_inserter(postfix),
"**Additional context**\n\n<details><summary>vcpkg.json</summary>\n\n```\n{}\n```\n</details>\n",
Json::stringify(manifest->manifest));
}
}

return result;
if (issue_body.size() + postfix.size() < MAX_ISSUE_SIZE)
{
size_t remaining_body_size = MAX_ISSUE_SIZE - issue_body.size() - postfix.size();
auto logs = Util::fmap(build_result.error_logs, [&](auto&& path) -> std::pair<Path, std::string> {
return {path, fs.read_contents(path, VCPKG_LINE_INFO)};
});
append_logs(std::move(logs), remaining_body_size, issue_body);
}

issue_body.append(postfix);

return issue_body;
}

static std::string make_gh_issue_search_url(const std::string& spec_name)
{
return "https://github.com/microsoft/vcpkg/issues?q=is%3Aissue+is%3Aopen+in%3Atitle+" + spec_name;
}

static std::string make_gh_issue_open_url(StringView spec_name, StringView path)
static std::string make_gh_issue_open_url(StringView spec_name, StringView triplet, StringView path)
{
return Strings::concat("https://github.com/microsoft/vcpkg/issues/new?title=[",
spec_name,
"]+Build+error&body=Copy+issue+body+from+",
"]+Build+error+on+",
triplet,
"&body=Copy+issue+body+from+",
Strings::percent_encode(path));
}

Expand All @@ -1589,18 +1631,19 @@ namespace vcpkg
const Optional<Path>& issue_body)
{
const auto& spec_name = action.spec.name();
const auto& triplet_name = action.spec.triplet().to_string();
LocalizedString result = msg::format(msgBuildTroubleshootingMessage1).append_raw('\n');
result.append_indent().append_raw(make_gh_issue_search_url(spec_name)).append_raw('\n');
result.append(msgBuildTroubleshootingMessage2).append_raw('\n');
if (issue_body.has_value())
{
const auto path = issue_body.get()->generic_u8string();
result.append_indent().append_raw(make_gh_issue_open_url(spec_name, path)).append_raw('\n');
result.append_indent().append_raw(make_gh_issue_open_url(spec_name, triplet_name, path)).append_raw('\n');
if (!paths.get_filesystem().find_from_PATH("gh").empty())
{
Command gh("gh");
gh.string_arg("issue").string_arg("create").string_arg("-R").string_arg("microsoft/vcpkg");
gh.string_arg("--title").string_arg(fmt::format("[{}] Build failure", spec_name));
gh.string_arg("--title").string_arg(fmt::format("[{}] Build failure on {}", spec_name, triplet_name));
gh.string_arg("--body-file").string_arg(path);

result.append(msgBuildTroubleshootingMessageGH).append_raw('\n');
Expand All @@ -1613,7 +1656,9 @@ namespace vcpkg
.append_raw("https://github.com/microsoft/vcpkg/issues/"
"new?template=report-package-build-failure.md&title=[")
.append_raw(spec_name)
.append_raw("]+Build+error\n");
.append_raw("]+Build+error+on+")
.append_raw(triplet_name)
.append_raw("\n");
result.append(msgBuildTroubleshootingMessage3, msg::package_name = spec_name).append_raw('\n');
result.append_raw(paths.get_toolver_diagnostics()).append_raw('\n');
}
Expand Down
10 changes: 7 additions & 3 deletions src/vcpkg/commands.install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,8 @@ namespace vcpkg
const VcpkgPaths& paths,
StatusParagraphs& status_db,
BinaryCache& binary_cache,
const IBuildLogsRecorder& build_logs_recorder)
const IBuildLogsRecorder& build_logs_recorder,
bool include_manifest_in_github_issue)
{
const ElapsedTimer timer;
std::vector<SpecSummary> results;
Expand Down Expand Up @@ -584,7 +585,9 @@ namespace vcpkg
print_user_troubleshooting_message(action, paths, result.stdoutlog.then([&](auto&) -> Optional<Path> {
auto issue_body_path = paths.installed().root() / "vcpkg" / "issue_body.md";
paths.get_filesystem().write_contents(
issue_body_path, create_github_issue(args, result, paths, action), VCPKG_LINE_INFO);
issue_body_path,
create_github_issue(args, result, paths, action, include_manifest_in_github_issue),
VCPKG_LINE_INFO);
return issue_body_path;
}));
Checks::exit_fail(VCPKG_LINE_INFO);
Expand Down Expand Up @@ -1236,7 +1239,8 @@ namespace vcpkg
host_triplet,
keep_going,
only_downloads,
print_cmake_usage);
print_cmake_usage,
true);
}

auto registry_set = paths.make_registry_set();
Expand Down
16 changes: 12 additions & 4 deletions src/vcpkg/commands.set-installed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ namespace vcpkg
Triplet host_triplet,
const KeepGoing keep_going,
const bool only_downloads,
const PrintUsage print_cmake_usage)
const PrintUsage print_cmake_usage,
bool include_manifest_in_github_issue)
{
auto& fs = paths.get_filesystem();

Expand Down Expand Up @@ -243,8 +244,14 @@ namespace vcpkg
auto binary_cache = only_downloads ? BinaryCache(paths.get_filesystem())
: BinaryCache::make(args, paths, stdout_sink).value_or_exit(VCPKG_LINE_INFO);
binary_cache.fetch(action_plan.install_actions);
const auto summary = install_execute_plan(
args, action_plan, keep_going, paths, status_db, binary_cache, null_build_logs_recorder());
const auto summary = install_execute_plan(args,
action_plan,
keep_going,
paths,
status_db,
binary_cache,
null_build_logs_recorder(),
include_manifest_in_github_issue);

if (keep_going == KeepGoing::YES && summary.failed())
{
Expand Down Expand Up @@ -340,6 +347,7 @@ namespace vcpkg
host_triplet,
keep_going,
only_downloads,
print_cmake_usage);
print_cmake_usage,
false);
}
} // namespace vcpkg

0 comments on commit c9e4be1

Please sign in to comment.