tests/sizes: Check duplicate file doesn't add sizes entry

A duplicate file will resolve to the same object, so it shouldn't add
any entries to the sizes metadata.

src/libostree/ostree-repo-commit.c

@@ -365,15 +365,38 @@ repo_setup_generate_sizes (OstreeRepo               *self,
 }
 
 static void
-repo_store_size_entry (OstreeRepo       *self,
+repo_ensure_size_entries (OstreeRepo *self)
-                       const gchar      *checksum,
-                       goffset           unpacked,
-                       goffset           archived)
 {
   if (G_UNLIKELY (self->object_sizes == NULL))
     self->object_sizes = g_hash_table_new_full (g_str_hash, g_str_equal,
                                                 g_free, content_size_cache_entry_free);
+}
 
+static gboolean
+repo_has_size_entry (OstreeRepo       *self,
+                     OstreeObjectType  objtype,
+                     const gchar      *checksum)
+{
+  /* Only file, dirtree and dirmeta objects appropriate for size metadata */
+  if (objtype > OSTREE_OBJECT_TYPE_DIR_META)
+    return TRUE;
+
+  repo_ensure_size_entries (self);
+  return (g_hash_table_lookup (self->object_sizes, checksum) != NULL);
+}
+
+static void
+repo_store_size_entry (OstreeRepo       *self,
+                       OstreeObjectType  objtype,
+                       const gchar      *checksum,
+                       goffset           unpacked,
+                       goffset           archived)
+{
+  /* Only file, dirtree and dirmeta objects appropriate for size metadata */
+  if (objtype > OSTREE_OBJECT_TYPE_DIR_META)
+    return;
+
+  repo_ensure_size_entries (self);
   g_hash_table_replace (self->object_sizes,
                         g_strdup (checksum),
                         content_size_cache_entry_new (unpacked, archived));
@@ -1031,6 +1054,11 @@ write_content_object (OstreeRepo         *self,
 
           unpacked_size = g_file_info_get_size (file_info);
         }
+      else
+        {
+          /* For a symlink, the size is the length of the target */
+          unpacked_size = strlen (g_file_info_get_symlink_target (file_info));
+        }
 
       if (!g_output_stream_flush (temp_out, cancellable, error))
         return FALSE;
@@ -1061,16 +1089,16 @@ write_content_object (OstreeRepo         *self,
   g_assert (actual_checksum != NULL); /* Pacify static analysis */
 
   /* Update size metadata if configured and entry missing */
-  if (self->generate_sizes && object_file_type == G_FILE_TYPE_REGULAR &&
+  if (self->generate_sizes &&
-      (self->object_sizes == NULL ||
+      !repo_has_size_entry (self, OSTREE_OBJECT_TYPE_FILE, actual_checksum))
-       g_hash_table_lookup (self->object_sizes, actual_checksum) == NULL))
     {
       struct stat stbuf;
 
       if (!glnx_fstat (tmpf.fd, &stbuf, error))
         return FALSE;
 
-      repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size);
+      repo_store_size_entry (self, OSTREE_OBJECT_TYPE_FILE, actual_checksum,
+                             unpacked_size, stbuf.st_size);
     }
 
   /* See whether or not we have the object, now that we know the
@@ -1329,6 +1357,11 @@ write_metadata_object (OstreeRepo         *self,
        */
       if (have_obj)
         {
+          /* Update size metadata if needed */
+          if (self->generate_sizes &&
+              !repo_has_size_entry (self, objtype, actual_checksum))
+            repo_store_size_entry (self, objtype, actual_checksum, len, len);
+
           g_mutex_lock (&self->txn_lock);
           self->txn.stats.metadata_objects_total++;
           g_mutex_unlock (&self->txn_lock);
@@ -1350,6 +1383,11 @@ write_metadata_object (OstreeRepo         *self,
   gsize len;
   const guint8 *bufp = g_bytes_get_data (buf, &len);
 
+  /* Update size metadata if needed */
+  if (self->generate_sizes &&
+      !repo_has_size_entry (self, objtype, actual_checksum))
+    repo_store_size_entry (self, objtype, actual_checksum, len, len);
+
   /* Write the metadata to a temporary file */
   g_auto(GLnxTmpfile) tmpf = { 0, };
   if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC,
@@ -2365,6 +2403,16 @@ ostree_repo_write_metadata (OstreeRepo         *self,
         return FALSE;
       if (have_obj)
         {
+          /* Update size metadata if needed */
+          if (self->generate_sizes &&
+              !repo_has_size_entry (self, objtype, expected_checksum))
+            {
+              /* Make sure we have a fully serialized object */
+              g_autoptr(GVariant) trusted = g_variant_get_normal_form (object);
+              gsize size = g_variant_get_size (trusted);
+              repo_store_size_entry (self, objtype, expected_checksum, size, size);
+            }
+
           if (out_csum)
             *out_csum = ostree_checksum_to_bytes (expected_checksum);
           return TRUE;

tests/test-sizes.js

@@ -123,6 +123,10 @@ print('1..3')
 let testDataDir = Gio.File.new_for_path('test-data');
 testDataDir.make_directory(null);
 testDataDir.get_child('some-file').replace_contents("hello world!", null, false, 0, null);
+testDataDir.get_child('some-file').copy(testDataDir.get_child('duplicate-file'),
+                                        Gio.FileCopyFlags.OVERWRITE,
+                                        null, null);
+testDataDir.get_child('link-file').make_symbolic_link('some-file', null);
 testDataDir.get_child('another-file').replace_contents("hello world again!", null, false, 0, null);
 
 let repoPath = Gio.File.new_for_path('repo');
@@ -152,15 +156,21 @@ repo.commit_transaction(null);
 let expectedFiles = {
     'f5ee222a21e2c96edbd6f2543c4bc8a039f827be3823d04777c9ee187778f1ad': [54, 18],
     'd35bfc50864fca777dbeead3ba3689115b76674a093210316589b1fe5cc3ff4b': [48, 12],
+    '8322876a078e79d8c960b8b4658fe77e7b2f878f8a6cf89dbb87c6becc8128a0': [43, 9],
+    '1c77033ca06eae77ed99cb26472969964314ffd5b4e4c0fd3ff6ec4265c86e51': [185, 185],
+    '446a0ef11b7cc167f3b603e585c7eeeeb675faa412d5ec73f62988eb0b6c5488': [12, 12],
 };
 validateSizes(repo, commit, expectedFiles);
 
 print("ok test-sizes");
 
 // Remove a file to make sure that metadata is not reused from the
-// previous commit
+// previous commit. Remove that file from the expected metadata and
+// replace the dirtree object.
 testDataDir.get_child('another-file').delete(null);
 delete expectedFiles['f5ee222a21e2c96edbd6f2543c4bc8a039f827be3823d04777c9ee187778f1ad'];
+delete expectedFiles['1c77033ca06eae77ed99cb26472969964314ffd5b4e4c0fd3ff6ec4265c86e51'];
+expectedFiles['a384660cc18ffdb60296961dde9a2d6f78f4fec095165652cb53aa81f6dc7539'] = [138, 138];
 
 repo.prepare_transaction(null);
 mtree = OSTree.MutableTree.new();