Skip to content

Commit 81c8bed

Browse files
BillyONealras0219-msftklalumiere
authored
Infrastructure: allow passing stdin to subprocesses. (#1134)
Also uses the new infrastructure to avoid needing a temporary file in: [dependencygraph] Fix "command line too long" when sending to GitHub's dependency-graph API #1144 from @klalumiere . Co-authored-by: Robert Schumacher <[email protected]> Co-authored-by: Kevin Lalumiere <[email protected]>
1 parent 5679873 commit 81c8bed

File tree

13 files changed

+1138
-417
lines changed

13 files changed

+1138
-417
lines changed

‎CMakeLists.txt‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,10 @@ if(DEFINE_DISABLE_METRICS OR VCPKG_DISABLE_METRICS)
3434
"file vcpkg.disable_metrics next to the binary.")
3535
endif()
3636

37-
set(LANGUAGES "CXX")
38-
if(VCPKG_BUILD_TLS12_DOWNLOADER)
39-
list(APPEND LANGUAGES "C")
40-
endif()
41-
4237
project(vcpkg
4338
DESCRIPTION "vcpkg helps you manage C and C++ libraries on Windows, Linux and MacOS."
4439
HOMEPAGE_URL "https://github.com/microsoft/vcpkg"
45-
LANGUAGES ${LANGUAGES}
40+
LANGUAGES C CXX
4641
)
4742

4843
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake")
@@ -449,9 +444,8 @@ if(VCPKG_ADD_SOURCELINK)
449444
REF "${VCPKG_VERSION}"
450445
)
451446
endif()
452-
if(VCPKG_PDB_SUFFIX)
453-
set_property(TARGET vcpkg PROPERTY PDB_NAME "vcpkg${VCPKG_PDB_SUFFIX}")
454-
endif()
447+
448+
set_property(TARGET vcpkg PROPERTY PDB_NAME "vcpkg${VCPKG_PDB_SUFFIX}")
455449

456450
# === Target: generate-message-map ===
457451
set(GENERATE_MESSAGE_MAP_DEPENDENCIES vcpkg)
@@ -481,6 +475,7 @@ if (BUILD_TESTING)
481475
"${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest"
482476
)
483477
target_link_libraries(vcpkg-test PRIVATE vcpkglib)
478+
set_property(TARGET vcpkg-test PROPERTY PDB_NAME "vcpkg-test${VCPKG_PDB_SUFFIX}")
484479
if(ANDROID)
485480
target_link_libraries(vcpkg-test PRIVATE log)
486481
endif()
@@ -499,27 +494,43 @@ if (BUILD_TESTING)
499494
endif()
500495

