kythe: Add --output_path to write extracted facts to a file The extractor could only write Kythe facts to stdout, so every caller had to capture the stream, and a large project produces a lot of facts. Add --output_path, which names a file to write the facts to. An empty value or "-" keeps the current stdout behavior. The file is opened in binary mode, because --print_kythe_facts=proto is not text. The tool exits 1 when it cannot open the file. All four print modes now share one std::ostream. To let the proto mode join them, KytheProtoOutput takes a std::ostream and wraps it in an OstreamOutputStream instead of taking a file descriptor. Fixes #736.
diff --git a/verible/verilog/tools/kythe/README.md b/verible/verilog/tools/kythe/README.md index b44aaf9..610f755 100644 --- a/verible/verilog/tools/kythe/README.md +++ b/verible/verilog/tools/kythe/README.md
@@ -30,4 +30,6 @@ File search will stop at the the first found among the listed directories. e.g --include_dir_paths directory1,directory2 if "A.sv" exists in both "directory1" and "directory2" the one in "directory1" is the one we will use) + --output_path (Path of the file to write the extracted Kythe facts to. + Writes to stdout if empty or "-"); default: ""; ```
diff --git a/verible/verilog/tools/kythe/kythe-proto-output.cc b/verible/verilog/tools/kythe/kythe-proto-output.cc index 0117329..76e5952 100644 --- a/verible/verilog/tools/kythe/kythe-proto-output.cc +++ b/verible/verilog/tools/kythe/kythe-proto-output.cc
@@ -14,9 +14,12 @@ #include "verible/verilog/tools/kythe/kythe-proto-output.h" +#include <memory> +#include <ostream> #include <string> #include "google/protobuf/io/coded_stream.h" +#include "google/protobuf/io/zero_copy_stream.h" #include "google/protobuf/io/zero_copy_stream_impl.h" #include "third_party/proto/kythe/storage.pb.h" #include "verible/verilog/tools/kythe/kythe-facts.h" @@ -26,7 +29,8 @@ namespace { using ::google::protobuf::io::CodedOutputStream; -using ::google::protobuf::io::FileOutputStream; +using ::google::protobuf::io::OstreamOutputStream; +using ::google::protobuf::io::ZeroCopyOutputStream; using ::kythe::proto::Entry; // Returns the VName representation in Kythe's storage proto format. @@ -60,7 +64,7 @@ } // Output entry to the stream. -void OutputProto(const Entry &entry, FileOutputStream *stream) { +void OutputProto(const Entry &entry, ZeroCopyOutputStream *stream) { CodedOutputStream coded_stream(stream); coded_stream.WriteVarint32(entry.ByteSizeLong()); entry.SerializeToCodedStream(&coded_stream); @@ -68,14 +72,17 @@ } // namespace -KytheProtoOutput::KytheProtoOutput(int fd) : out_(fd) {} -KytheProtoOutput::~KytheProtoOutput() { out_.Close(); } +KytheProtoOutput::KytheProtoOutput(std::ostream &output_stream) + : out_(std::make_unique<OstreamOutputStream>(&output_stream)) {} + +// OstreamOutputStream flushes its remaining bytes when destroyed. +KytheProtoOutput::~KytheProtoOutput() = default; void KytheProtoOutput::Emit(const Fact &fact) { - OutputProto(ConvertFactToEntry(fact), &out_); + OutputProto(ConvertFactToEntry(fact), out_.get()); } void KytheProtoOutput::Emit(const Edge &edge) { - OutputProto(ConvertEdgeToEntry(edge), &out_); + OutputProto(ConvertEdgeToEntry(edge), out_.get()); } } // namespace kythe
diff --git a/verible/verilog/tools/kythe/kythe-proto-output.h b/verible/verilog/tools/kythe/kythe-proto-output.h index ba873cd..2ffa58e 100644 --- a/verible/verilog/tools/kythe/kythe-proto-output.h +++ b/verible/verilog/tools/kythe/kythe-proto-output.h
@@ -15,7 +15,10 @@ #ifndef VERIBLE_VERILOG_TOOLS_KYTHE_KYTHE_PROTO_OUTPUT_H_ #define VERIBLE_VERILOG_TOOLS_KYTHE_KYTHE_PROTO_OUTPUT_H_ -#include "google/protobuf/io/zero_copy_stream_impl.h" +#include <memory> +#include <ostream> + +#include "google/protobuf/io/zero_copy_stream.h" #include "verible/verilog/tools/kythe/kythe-facts-extractor.h" #include "verible/verilog/tools/kythe/kythe-facts.h" @@ -24,7 +27,10 @@ class KytheProtoOutput final : public KytheOutput { public: - explicit KytheProtoOutput(int output_fd); + // Writes to an already-open stream. The stream must outlive this object and + // must be opened in binary mode, as the proto entries are not text. + explicit KytheProtoOutput(std::ostream &output_stream); + ~KytheProtoOutput() final; // Output Kythe facts from the indexing data in proto format. @@ -32,7 +38,7 @@ void Emit(const Edge &edge) final; private: - ::google::protobuf::io::FileOutputStream out_; + std::unique_ptr<::google::protobuf::io::ZeroCopyOutputStream> out_; }; } // namespace kythe
diff --git a/verible/verilog/tools/kythe/verilog-kythe-extractor.cc b/verible/verilog/tools/kythe/verilog-kythe-extractor.cc index df43e52..f21fdf4 100644 --- a/verible/verilog/tools/kythe/verilog-kythe-extractor.cc +++ b/verible/verilog/tools/kythe/verilog-kythe-extractor.cc
@@ -12,7 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include <fstream> +#include <ios> #include <iostream> +#include <memory> +#include <ostream> #include <sstream> #include <string> #include <string_view> @@ -33,13 +37,6 @@ #include "verible/verilog/tools/kythe/kythe-facts.h" #include "verible/verilog/tools/kythe/kythe-proto-output.h" -#ifndef _WIN32 -#include <unistd.h> // for STDOUT_FILENO -#else -#include <stdio.h> -#define STDOUT_FILENO _fileno(stdout) -#endif - // for --print_kythe_facts flag enum class PrintMode { kJSON, @@ -109,14 +106,18 @@ ABSL_FLAG(std::string, verilog_project_name, "", "Verilog project name to use as Kythe corpus. Optional"); +ABSL_FLAG(std::string, output_path, "", + R"(Path of the file to write the extracted Kythe facts to. +Writes to stdout if empty or "-".)"); + namespace verilog { namespace kythe { -// Prints Kythe facts in proto format to stdout. +// Prints Kythe facts in proto format to the given stream. static void PrintKytheFactsProtoEntries( const IndexingFactNode &file_list_facts_tree, const VerilogProject &project, - int fd) { - KytheProtoOutput proto_output(fd); + std::ostream &stream) { + KytheProtoOutput proto_output(stream); StreamKytheFactsEntries(&proto_output, file_list_facts_tree, project); } @@ -134,7 +135,7 @@ static std::vector<absl::Status> ExtractTranslationUnits( std::string_view file_list_path, VerilogProject *project, - const std::vector<std::string> &file_names) { + const std::vector<std::string> &file_names, std::ostream &output_stream) { std::vector<absl::Status> errors; const verilog::kythe::IndexingFactNode file_list_facts_tree( verilog::kythe::ExtractFiles(file_list_path, project, file_names, @@ -149,17 +150,17 @@ // check how to output kythe facts. switch (absl::GetFlag(FLAGS_print_kythe_facts)) { case PrintMode::kJSON: - std::cout << KytheFactsPrinter(file_list_facts_tree, *project) - << std::endl; + output_stream << KytheFactsPrinter(file_list_facts_tree, *project) + << std::endl; break; case PrintMode::kJSONDebug: - std::cout << KytheFactsPrinter(file_list_facts_tree, *project, - /*debug=*/true) - << std::endl; + output_stream << KytheFactsPrinter(file_list_facts_tree, *project, + /*debug=*/true) + << std::endl; break; case PrintMode::kProto: PrintKytheFactsProtoEntries(file_list_facts_tree, *project, - STDOUT_FILENO); + output_stream); break; case PrintMode::kNone: KytheFactsNullPrinter(file_list_facts_tree, *project); @@ -211,9 +212,24 @@ absl::GetFlag(FLAGS_verilog_project_name), /*provide_lookup_file_origin=*/false); + // Send the facts to a file when asked to, otherwise to stdout. The file is + // opened in binary mode because --print_kythe_facts=proto is not text. + const std::string output_path = absl::GetFlag(FLAGS_output_path); + std::unique_ptr<std::ofstream> file_closer; + std::ostream *output_stream = &std::cout; + if (!output_path.empty() && output_path != "-") { + file_closer = std::make_unique<std::ofstream>( + output_path, std::ios::out | std::ios::binary); + if (!file_closer->good()) { + LOG(ERROR) << "Failed to create/open output file: " << output_path; + return 1; + } + output_stream = file_closer.get(); + } + const std::vector<absl::Status> errors( verilog::kythe::ExtractTranslationUnits(file_list_path, &project, - file_paths)); + file_paths, *output_stream)); if (!errors.empty()) { LOG(ERROR) << "Encountered some issues while indexing files (could result " "in missing indexing data):"
diff --git a/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh b/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh index bf3bd7c..1e7bcc9 100755 --- a/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh +++ b/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh
@@ -160,4 +160,90 @@ } ################################################################################ +echo "=== Write facts to --output_path instead of stdout." + +cat > "$MY_INPUT_FILE" <<EOF +localparam int fooo = 1; +localparam int barr = fooo; +EOF + +echo "myinput.txt" > "${TEST_TMPDIR}/file_list" + +# Every print mode has to honor --output_path, and has to write the same bytes +# it would have written to stdout. +for mode in json json_debug proto ; do + "$extractor" \ + --file_list_path "${TEST_TMPDIR}/file_list" \ + --file_list_root "$(dirname "$MY_INPUT_FILE")" \ + --print_kythe_facts="$mode" \ + --output_path "$MY_OUTPUT_FILE" 2>/dev/null + + status="$?" + [[ $status == 0 ]] || { + echo "Expected exit code 0 for --print_kythe_facts=$mode, but got $status" + exit 1 + } + + [[ -s "$MY_OUTPUT_FILE" ]] || { + echo "Expected --output_path file to be non-empty for mode $mode." + exit 1 + } + + "$extractor" \ + --file_list_path "${TEST_TMPDIR}/file_list" \ + --file_list_root "$(dirname "$MY_INPUT_FILE")" \ + --print_kythe_facts="$mode" \ + > "$MY_EXPECT_FILE" 2>/dev/null + + cmp "$MY_OUTPUT_FILE" "$MY_EXPECT_FILE" || { + echo "--output_path output differs from stdout output for mode $mode." + exit 1 + } +done + +################################################################################ +echo "=== '--output_path -' writes to stdout." + +"$extractor" \ + --file_list_path "${TEST_TMPDIR}/file_list" \ + --file_list_root "$(dirname "$MY_INPUT_FILE")" \ + --print_kythe_facts=json \ + --output_path - \ + > "$MY_OUTPUT_FILE" 2>/dev/null + +status="$?" +[[ $status == 0 ]] || { + echo "Expected exit code 0, but got $status" + exit 1 +} + +grep -q "signature" "$MY_OUTPUT_FILE" || { + echo "Expected \"signature\" in $MY_OUTPUT_FILE but didn't find it. Got:" + cat "$MY_OUTPUT_FILE" + exit 1 +} + +################################################################################ +echo "=== Expect failure on unwritable --output_path." + +"$extractor" \ + --file_list_path "${TEST_TMPDIR}/file_list" \ + --file_list_root "$(dirname "$MY_INPUT_FILE")" \ + --print_kythe_facts=json \ + --output_path "${TEST_TMPDIR}/nonexistent-dir/out.json" \ + > "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 1 ]] || { + echo "Expected exit code 1, but got $status" + exit 1 +} + +grep -q "Failed to create/open output file" "$MY_OUTPUT_FILE" || { + echo "Expected \"Failed to create/open output file\" in $MY_OUTPUT_FILE but didn't find it. Got:" + cat "$MY_OUTPUT_FILE" + exit 1 +} + +################################################################################ echo "PASS"