Skip to content

image-index: define platform.variant for arm #650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 17, 2017
Merged

image-index: define platform.variant for arm #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion image-index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,16 @@ For the media type(s) that this document is compatible with, see the [matrix][ma

- **`variant`** *string*

This OPTIONAL property specifies the variant of the CPU, for example `armv6l` to specify a particular CPU variant of the ARM CPU.
This OPTIONAL property specifies the variant of the CPU.
Image indexes SHOULD use, and implementations SHOULD understand, values listed in the following table.
When the variant of the CPU is not listed in the table, values are implementation-defined and SHOULD be submitted to this specification for standardization.

| ISA/ABI | `architecture` | `variant` |
|-----------------|----------------|-------------|
| ARM 32-bit, v6 | `arm` | `v6` |
| ARM 32-bit, v7 | `arm` | `v7` |
| ARM 32-bit, v8 | `arm` | `v8` |
| ARM 64-bit, v8 | `arm64` | `v8` |
Copy link
Contributor

@hqhq hqhq May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we scope arch and variant with this table, and remove features, we can't identify platforms with ilp32 support, are there other issues tracing this?

And is variant really ARM specific property?

Copy link
Contributor

@stevvooe stevvooe May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see https://golang.org/src/go/build/syslist.go for extra architecture values. I think ilp32 will be arm64p32. It is not here because it has low adoption at this point.

And is variant really ARM specific property?

It is not. Where did you see this implied?

Copy link

@imrehg imrehg May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to include a v5 variant? armv5/armel is still supported by go, and would be great to cover all options (and it's in use for some devices at us at resin.io)


- **`features`** *array of strings*

Expand Down
6 changes: 3 additions & 3 deletions schema/backwards_compatibility_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
fail bool
}{
{
digest: "sha256:219f4b61132fe9d09b0ec5c15517be2ca712e4744b0e0cc3be71295b35b2a467",
digest: "sha256:d0ed7cfe33821cb6a15624486e650149e92fff3192ff2014bda0c4b0206c1aa2",
imageIndex: `{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
"platform": {
"architecture": "arm",
Copy link
Contributor

@mattspencer-arm mattspencer-arm Apr 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that "arm" is what gets expressed by GOARCH, which is why it is used here. But would it not be better to disambiguate the property by calling it "arm32"?

I am not sure if this is the right place to add this, but I also think that for completeness we ought to have arch entries for "arm32", "arm64" and "arm64ilp32"?

Copy link
Member Author

@AkihiroSuda AkihiroSuda Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is defined by runtime-spec, and I don't think we can change anymore.
https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc3/config.md#platform

Copy link
Contributor

@mattspencer-arm mattspencer-arm Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, and as a result of a historical decisions.
ARM would rather see the use of arm32 to reference their 32 bit instruction set rather than the company name - which could be considered ambiguous?

Copy link
Contributor

@stevvooe stevvooe May 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARM would rather see the use of arm32 to reference their 32 bit instruction set rather than the company name - which could be considered ambiguous?

Please take this up with the Go team.

"os": "linux",
"variant": "armv7"
"variant": "v7"
}
},
{
Expand All @@ -101,7 +101,7 @@ func TestBackwardsCompatibilityImageIndex(t *testing.T) {
"platform": {
"architecture": "arm64",
"os": "linux",
"variant": "armv8"
"variant": "v8"
}
}
]
Expand Down
2 changes: 1 addition & 1 deletion specs-go/v1/descriptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Platform struct {
OSFeatures []string `json:"os.features,omitempty"`

// Variant is an optional field specifying a variant of the CPU, for
// example `ppc64le` to specify a little-endian version of a PowerPC CPU.
// example `v7` to specify ARMv7 when architecture is `arm`.
Variant string `json:"variant,omitempty"`

// Features is an optional field specifying an array of strings, each
Expand Down