fix log progress quotes (#580)

* add quotes to message in log-progress mode
* improved log progress for builds upload and download

4 files changed, 58 insertions, 15 deletions

diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp
index 76322013d..5633a90b7 100644
--- a/src/zen/cmds/builds_cmd.cpp
+++ b/src/zen/cmds/builds_cmd.cpp
@@ -2139,7 +2139,11 @@ namespace {
 													 .EnableTargetFolderScavenging	 = Options.EnableTargetFolderScavenging,
 													 .ExcludeFolders				 = DefaultExcludeFolders,
 													 .ExcludeExtensions				 = DefaultExcludeExtensions});
-			Updater.Execute(LocalFolderState);
+			{
+				ProgressBar::PushLogOperation(ProgressMode, "Download");
+				auto _ = MakeGuard([]() { ProgressBar::PopLogOperation(ProgressMode); });
+				Updater.Execute(LocalFolderState);
+			}
 
 			VerifyFolderStatistics VerifyFolderStats;
 			if (!AbortFlag)
diff --git a/src/zen/zen.cpp b/src/zen/zen.cpp
index 20ff6cb56..e60e5451b 100644
--- a/src/zen/zen.cpp
+++ b/src/zen/zen.cpp
@@ -372,9 +372,10 @@ ZenCmdBase::LogExecutableVersionAndPid()
 void
 ProgressBar::SetLogOperationName(Mode InMode, std::string_view Name)
 {
+	ZEN_ASSERT(Name.find('\"') == std::string_view::npos);
 	if (InMode == Mode::Log)
 	{
-		std::string String = fmt::format("@progress {}\n", Name);
+		std::string String = fmt::format("@progress \"{}\"\n", Name);
 		OutputToConsoleRaw(String);
 	}
 }
@@ -391,16 +392,36 @@ ProgressBar::SetLogOperationProgress(Mode InMode, uint32_t StepIndex, uint32_t S
 	}
 }
 
+void
+ProgressBar::PushLogOperation(Mode InMode, std::string_view Name)
+{
+	if (InMode == Mode::Log)
+	{
+		std::string String = fmt::format("@progress push \"{}\"\n", Name);
+		OutputToConsoleRaw(String);
+	}
+}
+
+void
+ProgressBar::PopLogOperation(Mode InMode)
+{
+	if (InMode == Mode::Log)
+	{
+		const std::string String("@progress pop\n");
+		OutputToConsoleRaw(String);
+	}
+}
+
 ProgressBar::ProgressBar(Mode InMode, std::string_view InSubTask)
 : m_Mode((!IsStdoutTty() && InMode == Mode::Pretty) ? Mode::Plain : InMode)
 , m_LastUpdateMS((uint64_t)-1)
 , m_PausedMS(0)
 , m_SubTask(InSubTask)
 {
-	if (!m_SubTask.empty() && InMode == Mode::Log)
+	ZEN_ASSERT(InSubTask.find('\"') == std::string_view::npos);
+	if (!m_SubTask.empty())
 	{
-		std::string String = fmt::format("@progress push {}\n", m_SubTask);
-		OutputToConsoleRaw(String);
+		PushLogOperation(InMode, m_SubTask);
 	}
 }
 
@@ -409,10 +430,9 @@ ProgressBar::~ProgressBar()
 	try
 	{
 		ForceLinebreak();
-		if (!m_SubTask.empty() && m_Mode == Mode::Log)
+		if (!m_SubTask.empty())
 		{
-			const std::string String("@progress pop\n");
-			OutputToConsoleRaw(String);
+			PopLogOperation(m_Mode);
 		}
 	}
 	catch (const std::exception& Ex)
