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.

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

Fix github issue body size #1257

Merged
merged 8 commits into from
Nov 7, 2023
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
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 @@ -1488,55 +1488,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 @@ -1548,43 +1574,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 @@ -1593,18 +1635,19 @@ namespace vcpkg
const Optional<Path>& issue_body)
{
const auto& spec_name = action.spec.name();
const auto& triplet_name = action.spec.triplet().to_string();
BillyONeal marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -1617,7 +1660,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 @@ -1235,7 +1238,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