Commit history matching Buildkit's Dockerfile2LLB logic

[hinshun]

Some external systems depends on history to exist for things like labels.

For context, the history comment from buildkit is buildkit.dockerfile.v0.

Example:

  "history": [
    {
      "created": "2021-03-31T20:10:06.686359124Z",
      "created_by": "/bin/sh -c #(nop) ADD file:7119167b56ff1228b2fb639c768955ce9db7a999cd947179240b216dfa5ccbb9 in / "
    },
    {
      "created": "2021-03-31T20:10:06.934368604Z",
      "created_by": "/bin/sh -c #(nop)  CMD [\"/bin/sh\"]",
      "empty_layer": true
    },
    {
      "created": "2021-03-31T20:10:06.934368604Z",
      "created_by": "LABEL image-name=foo",
      "comment": "hlb.v0",
      "empty_layer": true
    },
    {
      "created": "2021-03-31T20:10:06.934368604Z",
      "created_by": "RUN /bin/sh -c sleep 1",
      "comment": "hlb.v0",
      "empty_layer": true
    },
    {
      "created": "2021-03-31T20:10:06.934368604Z",
      "created_by": "LABEL image-name=bar",
      "comment": "hlb.v0",
      "empty_layer": true
    }
  ],

[aaronlehmann]

Is it correct for EmptyLayer to always be true? Is the implication that the downstream tools don't really pay attention to it? (Still, wondering if false would be safer.)

[hinshun]

EmptyLayer should be false for RUN, COPY, ADD, but true for everything else. Let me see if I can add history for copy as well.

[aaronlehmann]

Can you explain why we replace rather than append the label in this case? It might be worth a comment in the code.

[hinshun]

In Dockerfile it is LABEL key=value key2=value2 ... whereas in HLB it is fs label(string key, string value) singular. So we need to merge to produce the same history.

I also tested LABEL samekey=foo samekey=bar with Dockerfile and it does show up as is in the history without deduplication.

Will add a comment.

[aaronlehmann]

Ah, I missed that we were including the existing contents. How about this to make it slightly more obvious and succinct:

Suggested change

	fs.Image.History[numHistory-1].CreatedBy = fmt.Sprintf("%s %s=%s", fs.Image.History[numHistory-1].CreatedBy, key, value)
	fs.Image.History[numHistory-1].CreatedBy += fmt.Sprintf(" %s=%s", key, value)

[aaronlehmann]

LGTM

Only other thought is whether there's a convenient place to add test assertions that the correct history entries are generate...

[hinshun]

Filed an issue tracking history tests: #236

[coryb]

lgtm