[Bug 797177] Require tarball checksum verification for all recipes

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=797177

--- Comment #7 from Nirbheek Chauhan <nirbheek.chauhan at gmail.com> ---
(In reply to Nicolas Dufresne (ndufresne) from comment #4)
> Review of attachment 373707 [details] [review]:
> 
> Could have been just name "checksum" instead of "tarball_checksum" ... haha,
> kidding, I don't really care. Note, I didn't recalculate all the checksum :-D


Just to entertain the bikeshed for a moment, I thought `tarball_checksum` would
be good alongside `tarball_dirname` and `tarball_name`. :)

(In reply to Nicolas Dufresne (ndufresne) from comment #6)
> Review of attachment 373706 [details] [review]:
> 
> ::: cerbero/build/source.py
> @@ +128,3 @@
>          cached_file = os.path.join(self.config.cached_sources,
>                                     self.package_name, self.tarball_name)
> +        if not redownload and os.path.isfile(cached_file) and
> self.verify(cached_file, fatal=False):
> 
> I'm wondering if checking the cache won't be quite an overhead for the
> safety. What we could do instead is download to <filename>.unchecked, and if
> it succeed validate, move it back to it's real name. And then just trust the
> cache.
> 

It's actually not a lot of overhead at all. It takes <2 seconds to verify all
the tarball checksums in gstreamer-1.0, which is a trivial amount of time
compared to building stuff. I tested with: 

time ./cerbero-uninstalled fetch-package gstreamer-1.0

So I think it's good to keep the implementation simple. It's also a good way to
know and recover easily if a file in your cache got corrupted somehow.

> @@ +174,3 @@
> +        if checksum != self.tarball_checksum:
> +            movedto = fname + '.failed-checksum'
> +            os.replace(fname, movedto)
> 
> I'm not familiar with replace, does it override if there is already a
> .failed-checksum file ? Just asking if we need to remove ancient failed
> download first.

Yes, os.replace is a cross-platform API to overwrite the existing file:
https://docs.python.org/3/library/os.html#os.replace

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.