Fix writing state manifests on Windows

3 files changed, 153 insertions, 14 deletions

diff --git a/NEWS b/NEWS
index 89852d6..d212a74 100644
--- a/NEWS
+++ b/NEWS
@@ -3,8 +3,9 @@ lilv (0.24.11) unstable;
   * Allow connecting ports to structures in Python
   * Fix potential memory error when joining filesystem paths
   * Fix unlikely undefined behavior when saving state
+  * Fix writing state manifests on Windows
 
- -- David Robillard <d@drobilla.net>  Tue, 01 Dec 2020 12:19:48 +0000
+ -- David Robillard <d@drobilla.net>  Thu, 17 Dec 2020 22:05:55 +0000
 
 lilv (0.24.10) stable;
 
diff --git a/src/state.c b/src/state.c
index 914ff5b..1174b74 100644
--- a/src/state.c
+++ b/src/state.c
@@ -914,7 +914,6 @@ add_state_to_manifest(LilvWorld*      lworld,
 	if (rfd) {
 		// Read manifest into model
 		SerdReader* reader = sord_new_reader(model, env, SERD_TURTLE, NULL);
-		lilv_flock(rfd, true, true);
 		serd_reader_read_file_handle(reader, rfd, manifest.buf);
 		serd_reader_free(reader);
 	}
@@ -955,20 +954,36 @@ add_state_to_manifest(LilvWorld*      lworld,
 	             serd_node_from_string(SERD_URI,
 	                                   USTR(lilv_node_as_string(plugin_uri))));
 
-	// Write manifest model to file
-	write_manifest(lworld, env, model, &manifest);
+	// Close manifest since we're done reading
+	if (rfd) {
+		fclose(rfd);
+	}
+
+	/* Re-open manifest for locked writing.  We need to do this because it may
+	   need to be truncated, and the file can only be open once on Windows. */
+
+	FILE* wfd = fopen(manifest_path, "w");
+	int   st  = 0;
+	if (!wfd) {
+		LILV_ERRORF("Failed to open %s for writing (%s)\n",
+		            manifest_path,
+		            strerror(errno));
+		st = 1;
+	}
+
+	SerdWriter* writer = ttl_file_writer(wfd, &manifest, &env);
+	lilv_flock(wfd, true, true);
+	sord_write(model, writer, NULL);
+	lilv_flock(wfd, false, true);
+	serd_writer_free(writer);
+	fclose(wfd);
 
 	sord_free(model);
 	serd_node_free(&file);
 	serd_node_free(&manifest);
 	serd_env_free(env);
 
-	if (rfd) {
-		lilv_flock(rfd, false, true);
-		fclose(rfd);
-	}
-
-	return 0;
+	return st;
 }
 
 static bool
@@ -1223,10 +1238,15 @@ lilv_state_save(LilvWorld*       world,
 	fclose(fd);
 
 	// Add entry to manifest
-	char* const manifest = lilv_path_join(abs_dir, "manifest.ttl");
-	add_state_to_manifest(world, state->plugin_uri, manifest, uri, path);
+	if (!ret) {
+		char* const manifest = lilv_path_join(abs_dir, "manifest.ttl");
+
+		ret = add_state_to_manifest(
+			world, state->plugin_uri, manifest, uri, path);
+
+		free(manifest);
+	}
 
-	free(manifest);
 	free(abs_dir);
 	free(path);
 	return ret;
diff --git a/test/test_state.c b/test/test_state.c
index 330aa47..8dd2f23 100644
--- a/test/test_state.c
+++ b/test/test_state.c
@@ -529,6 +529,42 @@ test_string_round_trip(void)
 	test_context_free(ctx);
 }
 
+static SerdStatus
+count_sink(void* const              handle,
+           const SerdStatementFlags flags,
+           const SerdNode* const    graph,
+           const SerdNode* const    subject,
+           const SerdNode* const    predicate,
+           const SerdNode* const    object,
+           const SerdNode* const    object_datatype,
+           const SerdNode* const    object_lang)
+{
+	size_t* const n_statements = (size_t*)handle;
+
+	++(*n_statements);
+
+	return SERD_SUCCESS;
+}
+
+static size_t
+count_statements(const char* path)
+{
+	size_t n_statements = 0;
+
+	SerdReader* reader = serd_reader_new(
+	    SERD_TURTLE, &n_statements, NULL, NULL, NULL, count_sink, NULL);
+
+	SerdNode uri =
+	    serd_node_new_file_uri((const uint8_t*)path, NULL, NULL, true);
+
+	assert(!serd_reader_read_file(reader, uri.buf));
+
+	serd_node_free(&uri);
+	serd_reader_free(reader);
+
+	return n_statements;
+}
+
 static void
 test_to_files(void)
 {
@@ -574,7 +610,7 @@ test_to_files(void)
 	char* const recfile_copy_1 = lilv_path_join(dirs.copy, "recfile");
 	assert(lilv_path_exists(recfile_copy_1));
 
-	// Set a label and save state to a bundle
+	// Save state to a bundle
 	assert(!lilv_state_save(ctx->env->world,
 	                        &ctx->map,
 	                        &ctx->unmap,
@@ -583,6 +619,13 @@ test_to_files(void)
 	                        bundle_1_path,
 	                        "state.ttl"));
 
+	// Check that a manifest exists
+	char* const manifest_path = lilv_path_join(bundle_1_path, "manifest.ttl");
+	assert(lilv_path_exists(manifest_path));
+
+	// Check that the expected statements are in the manifest file
+	assert(count_statements(manifest_path) == 3);
+
 	// Check that a link to the recfile exists in the saved bundle
 	char* const recfile_link_1 = lilv_path_join(bundle_1_path, "recfile");
 	assert(lilv_path_exists(recfile_link_1));
@@ -633,6 +676,7 @@ test_to_files(void)
 	free(bundle_2_path);
 	free(recfile_link_1_real);
 	free(recfile_link_1);
+	free(manifest_path);
 	free(recfile_copy_1);
 	lilv_state_free(state_1);
 	free(bundle_1_path);
@@ -642,6 +686,79 @@ test_to_files(void)
 }
 
 static void
+test_multi_save(void)
+{
+	TestContext* const      ctx    = test_context_new();
+	TestDirectories         dirs   = create_test_directories();
+	const LilvPlugin* const plugin = load_test_plugin(ctx);
+
+	LV2_State_Make_Path make_path         = {&dirs, make_scratch_path};
+	LV2_Feature         make_path_feature = {LV2_STATE__makePath, &make_path};
+
+	const LV2_Feature* const instance_features[] = {&ctx->map_feature,
+	                                                &ctx->free_path_feature,
+	                                                &make_path_feature,
+	                                                NULL};
+
+	LilvInstance* const instance =
+	    lilv_plugin_instantiate(plugin, 48000.0, instance_features);
+
+	assert(instance);
+
+	// Get state
+	char* const      bundle_1_path = lilv_path_join(dirs.top, "state1.lv2");
+	LilvState* const state_1 =
+	    state_from_instance(plugin, instance, ctx, &dirs, bundle_1_path);
+
+	// Save state to a bundle
+	assert(!lilv_state_save(ctx->env->world,
+	                        &ctx->map,
+	                        &ctx->unmap,
+	                        state_1,
+	                        "http://example.org/state1",
+	                        bundle_1_path,
+	                        "state.ttl"));
+
+	// Check that a manifest exists
+	char* const manifest_path = lilv_path_join(bundle_1_path, "manifest.ttl");
+	assert(lilv_path_exists(manifest_path));
+
+	// Check that the state file exists
+	char* const state_path = lilv_path_join(bundle_1_path, "state.ttl");
+	assert(lilv_path_exists(state_path));
+
+	// Check that the expected statements are in the files
+	assert(count_statements(manifest_path) == 3);
+	assert(count_statements(state_path) == 21);
+
+	// Save state again to the same bundle
+	assert(!lilv_state_save(ctx->env->world,
+	                        &ctx->map,
+	                        &ctx->unmap,
+	                        state_1,
+	                        "http://example.org/state1",
+	                        bundle_1_path,
+	                        "state.ttl"));
+
+	// Check that everything is the same
+	assert(lilv_path_exists(manifest_path));
+	assert(lilv_path_exists(state_path));
+	assert(count_statements(manifest_path) == 3);
+	assert(count_statements(state_path) == 21);
+
+	lilv_dir_for_each(bundle_1_path, NULL, remove_file);
+	lilv_remove(bundle_1_path);
+	cleanup_test_directories(dirs);
+
+	free(state_path);
+	free(manifest_path);
+	lilv_state_free(state_1);
+	free(bundle_1_path);
+	lilv_instance_free(instance);
+	test_context_free(ctx);
+}
+
+static void
 test_files_round_trip(void)
 {
 	TestContext* const      ctx    = test_context_new();
@@ -995,6 +1112,7 @@ main(void)
 	test_to_string();
 	test_string_round_trip();
 	test_to_files();
+	test_multi_save();
 	test_files_round_trip();
 	test_world_round_trip();
 	test_label_round_trip();