libimage: fix computing history

libimage/history.go

@@ -2,9 +2,8 @@ package libimage
 
 import (
 	"context"
+	"fmt"
 	"time"
-
-	"github.com/containers/storage"
 )
 
 // ImageHistory contains the history information of an image.
@@ -29,17 +28,19 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
 		return nil, err
 	}
 
-	var allHistory []ImageHistory
-	var layer *storage.Layer
+	var nextNode *layerNode
 	if i.TopLayer() != "" {
-		layer, err = i.runtime.store.Layer(i.TopLayer())
+		layer, err := i.runtime.store.Layer(i.TopLayer())
 		if err != nil {
 			return nil, err
 		}
+		nextNode = layerTree.node(layer.ID)
 	}
 
 	// Iterate in reverse order over the history entries, and lookup the
-	// corresponding image ID, size and get the next later if needed.
+	// corresponding image ID, size.  If it's a non-empty history entry,
+	// pick the next "storage" layer by walking the layer tree.
+	var allHistory []ImageHistory
 	numHistories := len(ociImage.History) - 1
 	usedIDs := make(map[string]bool) // prevents assigning images IDs more than once
 	for x := numHistories; x >= 0; x-- {
@@ -50,31 +51,25 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
 			Comment:   ociImage.History[x].Comment,
 		}
 
-		if layer != nil {
-			if !ociImage.History[x].EmptyLayer {
-				history.Size = layer.UncompressedSize
+		if nextNode != nil && len(nextNode.images) > 0 {
+			id := nextNode.images[0].ID() // always use the first one
+			if _, used := usedIDs[id]; !used {
+				history.ID = id
+				usedIDs[id] = true
 			}
-			// Query the layer tree if it's the top layer of an
-			// image.
-			node := layerTree.node(layer.ID)
-			if len(node.images) > 0 {
-				id := node.images[0].ID() // always use the first one
-				if _, used := usedIDs[id]; !used {
-					history.ID = id
-					usedIDs[id] = true
-				}
-				for i := range node.images {
-					history.Tags = append(history.Tags, node.images[i].Names()...)
-				}
+			for i := range nextNode.images {
+				history.Tags = append(history.Tags, nextNode.images[i].Names()...)
 			}
-			if layer.Parent == "" {
-				layer = nil
-			} else if !ociImage.History[x].EmptyLayer {
-				layer, err = i.runtime.store.Layer(layer.Parent)
-				if err != nil {
-					return nil, err
-				}
+		}
+
+		if !ociImage.History[x].EmptyLayer {
+			if nextNode == nil { // If no layer's left, something's wrong.
+				return nil, fmt.Errorf("no layer left for non-empty history entry: %v", history)
 			}
+
+			history.Size = nextNode.layer.UncompressedSize
+
+			nextNode = nextNode.parent
 		}
 
 		allHistory = append(allHistory, history)

libimage/history_test.go

@@ -24,5 +24,7 @@ func TestHistory(t *testing.T) {
 	require.Len(t, history, 2)
 
 	require.Equal(t, []string{name}, history[0].Tags)
-	require.Len(t, history[1].Tags, 0)
+	require.Equal(t, history[1].Tags, []string{name})
+	require.NotEqual(t, history[0].Size, 0)
+	require.NotEqual(t, history[1].Size, 0)
 }