fix(features): write Feature ENV lines and options in declared order

docs/parity/parity-matrix.yaml

@@ -1079,6 +1079,20 @@ initial_cases:
     current_status: match
     notes: "Without BuildKit, TS builds the dev_container_feature_content_temp image from the features folder and COPYs /tmp/build-features/hello_0 to /tmp/dev-container-features/hello_0 (kept in the image), so verify_cmd also compares the staged devcontainer-features.env and the devcontainer-features-install.sh banner (getFeatureInstallWrapperScript). Go used to copy from the build context to /tmp/build-features/_dev_container_feature_0."
 
+  - id: build.features-env-declaration-order-success
+    lane: runtime
+    command: build
+    priority: p1
+    docker_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/sdk; printf '%s' '{\"id\":\"sdk\",\"version\":\"1.0.0\",\"name\":\"Sdk\",\"options\":{\"version\":{\"type\":\"string\",\"default\":\"latest\"}},\"containerEnv\":{\"SDKMAN_DIR\":\"/usr/local/sdkman\",\"PATH\":\"${SDKMAN_DIR}/bin:${PATH}\"}}' > $WORKSPACE/.devcontainer/sdk/devcontainer-feature.json; printf '%s\\n' '#!/bin/sh' 'set -e' 'cp devcontainer-features.env /features-env.txt' > $WORKSPACE/.devcontainer/sdk/install.sh; chmod +x $WORKSPACE/.devcontainer/sdk/install.sh; printf '%s' '{\"image\":\"alpine:3\",\"containerEnv\":{\"ZED\":\"1\",\"ALPHA\":\"2\"},\"features\":{\"./sdk\":{\"zz\":\"1\",\"version\":\"2\",\"aa\":\"3\"}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    ts_cmd: "build --workspace-folder ${WORKSPACE} --image-name parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    verify_cmd: "docker run --rm ${IMAGE_NAME} cat /features-env.txt; echo; docker image inspect -f '{{json .Config.Env}}' ${IMAGE_NAME}"
+    cleanup_cmd: "docker rmi -f parity-${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null 2>&1; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true"
+    asserts: [exit_code, stdout_normalized]
+    class: build-runtime
+    current_status: match
+    notes: "TS writes a Feature's containerEnv as ENV lines in devcontainer-feature.json's order and the config's containerEnv in devcontainer.json's (generateContainerEnvs, Object.keys), and devcontainer-features.env lists the options the Feature declares, then the user's other options in the user's order (getFeatureValueObject). The Feature's PATH line uses SDKMAN_DIR, which the line before it sets, so the image's PATH starts with /usr/local/sdkman/bin. Go sorted the keys: PATH came first and expanded an unset SDKMAN_DIR to /bin, ALPHA preceded ZED, and AA preceded ZZ."
+
   - id: up.compose-features-staging-layout-success
     lane: runtime
     command: up

internal/cli/build.go

