[proposal] Composite Features #208

joshspicer · 2023-03-08T22:39:10Z

A proposal for a Composite Dev Container Feature.

chrmarti · 2023-03-14T14:11:36Z

proposals/features-composite-dependencies.md

+|----------|------|-------------|
+| `id` | `string` | The ID of the Feature.  This follows the same semantics of the `id` property in the `devcontainer-feature.json` file. |
+| `version` | `string` | The version of the Feature.  This follows the same semantics of the `version` property in the `devcontainer-feature.json` file. |
+| `features` | `array` | An array of objects (in installation order) that define the Feature(s) that compose this Feature. |


Does the correct installation order depend on each of the install script's requirements?

We'd want to peek at each Feature's installsAfter, i think. That was the primary reason that I didn't want to (for now) enable nesting composite Features. This spec does NOT let you nest one composite feature in another, making this process easier to resolve (we can "flatten" the installation from composite Features into one install order.

This is something to discuss and finalize in the spec, but we could have installation follow installsAfter order with precedence within a composite Feature first, and then some algorithm for ordering composite Features based on that? Perhaps the other way around?

Since we are already thinking about composite features depending on each other: Wouldn't it be simpler to avoid the additional artifact type of a "composite feature" and instead allow features to have hard dependencies in addition to the soft dependencies supported with "installAfter"? We already compute the dependency graph including cycle detection for "installAfter". (Other package management systems also seem to have only one package type that can carry code/binaries and dependencies. Do we have an example that keeps these apart?)

We could add "dependencies" to the feature metadata as an object like "features" in the devcontainer.json, so installation order is decided by the same dependency graph algorithm. A dependency would implicitly go into the "installAfter" list of the feature depending on it.

proposals/features-composite-dependencies.md

chrmarti · 2023-03-14T14:12:57Z

proposals/features-composite-dependencies.md

+| `version` | `string` | The version of the Feature.  This follows the same semantics of the `version` property in the `devcontainer-feature.json` file. |
+| `features` | `array` | An array of objects (in installation order) that define the Feature(s) that compose this Feature. |
+| `features.id` | `string` | The ID of the Feature that this Feature depends on. |
+| `features.version` | `string` | The version of the Feature that this Feature depends on. |


What if a different version is configured in the devcontainer.json?

I think this brings us back to similar discussion over at #44.

Maybe it would be helpful to define behavior for installing a Feature more than once. I think a goal of Features should be that they are idempotent - perhaps this would be a good time to also define some "Feature best practices" to guide authors toward the behavior we want Features to exhibit?

Agreed. We already have that when you use a base image with a feature installed and you then install that feature again in a second devcontainer.json.

If the version and options objects are equal, we can optimize by running the feature only once.

Maybe a detail: The install order algorithm currently uses the feature ids to sort features that do not require a specific install order to arrive at a stable install order. With the same feature being allowed multiple times in the same install pass, we will need to include also the feature version and options object in this sorting to ensure a stable install order.

chrmarti · 2023-03-14T14:13:39Z

proposals/features-composite-dependencies.md

+| `features` | `array` | An array of objects (in installation order) that define the Feature(s) that compose this Feature. |
+| `features.id` | `string` | The ID of the Feature that this Feature depends on. |
+| `features.version` | `string` | The version of the Feature that this Feature depends on. |
+| `features.detect` | `string` | A command that will be executed in a shell to determine if the Feature should be installed.  If the command returns a non-zero exit code, the Feature will be installed.  If the command returns a zero exit code, the remaining install steps will be skipped. |


Does "remaining install steps will be skipped" mean, the remaining features listed in the array will be skipped?

I will clarify this - I meant that each Feature will have a detect that will cause the further installation and merging of dev container config for that Feature. Remaining Features in the array will still be executed (and their detect phase run, too).

What are the examples we want to cover with this? Would this run after all previous features have detected & installed, immediately before this feature's install script would run?

chrmarti · 2023-03-14T14:14:33Z

proposals/features-composite-dependencies.md