501496
# === Target: vcpkg-fuzz ===
502-
503-
file(GLOB VCPKG_FUZZ_SOURCES CONFIGURE_DEPENDS "src/vcpkg-fuzz/*.cpp")
497+
set(VCPKG_FUZZ_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg-fuzz/main.cpp")
504498
if(VCPKG_BUILD_FUZZING)
505499
add_executable(vcpkg-fuzz ${VCPKG_FUZZ_SOURCES} "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest")
506500
target_link_libraries(vcpkg-fuzz PRIVATE vcpkglib)
501+
set_property(TARGET vcpkg-fuzz PROPERTY PDB_NAME "vcpkg-fuzz${VCPKG_PDB_SUFFIX}")
507502
endif()
508503

509504

510505
# === Target: tls12-download ===
511-
512506
set(TLS12_DOWNLOAD_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/src/tls12-download.c")
513507
if(VCPKG_BUILD_TLS12_DOWNLOADER)
514508
add_executable(tls12-download ${TLS12_DOWNLOAD_SOURCES} "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest")
515509
set_property(TARGET tls12-download PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded")
516510
set_property(TARGET tls12-download APPEND PROPERTY LINK_OPTIONS "$<IF:$<CONFIG:Debug>,,/ENTRY:entry>")
517511
target_link_libraries(tls12-download winhttp wintrust shell32)
518-
if(VCPKG_PDB_SUFFIX)
519-
set_property(TARGET tls12-download PROPERTY PDB_NAME "tls12-download${VCPKG_PDB_SUFFIX}")
520-
endif()
512+
set_property(TARGET tls12-download PROPERTY PDB_NAME "tls12-download${VCPKG_PDB_SUFFIX}")
521513
endif()
522514

515+
if (BUILD_TESTING)
516+
# === Target: closes-stdin ===
517+
518+
set(CLOSES_STDIN_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/src/closes-stdin.c")
519+
add_executable(closes-stdin ${CLOSES_STDIN_SOURCES} "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest")
520+
set_property(TARGET closes-stdin PROPERTY PDB_NAME "closes-stdin${VCPKG_PDB_SUFFIX}")
521+
522+
# === Target: closes-stdout ===
523+
524+
set(CLOSES_STDOUT_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/src/closes-stdout.c")
525+
add_executable(closes-stdout ${CLOSES_STDOUT_SOURCES} "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest")
526+
set_property(TARGET closes-stdout PROPERTY PDB_NAME "closes-stdout${VCPKG_PDB_SUFFIX}")
527+
528+
# === Target: reads-stdin ===
529+
530+
set(READS_STDIN_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/src/reads-stdin.c")
531+
add_executable(reads-stdin ${READS_STDIN_SOURCES} "${CMAKE_CURRENT_SOURCE_DIR}/src/vcpkg.manifest")
532+
set_property(TARGET reads-stdin PROPERTY PDB_NAME "reads-stdin${VCPKG_PDB_SUFFIX}")
533+
endif()
523534

524535
# === Target: format ===
525536

@@ -540,6 +551,9 @@ if(CLANG_FORMAT)
540551

541552
COMMAND "${CLANG_FORMAT}" -i -verbose ${VCPKG_FUZZ_SOURCES}
542553
COMMAND "${CLANG_FORMAT}" -i -verbose ${TLS12_DOWNLOAD_SOURCES}
554+
COMMAND "${CLANG_FORMAT}" -i -verbose ${CLOSES_STDIN_SOURCES}
555+
COMMAND "${CLANG_FORMAT}" -i -verbose ${CLOSES_STDOUT_SOURCES}
556+
COMMAND "${CLANG_FORMAT}" -i -verbose ${READS_STDIN_SOURCES}
543557
)
544558
endif()
545559

‎include/vcpkg/base/downloads.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ namespace vcpkg
3636
View<std::pair<std::string, Path>> url_pairs,
3737
View<std::string> headers);
3838

39-
bool send_snapshot_to_api(const Filesystem& fs,
40-
const std::string& github_token,
39+
bool send_snapshot_to_api(const std::string& github_token,
4140
const std::string& github_repository,
4241
const Json::Object& snapshot);
4342
ExpectedL<int> put_file(const ReadOnlyFilesystem&,

‎include/vcpkg/base/files.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,10 @@ namespace vcpkg
330330
std::vector<std::string> exts;
331331
bool operator()(const Path& target) const;
332332
};
333+
334+
#if !defined(_WIN32)
335+
void close_mark_invalid(int& fd) noexcept;
336+
#endif // ^^^ !_WIN32
333337
}
334338

335339
VCPKG_FORMAT_AS(vcpkg::Path, vcpkg::StringView);

‎include/vcpkg/base/system-headers.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,16 @@
2121
#if __APPLE__
2222
extern "C"
2323
{
24-
#endif
24+
#endif // __APPLE__
2525

2626
#include <unistd.h>
2727

2828
#if __APPLE__
2929
}
30-
#endif
31-
32-
#endif
30+
#endif // __APPLE__
3331

3432
#include <sys/types.h>
3533
// glibc defines major and minor in sys/types.h, and should not
3634
#undef major
3735
#undef minor
36+
#endif // ^^^ Unix

‎include/vcpkg/base/system.process.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
#pragma once
22

33
#include <vcpkg/base/fwd/files.h>
4-
#include <vcpkg/base/fwd/optional.h>
54
#include <vcpkg/base/fwd/system.process.h>
65

76
#include <vcpkg/base/expected.h>
7+
#include <vcpkg/base/optional.h>
88
#include <vcpkg/base/path.h>
99
#include <vcpkg/base/span.h>
1010
#include <vcpkg/base/stringview.h>
@@ -134,24 +134,27 @@ namespace vcpkg
134134
const WorkingDirectory& wd = default_working_directory,
135135
const Environment& env = default_environment,
136136
Encoding encoding = Encoding::Utf8,
137-
EchoInDebug echo_in_debug = EchoInDebug::Hide);
137+
EchoInDebug echo_in_debug = EchoInDebug::Hide,
138+
StringView stdin_content = {});
138139

139140
std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(
140141
View<Command> cmd_lines,
141142
const WorkingDirectory& wd = default_working_directory,
142143
const Environment& env = default_environment);
143144

144145
ExpectedL<int> cmd_execute_and_stream_lines(const Command& cmd_line,
145-
std::function<void(StringView)> per_line_cb,
146+
const std::function<void(StringView)>& per_line_cb,
146147
const WorkingDirectory& wd = default_working_directory,
147148
const Environment& env = default_environment,
148-
Encoding encoding = Encoding::Utf8);
149+
Encoding encoding = Encoding::Utf8,
150+
StringView stdin_content = {});
149151

150152
ExpectedL<int> cmd_execute_and_stream_data(const Command& cmd_line,
151-
std::function<void(StringView)> data_cb,
153+
const std::function<void(StringView)>& data_cb,
152154
const WorkingDirectory& wd = default_working_directory,
153155
const Environment& env = default_environment,
154-
Encoding encoding = Encoding::Utf8);
156+
Encoding encoding = Encoding::Utf8,
157+
StringView stdin_content = {});
155158

