[PATCH 2/3] Make CachedRepo.resolve_ref handle non-existent SHA1

Richard Maw richard.maw at codethink.co.uk
Wed Oct 23 17:46:39 BST 2013


On Wed, Oct 23, 2013 at 05:29:40PM +0100, Lars Wirzenius wrote:
> ---
>  morphlib/cachedrepo.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/morphlib/cachedrepo.py b/morphlib/cachedrepo.py
> index 0a085bb..cbac787 100644
> --- a/morphlib/cachedrepo.py
> +++ b/morphlib/cachedrepo.py
> @@ -126,7 +126,10 @@ class CachedRepo(object):
>          except cliapp.AppException:
>              raise InvalidReferenceError(self, ref)
>  
> -        tree = self._show_tree_hash(absref)
> +        try:
> +            tree = self._show_tree_hash(absref)
> +        except cliapp.AppException:
> +            raise InvalidReferenceError(self, ref)
>          return absref, tree
>  
>      def cat(self, ref, filename):

Instead of just adding the check that the tree returns a valid reference,
I would have changed the initial rev parse from

    git rev-parse --verify $ref

to

    git rev-parse --verify $ref^{commit}

Since to me, it is clearly the intention that the rev-parse return a
valid commit, or fail entirely.

I would also like _show_tree_hash to be changed to do

    git rev-parse --verify $commit^{tree}

instead of messing about with formatting git log, but that's only a bug
if we find out using git log like that is too slow.


I'm not convinced that leaving the original rev-parse to falsely succeed
if the commit exists is a good idea, so I'm voting +0.



More information about the baserock-dev mailing list