+| `features.id` | `string` | The ID of the Feature that this Feature depends on. |
+| `features.version` | `string` | The version of the Feature that this Feature depends on. |
+| `features.detect` | `string` | A command that will be executed in a shell to determine if the Feature should be installed.  If the command returns a non-zero exit code, the Feature will be installed.  If the command returns a zero exit code, the remaining install steps will be skipped. |
+| `features.options` | `object` | An object of key/value pairs that will be passed to the Feature's `install.sh` script. |


How are these merged when the same feature is installed by the devcontainer.json?

Similar to #208 (comment)

chrmarti · 2023-03-14T14:15:39Z

proposals/features-composite-dependencies.md

+}
+```
+
+An optional `finalize.sh` script can be included, and will be fired after all Features have been installed.


Do we have good examples for that?

This could be omitted in the first pass if we find it to be unnecessary or without justification yet. I'll leave for discussion

I like that we are discussing various options. We should aim for as much simplicity as we can while still covering the important cases we have. It will be easier to later add to a simple design than a complex one.

Updating that featmake bash script has been deprecated in favor of a python cli program (which still going to be deprecated in the future in favor of this proposal)

bamurtaugh

Very thorough and clear, thanks for putting this together! Left a couple minor comments and questions.

proposals/features-composite-dependencies.md

Co-authored-by: Brigit Murtaugh <[email protected]>

…ible dependency models.

joshspicer · 2023-03-27T21:33:40Z

📢 Open Questions

Include a finalize.sh script?
Behavior when installing the same Feature >1 times? (eg: embedded in composite Feature and in user dev container)
- (Related to above) Installing the same Feature(s), different versions?
Install order precedence with existing mechanisms (eg: installsAfter)

joshspicer · 2023-06-12T17:52:57Z

closing in favor of #234

joshspicer added 2 commits March 8, 2023 22:14

introduction of spec

149035b

composite Features initial draft

9347bd2

joshspicer changed the title ~~Proposal: Composite Features~~ [proposal] Composite Features Mar 8, 2023

Update features-composite-dependencies.md

ce50238

joshspicer marked this pull request as ready for review March 13, 2023 17:45

joshspicer requested a review from a team as a code owner March 13, 2023 17:45

chrmarti requested changes Mar 14, 2023

View reviewed changes

Update features-composite-dependencies.md (#211)

5ad1c0d

Updating that featmake bash script has been deprecated in favor of a python cli program (which still going to be deprecated in the future in favor of this proposal)

bamurtaugh reviewed Mar 14, 2023

View reviewed changes

typos

7b82815

Co-authored-by: Brigit Murtaugh <[email protected]>

jcbhmr mentioned this pull request Mar 20, 2023

Incorporate ideas/content from main-OLD branch devcontainers-community/features#12

Closed

4 tasks

joshspicer added 3 commits March 27, 2023 18:46

typos

e3cd09d

add reasons why 'Composite features are not as flexible as other poss…

5419937

…ible dependency models.

namespace/repo

f441a0b

joshspicer mentioned this pull request May 2, 2023

[Proposal] Feature dependsOn #234

Merged

joshspicer closed this Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[proposal] Composite Features #208

[proposal] Composite Features #208

joshspicer commented Mar 8, 2023 •

edited

Loading

chrmarti Mar 14, 2023

joshspicer Mar 27, 2023 •

edited

Loading

chrmarti Mar 28, 2023

chrmarti Mar 14, 2023

joshspicer Mar 17, 2023

chrmarti Mar 28, 2023

chrmarti Mar 14, 2023

joshspicer Mar 27, 2023

chrmarti Mar 28, 2023

chrmarti Mar 14, 2023

joshspicer Mar 27, 2023

chrmarti Mar 14, 2023

joshspicer Mar 27, 2023

chrmarti Mar 28, 2023

bamurtaugh left a comment

joshspicer commented Mar 27, 2023 •

edited

Loading

joshspicer commented Jun 12, 2023

[proposal] Composite Features #208

[proposal] Composite Features #208

Conversation

joshspicer commented Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

joshspicer Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bamurtaugh left a comment

Choose a reason for hiding this comment

joshspicer commented Mar 27, 2023 • edited Loading

joshspicer commented Jun 12, 2023

joshspicer commented Mar 8, 2023 •

edited

Loading

joshspicer Mar 27, 2023 •

edited

Loading

joshspicer commented Mar 27, 2023 •

edited

Loading