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

Providing a set of values to a single feature option #57

Open
joshspicer opened this issue Jun 27, 2022 · 7 comments
Open

Providing a set of values to a single feature option #57

joshspicer opened this issue Jun 27, 2022 · 7 comments
Labels
proposal Still under discussion, collecting feedback

Comments

@joshspicer
Copy link
Member

joshspicer commented Jun 27, 2022

This proposal is an enhancement idea to the current dev container features proposal.


Breaking off from #44 (comment) to address @Chuxel 's option (1).

There are several ways we could pass an array of options to an individual features.

features: {
   apt: {
     "packages": ["vim", "jq", "curl"] // option A
     "packages": "vim jq curl" // option B 
   }
}

Under the hook, this would be converted to an environment variable for the underlying shell script to handle.

_BUILD_ARG_APT_GET_PACKAGES="vim jq curl"

@joshspicer
Copy link
Member Author

For option (B), we'd want to give hints in our devcontainer-feature.json to implementing UX tools that this is an array. The tooling could then simply write the set of chosen values as a space-separated value.

Like:

{
  "id": "apt",
  "options": {
    "packages": {
      "type": "array", // NEW!
      "proposals": [
        "vim",
        "jq",
        "wget",
        "curl"
      ],
      "default": "latest",
      "description": "Choose any subset of tools from the list above."
}

@Chuxel Chuxel added the proposal Still under discussion, collecting feedback label Jun 27, 2022
@jkeech
Copy link
Contributor

jkeech commented Jun 27, 2022

Just curious, is there a reason we'd prefer space as a delimiter over another character (such as a comma)?

@joshspicer
Copy link
Member Author

Thinking about the implementation side, it seems most natural to me (since the default $IFS value in bash includes whitespace).

A for loop going over the value would look like:

APT_GET_PACKAGES="vim jq curl"
for i in $APT_GET_PACKAGES
do
 apt install -y "'$i' is the substring"
done

We could tell people to override the $IFS in their features (or of course features can be implemented to however folks how they'd like).

@chrmarti
Copy link
Contributor

For the devcontainer.json the JSON array makes sense (to stay in JSON).

For the install script we can consider shortcuts (like the space separated string) that simplify the handling, but should also include a general approach (likely in parallel) like an env variable with the JSON string.

A devcontainer-feature.json could use a subset of JSON schema to define a feature's options (that will spare us a lot of decision points and will make it easier to run IntelliSense on top). This will also give us a clear path to supporting nested objects and arrays.

@joshspicer
Copy link
Member Author

@chrmarti - all sounds good to me!

@andreiborisov
Copy link

andreiborisov commented Nov 7, 2022

👋🏻 Any time estimate for this? We need it for our features.

@Chuxel
Copy link
Member

Chuxel commented Nov 9, 2022

We'd be happy to take a PR at https://github.com/devcontainers/cli if anyone wants to take a crack at adding this! Right now things in devcontainers/features are using a comma separated string as a near term workaround.

Converting this into an array is pretty easy if you do not need to support spaces in the array.

BASH_ARRAY=( ${COMMA_SEPARATED_OPTION//,/ } )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Still under discussion, collecting feedback
Projects
None yet
Development

No branches or pull requests

5 participants