@@ -576,7 +576,7 @@ func imageBuildFor(cfg *config.DevContainer, opts *buildOpts) imageBuild {
 }
 
 func checkLegacyFeatures(feats map[string]any) error {
-	userFeatures, _ := features.UpdateDeprecatedFeaturesIntoOptions(sortedUserFeatures(feats))
+	userFeatures, _ := features.UpdateDeprecatedFeaturesIntoOptions(sortedUserFeatures(feats, nil))
 	for _, f := range userFeatures {
 		id, _ := features.BackwardCompatibleID(f.UserFeatureID)
 		if features.ClassifyID(id) == features.SourceLegacyShorthand {
@@ -830,6 +830,8 @@ func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *con
 		Push:                        opts.push,
 		Output:                      opts.output,
 		ContainerEnv:                cfg.ContainerEnv,
+		ContainerEnvOrder:           cfg.ContainerEnvOrder,
+		FeatureOptionOrder:          cfg.FeatureOptionOrder,
 		FrozenLockfile:              opts.experimentalFrozenLockfile,
 		NoLockfile:                  opts.noLockfile,
 		ConfigPath:                  cfg.ConfigFilePath,

internal/cli/compose_up.go

@@ -274,7 +274,7 @@ func (r *upRunner) composeFeatureSets(ctx context.Context, cfg *config.DevContai
 			return nil, "", err
 		}
 	}
-	res, err := fetchFeatureSetsWithOrder(ctx, r.log, cfg.Features, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, lockfile, cfg.OverrideFeatureInstallOrder)
+	res, err := fetchFeatureSetsWithOrder(ctx, r.log, cfg.Features, cfg.FeatureOptionOrder, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, lockfile, cfg.OverrideFeatureInstallOrder)
 	if err != nil {
 		return nil, "", err
 	}
@@ -454,6 +454,7 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe
 			UseBuildKitContexts: useContexts,
 			Syntax:              syntax,
 			ConfigContainerEnv:  cfg.ContainerEnv,
+			ConfigEnvOrder:      cfg.ContainerEnvOrder,
 		})
 		if !useContexts {
 			if err := buildFeatureContentImage(ctx, r.docker, featuresFolder); err != nil {

internal/cli/feature_deps.go

@@ -47,7 +47,7 @@ func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local loc
 			if err != nil {
 				return nil, fmt.Errorf("ERR: Feature '%s' could not be processed.  %w", id, err)
 			}
-			meta.Value = node.Options
+			meta.Value, meta.ValueOrder = node.Options, node.OptionOrder
 			if meta.ID == "" {
 				meta.ID = id
 			}
@@ -64,7 +64,7 @@ func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local loc
 			if err != nil {
 				return nil, fmt.Errorf("ERR: Feature '%s' could not be processed.  %w", id, err)
 			}
-			meta.Value = node.Options
+			meta.Value, meta.ValueOrder = node.Options, node.OptionOrder
 			return &features.Set{
 				SourceInfo: &features.TarballSource{TarballURI: id, UserID: id},
 				Features:   []features.Feature{meta},
@@ -94,7 +94,7 @@ func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local loc
 			if annoID := ociAnnotationID(manifest); annoID != "" {
 				meta.ID = annoID
 			}
-			meta.Value = node.Options
+			meta.Value, meta.ValueOrder = node.Options, node.OptionOrder
 
 			return &features.Set{
 				SourceInfo: &features.OCISource{

internal/cli/feature_install.go

@@ -47,6 +47,8 @@ type FeatureBuildOptions struct {
 	Secrets                     []string
 	CABundle                    string
 	ContainerEnv                map[string]string
+	ContainerEnvOrder           []string
+	FeatureOptionOrder          map[string][]string
 	SkipPersistCustoms          bool
 	FeaturesBasePath            string
 	WorkspaceFolder             string
@@ -64,12 +66,12 @@ type fetchFeatureResult struct {
 	TmpDir      string
 }
 
-func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots) (*fetchFeatureResult, error) {
-	return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, local, nil, nil)
+func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, optionOrder map[string][]string, local localFeatureRoots) (*fetchFeatureResult, error) {
+	return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, optionOrder, local, nil, nil)
 }
 
-func sortedUserFeatures(featuresCfg map[string]any) []features.DevContainerFeature {
-	userFeatures := features.UserFeaturesToArray(featuresCfg)
+func sortedUserFeatures(featuresCfg map[string]any, optionOrder map[string][]string) []features.DevContainerFeature {
+	userFeatures := features.UserFeaturesToArray(featuresCfg, optionOrder)
 	sort.Slice(userFeatures, func(i, j int) bool {
 		return userFeatures[i].UserFeatureID < userFeatures[j].UserFeatureID
 	})
@@ -110,12 +112,12 @@ func createFeaturesFolder() (string, error) {
 	}
 }
 
-func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) {
+func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, optionOrder map[string][]string, local localFeatureRoots, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) {
 	if len(featuresCfg) == 0 {
 		return nil, nil
 	}
 
-	userFeatures := sortedUserFeatures(featuresCfg)
+	userFeatures := sortedUserFeatures(featuresCfg, optionOrder)
 	userFeatures, warnings := features.UpdateDeprecatedFeaturesIntoOptions(userFeatures)
 	for _, w := range warnings {
 		logger.Write(w, log.LevelWarning)
@@ -169,21 +171,23 @@ func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresC
 
 const featureStagePrefix = ".stage-"
 
-func featureFromMetadata(m features.Feature, id string, opts any, cachePath string) features.Feature {
+func featureFromMetadata(m features.Feature, id string, opts any, optOrder []string, cachePath string) features.Feature {
 	return features.Feature{
 		ID:                   id,
 		Version:              m.Version,
 		Name:                 m.Name,
 		Description:          m.Description,
 		DocumentationURL:     m.DocumentationURL,
 		Value:                opts,
+		ValueOrder:           optOrder,
 		UserOptions:          extractUserOptions(opts),
 		Options:              m.Options,
 		DependsOn:            m.DependsOn,
 		DependsOnOrder:       m.DependsOnOrder,
 		InstallsAfter:        m.InstallsAfter,
 		LegacyIds:            m.LegacyIds,
 		ContainerEnv:         m.ContainerEnv,
+		ContainerEnvOrder:    m.ContainerEnvOrder,
 		Mounts:               m.Mounts,
 		Init:                 m.Init,
 		Privileged:           m.Privileged,
@@ -212,7 +216,7 @@ func processInstallFeature(
 	nextStageDir func() string,
 ) (*features.Set, error) {
 	id := node.UserFeatureID
-	opts := node.Options
+	opts, optOrder := node.Options, node.OptionOrder
 	resolvedID, srcType, err := resolveFeatureID(logger, id)
 	if err != nil {
 		return nil, err
@@ -248,7 +252,7 @@ func processInstallFeature(
 		if featID == "" {
 			featID = filepath.Base(id)
 		}
-		feat := featureFromMetadata(m, featID, opts, featureDir)
+		feat := featureFromMetadata(m, featID, opts, optOrder, featureDir)
 		if feat.Name == "" {
 			feat.Name = id
 		}
@@ -287,7 +291,7 @@ func processInstallFeature(
 			featID = tarballFeatureID(id)
 		}
 
-		feat := featureFromMetadata(m, featID, opts, featureDir)
+		feat := featureFromMetadata(m, featID, opts, optOrder, featureDir)
 		if feat.Name == "" {
 			feat.Name = id
 		}
@@ -348,31 +352,33 @@ func processInstallFeature(
 		}
 
 		feat := features.Feature{
-			ID:               cmp.Or(m.ID, ref.ID),
-			Version:          m.Version,
-			Name:             m.Name,
-			Description:      m.Description,
-			DocumentationURL: m.DocumentationURL,
-			Value:            opts,
-			UserOptions:      extractUserOptions(opts),
-			Options:          m.Options,
-			DependsOn:        m.DependsOn,
-			DependsOnOrder:   m.DependsOnOrder,
-			InstallsAfter:    m.InstallsAfter,
-			LegacyIds:        m.LegacyIds,
-			ContainerEnv:     m.ContainerEnv,
-			Init:             m.Init,
-			Privileged:       m.Privileged,
-			CapAdd:           m.CapAdd,
-			SecurityOpt:      m.SecurityOpt,
-			Entrypoint:       m.Entrypoint,
-			Mounts:           m.Mounts,
-			Customizations:   m.Customizations,
-			Included:         true,
-			CachePath:        featureDir,
-			Metadata:         m.Metadata,
-			File:             m.Metadata,
-			SourceKeys:       features.OCIFeatureKeys,
+			ID:                cmp.Or(m.ID, ref.ID),
+			Version:           m.Version,
+			Name:              m.Name,
+			Description:       m.Description,
+			DocumentationURL:  m.DocumentationURL,
+			Value:             opts,
+			ValueOrder:        optOrder,
+			UserOptions:       extractUserOptions(opts),
+			Options:           m.Options,
+			DependsOn:         m.DependsOn,
+			DependsOnOrder:    m.DependsOnOrder,
+			InstallsAfter:     m.InstallsAfter,
+			LegacyIds:         m.LegacyIds,
+			ContainerEnv:      m.ContainerEnv,
+			ContainerEnvOrder: m.ContainerEnvOrder,
+			Init:              m.Init,
+			Privileged:        m.Privileged,
+			CapAdd:            m.CapAdd,
+			SecurityOpt:       m.SecurityOpt,
+			Entrypoint:        m.Entrypoint,
+			Mounts:            m.Mounts,
+			Customizations:    m.Customizations,
+			Included:          true,
+			CachePath:         featureDir,
+			Metadata:          m.Metadata,
+			File:              m.Metadata,
+			SourceKeys:        features.OCIFeatureKeys,
 		}
 		if feat.Version == "" {
 			feat.Version = ref.Tag
@@ -588,7 +594,7 @@ func extendImageWithFeatures(
 			return nil, err
 		}
 	}
-	result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, lockfile, fbOpts.OverrideFeatureInstallOrder)
+	result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, fbOpts.FeatureOptionOrder, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, lockfile, fbOpts.OverrideFeatureInstallOrder)
 	if err != nil {
 		return nil, err
 	}
@@ -656,6 +662,7 @@ func extendImageWithFeatures(
 		UseBuildKitContexts: useBuildx,
 		Syntax:              syntax,
 		ConfigContainerEnv:  fbOpts.ContainerEnv,
+		ConfigEnvOrder:      fbOpts.ContainerEnvOrder,
 		SecretIDs:           secretIDs,
 		CABundle:            caSecrets != nil,
 	})

internal/cli/feature_install_held_test.go

@@ -429,7 +429,7 @@ func TestTarballFeatureIDUsesTheURLPath(t *testing.T) {
 
 func TestFetchFeatureSetsReportsTheFirstSortedFailure(t *testing.T) {
 	for range 5 {
-		_, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./b-missing": true, "./a-missing": true, "./c-missing": true}, devcontainerRoots(t))
+		_, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./b-missing": true, "./a-missing": true, "./c-missing": true}, nil, devcontainerRoots(t))
 		if err == nil || !strings.Contains(err.Error(), "'./a-missing'") {
 			t.Fatalf("err = %v, want the failure of ./a-missing", err)
 		}

internal/cli/feature_install_test.go

@@ -148,7 +148,7 @@ func TestFetchFeatureSets(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			roots := devcontainerRoots(t)
 			tt.setup(t, roots.configDir)
-			result, err := fetchFeatureSets(t.Context(), log.Null, tt.entries, roots)
+			result, err := fetchFeatureSets(t.Context(), log.Null, tt.entries, nil, roots)
 			if result != nil && result.TmpDir != "" {
 				defer os.RemoveAll(result.TmpDir)
 			}
@@ -232,7 +232,7 @@ func TestFetchFeatureSets_StagingLayout(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(dirB, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil {
 		t.Fatal(err)
 	}
-	result, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./dir-a": true}, roots)
+	result, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./dir-a": true}, nil, roots)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -396,7 +396,7 @@ func TestFetchFeatureSetsRefusesLocalFeaturesOutsideDevcontainer(t *testing.T) {
 		t.Run(tc.id, func(t *testing.T) {
 			var logs bytes.Buffer
 			logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs})
-			result, err := fetchFeatureSets(t.Context(), logger, map[string]any{tc.id: true}, roots)
+			result, err := fetchFeatureSets(t.Context(), logger, map[string]any{tc.id: true}, nil, roots)
 			if result != nil {
 				t.Fatalf("result = %+v, want none", result)
 			}
@@ -420,7 +420,7 @@ func TestFetchFeatureSetsRefusesLocalFeaturesOutsideDevcontainer(t *testing.T) {
 func TestFetchFeatureSetsMovesDeprecatedFeaturesIntoOptions(t *testing.T) {
 	var logs bytes.Buffer
 	logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs})
-	result, err := fetchFeatureSets(t.Context(), logger, map[string]any{"gradle": map[string]any{}, "node:18": map[string]any{}}, devcontainerRoots(t))
+	result, err := fetchFeatureSets(t.Context(), logger, map[string]any{"gradle": map[string]any{}, "node:18": map[string]any{}}, nil, devcontainerRoots(t))
 	if result != nil || err == nil || err.Error() != fmt.Sprintf(msgLegacyFeature, "node:18") {
 		t.Fatalf("result = %+v, err = %v; want the legacy Feature error for node:18", result, err)
 	}
@@ -457,3 +457,54 @@ func TestCreateFeaturesFolderIsTheRunsAlone(t *testing.T) {
 		seen[dir] = true
 	}
 }
+
+func TestFetchedFeaturesKeepTheDeclaredOrder(t *testing.T) {
+	roots := devcontainerRoots(t)
+	write := func(name, metadata string) {
+		t.Helper()
+		dir := filepath.Join(roots.configDir, name)
+		if err := os.MkdirAll(dir, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, "devcontainer-feature.json"), []byte(metadata), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	write("sdk", `{"id": "sdk", "version": "1.0.0",
+		"options": {"version": {"type": "string", "default": "latest"}},
+		"containerEnv": {"SDKMAN_DIR": "/usr/local/sdkman", "PATH": "${SDKMAN_DIR}/bin:${PATH}"},
+		"dependsOn": {"./dep": {"zz": "1", "aa": "2"}}}`)
+	write("dep", `{"id": "dep", "version": "1.0.0"}`)
+
+	result, err := fetchFeatureSets(t.Context(), log.Null,
+		map[string]any{"./sdk": map[string]any{"zz": "1", "version": "2", "aa": "3"}},
+		map[string][]string{"./sdk": {"zz", "version", "aa"}}, roots)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(result.TmpDir)
+	if err := stageFeatureBuildFiles(result.TmpDir, result.FeatureSets, "root", "root"); err != nil {
+		t.Fatal(err)
+	}
+	wantEnv := map[string]string{"sdk": "VERSION=\"2\"\nZZ=\"1\"\nAA=\"3\"", "dep": "ZZ=\"1\"\nAA=\"2\""}
+	for _, set := range result.FeatureSets {
+		f := set.Features[0]
+		data, err := os.ReadFile(filepath.Join(f.CachePath, imagemeta.FeatureEnvFile))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got := string(data); got != wantEnv[f.ID] {
+			t.Errorf("%s %s = %q, want %q", f.ID, imagemeta.FeatureEnvFile, got, wantEnv[f.ID])
+		}
+	}
+
+	dockerfile := imagemeta.GenerateFeaturesBuildOptions(imagemeta.FeaturesBuildParams{
+		BaseName: "base", FeatureSets: result.FeatureSets, ContainerUser: "root", RemoteUser: "root", UseBuildKitContexts: true,
+	}).DockerfileContent
+	if want := "ENV SDKMAN_DIR=\"/usr/local/sdkman\"\nENV PATH=\"${SDKMAN_DIR}/bin:${PATH}\"\n"; !strings.Contains(dockerfile, want) {
+		t.Errorf("Dockerfile lacks the Feature's ENV lines in declaration order %q:\n%s", want, dockerfile)
+	}
+}

internal/cli/features_resolve_deps.go

@@ -38,7 +38,7 @@ func realFeaturesResolveDepsCmd() *cobra.Command {
 			}
 
 			cfg := loadResult.Config
-			userFeatures := features.UserFeaturesToArray(cfg.Features)
+			userFeatures := features.UserFeaturesToArray(cfg.Features, cfg.FeatureOptionOrder)
 			if len(userFeatures) == 0 {
 				logger.Write(fmt.Sprintf("Could not parse features object in configuration '%s'", cfg.ConfigFilePath), log.LevelError)
 				return &coreerrors.ExitCodeError{Code: 1}

internal/cli/helpers.go

@@ -29,6 +29,12 @@ func mergeAdditionalFeatures(cfg *config.DevContainer, additionalFeaturesJSON st
 		if _, exists := cfg.Features[k]; !exists {
 			cfg.Features[k] = v
 			additionalOnly[k] = true
+			if names := jsonc.MemberNames([]byte(additionalFeaturesJSON), k); len(names) > 0 {
+				if cfg.FeatureOptionOrder == nil {
+					cfg.FeatureOptionOrder = map[string][]string{}
+				}
+				cfg.FeatureOptionOrder[k] = names
+			}
 		}
 	}
 	return additionalOnly, nil