156159
uint64_t get_subproccess_stats();
157160

‎src/closes-stdin.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#if defined(_WIN32)
2+
#include <Windows.h>
3+
int main() { CloseHandle(GetStdHandle(STD_INPUT_HANDLE)); }
4+
#else
5+
#include <unistd.h>
6+
int main(void) { close(STDIN_FILENO); }
7+
#endif

‎src/closes-stdout.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#include <stdio.h>
2+
#if defined(_WIN32)
3+
#include <Windows.h>
4+
#else // ^^^ _WIN32 // !_WIN32
5+
#include <unistd.h>
6+
#endif // ^^^ !_WIN32
7+
8+
int main(void)
9+
{
10+
const char content[] = "hello world";
11+
fwrite(content, 1, sizeof(content) - 1, stdout);
12+
fflush(stdout);
13+
#if defined(_WIN32)
14+
CloseHandle(GetStdHandle(STD_OUTPUT_HANDLE));
15+
#else // ^^^ _WIN32 // !_WIN32
16+
close(STDOUT_FILENO);
17+
#endif // ^^^ !_WIN32
18+
}

‎src/reads-stdin.c‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#include <stdio.h>
2+
#include <string.h>
3+
4+
// This program reads stdin and asserts that it is the expected value repeated
5+
// until stdin ends
6+
7+
int main(int argc, char** argv)
8+
{
9+
char buffer[20];
10+
// The repeated string 'example' is intentionally a prime length to make hitting buffering edge
11+
// cases more likely
12+
const char expected[] = "exampleexampleexampleexamp";
13+
size_t offset = 0; // always between 0 and 6
14+
for (;;)
15+
{
16+
size_t read_amount = fread(buffer, 1, sizeof(buffer), stdin);
17+
if (argc > 1)
18+
{
19+
puts(argv[1]);
20+
fflush(stdout);
21+
}
22+
if (read_amount == 0)
23+
{
24+
if (feof(stdin))
25+
{
26+
puts("success");
27+
return 0;
28+
}
29+
return 1;
30+
}
31+
32+
if (memcmp(buffer, expected + offset, read_amount) != 0)
33+
{
34+
return 2;
35+
}
36+
offset = (offset + read_amount) % 7;
37+
}
38+
}

‎src/vcpkg-test/system.process.cpp‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#include <catch2/catch.hpp>
2+
3+
#include <vcpkg/base/strings.h>
4+
#include <vcpkg/base/system.process.h>
5+
6+
using namespace vcpkg;
7+
8+
TEST_CASE ("captures-output", "[system.process]")
9+
{
10+
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "reads-stdin";
11+
Command cmd{test_program};
12+
cmd.string_arg("this is printed when something is read");
13+
static constexpr std::size_t minimum_size = 1'000'000; // to exceed OS pipe buffer size
14+
constexpr StringLiteral example = "example";
15+
constexpr auto examples = (minimum_size / example.size()) + 1;
16+
std::string input;
17+
constexpr auto input_size = examples * example.size();
18+
for (std::size_t idx = 0; idx < examples; ++idx)
19+
{
20+
input.append(example.data(), example.size());
21+
}
22+
23+
std::string expected;
24+
constexpr StringLiteral repeat = "this is printed when something is read";
25+
constexpr auto repeats = (input_size / 20) + (input_size % 20 != 0) + 1;
26+
for (std::size_t idx = 0; idx < repeats; ++idx)
27+
{
28+
expected.append(repeat.data(), repeat.size());
29+
#if defined(_WIN32)
30+
expected.push_back('\r');
31+
#endif // ^^^ _WIN32
32+
expected.push_back('\n');
33+
}
34+
35+
expected.append("success");
36+
#if defined(_WIN32)
37+
expected.push_back('\r');
38+
#endif // ^^^ _WIN32
39+
expected.push_back('\n');
40+
41+
auto run = cmd_execute_and_capture_output(
42+
cmd, default_working_directory, default_environment, Encoding::Utf8, EchoInDebug::Hide, input)
43+
.value_or_exit(VCPKG_LINE_INFO);
44+
REQUIRE(run.exit_code == 0);
45+
REQUIRE(run.output == expected);
46+
}
47+
48+
TEST_CASE ("no closes-stdin crash", "[system.process]")
49+
{
50+
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "closes-stdin";
51+
Command cmd{test_program};
52+
auto run = cmd_execute_and_capture_output(cmd,
53+
default_working_directory,
54+
default_environment,
55+
Encoding::Utf8,
56+
EchoInDebug::Hide,
57+
"this is some input that will be intentionally not read")
58+
.value_or_exit(VCPKG_LINE_INFO);
59+
REQUIRE(run.exit_code == 0);
60+
REQUIRE(run.output.empty());
61+
}
62+
63+
TEST_CASE ("no closes-stdout crash", "[system.process]")
64+
{
65+
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "closes-stdout";
66+
Command cmd{test_program};
67+
auto run = cmd_execute_and_capture_output(cmd,
68+
default_working_directory,
69+
default_environment,
70+
Encoding::Utf8,
71+
EchoInDebug::Hide,
72+
"this is some input that will be read")
73+
.value_or_exit(VCPKG_LINE_INFO);
74+
REQUIRE(run.exit_code == 0);
75+
REQUIRE(run.output == "hello world");
76+
}

‎src/vcpkg/base/downloads.cpp‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include <vcpkg/base/system.process.h>
1313
#include <vcpkg/base/system.proxy.h>
1414
#include <vcpkg/base/util.h>
15-
#include <vcpkg/base/uuid.h>
1615

1716
#include <vcpkg/commands.version.h>
1817
#include <vcpkg/metrics.h>
@@ -522,8 +521,7 @@ namespace vcpkg
522521
return ret;
523522
}
524523

525-
bool send_snapshot_to_api(const Filesystem& fs,
526-
const std::string& github_token,
524+
bool send_snapshot_to_api(const std::string& github_token,
527525
const std::string& github_repository,
528526
const Json::Object& snapshot)
529527
{
@@ -540,24 +538,24 @@ namespace vcpkg
540538
cmd.string_arg("-H").string_arg("X-GitHub-Api-Version: 2022-11-28");
541539
cmd.string_arg(
542540
Strings::concat("https://api.github.com/repos/", github_repository, "/dependency-graph/snapshots"));
543-
544-
const auto tmp_dir = fs.create_or_get_temp_directory(VCPKG_LINE_INFO);
545-
const auto unique_file_name = fmt::format("dependency_snapshot_{}.json", generate_random_UUID());
546-
const auto snapshot_file_path = tmp_dir / unique_file_name;
547-
fs.write_contents(snapshot_file_path, Json::stringify(snapshot), VCPKG_LINE_INFO);
548-
549-
cmd.string_arg("-d").string_arg(fmt::format("@{}", snapshot_file_path));
541+
cmd.string_arg("-d").string_arg("@-");
550542
int code = 0;
551-
auto result = cmd_execute_and_stream_lines(cmd, [&code](StringView line) {
552-
if (Strings::starts_with(line, guid_marker))
553-
{
554-
code = std::strtol(line.data() + guid_marker.size(), nullptr, 10);
555-
}
556-
else
557-
{
558-
Debug::println(line);
559-
}
560-
});
543+
auto result = cmd_execute_and_stream_lines(
544+
cmd,
545+
[&code](StringView line) {
546+
if (Strings::starts_with(line, guid_marker))
547+
{
548+
code = std::strtol(line.data() + guid_marker.size(), nullptr, 10);
549+
}
550+
else
551+
{
552+
Debug::println(line);
553+
}
554+
},
555+
default_working_directory,
556+
default_environment,
557+
Encoding::Utf8,
558+
Json::stringify(snapshot));
561559

562560
auto r = result.get();
563561
if (r && *r == 0 && code >= 200 && code < 300)

0 commit comments

Comments
 (0)