@@ -425,6 +445,7 @@ void
 ProgressBar::UpdateState(const State& NewState, bool DoLinebreak)
 {
 	ZEN_ASSERT(NewState.TotalCount >= NewState.RemainingCount);
+	ZEN_ASSERT(NewState.Task.find('\"') == std::string::npos);
 	if (DoLinebreak == false && m_State == NewState)
 	{
 		return;
@@ -563,8 +584,15 @@ ProgressBar::UpdateState(const State& NewState, bool DoLinebreak)
 		if (m_State.Task != NewState.Task ||
 			m_State.Details != NewState.Details)  // TODO: Should we output just because details change? Will this spam the log collector?
 		{
-			const std::string Details = (!NewState.Details.empty()) ? fmt::format(": {}", NewState.Details) : "";
-			const std::string Message = fmt::format("@progress {} ({}){}\n", NewState.Task, NiceTimeSpanMs(ElapsedTimeMS), Details);
+			std::string Details = (!NewState.Details.empty()) ? fmt::format(": {}", NewState.Details) : "";
+			for (std::string::value_type& Char : Details)
+			{
+				if (Char == '"')
+				{
+					Char = '\'';
+				}
+			}
+			const std::string Message = fmt::format("@progress \"{} ({}){}\"\n", NewState.Task, NiceTimeSpanMs(ElapsedTimeMS), Details);
 			OutputToConsoleRaw(Message);
 		}
 
diff --git a/src/zen/zen.h b/src/zen/zen.h
index ffb35e5ca..80df8c17f 100644
--- a/src/zen/zen.h
+++ b/src/zen/zen.h
@@ -118,6 +118,8 @@ public:
 
 	static void SetLogOperationName(Mode InMode, std::string_view Name);
 	static void SetLogOperationProgress(Mode InMode, uint32_t StepIndex, uint32_t StepCount);
+	static void PushLogOperation(Mode InMode, std::string_view Name);
+	static void PopLogOperation(Mode InMode);
 
 	explicit ProgressBar(Mode InMode, std::string_view InSubTask);
 	~ProgressBar();
diff --git a/src/zenremotestore/builds/buildstorageoperations.cpp b/src/zenremotestore/builds/buildstorageoperations.cpp
index ef180935e..431f1caf3 100644
--- a/src/zenremotestore/builds/buildstorageoperations.cpp
+++ b/src/zenremotestore/builds/buildstorageoperations.cpp
@@ -652,6 +652,7 @@ BuildsOperationUpdateFolder::Execute(FolderContent& OutLocalFolderState)
 		WriteChunks,
 		PrepareTarget,
 		FinalizeTarget,
+		Cleanup,
 		StepCount
 	};
 
@@ -2282,6 +2283,8 @@ BuildsOperationUpdateFolder::Execute(FolderContent& OutLocalFolderState)
 		}
 	}
 
+	m_LogOutput.SetLogOperationProgress(TaskSteps::FinalizeTarget, TaskSteps::StepCount);
+
 	if (m_Options.WipeTargetFolder)
 	{
 		ZEN_TRACE_CPU("WipeTarget");
@@ -2303,8 +2306,6 @@ BuildsOperationUpdateFolder::Execute(FolderContent& OutLocalFolderState)
 	{
 		ZEN_TRACE_CPU("FinalizeTree");
 
-		m_LogOutput.SetLogOperationProgress(TaskSteps::FinalizeTarget, TaskSteps::StepCount);
-
 		Stopwatch Timer;
 
 		std::unique_ptr<BuildOpLogOutput::ProgressBar> RebuildProgressBarPtr(m_LogOutput.CreateProgressBar("Rebuild State"));
@@ -2611,6 +2612,7 @@ BuildsOperationUpdateFolder::Execute(FolderContent& OutLocalFolderState)
 		m_RebuildFolderStateStats.FinalizeTreeElapsedWallTimeUs = Timer.GetElapsedTimeUs();
 		RebuildProgressBar.Finish();
 	}
+	m_LogOutput.SetLogOperationProgress(TaskSteps::Cleanup, TaskSteps::StepCount);
 }
 
 void
@@ -4578,8 +4580,10 @@ BuildsOperationUploadFolder::Execute()
 	{
 		PrepareBuild,
 		CalculateDelta,
+		GenerateBlocks,
+		BuildPartManifest,
 		Upload,
-		//		Validate,
+		FinalizeBuild,
 		Cleanup,
 		StepCount
 	};
@@ -4945,10 +4949,9 @@ BuildsOperationUploadFolder::Execute()
 				   NiceTimeSpanMs(BlockArrangeTimer.GetElapsedTimeMs()));
 	}
 
+	m_LogOutput.SetLogOperationProgress(TaskSteps::GenerateBlocks, TaskSteps::StepCount);
 	GeneratedBlocks NewBlocks;
 
-	m_LogOutput.SetLogOperationProgress(TaskSteps::Upload, TaskSteps::StepCount);
-
 	if (!NewBlockChunks.empty())
 	{
 		Stopwatch GenerateBuildBlocksTimer;
@@ -4971,6 +4974,8 @@ BuildsOperationUploadFolder::Execute()
 		GenerateBuildBlocks(LocalContent, LocalLookup, NewBlockChunks, NewBlocks);
 	}
 
+	m_LogOutput.SetLogOperationProgress(TaskSteps::BuildPartManifest, TaskSteps::StepCount);
+
 	CbObject PartManifest;
 	{
 		CbObjectWriter PartManifestWriter;
@@ -5128,6 +5133,8 @@ BuildsOperationUploadFolder::Execute()
 		PartManifest = PartManifestWriter.Save();
 	}
 
+	m_LogOutput.SetLogOperationProgress(TaskSteps::Upload, TaskSteps::StepCount);
+
 	Stopwatch							   PutBuildPartResultTimer;
 	std::pair<IoHash, std::vector<IoHash>> PutBuildPartResult =
 		m_Storage.BuildStorage->PutBuildPart(m_BuildId, m_BuildPartId, m_BuildPartName, PartManifest);
@@ -5244,6 +5251,8 @@ BuildsOperationUploadFolder::Execute()
 		UploadAttachments(Needs);
 	}
 
+	m_LogOutput.SetLogOperationProgress(TaskSteps::FinalizeBuild, TaskSteps::StepCount);
+
 	if (m_CreateBuild && !m_AbortFlag)
 	{
 		Stopwatch FinalizeBuildTimer;