devcontainers / devcontainers/spec

CLI sorts Feature options fewest-first, against spec

Open
#755 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
No language data
Stars
5.7k
Forks
496
PR merge metrics
No merged PRs in 30d

Description

> **Note on repo choice:** Per the [`devcontainers/cli` CONTRIBUTING guide](https://github.com/devcontainers/cli/blob/main/CONTRIBUTING.md), issues concerning the CLI should be filed in this spec repository, so I've opened it here even though it's about `devcontainers/cli` behavior. Please let me know if you'd prefer it moved to the `devcontainers/cli` repo instead.

### Summary

When sorting Features within an installation round, the spec says Features are ordered by the **greatest** number of user-defined options first, but `containerFeaturesOrder.ts` orders by the **fewest** options first.

### Specification

[Features spec — Definition: Round Stable Sort](https://containers.dev/implementors/features/#definition-feature-equality):

> Greatest number of user-defined options (note omitting an option will default that value to the Feature's default value and is not considered a user-defined option)

### Implementation

`optionsCompareTo` returns `aKeys.length - bKeys.length`, which is negative when `a` has *fewer* options, so the Feature with fewer options sorts *earlier*:

https://github.com/devcontainers/cli/blob/f683c29f64a20109b4453e5149807e390ff65133/src/spec-configuration/containerFeaturesOrder.ts#L123-L125

This is the option tiebreaker for every source type (OCI, file-path, direct-tarball, legacy).

### Test evidence

The existing test `valid dependsOn with round sorting based on options` (fixture `configs/feature-dependencies/dependsOn/local-with-options`) pins the fewest-first behavior — the `./b` variant with `{}` (zero options) is expected first, ahead of the two-option variants:

https://github.com/devcontainers/cli/blob/f683c29f64a20109b4453e5149807e390ff65133/src/test/container-features/containerFeaturesOrder.test.ts#L135-L209

So the implementation and its tests are internally consistent; only the behavior disagrees with the spec.

### Proposed fix

Sort by the greatest option count to match the spec (`bKeys.length - aKeys.length`), with the fixture/expected order updated accordingly. This changes the install order of the same Feature requested with differing option sets within a round.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.