Skip to content
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.

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

Support toml markup for Platform struct #1073

Open
kiashok opened this issue Jun 6, 2023 · 5 comments
Open

Support toml markup for Platform struct #1073

kiashok opened this issue Jun 6, 2023 · 5 comments

Comments

@kiashok
Copy link

kiashok commented Jun 6, 2023

It would be very helpful to have toml markup for Platform struct in https://github.com/opencontainers/image-spec/blob/main/specs-go/v1/descriptor.go#L53 .
For example, we are trying to add an option of Platform type in containerd's config.toml and toml markup for this struct would be handy:

// Platform describes the platform which the image in the manifest runs on.
type Platform struct {
	// Architecture field specifies the CPU architecture, for example
	// `amd64` or `ppc64le`.
	Architecture string `toml:"architecture" json:"architecture"`

	// OS specifies the operating system, for example `linux` or `windows`.
	OS string `toml:"os" json:"os"`

	// OSVersion is an optional field specifying the operating system
	// version, for example on Windows `10.0.14393.1066`.
	OSVersion string `toml:"os_version" json:"os.version,omitempty"`

	// OSFeatures is an optional field specifying an array of strings,
	// each listing a required OS feature (for example on Windows `win32k`).
	OSFeatures []string `toml:"os_features" json:"os.features,omitempty"`

	// Variant is an optional field specifying a variant of the CPU, for
	// example `v7` to specify ARMv7 when architecture is `arm`.
	Variant string `toml:"variant" json:"variant,omitempty"`
}

Thoughts?

@tianon
Copy link
Member

tianon commented Jun 12, 2023

I worry that this might perhaps encourage other formats to also request inclusion in our tags here, and I'm not sure it makes sense to maintain all that here 😅 I think this is an unfortunate limitation of Go's struct tagging conventions 😞

Is there a way to convince toml to read the json tags, perhaps?

@sparr
Copy link
Contributor

sparr commented Jun 13, 2023

Both go-toml and encoding/json support encoding and decoding without tags if the markup name for a field is the same as the struct name, and a case-insensitive match is the fallback if a case-sensitive match is missing.

That is, the existing lack of tags should be exactly equivalent to the following (json omitted for clarity):

type Platform struct {
	Architecture string `toml:"Architecture"`
	OS string `toml:"OS"`
	OSVersion string `toml:"OSVersion"`
	OSFeatures []string `toml:"OSFeatures"`
	Variant string `toml:"Variant"`
}

and compatible with the following which differs from your desired use case only in lack of underscores:

type Platform struct {
	Architecture string `toml:"architecture"`
	OS string `toml:"os"`
	OSVersion string `toml:"osversion"`
	OSFeatures []string `toml:"osfeatures"`
	Variant string `toml:"variant"`
}

@kiashok
Copy link
Author

kiashok commented Jun 13, 2023

I worry that this might perhaps encourage other formats to also request inclusion in our tags here, and I'm not sure it makes sense to maintain all that here 😅 I think this is an unfortunate limitation of Go's struct tagging conventions 😞

Is there a way to convince toml to read the json tags, perhaps?

I think toml is pretty standard and used all over containerd and k8s codebase so would it be ok to support that here as well?

@kiashok
Copy link
Author

kiashok commented Jun 13, 2023

Both go-toml and encoding/json support encoding and decoding without tags if the markup name for a field is the same as the struct name

Not sure I understand this - "Both go-toml and encoding/json support encoding and decoding without tags if the markup name for a field is the same as the struct name" .
Does it mean we don't need explicit toml markups if we reference the struct members as is (case sensitive) from a go file?

@sparr
Copy link
Contributor

sparr commented Jun 13, 2023

Yes. It also works case-insensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants