GNOME Bugzilla – Bug 750459
Never delete .commitmeta files
Last modified: 2015-11-23 19:33:47 UTC
Bit of a followup to https://github.com/GNOME/ostree/pull/94, where I mused: It occurs to me the patch doesn't handle the case of deleted detached metadata, such as deleting all GPG signatures from a commit which may also delete the .commitmeta file. The client will retain its .commitmeta file during a pull, even though it's been deleted on the server. But this seems like a pretty remote corner case, so maybe not worth worrying about right now. A simple solution to this is just never delete a .commitmeta file, even if its content is now empty. That way clients will pull the empty file and overwrite the previous .commitmeta content. Duh.
Created attachment 304658 [details] [review] Patch + test case
Created attachment 316101 [details] [review] Patch + test case Rebased patch. I forgot this was still outstanding. Covers a corner case, probably rare in practice, but still good to have.
Review of attachment 316101 [details] [review]: ::: src/ostree/ot-builtin-trivial-httpd.c @@ +274,3 @@ buffer_length, mapping, (GDestroyNotify)g_mapped_file_unref); + if (buffer->length > 0) Optional: We could hoist this conditional higher as well, right? No point creating the buffer only to free it.
(In reply to Colin Walters from comment #3) > Optional: We could hoist this conditional higher as well, right? No point > creating the buffer only to free it. Creating the buffer has the side-effect of taking ownership of the GMappedFile, but I can work around that with g_autoptr(GMappedFile). Do you want to see another patch?
Ah, right, good catch. Up to you either way, I don't see a need for another roundtrip review.
Thanks for the review. Pushed with change to avoid unnecessarily creating the SoupBuffer, but without leaking the GMappedFile. https://git.gnome.org/browse/ostree/commit/?id=df75fc232a3d9191be6e2393400a2eff9602f43a
Gah, forgot to include the extra changes in what got pushed. Followup commit: https://git.gnome.org/browse/ostree/commit/?id=74d8e5f15969e2bf832d060e18c7f8c9f3f11a55