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

pack buildpack new should accept --targets #1918

Closed
1 task done
natalieparellano opened this issue Sep 22, 2023 · 7 comments · Fixed by #1921
Closed
1 task done

pack buildpack new should accept --targets #1918

natalieparellano opened this issue Sep 22, 2023 · 7 comments · Fixed by #1921
Labels
good first issue A good first issue to get started with. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.

Comments

@natalieparellano
Copy link
Member

Description

pack buildpack new accepts --stacks but stacks are deprecated. It should accept --targets and print a warning when --stacks are used.

Proposed solution

From github.com/buildpacks/lifecycle/buildpack package - the structure of targets is a list of

type TargetMetadata struct {
	OS          string     `json:"os" toml:"os"`
	Arch        string     `json:"arch" toml:"arch"`
	ArchVariant string     `json:"arch-variant,omitempty" toml:"arch-variant"`
	Distros     []OSDistro `json:"distros,omitempty" toml:"distros"`
}
type OSDistro struct {
	Name    string `json:"name" toml:"name"`
	Version string `json:"version" toml:"version"`
}

For each target, all fields are optional (but at least one should be specified).

Additional context

  • This feature should be documented somewhere - we should update our use of pack buildpack new in the docs to include targets
@natalieparellano natalieparellano added type/enhancement Issue that requests a new feature or improvement. good first issue A good first issue to get started with. status/ready Issue ready to be worked on. labels Sep 22, 2023
@WYGIN
Copy link
Contributor

WYGIN commented Sep 25, 2023

/assign

@WYGIN
Copy link
Contributor

WYGIN commented Sep 25, 2023

Description

pack buildpack new accepts --stacks but stacks are deprecated. It should accept --targets and print a warning when --stacks are used.

Proposed solution

From github.com/buildpacks/lifecycle/buildpack package - the structure of targets is a list of

type TargetMetadata struct {
	OS          string     `json:"os" toml:"os"`
	Arch        string     `json:"arch" toml:"arch"`
	ArchVariant string     `json:"arch-variant,omitempty" toml:"arch-variant"`
	Distros     []OSDistro `json:"distros,omitempty" toml:"distros"`
}
type OSDistro struct {
	Name    string `json:"name" toml:"name"`
	Version string `json:"version" toml:"version"`
}

For each target, all fields are optional (but at least one should be specified).

Additional context

  • This feature should be documented somewhere - we should update our use of pack buildpack new in the docs to include targets

type Target struct {
OS string `json:"os" toml:"os"`
Arch string `json:"arch" toml:"arch"`
Distributions []Distribution `json:"distributions,omitempty" toml:"distributions,omitempty"`
}
has targets already defined with similar type except the absence of ArchVariant & Distros &
Distribution having similar type as Distro but where a distro has type with single version with name & where Distribution has a single name with array of versions
if this is the type i need to work with should i make changes to the existing Target type ? & what about ArchVarient

@WYGIN
Copy link
Contributor

WYGIN commented Sep 25, 2023

@natalieparellano

Description

pack buildpack new accepts --stacks but stacks are deprecated. It should accept --targets and print a warning when --stacks are used.

Proposed solution

From github.com/buildpacks/lifecycle/buildpack package - the structure of targets is a list of

type TargetMetadata struct {
	OS          string     `json:"os" toml:"os"`
	Arch        string     `json:"arch" toml:"arch"`
	ArchVariant string     `json:"arch-variant,omitempty" toml:"arch-variant"`
	Distros     []OSDistro `json:"distros,omitempty" toml:"distros"`
}
type OSDistro struct {
	Name    string `json:"name" toml:"name"`
	Version string `json:"version" toml:"version"`
}

For each target, all fields are optional (but at least one should be specified).

Additional context

  • This feature should be documented somewhere - we should update our use of pack buildpack new in the docs to include targets

type Target struct {
OS string `json:"os" toml:"os"`
Arch string `json:"arch" toml:"arch"`
Distributions []Distribution `json:"distributions,omitempty" toml:"distributions,omitempty"`
}

