Created attachment 293690 [details] [review]
packagedb: properly terminate manifest files

Manifest files are missing the final newline.  Add it.

Created attachment 293691 [details] [review]
packagedb: remove ancient compat code

Nobody stores manifests in .xml anymore.

Created attachment 293692 [details] [review]
packagedb: write only one manifest

Because of the fact that the 'manifest' attribute on PackageEntry items
is a property and because of the fact that we check it each time we
convert a node to XML, we end up reading every manifest file every time
we go to update the packagedb (ie: every time we install a package).

Even worse: because self.manifest is now populated, we also write every
single manifest file each time we update the packagedb.

Avoid the problem by making the write explicit for the module being
added, and no other.

Created attachment 293693 [details] [review]
packagedb: remove manifest files on uninstall

These have been left dangling.  Make sure we delete them.

Created attachment 293694 [details] [review]
packagedb: move safe file writing to fileutils

Add a SafeWriter class to fileutils that handles the usual dance of
creating a tmpfile, writing to it, flushing it, fsyncing it, closing it
and renaming it.

Use it from the two locations that were doing this in packagedb.

Created attachment 293695 [details] [review]
packagedb: remove unused parameter

Review of attachment 293690 [details] [review]:

Ok.

Review of attachment 293691 [details] [review]:

I'd like to see some at least very brief attempt at backing the assertion in the commit message; something like "The manifests were switched to external files ~3.5 years ago in bug 655417, and we expect jhbuild users to stay close to git master".

Review of attachment 293692 [details] [review]:

Looks fine to me.

Review of attachment 293693 [details] [review]:

::: jhbuild/utils/packagedb.py
@@ +106,3 @@
 
+    def remove(self):
+        os.unlink(os.path.join(self.manifests_dir, self.package))

Would prefer a bit if we handled the ENOENT case, so we don't traceback if we're interrupted.

Review of attachment 293694 [details] [review]:

Definitely a nice cleanup.

Review of attachment 293695 [details] [review]:

OK.

Created attachment 293724 [details] [review]
packagedb: remove manifest files on uninstall

These have been left dangling.  Make sure we delete them.

Add a ensure_unlinked() function to fileutils to help with the job.

Review of attachment 293724 [details] [review]:

Yep.

Attachment 293690 [details] pushed as 6686953 - packagedb: properly terminate manifest files
Attachment 293691 [details] pushed as e385291 - packagedb: remove ancient compat code
Attachment 293692 [details] pushed as c2904cd - packagedb: write only one manifest
Attachment 293694 [details] pushed as 62a7097 - packagedb: move safe file writing to fileutils
Attachment 293695 [details] pushed as a83b728 - packagedb: remove unused parameter
Attachment 293724 [details] pushed as 5dd4fd9 - packagedb: remove manifest files on uninstall