Add ImageLayoutVersion const

[coolljt0725]

Add a ImageLayoutVersion = 1.0.0 const so implementation can
refer to rather than hard code in implementation just like
https://github.com/containers/image/blob/master/oci/layout/oci_dest.go#L173

if err := ioutil.WriteFile(d.ref.ociLayoutPath(), []byte(`{"imageLayoutVersion": "1.0.0"}`), 0644); err != nil {
        return err
}

Signed-off-by: Lei Jitang leijitang@huawei.com

ping @vbatts

[wking]

I'm usually against defining constants to replace string literals, and feel like those same “what do you gain?” arguments apply here too. Having the image-layout version auto-bump seems like it makes it easy to get into trouble.

For example if you bump your image-spec dependency and have ImageLayoutVersion shift to 1.1.0. But you haven't actually taught your image-layout logic about the new-in-1.1.0 feature, and so you miss silently ignore the important information that feature contains.

On the other hand, if you have hard-coded the version string locally, you're only likely to bump the string when you actually bump your implementation.

[coolljt0725]

@wking
I think the implementation should use a value defined in spec and we can know why we set to that value from the source code. I think We should define this as we do with Version, it's the user's choice to use it or not. And I don't quite understand your concerns, I don't think that's a problem. :)

/cc @runcom

[wking]

On Mon, Oct 24, 2016 at 11:04:57PM -0700, Lei Jitang wrote:

I think the implementation should use a value defined in spec and we can know why we set to that value from the source code.

I don't see how value discovery becomes easier by adding a variable. Are you aiming to make image-layout implementation easier for folks who don't read image-layout.md? I hope the set of implementers who haven't read that file is empty ;).

I think We should define this as we do with Version…

Version is strange (especially for image-spec), because each of the components are media typed or otherwise versioned independently. The repo-wide Version ends up being convenient shorthand that's not actually tied to any particular implementation code. So there may not be backing code to bump when Version changes, but there is backing code to bump when the image-layout version changes.

… it's the user's choice to use it or not.

Fair enough. I don't think defining this constant is useful, and sloppy implementers could get in trouble if they use it. But sloppy implementers are likely going to get in trouble anyway, and we're all consenting adults ;). If my case against the constant isn't convincing, I won't be terribly put out if the constant lands :p.

[vbatts]

So, i'm wondering whether this ought to have its own variable to be versioned outside of specs.Version?

[wking]

So, i'm wondering whether this ought to have its own variable to be versioned outside of specs.Version?

Isn't that what this PR is adding?

[vbatts]

no, it is adding an additional versioning, which presumably would then be independent of the spec version presented in the laid out files.

[wking]

Right. How is that different from the "outside of specs.Version" you're asking for?

Or was your original comment trying to say "I'm not sure we want independent version numbers for image-layout and the spec as a whole"? I think we do want independent versions, because I expect image-layout to be bumped less frequently.

[vbatts]

I'm not particularly in favor of it being versioned independently, but can understand the usefulness in that.
I've opened #434 to clarify the version.

[vbatts]

@coolljt0725 is this still needed?

[coolljt0725]

@vbatts yes, I think this still needed

[vbatts]

LGTM

[stevvooe]

LGTM