has targets already defined with similar type except the absence of ArchVariant & Distros &
Distribution having similar type as Distro but where a distro has type with single version with name & where Distribution has a single name with array of versions
if this is the type i need to work with should i make changes to the existing Target type ? & what about ArchVarient

@natalieparellano need help

@jjbustamante
Copy link
Member

Hi @WYGIN

Thank you for working on this!

As Natalie mentioned, we have a similar structure in the lifecycle.

  • Yes, that's the struct you need to do your changes
  • Yes, we need to add ArchVariant to the struct
  • I think the reason of the difference you mentioned here:

but where a distro has type with single version with name & where Distribution has a single name with array of versions

is expected from pack side, if you checked this RFC what pack is doing is to create the structure for the buildpack.toml file with the new target field, and we are expecting a distribution to have more than one version.

@jjbustamante
Copy link
Member

jjbustamante commented Sep 25, 2023

@natalieparellano

I started thinking on this for the multi-arch support, but I called the flag platform I am ok with using targets.

I would like to suggest the following format [os][/arch][/variant]:[name@osversion] for the flag.

For example:

Example 1

Basic case for two different architectures

pack buildpack new [.. some flags ..] 
--targets "linux/amd64" --targets "linux/arm64" 

Output:

[[targets]]
os = "linux"
arch = "amd64"

[[targets]]
os = "linux"
arch = "arm64"

Example 2

Case for a distribution version, see how you could passthrough the distribution.name value

pack buildpack new [.. some flags ..]   --targets "windows/amd64:windows-nano@10.0.19041.1415"

Output:

[[targets]]
os = "windows"
arch = "amd64"

[[targets.distributions]]
name = "windows-nano"
versions = ["10.0.19041.1415"]

Example 3

Case for all the fields, and two different versions. not sure if we could provide a better UX.

pack buildpack new [.. some flags ..]   --targets "linux/arm/v6:ubuntu@14.04"  --targets "linux/arm/v6:ubuntu@16.04"

Output:

[[targets]]
os = "linux"
arch = "arm"
variant = "v6"
[[targets.distributions]]
name = "ubuntu"                              
versions = ["14.04", "16.04"]

This proposal is not final, I am just thinking on different use cases and we can define a good UX for it.

@WYGIN
Copy link
Contributor

WYGIN commented Sep 26, 2023

@jjbustamante i am trying to implement it but i have few doubts regarding the format [os][/arch][/variant]:[name@osversion].

  • if [os] is defined then only [/arch] can be defined ? if not should the user need to add / before the [arch]
  • if both [os] & [/arch] is defined then only [/variant] defined ?
  • should the [os] need to be defined to define distros?

i am thinking to distros have format name@version1@version2 to define multiple versions to the same distro name & name1@version1@version2;name2@version1@version2 to define multiple distros in a single target command along with the examples provided by you
& please point me the logic where these flags are used to customize the build output

@jjbustamante
Copy link
Member

Hi @WYGIN.

Remember the pack buildpack new command is being used once to create the basic scaffolding of a buildpack repository, if you run the command against an existing folder it will throw an error message saying the directory already exists.

I like your idea of using name1@version1@version2;name2@version1@version2 to define multiple distros, let's use it.

Assuming the format is defined as [os][/arch][/variant]:[name1@version1@version2;name2@version1@version2] the idea is to be flexible enough to allow users to fill the buildpack.toml when they are creating the scaffolding. I don't want to add a lot of validations because the command is just for filling the file.

Are your doubts about how to allow different combinations of the values in the flag, right?

  • if [os] is defined then only [/arch] can be defined ? if not should the user need to add / before the [arch]
  • if both [os] & [/arch] is defined then only [/variant] defined ?

The [os] and [arch] are defined in the flag or in an existent buildpack.toml?

  • should the [os] need to be defined to define distros?

I think so, at the end, we need to fill the .toml file with:

[[targets]]
os = "<operating system>"
arch = "<system architecture>"
variant = "<architecture variant>"
[[targets.distributions]]
name = "<distribution ID>"
versions = ["<distribution version>"]

I do not see how we can fill targets.distribution without having at least targets.os

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A good first issue to get started with. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants