aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAdrian Kummerlaender2016-02-06 22:20:46 +0100
committerAdrian Kummerlaender2016-02-06 22:20:46 +0100
commitf08930c9c9ce91007c2f576c25d3f874abb96a00 (patch)
treed9d2f670b833874171b4c7ec77488421f55b983b /src
parent0f083843bc31bf985991728125ef74efca6ef7b7 (diff)
downloadchange-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar.gz
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar.bz2
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar.lz
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar.xz
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.tar.zst
change-f08930c9c9ce91007c2f576c25d3f874abb96a00.zip
Preserve pre-change file content as tracking state
Relying on `diff` for storing the pre-change content did not work out as it opens the file descriptors to soon and only reads the first block of pre-change file content until the remaining content is actually required. This obviously led to problems when tracking actually changing files.
Diffstat (limited to 'src')
-rw-r--r--src/tracking/change_tracker.cc66
-rw-r--r--src/tracking/change_tracker.h7
2 files changed, 38 insertions, 35 deletions
diff --git a/src/tracking/change_tracker.cc b/src/tracking/change_tracker.cc
index ca07214..9416576 100644
--- a/src/tracking/change_tracker.cc
+++ b/src/tracking/change_tracker.cc
@@ -8,7 +8,7 @@ namespace {
// constants for increasing pair access readability
constexpr unsigned int EMPLACE_SUCCESS = 1;
constexpr unsigned int FILE_NAME = 0;
-constexpr unsigned int DIFF_PROCESS = 1;
+constexpr unsigned int FILE_CONTENT = 1;
boost::process::context getDefaultContext() {
boost::process::context context;
@@ -20,6 +20,15 @@ boost::process::context getDefaultContext() {
return context;
}
+// Build `diff` command to be used to display the tracked changes in a
+// human readable fashion.
+//
+// As the original file contents are read by this library and forwarded
+// to `diff` via its standard input the command has to be structured as
+// follows:
+//
+// diff -u --label $file_path - $file_path
+//
std::string getDiffCommand(
const std::string& diff_cmd, const std::string& full_path) {
return diff_cmd + " --label " + full_path + " - " + full_path;
@@ -39,11 +48,19 @@ ChangeTracker::ChangeTracker(utility::Logger* logger):
ChangeTracker::~ChangeTracker() {
for ( auto&& tracked : this->children_ ) {
- std::get<DIFF_PROCESS>(tracked)->get_stdin().close();
+ boost::process::child diffProcess{
+ boost::process::launch_shell(
+ getDiffCommand(this->diff_cmd_, std::get<FILE_NAME>(tracked)),
+ getDefaultContext()
+ )
+ };
- this->logger_->forward(std::get<DIFF_PROCESS>(tracked)->get_stdout());
+ diffProcess.get_stdin() << std::get<FILE_CONTENT>(tracked)->rdbuf();
+ diffProcess.get_stdin().close();
- std::get<DIFF_PROCESS>(tracked)->wait();
+ this->logger_->forward(diffProcess.get_stdout());
+
+ diffProcess.wait();
}
}
@@ -55,27 +72,18 @@ bool ChangeTracker::is_tracked(const std::string& file_path) const {
// Begins tracking changes to a file reachable by a given path
//
-// The actual tracking is performed by a `diff` instance that is
-// spawned by this method and managed by this class.
-// As `diff` is called as a new subprocess and because there is no
-// straight forward way for checking / enforcing if it has already
-// completed reading the initial contents of the file the command
-// is structured in the following sequence:
-//
-// diff -p - $file_path
-//
-// This means that reading the final file contents is delegated to
-// `diff` while the initial file contents are read by this method
-// and written to `diff`'s standard input.
+// _Tracking_ consists of adding the full file path to the `children_`
+// map and reading the full pre-change file contents into the mapped
+// `std::stringstream` instance.
//
-// If the `-` and `$file_path` arguments were exchanged there would
-// be no way to:
-// - be sure the initial contents are read before the changing
-// syscall that triggered this tracking in the first place is
-// performed
-// - be sure that the initial file contents are read completly
-// as `diff` seemingly only reads the first block of the first
-// file provided if the second file argument is standard input
+// This seems to be the most workable of various tested approaches
+// to providing `diff` with proper input in all circumstances. Leaving
+// the intermediate preservation of the original file content to
+// `diff` sadly failed randomly as it is _too intelligent_ in actually
+// reading the file from permanent storage. e.g. it opens all relevant
+// file descriptors at launch and only reads the first block of the
+// file contents until more is required. This leads to problems when
+// the tracked file is modified after `diff` has been spawned.
//
bool ChangeTracker::track(const std::string& file_path) {
const std::string full_file_path{
@@ -86,12 +94,7 @@ bool ChangeTracker::track(const std::string& file_path) {
auto result = this->children_.emplace(
full_file_path,
- std::make_unique<boost::process::child>(
- boost::process::launch_shell(
- getDiffCommand(this->diff_cmd_, full_file_path),
- getDefaultContext()
- )
- )
+ std::make_unique<std::stringstream>()
);
guard.unlock();
@@ -102,7 +105,8 @@ bool ChangeTracker::track(const std::string& file_path) {
);
if ( file.is_open() ) {
- std::get<DIFF_PROCESS>(*result.first)->get_stdin() << file.rdbuf();
+ *std::get<FILE_CONTENT>(*result.first) << file.rdbuf();
+ std::get<FILE_CONTENT>(*result.first)->sync();
}
return true;
diff --git a/src/tracking/change_tracker.h b/src/tracking/change_tracker.h
index e694b56..05f4021 100644
--- a/src/tracking/change_tracker.h
+++ b/src/tracking/change_tracker.h
@@ -1,10 +1,9 @@
#ifndef CHANGE_SRC_TRACKING_CHANGE_TRACKER_H_
#define CHANGE_SRC_TRACKING_CHANGE_TRACKER_H_
-#include <unordered_map>
#include <mutex>
-
-#include <boost/process.hpp>
+#include <sstream>
+#include <unordered_map>
#include "utility/logger.h"
@@ -26,7 +25,7 @@ class ChangeTracker {
std::mutex write_mutex_;
std::unordered_map<
- std::string, std::unique_ptr<boost::process::child>
+ std::string, std::unique_ptr<std::stringstream>
> children_;
};