From eebacc50c8e248f54b50d209b5d23fd244f20f5b Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 12:04:30 -0700
Subject: [PATCH 01/30] First changes for features

---
 src/spec-common/injectHeadless.ts             |   7 +-
 src/spec-configuration/configuration.ts       |  11 +-
 .../containerFeaturesConfiguration.ts         | 805 +++++++++---------
 src/spec-node/containerFeatures.ts            | 104 ++-
 src/spec-node/devContainersSpecCLI.ts         |   2 +-
 src/spec-utils/pfs.ts                         |   2 +
 .../helpers.offline.test.ts                   | 143 +++-
 7 files changed, 574 insertions(+), 500 deletions(-)

diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts
index a7a616e8b..292b0647b 100644
--- a/src/spec-common/injectHeadless.ts
+++ b/src/spec-common/injectHeadless.ts
@@ -105,13 +105,18 @@ export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand'
 
 const defaultWaitFor: DevContainerConfigCommand = 'updateContentCommand';
 
+export interface DevContainerFeature{
+	id: string;
+	options: boolean | string | Record<string, boolean | string | undefined>;
+}
+
 export interface CommonDevContainerConfig {
 	configFilePath?: URI;
 	remoteEnv?: Record<string, string | null>;
 	forwardPorts?: (number | string)[];
 	portsAttributes?: Record<string, PortAttributes>;
 	otherPortsAttributes?: PortAttributes;
-	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
 	onCreateCommand?: string | string[];
 	updateContentCommand?: string | string[];
 	postCreateCommand?: string | string[];
diff --git a/src/spec-configuration/configuration.ts b/src/spec-configuration/configuration.ts
index f10d6b18f..0c515d8d9 100644
--- a/src/spec-configuration/configuration.ts
+++ b/src/spec-configuration/configuration.ts
@@ -26,6 +26,11 @@ export interface HostRequirements {
 	storage?: string;
 }
 
+export interface DevContainerFeature{
+	id: string;
+	options: boolean | string | Record<string, boolean | string | undefined>;
+}
+
 export interface DevContainerFromImageConfig {
 	configFilePath: URI;
 	image: string;
@@ -54,7 +59,7 @@ export interface DevContainerFromImageConfig {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
 	hostRequirements?: HostRequirements;
 }
 
@@ -85,7 +90,7 @@ export type DevContainerFromDockerfileConfig = {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
 	hostRequirements?: HostRequirements;
 } & (
 	{
@@ -132,7 +137,7 @@ export interface DevContainerFromDockerComposeConfig {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
 	hostRequirements?: HostRequirements;
 }
 
diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index fbc8f5280..a926a63ef 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -5,20 +5,32 @@
 
 import * as jsonc from 'jsonc-parser';
 import * as path from 'path';
-import * as semver from 'semver';
+//import * as semver from 'semver';
 import * as URL from 'url';
 import * as tar from 'tar';
 import { DevContainerConfig } from './configuration';
-import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile } from '../spec-utils/pfs';
+import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal } from '../spec-utils/pfs';
 import { Log, LogLevel } from '../spec-utils/log';
 import { request } from '../spec-utils/httpRequest';
+import { DevContainerFeature } from './configuration';
+import { existsSync } from 'fs';
 
 const ASSET_NAME = 'devcontainer-features.tgz';
 
 export interface Feature {
 	id: string;
 	name: string;
+	description?: string;
+	filename?: string;
+	runApp?: string;
+	runParams?: string;
+	infoString?: string;
+	internalVersion?: string;
+	tempLocalPath?: string;
+	consecutiveId?: string;
+	install?: Record<string, string>;
 	documentationURL?: string;
+	licenseURL?: string;
 	options?: Record<string, FeatureOption>;
 	buildArg?: string; // old properties for temporary compatibility
 	containerEnv?: Record<string, string>;
@@ -99,6 +111,7 @@ export interface FilePathSourceInformation extends BaseSourceInformation {
 
 export interface FeatureSet {
 	features: Feature[];
+	internalVersion?: string;
 	sourceInformation: SourceInformation;
 }
 
@@ -145,6 +158,13 @@ export function collapseFeaturesConfig(original: FeaturesConfig | undefined): Co
 
 export const multiStageBuildExploration = false;
 
+// TODO: Cleanup
+let counter = 1;
+
+function getCounter() {
+	return counter++;
+}
+
 const isTsnode = path.basename(process.argv[0]) === 'ts-node' || process.argv.indexOf('ts-node/register') !== -1;
 
 export function getContainerFeaturesFolder(_extensionPath: string | { distFolder: string }) {
@@ -161,13 +181,13 @@ export function getSourceInfoString(srcInfo: SourceInformation): string {
 	const { type } = srcInfo;
 	switch (type) {
 		case 'local-cache':
-			return 'local-cache';
+			return getCounter() + '-local-cache';
 		case 'direct-tarball':
-			return Buffer.from(srcInfo.tarballUri).toString('base64');
+			return getCounter() + '-' + srcInfo.tarballUri;
 		case 'github-repo':
-			return `github-${srcInfo.owner}-${srcInfo.repo}-${srcInfo.isLatest ? 'latest' : srcInfo.tag}`;
+			return `${getCounter()}-github-${srcInfo.owner}-${srcInfo.repo}-${srcInfo.isLatest ? 'latest' : srcInfo.tag}`;
 		case 'file-path':
-			return Buffer.from(srcInfo.filePath).toString('base64');
+			return getCounter() + '-' + srcInfo.filePath;
 	}
 }
 
@@ -197,17 +217,50 @@ USER $IMAGE_USER
 
 export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 	let result = '';
-	const folders = (featuresConfig.featureSets || []).map(x => getSourceInfoString(x.sourceInformation));
+
+	// Features version 1
+	const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => getSourceInfoString(x.sourceInformation));
 	folders.forEach(folder => {
 		result += `RUN cd /tmp/build-features/${folder} \\
 && chmod +x ./install.sh \\
 && ./install.sh
 
 `;
+	
+});
+	// Features version 2
+	featuresConfig.featureSets.filter(y => y.internalVersion === '2').forEach(featureSet => {
+		featureSet.features.forEach(feature => {
+				result += generateContainerEnvs(feature);
+				result += `
+				
+RUN cd /tmp/build-features/${feature.consecutiveId} \\
+&& export $(cat devcontainer-features.env | xargs) \\
+&& echo $PATH \\
+&& chmod +x ./${feature.runParams} \\
+&& ./${feature.runParams}
+
+`;	
+		})
 	});
 	return result;
 }
 
+// Used for features version 2
+export function generateContainerEnvs(feature: Feature) {
+	let result = '';
+	if(!feature.containerEnv)
+	{
+		return result;
+	}
+	let keys = Object.keys(feature.containerEnv);
+	result = keys.map(k => `ENV ${k}=${feature.containerEnv![k]}`).join('\n');
+
+	return result;
+}
+
+const allowedFeatureIdRegex = new RegExp('^[a-zA-Z0-9_-]*$');
+
 // Parses a declared feature in user's devcontainer file into
 // a usable URI to download remote features.
 // RETURNS
@@ -216,158 +269,6 @@ export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 //  sourceInformation  <----- Source information (is this locally cached, a GitHub remote feature, etc..), including tarballUri if applicable.
 // }
 //
-export function parseFeatureIdentifier(input: string, output: Log): { id: string; sourceInformation: SourceInformation } | undefined {
-	// A identifier takes this form:
-	//      (0)  <feature>
-	//      (1)  <publisher>/<feature-set>/<feature>@version
-	//      (2)  https://<../URI/..>/devcontainer-features.tgz#<feature>
-	//      (3) ./<local-path>#<feature>  -or-  ../<local-path>#<feature>  -or-   /<local-path>#<feature>
-	// 
-	//  (0) This is a locally cached feature. The function should return `undefined` for tarballUrl
-	//
-	//  (1) Our "registry" is backed by GitHub public repositories (or repos visible with the environment's GITHUB_TOKEN).
-	//      Say organization 'octocat' has a repo titled 'myfeatures' with a set of feature definitions.
-	//      One of the [1..n] features in this repo has an id of 'helloworld'.
-	//
-	//      eg: octocat/myfeatures/helloworld
-	//
-	//      The above example assumes the 'latest' GitHub release, and internally will 
-	//      fetch the devcontainer-features.tgz artifact from that release.
-	//      To specify a certain release tag, append the tag with an @ symbol
-	//
-	//      eg: octocat/myfeatures/helloworld@v0.0.2
-	//
-	//  (2) A fully-qualified https URI to a devcontainer-features.tgz file can be provided instead
-	//      of a using the GitHub registry "shorthand". Note this is identified by a
-	//      s.StartsWith("https://" ||  "http://").
-	//
-	//      eg: https://example.com/../../devcontainer-features.tgz#helloworld
-	//
-	//  (3) This is a local path to a directory on disk following the expected file convention
-	//      The path can either be:
-	//          -  a relative file path to the .devcontainer file (prepended by a ./  or ../)
-	//          -  an absolute file path (prepended by a /)
-	//
-	//      No version can be provided, as the directory is copied 'as is' and is inherently taking the 'latest'
-
-	// Regexes
-	const allowedFeatureIdRegex = new RegExp('^[a-zA-Z0-9_-]*$');
-
-	// Case (0): Cached feature
-	if (!input.includes('/')) {
-		output.write(`[${input}] - No slash, must be locally cached feature.`, LogLevel.Trace);
-		return {
-			id: input,
-			sourceInformation: { type: 'local-cache' },
-		};
-	}
-
-	// Case (2): Direct URI to a tgz
-	if (input.startsWith('http://') || input.startsWith('https://')) {
-		output.write(`[${input}] - Direct URI`, LogLevel.Trace);
-
-		// Trim any trailing slash character to make parsing easier.
-		// A slash at the end of the direct tgz identifier is not important.
-		input = input.replace(/\/+$/, '');
-
-		// Parse out feature ID by splitting on final slash character.
-		const featureIdDelimiter = input.lastIndexOf('#');
-		const id = input.substring(featureIdDelimiter + 1);
-		// Ensure feature id only contains the expected set of characters.
-		if (id === '' || !allowedFeatureIdRegex.test(id)) {
-			output.write(`Parse error. Specify a feature id with alphanumeric, dash, or underscore characters. Provided: ${id}.`, LogLevel.Error);
-			return undefined;
-		}
-		const tarballUri =
-			new URL.URL(input.substring(0, featureIdDelimiter))
-				.toString();
-
-		output.write(`[${input}] - uri: ${tarballUri} , id: ${id}`, LogLevel.Trace);
-		return {
-			id,
-			sourceInformation: { 'type': 'direct-tarball', tarballUri }
-		};
-	}
-
-	// Case (3): Local disk relative/absolute path to directory
-	if (input.startsWith('/') || input.startsWith('./') || input.startsWith('../')) {
-		// Currently unimplemented.
-		return undefined;
-
-		// const splitOnHash = input.split('#');
-		// if (!splitOnHash || splitOnHash.length !== 2) {
-		// 	output.write(`Parse error. Relative or absolute path to directory should be of the form: <PATH>#<FEATURE>`, LogLevel.Error);
-		// 	return undefined;
-		// }
-		// const filePath = splitOnHash[0];
-		// const id = splitOnHash[1];
-		// if (!allowedFeatureIdRegex.test(id)) {
-		// 	output.write(`Parse error. Specify a feature id with alphanumeric, dash, or underscore characters. Provided: ${id}.`, LogLevel.Error);
-		// 	return undefined;
-		// }
-		// return {
-		// 	id,
-		// 	sourceInformation: { 'type': 'file-path', filePath, isRelative: input.startsWith('./') }
-		// };
-	}
-
-	// Must be case (1) - GH
-	let version = 'latest';
-	let splitOnAt = input.split('@');
-	if (splitOnAt.length > 2) {
-		output.write(`Parse error. Use the '@' symbol only to designate a version tag.`, LogLevel.Error);
-		return undefined;
-	}
-	if (splitOnAt.length === 2) {
-		output.write(`[${input}] has version ${splitOnAt[1]}`, LogLevel.Trace);
-		version = splitOnAt[1];
-	}
-	// Remaining info must be in the first part of the split.
-	const featureBlob = splitOnAt[0];
-	const splitOnSlash = featureBlob.split('/');
-	// We expect all GitHub/registry features to follow the triple slash pattern at this point
-	//  eg: <publisher>/<feature-set>/<feature>
-	if (splitOnSlash.length !== 3 || splitOnSlash.some(x => x === '') || !allowedFeatureIdRegex.test(splitOnSlash[2])) {
-		output.write(`Invalid parse for GitHub/registry feature identifier. Follow format: '<publisher>/<feature-set>/<feature>'`, LogLevel.Error);
-		return undefined;
-	}
-	const owner = splitOnSlash[0];
-	const repo = splitOnSlash[1];
-	const id = splitOnSlash[2];
-
-	// Return expected tarball URI for a latest release on the parsed repo.
-	const ghSrcInfo = createGitHubSourceInformation({ owner, repo, tag: version });
-	return {
-		id,
-		sourceInformation: ghSrcInfo
-	};
-}
-
-export function createGitHubSourceInformation(params: GithubSourceInformationInput): GithubSourceInformation {
-	const { owner, repo, tag } = params;
-	if (tag === 'latest') {
-		return {
-			type: 'github-repo',
-			apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/latest`,
-			unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download/${ASSET_NAME}`,
-			owner,
-			repo,
-			isLatest: true
-		};
-	} else {
-		// We must have a tag, return a tarball URI for the tagged version. 
-		return {
-			type: 'github-repo',
-			apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/tags/${tag}`,
-			unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${tag}/${ASSET_NAME}`,
-			owner,
-			repo,
-			tag,
-			isLatest: false
-		};
-	}
-}
-
 
 const cleanupIterationFetchAndMerge = async (tempTarballPath: string, output: Log) => {
 	// Non-fatal, will just get overwritten if we don't do the cleaned up.
@@ -400,175 +301,6 @@ function getRequestHeaders(sourceInformation: SourceInformation, env: NodeJS.Pro
    return headers;
 }
 
-async function fetchAndMergeRemoteFeaturesAsync(params: { extensionPath: string; output: Log; env: NodeJS.ProcessEnv }, featuresConfig: FeaturesConfig, config: DevContainerConfig) {
-
-	const { output, env } = params;
-	const { dstFolder } = featuresConfig;
-	let buildFoldersCreatedAlready: String[] = [];
-
-	// The requested features from the user's devcontainer
-	const features = config.features;
-	if (!features || !Object.keys(features).length) {
-		return undefined;
-	}
-
-	// We need a dstFolder to know where to download remote resources to
-	if (!dstFolder) {
-		return undefined;
-	}
-
-	const tempTarballPath = path.join(dstFolder, ASSET_NAME);
-
-	output.write(`Preparing to parse declared features and fetch remote features.`);
-
-	for await (const id of Object.keys(features)) {
-		const remoteFeatureParsed = parseFeatureIdentifier(id, output);
-
-		if (remoteFeatureParsed === undefined) {
-			output.write(`Failed to parse key: ${id}`, LogLevel.Error);
-			// Failed to parse.
-			// TODO: Should be more fatal.
-			continue;
-		}
-
-		// -- Next section handles each possible type of "SourceInformation"
-
-		const featureName = remoteFeatureParsed.id;
-		const sourceInformation = remoteFeatureParsed.sourceInformation;
-		const sourceType = sourceInformation.type;
-
-		if (sourceType === 'local-cache') {
-			output.write(`Detected local feature set. Continuing...`);
-			continue;
-		}
-
-		const buildFolderName = getSourceInfoString(remoteFeatureParsed.sourceInformation);
-		// Calculate some predictable caching paths.
-		// Don't create the folder on-disk until we need it.
-		const featCachePath = path.join(dstFolder, buildFolderName);
-
-		// Break out earlier if already copied over remote features to dstFolder
-		const alreadyExists = buildFoldersCreatedAlready.some(x => x === buildFolderName);
-		if (alreadyExists) {
-			output.write(`Already pulled remote resource for '${buildFolderName}'. No need to re-fetch.`); //TODO: not true, might have been updated on the repo since if we pulled `local`.  Should probably use commit SHA? 
-			continue;
-		}
-
-		output.write(`Fetching: featureSet = ${buildFolderName}, feature = ${featureName}, Type = ${sourceType}`);
-
-		if (sourceType === 'file-path') {
-			output.write(`Local file-path to features on disk is unimplemented. Continuing...`);
-			continue;
-		} else {
-			let tarballUri: string | undefined = undefined;
-			const headers = getRequestHeaders(sourceInformation, env, output);
-
-			// If this is 'github-repo', we need to do an API call to fetch the appropriate asset's tarballUri
-			if (sourceType === 'github-repo') {
-				output.write('Determining tarball URI for provided github repo.', LogLevel.Trace);
-				if (headers.Authorization && headers.Authorization !== '') {
-					output.write('Authenticated. Fetching from GH API.', LogLevel.Trace);
-					tarballUri = await askGitHubApiForTarballUri(sourceInformation, headers, output);
-					headers.Accept = 'Accept: application/octet-stream';
-				} else {
-					output.write('Not authenticated. Fetching from unauthenticated uri', LogLevel.Trace);
-					tarballUri = sourceInformation.unauthenticatedUri;
-				}
-			} else if (sourceType === 'direct-tarball') {
-				tarballUri = sourceInformation.tarballUri;
-			} else {
-				output.write(`Unhandled source type: ${sourceType}`, LogLevel.Error);
-				continue;  // TODO: Should be more fatal?
-			}
-
-			// uri direct to the tarball either acquired at this point, or failed.
-			if (tarballUri !== undefined && tarballUri !== '') {
-				const options = {
-					type: 'GET',
-					url: tarballUri,
-					headers
-				};
-				output.write(`Fetching tarball at ${options.url}`);
-				output.write(`Headers: ${JSON.stringify(options)}`, LogLevel.Trace);
-				const tarball = await request(options, output);
-
-				if (!tarball || tarball.length === 0) {
-					output.write(`Did not receive a response from tarball download URI`, LogLevel.Error);
-					// Continue loop to the next remote feature.
-					// TODO: Should be more fatal.
-					await cleanupIterationFetchAndMerge(tempTarballPath, output);
-					continue;
-				}
-
-				// Filter what gets emitted from the tar.extract().
-				const filter = (file: string, _: tar.FileStat) => {
-					// Don't include .dotfiles or the archive itself.
-					if (file.startsWith('./.') || file === `./${ASSET_NAME}` || file === './.') {
-						return false;
-					}
-					return true;
-				};
-
-				output.write(`Preparing to unarchive received tgz.`, LogLevel.Trace);
-				// Create the directory to cache this feature-set in.
-				await mkdirpLocal(featCachePath);
-				await writeLocalFile(tempTarballPath, tarball);
-				await tar.x(
-					{
-						file: tempTarballPath,
-						cwd: featCachePath,
-						filter
-					}
-				);
-
-			} else {
-				output.write(`Could not fetch features from constructed tarball URL`, LogLevel.Error);
-				// Continue loop to the next remote feature.
-				// TODO: Should be more fatal.
-				await cleanupIterationFetchAndMerge(tempTarballPath, output);
-				continue;
-			}
-		}
-
-		// -- Whichever modality the feature-set was stored, at this point that process of retrieving and extracting a feature-set has completed successfully.
-		//    Now, load in the devcontainer-features.json from the `featureCachePath` and continue merging into the featuresConfig.
-
-		output.write('Attempting to load devcontainer-features.json', LogLevel.Trace);
-		let newFeaturesSet: FeatureSet | undefined = await loadFeaturesJsonFromDisk(featCachePath, output);
-
-		if (!newFeaturesSet || !newFeaturesSet.features || newFeaturesSet.features.length === 0) {
-			output.write(`Unable to parse received devcontainer-features.json.`, LogLevel.Error);
-			// TODO: Should be more fatal?
-			await cleanupIterationFetchAndMerge(tempTarballPath, output);
-			continue;
-		}
-		output.write(`Done loading FeatureSet ${buildFolderName} into from disk into memory`, LogLevel.Trace);
-
-		// Merge sourceInformation if the remote featureSet provides one.
-		// Priority is to maintain the values we had calculated previously.
-		if (newFeaturesSet.sourceInformation) {
-			newFeaturesSet = {
-				...newFeaturesSet,
-				sourceInformation: { ...newFeaturesSet.sourceInformation, ...sourceInformation },
-			};
-		}
-		output.write(`Merged sourceInfomation`, LogLevel.Trace);
-
-		// Add this new feature set to our featuresConfig
-		featuresConfig.featureSets.push(newFeaturesSet);
-		// Remember that we've succeeded in fetching this featureSet
-		buildFoldersCreatedAlready.push(buildFolderName);
-
-		// Clean-up
-		await cleanupIterationFetchAndMerge(tempTarballPath, output);
-		output.write(`Succeeded in fetching feature set ${buildFolderName}`, LogLevel.Trace);
-	}
-
-	// Return updated featuresConfig
-	return featuresConfig;
-}
-
-
 async function askGitHubApiForTarballUri(sourceInformation: GithubSourceInformation, headers: { 'user-agent': string; 'Authorization'?: string; 'Accept'?: string }, output: Log) {
 	const options = {
 		type: 'GET',
@@ -643,11 +375,22 @@ function updateFromOldProperties<T extends { features: (Feature & { extensions?:
 
 // Generate a base featuresConfig object with the set of locally-cached features, 
 // as well as downloading and merging in remote feature definitions.
-export async function generateFeaturesConfig(params: { extensionPath: string; output: Log; env: NodeJS.ProcessEnv }, dstFolder: string, config: DevContainerConfig, imageLabels: () => Promise<Record<string, string | undefined>>, getLocalFolder: (d: string) => string) {
+export async function generateFeaturesConfig(params: { extensionPath: string; cwd: string ;output: Log; env: NodeJS.ProcessEnv }, dstFolder: string, config: DevContainerConfig, imageLabels: () => Promise<Record<string, string | undefined>>, getLocalFolder: (d: string) => string) {
 	const { output } = params;
 
-	const userDeclaredFeatures = config.features;
-	if (!userDeclaredFeatures || !Object.keys(userDeclaredFeatures).length) {
+	if (!config.features)
+	{
+		return undefined;
+	}
+
+	if (!imageLabels)
+	{
+		return undefined;
+	}
+
+	const userFeatures = convertOldFeatures(params, config);
+
+	if(!userFeatures) {
 		return undefined;
 	}
 
@@ -658,125 +401,335 @@ export async function generateFeaturesConfig(params: { extensionPath: string; ou
 		dstFolder
 	};
 
+	// load local cache of features;
+	// TODO: Update so that cached features are always version 2
 	let locallyCachedFeatureSet = await loadFeaturesJsonFromDisk(getLocalFolder(params.extensionPath), output); // TODO: Pass dist folder instead to also work with the devcontainer.json support package.
 	if (!locallyCachedFeatureSet) {
 		output.write('Failed to load locally cached features', LogLevel.Error);
 		return undefined;
 	}
 
-	// Add in the locally cached features
-	locallyCachedFeatureSet = {
-		...locallyCachedFeatureSet,
-		sourceInformation: { 'type': 'local-cache' },
-	};
+	// Read features and get the type.
+	featuresConfig = await processUserFeatures(params.output, userFeatures, featuresConfig);
+
+	// Fetch features and get version information
+	await fetchFeatures(params, featuresConfig, locallyCachedFeatureSet, dstFolder);
+
+	return featuresConfig;
+}
 
-	// Push feature set to FeaturesConfig
-	featuresConfig.featureSets.push(locallyCachedFeatureSet);
+// Convert features from object sintax to array sintax
+function convertOldFeatures(params: { output: Log }, config: DevContainerConfig): DevContainerFeature[] | undefined 
+{
+	params.output.write('');
 
-	// Parse, fetch, and merge information on remote features (if any).
-	// TODO: right now if anything fails in this method and we return `undefined`, we fallback to just the prior state of featureConfig (locally cached only). Is that what we want??
-	featuresConfig = await fetchAndMergeRemoteFeaturesAsync(params, featuresConfig, config) ?? featuresConfig;
+	let userFeatures: DevContainerFeature[] = [];
+	if (Array.isArray(config.features))
+	{
+		userFeatures = config.features;
+	}
+	else {
+		if (!config.features || !Object.keys(config.features).length) {
+			return undefined;
+		}
 
-	// Run filtering and include user options into config.
-	featuresConfig = await doReadUserDeclaredFeatures(params, config, featuresConfig, imageLabels);
-	if (featuresConfig.featureSets.every(set =>
-		set.features.every(feature => feature.value === false))) {
-		return undefined;
+		for (const userFeatureKey of Object.keys(config.features)) {
+			const userFeatureValue = config.features[userFeatureKey];
+			if(userFeatureKey)
+			{
+				let feature : DevContainerFeature = {
+					id: userFeatureKey,
+					options: userFeatureValue
+				};
+				userFeatures.push(feature);
+			}
+		}
 	}
 
+	return userFeatures;
+}
+
+// Process features contained in devcontainer.json
+async function processUserFeatures(output: Log, userFeatures: DevContainerFeature[], featuresConfig: FeaturesConfig) : Promise<FeaturesConfig>
+{
+	userFeatures.forEach(userFeature => {
+			const newFeatureSet = parseFeatureIdentifier(output, userFeature);
+			if(newFeatureSet) {
+				featuresConfig.featureSets.push(newFeatureSet);
+			}
+		}
+	);
 	return featuresConfig;
 }
 
-const getUniqueFeatureId = (id: string, srcInfo: SourceInformation) => `${id}-${getSourceInfoString(srcInfo)}`;
+export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFeature) : FeatureSet | undefined {
+	output.write(`Processing feature: ${userFeature.id}`)
+			// cached feature
+			if (!userFeature.id.includes('/')) {
+				output.write(`Cached feature found.`);
 
-// Given an existing featuresConfig, parse the user's features as they declared them in their devcontainer.
-export async function doReadUserDeclaredFeatures(params: { output: Log }, config: DevContainerConfig, featuresConfig: FeaturesConfig, imageLabels: () => Promise<Record<string, string | undefined>>) {
+				let feat: Feature = {
+					id: userFeature.id,
+					name: userFeature.id,
+					value: userFeature.options,
+					included: true,
+				}
 
-	const { output } = params;
-	const labels = await imageLabels();
-	const definition = labels['com.visualstudio.code.devcontainers.id'];
-	const version = labels['version'];
-
-	// Map user's declared features to its appropriate feature-set feature.
-	let configFeatures = config.features || {};
-	let userValues: Record<string, string | boolean | Record<string, string | boolean>> = {};
-	for (const feat of Object.keys(configFeatures)) {
-		const { id, sourceInformation } = parseFeatureIdentifier(feat, output) ?? {};
-		if (id && sourceInformation) {
-			const uniqueId = getUniqueFeatureId(id, sourceInformation);
-			userValues[uniqueId] = configFeatures[feat];
-		} else {
-			output.write(`Failed to read user declared feature ${feat}. Skipping.`, LogLevel.Error);
-			continue;
-		}
-	}
+				let newFeaturesSet: FeatureSet = {
+					sourceInformation: {
+						type: 'local-cache', 
+					},
+					features: [feat],
+				};
+				
+				return newFeaturesSet;
+			}
 
-	const included = {} as Record<string, boolean | undefined>;
-	for (const featureSet of featuresConfig.featureSets) {
-		for (const feature of featureSet.features) {
-			updateFeature(feature); // REMOVEME: Temporary migration.
-			const uniqueFeatureId = getUniqueFeatureId(feature.id, featureSet.sourceInformation);
-
-			// Compare the feature to the base definition.
-			if (definition && (feature.exclude || []).some(e => matches(e, definition, version))) {
-				// The feature explicitly excludes the detected base definition
-				feature.included = false;
-			} else if ('include' in feature) {
-				// The feature explicitly includes one or more base definitions
-				// Set the included flag to true IFF we have detected a base definition, and its in the feature's list of includes
-				feature.included = !!definition && (feature.include || []).some(e => matches(e, definition, version));
-			} else {
-				// The feature doesn't define any base definitions to "include" or "exclude" in which we can filter on.
-				// By default, include it.
-				feature.included = true;
+			// remote tar file
+			if (userFeature.id.startsWith('http://') || userFeature.id.startsWith('https://'))
+			{
+				output.write(`Remote tar file found.`);
+				let input = userFeature.id.replace(/\/+$/, '');
+				const featureIdDelimiter = input.lastIndexOf('#');
+				const id = input.substring(featureIdDelimiter + 1);
+
+				const tarballUri = new URL.URL(input.substring(0, featureIdDelimiter)).toString();
+				let feat: Feature = {
+					id: id,
+					name: userFeature.id,
+					value: userFeature.options,
+					included: true,
+				}
+
+				let newFeaturesSet: FeatureSet = {
+					sourceInformation: {
+						type: 'direct-tarball', 
+						tarballUri: tarballUri 
+					},
+					features: [feat],
+				};
+
+				return newFeaturesSet;
 			}
 
-			// Mark feature as with its state of inclusion
-			included[uniqueFeatureId] = included[uniqueFeatureId] || feature.included;
+			// local disk
+			if (userFeature.id.startsWith('./') || userFeature.id.startsWith('../') || userFeature.id.startsWith('/')) {
+				output.write(`Local disk feature.`);
+				const filePath = userFeature.id;
+				const splitPath = userFeature.id.split('/');
+				const id = splitPath[splitPath.length - 1];
+				const isRelative = !userFeature.id.startsWith('/');
+				
+				let feat: Feature = {
+					id: id,
+					name: userFeature.id,
+					value: userFeature.options,
+					included: true,
+				}
 
-			// Set the user-defined values from the user's devcontainer onto the feature config.
-			feature.value = userValues[uniqueFeatureId] || false;
-		}
-	}
-	params.output.write('Feature configuration:\n' + JSON.stringify({ ...featuresConfig, imageDetails: undefined }, undefined, '  '), LogLevel.Trace);
-
-	// Filter
-	for (const featureSet of featuresConfig.featureSets) {
-		featureSet.features = featureSet.features.filter(feature => {
-			const uniqueFeatureId = getUniqueFeatureId(feature.id, featureSet.sourceInformation);
-			// Ensure we are not including duplicate features.
-			// Note: Takes first feature even if f.included == false.
-			if (uniqueFeatureId in included && feature.included === included[uniqueFeatureId]) {  	// TODO: This logic should be revisited.
-				delete included[feature.id];
-				return true;
+				let newFeaturesSet: FeatureSet = {
+					sourceInformation: {
+						type: 'file-path', 
+						filePath, 
+						isRelative: isRelative
+					},
+					features: [feat],
+				};
+
+				return newFeaturesSet;
 			}
-			return false;
-		});
-	}
-	return featuresConfig;
-}
 
-function updateFeature(feature: Feature & { type?: 'option' | 'choice'; values?: string[]; customValues?: boolean; hint?: string }) {
-	// Update old to new properties for temporary compatiblity.
-	if (feature.values) {
-		const options = feature.options || (feature.options = {});
-		options.version = {
-			type: 'string',
-			[feature.customValues ? 'proposals' : 'enum']: feature.values,
-			default: feature.values[0],
-			description: feature.hint,
-		};
-	}
-	delete feature.type;
-	delete feature.values;
-	delete feature.customValues;
-	delete feature.hint;
+			output.write(`Github feature.`);
+			// Github repository source.
+			let version = 'latest';
+			let splitOnAt = userFeature.id.split('@');
+			if (splitOnAt.length > 2) {
+				output.write(`Parse error. Use the '@' symbol only to designate a version tag.`, LogLevel.Error);
+				return undefined;
+			}
+			if (splitOnAt.length === 2) {
+				output.write(`[${userFeature.id}] has version ${splitOnAt[1]}`, LogLevel.Trace);
+				version = splitOnAt[1];
+			}
+
+			// Remaining info must be in the first part of the split.
+			const featureBlob = splitOnAt[0];
+			const splitOnSlash = featureBlob.split('/');
+			// We expect all GitHub/registry features to follow the triple slash pattern at this point
+			//  eg: <publisher>/<feature-set>/<feature>
+			if (splitOnSlash.length !== 3 || splitOnSlash.some(x => x === '') || !allowedFeatureIdRegex.test(splitOnSlash[2])) {
+				output.write(`Invalid parse for GitHub/registry feature identifier. Follow format: '<publisher>/<feature-set>/<feature>'`, LogLevel.Error);
+				return undefined;
+			}
+			const owner = splitOnSlash[0];
+			const repo = splitOnSlash[1];
+			const id = splitOnSlash[2];
+
+			let feat: Feature = {
+				id: owner + repo + id,
+				name: userFeature.id,
+				value: userFeature.options,
+				included: true,
+			};
+
+			if (version === 'latest') {
+				let newFeaturesSet: FeatureSet = {
+					sourceInformation : {
+					type: 'github-repo',
+					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/latest`,
+					unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download/${ASSET_NAME}`,
+					owner,
+					repo,
+					isLatest: true
+					},
+					features: [feat],
+				};
+				return newFeaturesSet;
+			} else {
+				// We must have a tag, return a tarball URI for the tagged version. 
+				let newFeaturesSet: FeatureSet = {
+					sourceInformation : {
+					type: 'github-repo',
+					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/tags/${version}`,
+					unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${version}/${ASSET_NAME}`,
+					owner,
+					repo,
+					tag: version,
+					isLatest: false
+					},
+					features: [feat],
+				};
+				return newFeaturesSet;
+			}
 }
 
-function matches(spec: string, definition: string, version: string | undefined) {
-	const i = spec.indexOf('@');
-	const [specDefinition, specVersion] = i !== -1 ? [spec.slice(0, i), spec.slice(i + 1)] : [spec, undefined];
-	return definition === specDefinition && (!specVersion || !version || semver.satisfies(version, specVersion));
+async function fetchFeatures(params: { extensionPath: string; cwd: string; output: Log; env: NodeJS.ProcessEnv }, featuresConfig: FeaturesConfig, localFeatures: FeatureSet, dstFolder: string) {
+	for(const featureSet of featuresConfig.featureSets)	{
+		try {
+			if (!featureSet || !featureSet.features || !featureSet.sourceInformation)
+			{
+				return;
+			}
+
+			if(!localFeatures)
+			{
+				return;
+			}
+
+			const consecutiveId = featureSet.features[0].id + '_' + getCounter();
+			// Calculate some predictable caching paths.
+			const featCachePath = path.join(dstFolder, consecutiveId);
+
+			featureSet.features[0].infoString = featCachePath;
+			featureSet.features[0].consecutiveId = consecutiveId;
+
+			if(featureSet.sourceInformation?.type === 'local-cache') {
+				params.output.write(`Detected local feature set. Continuing...`);
+
+				
+				return;
+			}
+		
+			if(featureSet.sourceInformation?.type === 'file-path') {
+				params.output.write(`Detected local file path`);
+				
+				const executionPath = path.join(params.cwd, featureSet.sourceInformation?.filePath);
+				const jsonPath = path.join(executionPath, 'feature.json');
+
+				const jsonString: Buffer = await readLocalFile(jsonPath);
+				const featureJson = jsonc.parse(jsonString.toString());
+
+				featureSet.features[0].runApp = featureJson.install.app ?? '';
+				featureSet.features[0].runParams = featureJson.install.file ?? 'install.sh';
+				featureSet.features[0].containerEnv = featureJson.containerEnv;
+
+				// We only support version 2 for local features.
+				featureSet.internalVersion = '2';
+
+				await mkdirpLocal(featCachePath);
+				await cpDirectoryLocal(executionPath, featCachePath);
+				continue;
+			}
+
+			
+			const tempTarballPath = path.join(dstFolder, ASSET_NAME);
+			params.output.write(`Detected tarball`);
+			const headers = getRequestHeaders(featureSet.sourceInformation, params.env, params.output);
+			let tarballUri: string | undefined = undefined;
+			if (featureSet.sourceInformation.type === 'github-repo') {
+				params.output.write('Determining tarball URI for provided github repo.', LogLevel.Trace);
+				if (headers.Authorization && headers.Authorization !== '') {
+					params.output.write('Authenticated. Fetching from GH API.', LogLevel.Trace);
+					tarballUri = await askGitHubApiForTarballUri(featureSet.sourceInformation, headers, params.output);
+					headers.Accept = 'Accept: application/octet-stream';
+				} else {
+					params.output.write('Not authenticated. Fetching from unauthenticated uri', LogLevel.Trace);
+					tarballUri = featureSet.sourceInformation.unauthenticatedUri;
+				}
+			} else {
+				tarballUri = featureSet.sourceInformation.tarballUri;
+			}
+			
+			if(tarballUri) {
+				const options = {
+					type: 'GET',
+					url: tarballUri,
+					headers
+				};
+				params.output.write(`Fetching tarball at ${options.url}`);
+				params.output.write(`Headers: ${JSON.stringify(options)}`, LogLevel.Trace);
+				const tarball = await request(options, params.output);
+
+				if (!tarball || tarball.length === 0) {
+					params.output.write(`Did not receive a response from tarball download URI`, LogLevel.Error);
+					// Continue loop to the next remote feature.
+					// TODO: Should be more fatal.
+					await cleanupIterationFetchAndMerge(tempTarballPath, params.output);
+					continue;
+				}
+
+				// Filter what gets emitted from the tar.extract().
+				const filter = (file: string, _: tar.FileStat) => {
+					// Don't include .dotfiles or the archive itself.
+					if (file.startsWith('./.') || file === `./${ASSET_NAME}` || file === './.') {
+						return false;
+					}
+					return true;
+				};
+
+				params.output.write(`Preparing to unarchive received tgz.`, LogLevel.Trace);
+				// Create the directory to cache this feature-set in.
+				await mkdirpLocal(featCachePath);
+				await writeLocalFile(tempTarballPath, tarball);
+				await tar.x(
+					{
+						file: tempTarballPath,
+						cwd: featCachePath,
+						filter
+					}
+				);
+
+				// Read version information.
+				const jsonPath = path.join(featCachePath, 'feature.json');
+
+				if (existsSync(jsonPath)) {
+					const jsonString: Buffer = await readLocalFile(jsonPath);
+					const featureJson = jsonc.parse(jsonString.toString());
+					featureSet.features[0].runApp = featureJson.install.app ?? '';
+					featureSet.features[0].runParams = featureJson.install.file ?? 'install.sh';
+					featureSet.features[0].containerEnv = featureJson.containerEnv;
+					featureSet.internalVersion === '2';
+				} else {
+					featureSet.internalVersion === '1';
+				}
+
+			}
+			continue;
+		}
+		catch (e) {
+			params.output.write(`Exception: ${e?.Message} `, LogLevel.Trace);
+		}
+	}
 }
 
 export function getFeatureMainProperty(feature: Feature) {
diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
index e5ad42033..08e9ff593 100644
--- a/src/spec-node/containerFeatures.ts
+++ b/src/spec-node/containerFeatures.ts
@@ -30,11 +30,15 @@ export async function extendImage(params: DockerResolverParameters, config: DevC
 export function generateContainerEnvs(featuresConfig: FeaturesConfig) {
 	let result = '';
 	for (const fSet of featuresConfig.featureSets) {
-		result += fSet.features
-			.filter(f => (includeAllConfiguredFeatures || f.included) && f.value)
-			.reduce((envs, f) => envs.concat(Object.keys(f.containerEnv || {})
-				.map(k => `ENV ${k}=${f.containerEnv![k]}`)), [] as string[])
-			.join('\n');
+		// We only need to generate this ENV references for the old features spec.
+		if(fSet.internalVersion !== '2')
+		{
+			result += fSet.features
+				.filter(f => (includeAllConfiguredFeatures || f.included) && f.value)
+				.reduce((envs, f) => envs.concat(Object.keys(f.containerEnv || {})
+					.map(k => `ENV ${k}=${f.containerEnv![k]}`)), [] as string[])
+				.join('\n');
+		}
 	}
 	return result;
 }
@@ -117,28 +121,43 @@ async function addContainerFeatures(params: DockerResolverParameters, featuresCo
 	await cliHost.writeFile(cliHost.path.join(dstFolder, 'Dockerfile'), Buffer.from(dockerfile));
 
 	// Build devcontainer-features.env file(s) for each features source folder
-	await Promise.all([...featuresConfig.featureSets].map(async (featureSet, i) => {
-		const featuresEnv = ([] as string[]).concat(
-			...featureSet.features
-				.filter(f => (includeAllConfiguredFeatures || f.included) && f.value && !buildStageScripts[i][f.id]?.hasAcquire)
-				.map(getFeatureEnvVariables)
-		).join('\n');
-		const envPath = cliHost.path.join(dstFolder, getSourceInfoString(featureSet.sourceInformation), 'devcontainer-features.env'); // next to install.sh
-		await Promise.all([
-			cliHost.writeFile(envPath, Buffer.from(featuresEnv)),
-			...featureSet.features
-			.filter(f => (includeAllConfiguredFeatures || f.included) && f.value && buildStageScripts[i][f.id]?.hasAcquire)
-			.map(f => {
-					const featuresEnv = [
-						...getFeatureEnvVariables(f),
-						`_BUILD_ARG_${getFeatureSafeId(f)}_TARGETPATH=${path.posix.join('/usr/local/devcontainer-features', getSourceInfoString(featureSet.sourceInformation), f.id)}`
-					]
-						.join('\n');
-					const envPath = cliHost.path.join(dstFolder, getSourceInfoString(featureSet.sourceInformation), 'features', f.id, 'devcontainer-features.env'); // next to bin/acquire
-					return cliHost.writeFile(envPath, Buffer.from(featuresEnv));
-				})
-		]);
-	}));
+	for await (const fSet of featuresConfig.featureSets) {
+		let i = 0;
+		if(fSet.internalVersion === '2')
+		{
+			for await (const fe of fSet.features) {
+				if (fe.infoString)
+				{
+					fe.internalVersion = '2';
+					const envPath = cliHost.path.join(fe.infoString, 'devcontainer-features.env');
+					const variables = getFeatureEnvVariables(fe);
+					await cliHost.writeFile(envPath, Buffer.from(variables.join('\n')));
+				}
+			}
+		} else {
+			const featuresEnv = ([] as string[]).concat(
+				...fSet.features
+					.filter(f => (includeAllConfiguredFeatures|| f.included) && f.value && !buildStageScripts[i][f.id]?.hasAcquire)
+					.map(getFeatureEnvVariables)
+			).join('\n');
+			const envPath = cliHost.path.join(dstFolder, getSourceInfoString(fSet.sourceInformation), 'devcontainer-features.env'); // next to install.sh
+			await Promise.all([
+				cliHost.writeFile(envPath, Buffer.from(featuresEnv)),
+				...fSet.features
+				.filter(f => (includeAllConfiguredFeatures || f.included) && f.value && buildStageScripts[i][f.id]?.hasAcquire)
+				.map(f => {
+						const featuresEnv = [
+							...getFeatureEnvVariables(f),
+							`_BUILD_ARG_${getFeatureSafeId(f)}_TARGETPATH=${path.posix.join('/usr/local/devcontainer-features', getSourceInfoString(fSet.sourceInformation), f.id)}`
+						]
+							.join('\n');
+						const envPath = cliHost.path.join(dstFolder, getSourceInfoString(fSet.sourceInformation), 'features', f.id, 'devcontainer-features.env'); // next to bin/acquire
+						return cliHost.writeFile(envPath, Buffer.from(featuresEnv));
+					})
+			]);
+		}
+		i++;
+	}
 
 	const args = [
 		'build',
@@ -182,15 +201,28 @@ function getFeatureEnvVariables(f: Feature) {
 	const values = getFeatureValueObject(f);
 	const idSafe = getFeatureSafeId(f);
 	const variables = [];
-	if (values) {
-		variables.push(...Object.keys(values)
-			.map(name => `_BUILD_ARG_${idSafe}_${name.toUpperCase()}="${values[name]}"`));
-		variables.push(`_BUILD_ARG_${idSafe}=true`);
-	}
-	if (f.buildArg) {
-		variables.push(`${f.buildArg}=${getFeatureMainValue(f)}`);
-	}
-	return variables;
+	
+	if(f.internalVersion !== '2')
+	{
+		if (values) {
+			variables.push(...Object.keys(values)
+				.map(name => `_BUILD_ARG_${idSafe}_${name.toUpperCase()}="${values[name]}"`));
+			variables.push(`_BUILD_ARG_${idSafe}=true`);
+		}
+		if (f.buildArg) {
+			variables.push(`${f.buildArg}=${getFeatureMainValue(f)}`);
+		}
+		return variables;
+	} else {
+		if (values) {
+			variables.push(...Object.keys(values)
+			.map(name => `${idSafe}_${name.toUpperCase()}="${values[name]}"`));
+		}
+		if (f.buildArg) {
+			variables.push(`${f.buildArg}=${getFeatureMainValue(f)}`);
+		}
+		return variables;
+	}	
 }
 
 function getFeatureSafeId(f: Feature) {
diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts
index 1abadd574..942e52b0a 100644
--- a/src/spec-node/devContainersSpecCLI.ts
+++ b/src/spec-node/devContainersSpecCLI.ts
@@ -604,7 +604,7 @@ async function readConfiguration({
 		if (!configs) {
 			throw new ContainerError({ description: `Dev container config (${uriToFsPath(configFile || getDefaultDevContainerConfigPath(cliHost, workspace!.configFolderPath), cliHost.platform)}) not found.` });
 		}
-		const featuresConfiguration = includeFeaturesConfig ? await generateFeaturesConfig({ extensionPath, output, env: cliHost.env }, (await createFeaturesTempFolder({ cliHost, package: pkg })), configs.config, async () => /* TODO: ? (await imageDetails()).Config.Labels || */ ({}), getContainerFeaturesFolder) : undefined;
+		const featuresConfiguration = includeFeaturesConfig ? await generateFeaturesConfig({ extensionPath, cwd: process.cwd(), output, env: cliHost.env }, (await createFeaturesTempFolder({ cliHost, package: pkg })), configs.config, async () => /* TODO: ? (await imageDetails()).Config.Labels || */ ({}), getContainerFeaturesFolder) : undefined;
 		await new Promise<void>((resolve, reject) => {
 			process.stdout.write(JSON.stringify({
 				configuration: configs.config,
diff --git a/src/spec-utils/pfs.ts b/src/spec-utils/pfs.ts
index 9fd36ebc4..1e42a927c 100644
--- a/src/spec-utils/pfs.ts
+++ b/src/spec-utils/pfs.ts
@@ -6,6 +6,7 @@
 import * as fs from 'fs';
 import { promisify } from 'util';
 import * as path from 'path';
+import * as ncp from 'ncp';
 
 import { URI } from 'vscode-uri';
 
@@ -27,6 +28,7 @@ export const mkdirpLocal = (path: string) => new Promise<void>((res, rej) => fs.
 export const rmdirLocal = promisify(fs.rmdir);
 export const rmLocal = promisify(fs.rm);
 export const cpLocal = promisify(fs.copyFile);
+export const cpDirectoryLocal = promisify(ncp.ncp);
 
 export interface FileHost {
 	platform: NodeJS.Platform;
diff --git a/src/test/container-features/helpers.offline.test.ts b/src/test/container-features/helpers.offline.test.ts
index 3f974c97b..9fe385b66 100644
--- a/src/test/container-features/helpers.offline.test.ts
+++ b/src/test/container-features/helpers.offline.test.ts
@@ -1,23 +1,33 @@
 import { assert } from 'chai';
+import { DevContainerFeature } from '../../spec-configuration/configuration';
 import { getSourceInfoString, parseFeatureIdentifier, SourceInformation } from '../../spec-configuration/containerFeaturesConfiguration';
 import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';
 export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
 
 describe('validate function parseRemoteFeatureToDownloadUri', function () {
 
-    // -- Valid 
+    // // -- Valid 
 
     it('should parse local features and return an undefined tarballUrl', async function () {
-        const result = parseFeatureIdentifier('helloworld', output);
+        const feature: DevContainerFeature = {
+            id: 'helloworld',
+            options: {},
+        } 
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.strictEqual(result?.sourceInformation.type, 'local-cache');
     });
 
     it('should parse gitHub without version', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures/helloworld', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures/helloworld',
+            options: {},
+        }
+        const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
@@ -28,9 +38,14 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
     });
 
     it('should parse gitHub with version', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures/helloworld@v0.0.4', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures/helloworld@v0.0.4',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
@@ -42,81 +57,143 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
     });
 
     it('should parse generic tar', async function () {
-        const result = parseFeatureIdentifier('https://example.com/some/long/path/devcontainer-features.tgz#helloworld', output);
+        const feature: DevContainerFeature = {
+            id: 'https://example.com/some/long/path/devcontainer-features.tgz#helloworld',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'direct-tarball', tarballUri: 'https://example.com/some/long/path/devcontainer-features.tgz' });
     });
 
     it('should parse when provided a local-filesystem relative path', async function () {
-        const result = parseFeatureIdentifier('./some/long/path/to/features#helloworld', output);
-        assert.notExists(result);
-        // assert.exists(result);
-        // assert.strictEqual(result?.id, 'helloworld');
-        // assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: './some/long/path/to/features', isRelative: true });
+        const feature: DevContainerFeature = {
+            id: './some/long/path/to/helloworld',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
+        assert.exists(result);
+        assert.strictEqual(result?.features[0].id, 'helloworld');
+        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: './some/long/path/to/helloworld', isRelative: true });
     });
 
     it('should parse when provided a local-filesystem relative path, starting with ../', async function () {
-        const result = parseFeatureIdentifier('../some/long/path/to/features#helloworld', output);
-        assert.notExists(result);
-        // assert.exists(result);
-        // assert.strictEqual(result?.id, 'helloworld');
-        // assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '../some/long/path/to/features', isRelative: true });
+        const feature: DevContainerFeature = {
+            id: '../some/long/path/to/helloworld',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
+        
+        assert.exists(result);
+        assert.strictEqual(result?.features[0].id, 'helloworld');
+        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '../some/long/path/to/features', isRelative: true });
     });
 
     it('should parse when provided a local-filesystem absolute path', async function () {
-        const result = parseFeatureIdentifier('/some/long/path/to/features#helloworld', output);
-        assert.notExists(result);
-        // assert.exists(result);
-        // assert.strictEqual(result?.id, 'helloworld');
-        // assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '/some/long/path/to/features', isRelative: false });
+        const feature: DevContainerFeature = {
+            id: '/some/long/path/to/helloworld',
+            options: {},
+        }
+        const result = parseFeatureIdentifier(output, feature);
+        assert.exists(result);
+        assert.strictEqual(result?.features[0].id, 'helloworld');
+        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '/some/long/path/to/features', isRelative: false });
     });
 
 
     // -- Invalid
 
     it('should fail parsing a generic tar with no feature and trailing slash', async function () {
-        const result = parseFeatureIdentifier('https://example.com/some/long/path/devcontainer-features.tgz/', output);
+        const feature: DevContainerFeature = {
+            id: 'https://example.com/some/long/path/devcontainer-features.tgz/',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should not parse gitHub without triple slash', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures#helloworld', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures#helloworld',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a generic tar with no feature and no trailing slash', async function () {
-        const result = parseFeatureIdentifier('https://example.com/some/long/path/devcontainer-features.tgz', output);
+        const feature: DevContainerFeature = {
+            id: 'https://example.com/some/long/path/devcontainer-features.tgz',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a generic tar with a hash but no feature', async function () {
-        const result = parseFeatureIdentifier('https://example.com/some/long/path/devcontainer-features.tgz#', output);
+        const feature: DevContainerFeature = {
+            id: 'https://example.com/some/long/path/devcontainer-features.tgz#',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a marketplace shorthand with only two segments and a hash with no feature', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures#', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures#',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a marketplace shorthand with only two segments (no feature)', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a marketplace shorthand with an invalid feature name (1)', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures/@mycoolfeature', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures/@mycoolfeature',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a marketplace shorthand with an invalid feature name (2)', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures/MY_$UPER_COOL_FEATURE', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures/MY_$UPER_COOL_FEATURE',
+            options: {},
+        }
+
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 
     it('should fail parsing a marketplace shorthand with only two segments, no hash, and with a version', async function () {
-        const result = parseFeatureIdentifier('octocat/myfeatures@v0.0.1', output);
+        const feature: DevContainerFeature = {
+            id: 'octocat/myfeatures@v0.0.1',
+            options: {},
+        }
+        
+        const result = parseFeatureIdentifier(output, feature);
         assert.notExists(result);
     });
 });

From e82569204e4727e47b83e1b24bbec6b204797cf4 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 14:03:48 -0700
Subject: [PATCH 02/30] Missing reference.

---
 package.json |  2 ++
 yarn.lock    | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/package.json b/package.json
index 622c1c15c..c77e64450 100644
--- a/package.json
+++ b/package.json
@@ -63,6 +63,8 @@
 		"vinyl-fs": "^3.0.3"
 	},
 	"dependencies": {
+		"@types/ncp": "^2.0.5",
+		"ncp": "^2.0.0",
 		"follow-redirects": "^1.14.8",
 		"js-yaml": "^4.1.0",
 		"jsonc-parser": "^3.0.0",
diff --git a/yarn.lock b/yarn.lock
index de62a6b00..f3ed29a1e 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -151,6 +151,13 @@
   resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-9.1.0.tgz#baf17ab2cca3fcce2d322ebc30454bff487efad5"
   integrity sha512-QCWHkbMv4Y5U9oW10Uxbr45qMMSzl4OzijsozynUAgx3kEHUdXB00udx2dWDQ7f2TU2a2uuiFaRZjCe3unPpeg==
 
+"@types/ncp@^2.0.5":
+  version "2.0.5"
+  resolved "https://registry.yarnpkg.com/@types/ncp/-/ncp-2.0.5.tgz#5c53b229a321946102a188b603306162137f4fb9"
+  integrity sha512-ocK0p8JuFmX7UkMabFPjY0F7apPvQyLWt5qtdvuvQEBz9i4m2dbzV+6L1zNaUp042RfnL6pHnxDE53OH6XQ9VQ==
+  dependencies:
+    "@types/node" "*"
+
 "@types/node@*":
   version "14.14.41"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-14.14.41.tgz#d0b939d94c1d7bd53d04824af45f1139b8c45615"
@@ -1984,6 +1991,11 @@ natural-compare@^1.4.0:
   resolved "https://registry.yarnpkg.com/natural-compare/-/natural-compare-1.4.0.tgz#4abebfeed7541f2c27acfb29bdbbd15c8d5ba4f7"
   integrity sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc=
 
+ncp@^2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/ncp/-/ncp-2.0.0.tgz#195a21d6c46e361d2fb1281ba38b91e9df7bdbb3"
+  integrity sha1-GVoh1sRuNh0vsSgbo4uR6d9727M=
+
 nice-try@^1.0.4:
   version "1.0.5"
   resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366"

From 1047e01dcf1b1f6e77c3d26368ba372f7a2eab55 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 14:12:10 -0700
Subject: [PATCH 03/30] Fix test

---
 .../container-features/generateFeaturesConfig.offline.test.ts   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index f72ad3000..5ba4f449e 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -14,7 +14,7 @@ describe('validate (offline) generateFeaturesConfig()', function () {
 
     // Setup
     const env = { 'SOME_KEY': 'SOME_VAL'};
-    const params = { extensionPath: '', output, env, persistedFolder: '' };
+    const params = { extensionPath: '', cwd: '', output, env, persistedFolder: '' };
 
     // Mocha executes with the root of the project as the cwd.
     const localFeaturesFolder = (_: string) => {

From 40aa793ff700c716006e52daebbc82ab28d48fff Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 14:44:17 -0700
Subject: [PATCH 04/30] Fixes.

---
 .../containerFeaturesConfiguration.ts         |   5 +
 .../generateFeaturesConfig.offline.test.ts    | 110 +++++++++---------
 .../helpers.offline.test.ts                   |  14 +--
 3 files changed, 68 insertions(+), 61 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index a926a63ef..ca050eab4 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -493,6 +493,11 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 				const featureIdDelimiter = input.lastIndexOf('#');
 				const id = input.substring(featureIdDelimiter + 1);
 
+				if (id === '' || !allowedFeatureIdRegex.test(id)) {
+					output.write(`Parse error. Specify a feature id with alphanumeric, dash, or underscore characters. Provided: ${id}.`, LogLevel.Error);
+					return undefined;
+				}
+
 				const tarballUri = new URL.URL(input.substring(0, featureIdDelimiter)).toString();
 				let feat: Feature = {
 					id: id,
diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 5ba4f449e..3366e6792 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -34,61 +34,63 @@ describe('validate (offline) generateFeaturesConfig()', function () {
 
     it('should correctly return a featuresConfig with just local features', async function () {
 
-        const version = 'unittest';
-        const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
-        await mkdirpLocal(tmpFolder);
-
-        const config: DevContainerConfig = {
-            configFilePath: URI.from({ 'scheme': 'https' }),
-            dockerFile: '.',
-            features: {
-                first: {
-                    'version': 'latest',
-                    'option1': true
-                },
-                third: 'latest'
-            },
-        };
-
-        const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);
-        if (!featuresConfig) {
-            assert.fail();
-        }
-
-        const localFeatureSet = (featuresConfig?.featureSets.find(set => set.sourceInformation.type === 'local-cache'));
-        assert.exists(localFeatureSet);
-        assert.strictEqual(localFeatureSet?.features.length, 3);
-
-        const first = localFeatureSet?.features.find((f) => f.id === 'first');
-        assert.exists(first);
-
-        const second = localFeatureSet?.features.find((f) => f.id === 'second');
-        assert.exists(second);
-
-        const third = localFeatureSet?.features.find((f) => f.id === 'third');
-        assert.exists(third);
-
-        assert.isObject(first?.value);
-        assert.isBoolean(second?.value);
-        assert.isString(third?.value);
-
-        // -- Test containerFeatures.ts helper functions
-
-        // generateContainerEnvs
-// TODO
-//         const actualEnvs = generateContainerEnvs(featuresConfig);
-//         const expectedEnvs = `ENV MYKEYONE=MYRESULTONE
-// ENV MYKEYTHREE=MYRESULTHREE`;
-//         assert.strictEqual(actualEnvs, expectedEnvs);
+        // TODO, rewrite
+//         const version = 'unittest';
+//         const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
+//         await mkdirpLocal(tmpFolder);
+
+//         const config: DevContainerConfig = {
+//             configFilePath: URI.from({ 'scheme': 'https' }),
+//             dockerFile: '.',
+//             features: {
+//                 first: {
+//                     'version': 'latest',
+//                     'option1': true
+//                 },
+//                 second: 'latest',
+//                 third: 'latest'
+//             },
+//         };
+
+//         const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);
+//         if (!featuresConfig) {
+//             assert.fail();
+//         }
+
+//         const localFeatureSet = (featuresConfig?.featureSets.find(set => set.sourceInformation.type === 'local-cache'));
+//         assert.exists(localFeatureSet);
+//         assert.strictEqual(localFeatureSet?.features.length, 1);
+
+//         const first = localFeatureSet?.features.find((f) => f.id === 'first');
+//         assert.exists(first);
+
+//         const second = localFeatureSet?.features.find((f) => f.id === 'second');
+//         assert.exists(second);
+
+//         const third = localFeatureSet?.features.find((f) => f.id === 'third');
+//         assert.exists(third);
+
+//         assert.isObject(first?.value);
+//         assert.isBoolean(second?.value);
+//         assert.isString(third?.value);
+
+//         // -- Test containerFeatures.ts helper functions
+
+//         // generateContainerEnvs
+// // TODO
+// //         const actualEnvs = generateContainerEnvs(featuresConfig);
+// //         const expectedEnvs = `ENV MYKEYONE=MYRESULTONE
+// // ENV MYKEYTHREE=MYRESULTHREE`;
+// //         assert.strictEqual(actualEnvs, expectedEnvs);
     
-        // getFeatureLayers
-        const actualLayers = await getFeatureLayers(featuresConfig);
-        const expectedLayers = `RUN cd /tmp/build-features/local-cache \\
-&& chmod +x ./install.sh \\
-&& ./install.sh
-
-`;
-        assert.strictEqual(actualLayers, expectedLayers);
+//         // getFeatureLayers
+//         const actualLayers = await getFeatureLayers(featuresConfig);
+//         const expectedLayers = `RUN cd /tmp/build-features/local-cache \\
+// && chmod +x ./install.sh \\
+// && ./install.sh
+
+// `;
+//         assert.strictEqual(actualLayers, expectedLayers);
     });
 
 
diff --git a/src/test/container-features/helpers.offline.test.ts b/src/test/container-features/helpers.offline.test.ts
index 9fe385b66..bf093db88 100644
--- a/src/test/container-features/helpers.offline.test.ts
+++ b/src/test/container-features/helpers.offline.test.ts
@@ -27,7 +27,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
         }
         const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.features[0].id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'octocatmyfeatureshelloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
@@ -45,7 +45,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
 
         const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.features[0].id, 'helloworld');
+        assert.strictEqual(result?.features[0].id, 'octocatmyfeatureshelloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
@@ -90,7 +90,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
         
         assert.exists(result);
         assert.strictEqual(result?.features[0].id, 'helloworld');
-        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '../some/long/path/to/features', isRelative: true });
+        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '../some/long/path/to/helloworld', isRelative: true });
     });
 
     it('should parse when provided a local-filesystem absolute path', async function () {
@@ -101,7 +101,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
         const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
         assert.strictEqual(result?.features[0].id, 'helloworld');
-        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '/some/long/path/to/features', isRelative: false });
+        assert.deepEqual(result?.sourceInformation, { type: 'file-path', filePath: '/some/long/path/to/helloworld', isRelative: false });
     });
 
 
@@ -206,7 +206,7 @@ describe('validate function getSourceInfoString', function () {
             type : 'local-cache'
         };
         const output = getSourceInfoString(srcInfo);
-        assert.strictEqual(output, 'local-cache');
+        assert.include(output, 'local-cache');
     });
 
     it('should work for github-repo without a tag (implicit latest)', async function () {
@@ -219,7 +219,7 @@ describe('validate function getSourceInfoString', function () {
             unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/latest/download/devcontainer-features.tgz'
         };
         const output = getSourceInfoString(srcInfo);
-        assert.strictEqual(output, 'github-bob-mobileapp-latest');
+        assert.include(output, 'github-bob-mobileapp-latest');
     });
 
     it('should work for github-repo with a tag', async function () {
@@ -233,6 +233,6 @@ describe('validate function getSourceInfoString', function () {
             unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/download/v0.0.4/devcontainer-features.tgz'
         };
         const output = getSourceInfoString(srcInfo);
-        assert.strictEqual(output, 'github-bob-mobileapp-v0.0.4');
+        assert.include(output, 'github-bob-mobileapp-v0.0.4');
     });
 });
\ No newline at end of file

From 83afa1bd39b9dcc0ee86e9650c0f5a3c6e3910d8 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 14:44:46 -0700
Subject: [PATCH 05/30] Run config

---
 .vscode/launch.json | 17 +++++++++++++++++
 tsconfig.base.json  |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 .vscode/launch.json

diff --git a/.vscode/launch.json b/.vscode/launch.json
new file mode 100644
index 000000000..e5639a4b7
--- /dev/null
+++ b/.vscode/launch.json
@@ -0,0 +1,17 @@
+{
+    // Use IntelliSense to learn about possible attributes.
+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "type": "node",
+            "request": "launch",
+            "name": "Launch cli - up",
+            "program": "${workspaceFolder}/src/spec-node/devContainersSpecCLI.ts",
+            "cwd": "${workspaceFolder}",
+            "args": ["up", "--workspace-folder", "../features-playground", "--log-level", "debug", ],
+            "console": "integratedTerminal",
+        }
+    ]
+}
\ No newline at end of file
diff --git a/tsconfig.base.json b/tsconfig.base.json
index 77b55edf1..2bfc3ce9a 100644
--- a/tsconfig.base.json
+++ b/tsconfig.base.json
@@ -13,6 +13,7 @@
 		"noUnusedLocals": true,
 		"noUnusedParameters": true,
 		"useUnknownInCatchVariables": false,
-		"newLine": "LF"
+		"newLine": "LF",
+		"sourceMap": true
 	}
 }
\ No newline at end of file

From 39cff54ea3ddc74b9efabcec67113ffe3329691f Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 9 May 2022 14:50:51 -0700
Subject: [PATCH 06/30] Comment out test

---
 .../container-features/generateFeaturesConfig.offline.test.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 3366e6792..f37a61229 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -32,7 +32,7 @@ describe('validate (offline) generateFeaturesConfig()', function () {
         return record;
     };
 
-    it('should correctly return a featuresConfig with just local features', async function () {
+   // it('should correctly return a featuresConfig with just local features', async function () {
 
         // TODO, rewrite
 //         const version = 'unittest';
@@ -91,7 +91,7 @@ describe('validate (offline) generateFeaturesConfig()', function () {
 
 // `;
 //         assert.strictEqual(actualLayers, expectedLayers);
-    });
+  //  });
 
 
 

From 12d39a2c48df57b0bae6c4ce624f775341440bd4 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 10 May 2022 14:54:17 -0700
Subject: [PATCH 07/30] Fixes for absolute paths

---
 .../containerFeaturesConfiguration.ts         | 33 +++++++++----------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index ca050eab4..d70ad9c49 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -181,13 +181,13 @@ export function getSourceInfoString(srcInfo: SourceInformation): string {
 	const { type } = srcInfo;
 	switch (type) {
 		case 'local-cache':
-			return getCounter() + '-local-cache';
+			return 'local-cache-' + getCounter();
 		case 'direct-tarball':
-			return getCounter() + '-' + srcInfo.tarballUri;
+			return srcInfo.tarballUri + getCounter();
 		case 'github-repo':
-			return `${getCounter()}-github-${srcInfo.owner}-${srcInfo.repo}-${srcInfo.isLatest ? 'latest' : srcInfo.tag}`;
+			return `github-${srcInfo.owner}-${srcInfo.repo}-${srcInfo.isLatest ? 'latest' : srcInfo.tag}-${getCounter()}`;
 		case 'file-path':
-			return getCounter() + '-' + srcInfo.filePath;
+			return srcInfo.filePath + '-' + getCounter();
 	}
 }
 
@@ -218,7 +218,7 @@ USER $IMAGE_USER
 export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 	let result = '';
 
-	// Features version 1
+	// Features version 1  FIX
 	const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => getSourceInfoString(x.sourceInformation));
 	folders.forEach(folder => {
 		result += `RUN cd /tmp/build-features/${folder} \\
@@ -236,7 +236,6 @@ export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 				
 RUN cd /tmp/build-features/${feature.consecutiveId} \\
 && export $(cat devcontainer-features.env | xargs) \\
-&& echo $PATH \\
 && chmod +x ./${feature.runParams} \\
 && ./${feature.runParams}
 
@@ -418,7 +417,7 @@ export async function generateFeaturesConfig(params: { extensionPath: string; cw
 	return featuresConfig;
 }
 
-// Convert features from object sintax to array sintax
+// Convert features from object syntax to array syntax
 function convertOldFeatures(params: { output: Log }, config: DevContainerConfig): DevContainerFeature[] | undefined 
 {
 	params.output.write('');
@@ -465,7 +464,7 @@ async function processUserFeatures(output: Log, userFeatures: DevContainerFeatur
 export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFeature) : FeatureSet | undefined {
 	output.write(`Processing feature: ${userFeature.id}`)
 			// cached feature
-			if (!userFeature.id.includes('/')) {
+			if (!userFeature.id.includes('/') && !userFeature.id.includes('\\')) {
 				output.write(`Cached feature found.`);
 
 				let feat: Feature = {
@@ -518,12 +517,12 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 			}
 
 			// local disk
-			if (userFeature.id.startsWith('./') || userFeature.id.startsWith('../') || userFeature.id.startsWith('/')) {
+			const userFeaturePath = path.parse(userFeature.id);
+			if (userFeaturePath) {
 				output.write(`Local disk feature.`);
 				const filePath = userFeature.id;
-				const splitPath = userFeature.id.split('/');
-				const id = splitPath[splitPath.length - 1];
-				const isRelative = !userFeature.id.startsWith('/');
+				const id = userFeaturePath.name;
+				const isRelative = !path.isAbsolute(userFeature.id);
 				
 				let feat: Feature = {
 					id: id,
@@ -613,12 +612,12 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 		try {
 			if (!featureSet || !featureSet.features || !featureSet.sourceInformation)
 			{
-				return;
+				continue;
 			}
 
 			if(!localFeatures)
 			{
-				return;
+				continue;
 			}
 
 			const consecutiveId = featureSet.features[0].id + '_' + getCounter();
@@ -630,15 +629,13 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 
 			if(featureSet.sourceInformation?.type === 'local-cache') {
 				params.output.write(`Detected local feature set. Continuing...`);
-
-				
-				return;
+				continue;
 			}
 		
 			if(featureSet.sourceInformation?.type === 'file-path') {
 				params.output.write(`Detected local file path`);
 				
-				const executionPath = path.join(params.cwd, featureSet.sourceInformation?.filePath);
+				const executionPath = featureSet.sourceInformation.isRelative ? path.join(params.cwd, featureSet.sourceInformation.filePath) : featureSet.sourceInformation.filePath;
 				const jsonPath = path.join(executionPath, 'feature.json');
 
 				const jsonString: Buffer = await readLocalFile(jsonPath);

From e769e66995cc1f9fae5a23778b9a763b4c69bd29 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 10 May 2022 15:02:27 -0700
Subject: [PATCH 08/30] Comments on pr.

---
 .../containerFeaturesConfiguration.ts         | 36 +++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index d70ad9c49..3e22e880a 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -226,8 +226,7 @@ export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 && ./install.sh
 
 `;
-	
-});
+  });
 	// Features version 2
 	featuresConfig.featureSets.filter(y => y.internalVersion === '2').forEach(featureSet => {
 		featureSet.features.forEach(feature => {
@@ -462,6 +461,39 @@ async function processUserFeatures(output: Log, userFeatures: DevContainerFeatur
 }
 
 export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFeature) : FeatureSet | undefined {
+	// A identifier takes this form:
+	//      (0)  <feature>
+	//      (1)  <publisher>/<feature-set>/<feature>@version
+	//      (2)  https://<../URI/..>/devcontainer-features.tgz#<feature>
+	//      (3) ./<local-path>#<feature>  -or-  ../<local-path>#<feature>  -or-   /<local-path>#<feature>
+	// 
+	//  (0) This is a locally cached feature.
+	//
+	//  (1) Our "registry" is backed by GitHub public repositories (or repos visible with the environment's GITHUB_TOKEN).
+	//      Say organization 'octocat' has a repo titled 'myfeatures' with a set of feature definitions.
+	//      One of the [1..n] features in this repo has an id of 'helloworld'.
+	//
+	//      eg: octocat/myfeatures/helloworld
+	//
+	//      The above example assumes the 'latest' GitHub release, and internally will 
+	//      fetch the devcontainer-features.tgz artifact from that release.
+	//      To specify a certain release tag, append the tag with an @ symbol
+	//
+	//      eg: octocat/myfeatures/helloworld@v0.0.2
+	//
+	//  (2) A fully-qualified https URI to a devcontainer-features.tgz file can be provided instead
+	//      of a using the GitHub registry "shorthand". Note this is identified by a
+	//      s.StartsWith("https://" ||  "http://").
+	//
+	//      eg: https://example.com/../../devcontainer-features.tgz#helloworld
+	//
+	//  (3) This is a local path to a directory on disk following the expected file convention
+	//      The path can either be:
+	//          -  a relative file path to the .devcontainer file (prepended by a ./  or ../)
+	//          -  an absolute file path (prepended by a /)
+	//
+	//      No version can be provided, as the directory is copied 'as is' and is inherently taking the 'latest'
+	
 	output.write(`Processing feature: ${userFeature.id}`)
 			// cached feature
 			if (!userFeature.id.includes('/') && !userFeature.id.includes('\\')) {

From b35174c4ac9f1518e32287926c8cf4181fbd660f Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Thu, 12 May 2022 15:56:54 -0700
Subject: [PATCH 09/30] Fix for github features.

---
 src/spec-configuration/containerFeaturesConfiguration.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 3e22e880a..db4434954 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -550,7 +550,9 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 
 			// local disk
 			const userFeaturePath = path.parse(userFeature.id);
-			if (userFeaturePath) {
+			 // If its a valid path
+			 if (userFeature.id.startsWith('./') || userFeature.id.startsWith('../') || (userFeaturePath && path.isAbsolute(userFeature.id))) {
+			 //if (userFeaturePath && ((path.isAbsolute(userFeature.id) && existsSync(userFeature.id)) || !path.isAbsolute(userFeature.id))) {
 				output.write(`Local disk feature.`);
 				const filePath = userFeature.id;
 				const id = userFeaturePath.name;

From 1e9a5bb9c9ae5f1f79ca06863d174f3bf5f6377f Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Mon, 23 May 2022 16:19:10 -0700
Subject: [PATCH 10/30] Fixes for locally cacched features.

---
 .../containerFeaturesConfiguration.ts         | 11 +++++--
 src/spec-node/containerFeatures.ts            | 29 ++++++++++++-------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 73e15c290..3f6d2343f 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -214,7 +214,7 @@ export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 	let result = '';
 
 	// Features version 1  FIX
-	const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => getSourceInfoString(x.sourceInformation));
+	const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId);
 	folders.forEach(folder => {
 		result += `RUN cd /tmp/build-features/${folder} \\
 && chmod +x ./install.sh \\
@@ -376,6 +376,11 @@ export async function generateFeaturesConfig(params: { extensionPath: string; cw
 		return undefined;
 	}
 
+	if(!imageLabelDetails)
+	{
+		return undefined;
+	}
+
 	const userFeatures = convertOldFeatures(params, config);
 
 	if(!userFeatures) {
@@ -652,7 +657,9 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 			featureSet.features[0].consecutiveId = consecutiveId;
 
 			if(featureSet.sourceInformation?.type === 'local-cache') {
-				params.output.write(`Detected local feature set. Continuing...`);
+				
+				await mkdirpLocal(featCachePath);
+				await cpDirectoryLocal(path.join(dstFolder, 'local-cache'), featCachePath);
 				continue;
 			}
 		
diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
index 5a4ddfd40..127cd2ec7 100644
--- a/src/spec-node/containerFeatures.ts
+++ b/src/spec-node/containerFeatures.ts
@@ -87,11 +87,14 @@ export async function extendImage(params: DockerResolverParameters, config: DevC
 
 export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: DevContainerConfig, baseName: string, imageUser: string, imageLabelDetails: () => Promise<{ definition: string | undefined; version: string | undefined }>) {
 
-	const featuresConfig = await generateFeaturesConfig(params.common, (await createFeaturesTempFolder(params.common)), config, imageLabelDetails, getContainerFeaturesFolder);
+	const tempFolder = await createFeaturesTempFolder(params.common);
+	await createLocalFeatures(params, tempFolder);
+	const featuresConfig = await generateFeaturesConfig(params.common, tempFolder, config, imageLabelDetails, getContainerFeaturesFolder);
 	if (!featuresConfig) {
 		return null;
 	}
 
+
 	const collapsedFeaturesConfig = collapseFeaturesConfig(featuresConfig);
 	const featureBuildInfo = await getContainerFeaturesBuildInfo(params, featuresConfig, baseName, imageUser);
 	if (!featureBuildInfo) {
@@ -118,18 +121,13 @@ export function generateContainerEnvs(featuresConfig: FeaturesConfig) {
 	return result;
 }
 
-async function getContainerFeaturesBuildInfo(params: DockerResolverParameters, featuresConfig: FeaturesConfig, baseName: string, imageUser: string): Promise<{ dstFolder: string; dockerfileContent: string; dockerfilePrefixContent: string; buildArgs: Record<string, string>; buildKitContexts: Record<string, string> } | null> {
+async function createLocalFeatures(params: DockerResolverParameters, dstFolder: string)
+{
 	const { common } = params;
 	const { cliHost, output } = common;
-	const { dstFolder } = featuresConfig;
-
-	if (!dstFolder || dstFolder === '') {
-		output.write('dstFolder is undefined or empty in addContainerFeatures', LogLevel.Error);
-		return null;
-	}
 
 	// Calculate name of the build folder where localcache has been copied to.
-	const localCacheBuildFolderName = getSourceInfoString({ type: 'local-cache' });
+	const localCacheBuildFolderName = 'local-cache';
 
 	const srcFolder = getContainerFeaturesFolder(common.extensionPath);
 	output.write(`local container features stored at: ${srcFolder}`);
@@ -163,6 +161,17 @@ async function getContainerFeaturesBuildInfo(params: DockerResolverParameters, f
 	create.pipe(extract.stdin);
 	await extract.exit;
 	await createExit; // Allow errors to surface.
+}
+
+async function getContainerFeaturesBuildInfo(params: DockerResolverParameters, featuresConfig: FeaturesConfig, baseName: string, imageUser: string): Promise<{ dstFolder: string; dockerfileContent: string; dockerfilePrefixContent: string; buildArgs: Record<string, string>; buildKitContexts: Record<string, string> } | null> {
+	const { common } = params;
+	const { cliHost, output } = common;
+	const { dstFolder } = featuresConfig;
+
+	if (!dstFolder || dstFolder === '') {
+		output.write('dstFolder is undefined or empty in addContainerFeatures', LogLevel.Error);
+		return null;
+	}
 
 	const buildStageScripts = await Promise.all(featuresConfig.featureSets
 		.map(featureSet => multiStageBuildExploration ? featureSet.features
@@ -224,7 +233,7 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=mcr.microsoft.com/vscode/devcontainers/base:buste
 					.filter(f => (includeAllConfiguredFeatures|| f.included) && f.value && !buildStageScripts[i][f.id]?.hasAcquire)
 					.map(getFeatureEnvVariables)
 			).join('\n');
-			const envPath = cliHost.path.join(dstFolder, getSourceInfoString(fSet.sourceInformation), 'devcontainer-features.env'); // next to install.sh
+			const envPath = cliHost.path.join(fSet.features[0].infoString!, 'devcontainer-features.env');
 			await Promise.all([
 				cliHost.writeFile(envPath, Buffer.from(featuresEnv)),
 				...fSet.features

From 5923ef77a8ce8f20d97313d5d253cf8b483738b8 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 24 May 2022 14:28:35 -0700
Subject: [PATCH 11/30] Fix old features.

---
 .../containerFeaturesConfiguration.ts         | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 3f6d2343f..3372ef8c8 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -649,17 +649,23 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				continue;
 			}
 
-			const consecutiveId = featureSet.features[0].id + '_' + getCounter();
+			const feature = featureSet.features[0];
+			const consecutiveId = feature.id + '_' + getCounter();
 			// Calculate some predictable caching paths.
 			const featCachePath = path.join(dstFolder, consecutiveId);
 
-			featureSet.features[0].infoString = featCachePath;
-			featureSet.features[0].consecutiveId = consecutiveId;
+			feature.infoString = featCachePath;
+			feature.consecutiveId = consecutiveId;
 
 			if(featureSet.sourceInformation?.type === 'local-cache') {
 				
 				await mkdirpLocal(featCachePath);
 				await cpDirectoryLocal(path.join(dstFolder, 'local-cache'), featCachePath);
+
+				const local = localFeatures.features.find(x => x.id === feature.id);
+				feature.buildArg = local?.buildArg;
+				feature.options = local?.options;
+
 				continue;
 			}
 		
@@ -672,9 +678,9 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				const jsonString: Buffer = await readLocalFile(jsonPath);
 				const featureJson = jsonc.parse(jsonString.toString());
 
-				featureSet.features[0].runApp = featureJson.install.app ?? '';
-				featureSet.features[0].runParams = featureJson.install.file ?? 'install.sh';
-				featureSet.features[0].containerEnv = featureJson.containerEnv;
+				feature.runApp = featureJson.install.app ?? '';
+				feature.runParams = featureJson.install.file ?? 'install.sh';
+				feature.containerEnv = featureJson.containerEnv;
 
 				// We only support version 2 for local features.
 				featureSet.internalVersion = '2';
@@ -748,9 +754,9 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				if (existsSync(jsonPath)) {
 					const jsonString: Buffer = await readLocalFile(jsonPath);
 					const featureJson = jsonc.parse(jsonString.toString());
-					featureSet.features[0].runApp = featureJson.install.app ?? '';
-					featureSet.features[0].runParams = featureJson.install.file ?? 'install.sh';
-					featureSet.features[0].containerEnv = featureJson.containerEnv;
+					feature.runApp = featureJson.install.app ?? '';
+					feature.runParams = featureJson.install.file ?? 'install.sh';
+					feature.containerEnv = featureJson.containerEnv;
 					featureSet.internalVersion === '2';
 				} else {
 					featureSet.internalVersion === '1';

From 23e87ff4c51161b698b059c303cc8923bc52d43d Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 24 May 2022 14:43:26 -0700
Subject: [PATCH 12/30] Other fixes.

---
 src/spec-configuration/containerFeaturesConfiguration.ts | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 3372ef8c8..87e246898 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -658,14 +658,13 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 			feature.consecutiveId = consecutiveId;
 
 			if(featureSet.sourceInformation?.type === 'local-cache') {
-				
+				// create copy of the local features to set the environment variables for them.
 				await mkdirpLocal(featCachePath);
 				await cpDirectoryLocal(path.join(dstFolder, 'local-cache'), featCachePath);
 
 				const local = localFeatures.features.find(x => x.id === feature.id);
 				feature.buildArg = local?.buildArg;
 				feature.options = local?.options;
-
 				continue;
 			}
 		
@@ -681,6 +680,8 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				feature.runApp = featureJson.install.app ?? '';
 				feature.runParams = featureJson.install.file ?? 'install.sh';
 				feature.containerEnv = featureJson.containerEnv;
+				feature.buildArg = featureJson.buildArg;
+				feature.options = featureJson.options;
 
 				// We only support version 2 for local features.
 				featureSet.internalVersion = '2';
@@ -758,6 +759,8 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 					feature.runParams = featureJson.install.file ?? 'install.sh';
 					feature.containerEnv = featureJson.containerEnv;
 					featureSet.internalVersion === '2';
+					feature.buildArg = featureJson.buildArg;
+					feature.options = featureJson.options;
 				} else {
 					featureSet.internalVersion === '1';
 				}

From 9bce15f7b75c3c3157d696ea33ef7e0dfd5285e1 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 24 May 2022 15:29:29 -0700
Subject: [PATCH 13/30] Some comments and formating.

---
 .../containerFeaturesConfiguration.ts                 | 10 ++++++----
 src/spec-node/containerFeatures.ts                    | 11 ++++++++---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 87e246898..9150eb833 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -153,9 +153,8 @@ export function collapseFeaturesConfig(original: FeaturesConfig): CollapsedFeatu
 
 export const multiStageBuildExploration = false;
 
-// TODO: Cleanup
+// Counter to ensure that no two folders are the same even if we are executing the same feature multiple times.
 let counter = 1;
-
 function getCounter() {
 	return counter++;
 }
@@ -213,7 +212,7 @@ USER $_DEV_CONTAINERS_IMAGE_USER
 export function getFeatureLayers(featuresConfig: FeaturesConfig) {
 	let result = '';
 
-	// Features version 1  FIX
+	// Features version 1
 	const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId);
 	folders.forEach(folder => {
 		result += `RUN cd /tmp/build-features/${folder} \\
@@ -239,7 +238,7 @@ RUN cd /tmp/build-features/${feature.consecutiveId} \\
 	return result;
 }
 
-// Used for features version 2
+// Features version two export their environment variables as part of the Dockerfile to make them available to subsequent features.
 export function generateContainerEnvs(feature: Feature) {
 	let result = '';
 	if(!feature.containerEnv)
@@ -381,6 +380,7 @@ export async function generateFeaturesConfig(params: { extensionPath: string; cw
 		return undefined;
 	}
 
+	// Converts from object notation to array notation.
 	const userFeatures = convertOldFeatures(params, config);
 
 	if(!userFeatures) {
@@ -443,6 +443,7 @@ function convertOldFeatures(params: { output: Log }, config: DevContainerConfig)
 }
 
 // Process features contained in devcontainer.json
+// Creates one feature set per feature to aid in support of the previous structure.
 async function processUserFeatures(output: Log, userFeatures: DevContainerFeature[], featuresConfig: FeaturesConfig) : Promise<FeaturesConfig>
 {
 	userFeatures.forEach(userFeature => {
@@ -752,6 +753,7 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				// Read version information.
 				const jsonPath = path.join(featCachePath, 'feature.json');
 
+				// TODO: load features contained in a devcontainer-collection.json for version 2
 				if (existsSync(jsonPath)) {
 					const jsonString: Buffer = await readLocalFile(jsonPath);
 					const featureJson = jsonc.parse(jsonString.toString());
diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
index b5393b382..de46c6715 100644
--- a/src/spec-node/containerFeatures.ts
+++ b/src/spec-node/containerFeatures.ts
@@ -98,14 +98,19 @@ export async function extendImage(params: DockerResolverParameters, config: DevC
 
 export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: DevContainerConfig, baseName: string, imageUser: string, imageLabelDetails: () => Promise<{ definition: string | undefined; version: string | undefined }>) {
 
+	// Creates the folder where the working files will be setup.
 	const tempFolder = await createFeaturesTempFolder(params.common);
+
+	// Extracts the local cache of features.
 	await createLocalFeatures(params, tempFolder);
+
+	// Processes the user's configuration.
 	const featuresConfig = await generateFeaturesConfig(params.common, tempFolder, config, imageLabelDetails, getContainerFeaturesFolder);
 	if (!featuresConfig) {
 		return null;
 	}
 
-
+	// Generates the end configuration.
 	const collapsedFeaturesConfig = collapseFeaturesConfig(featuresConfig);
 	const featureBuildInfo = await getContainerFeaturesBuildInfo(params, featuresConfig, baseName, imageUser);
 	if (!featureBuildInfo) {
@@ -119,7 +124,7 @@ export async function getExtendImageBuildInfo(params: DockerResolverParameters,
 export function generateContainerEnvs(featuresConfig: FeaturesConfig) {
 	let result = '';
 	for (const fSet of featuresConfig.featureSets) {
-		// We only need to generate this ENV references for the old features spec.
+		// We only need to generate this ENV references for the initial features specification.
 		if(fSet.internalVersion !== '2')
 		{
 			result += fSet.features
@@ -137,7 +142,7 @@ async function createLocalFeatures(params: DockerResolverParameters, dstFolder:
 	const { common } = params;
 	const { cliHost, output } = common;
 
-	// Calculate name of the build folder where localcache has been copied to.
+	// Name of the local cache folder inside the working directory
 	const localCacheBuildFolderName = 'local-cache';
 
 	const srcFolder = getContainerFeaturesFolder(common.extensionPath);

From 91edf380a89e113b2b552fed47b00154ab7e5daa Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Tue, 24 May 2022 23:14:27 +0000
Subject: [PATCH 14/30] Test fix

---
 src/spec-configuration/containerFeaturesConfiguration.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 9150eb833..2a2ac85e7 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -600,7 +600,7 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 			const id = splitOnSlash[2];
 
 			let feat: Feature = {
-				id: owner + repo + id,
+				id: id,
 				name: userFeature.id,
 				value: userFeature.options,
 				included: true,

From 971bd56e48369c365626017d9ee56b6caadf2351 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Tue, 24 May 2022 23:34:17 +0000
Subject: [PATCH 15/30] Fix

---
 .../generateFeaturesConfig.offline.test.ts    | 128 +++++++++---------
 1 file changed, 63 insertions(+), 65 deletions(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index f37a61229..fedb5b2a2 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -13,7 +13,7 @@ export const output = makeLog(createPlainLog(text => process.stdout.write(text),
 describe('validate (offline) generateFeaturesConfig()', function () {
 
     // Setup
-    const env = { 'SOME_KEY': 'SOME_VAL'};
+    const env = { 'SOME_KEY': 'SOME_VAL' };
     const params = { extensionPath: '', cwd: '', output, env, persistedFolder: '' };
 
     // Mocha executes with the root of the project as the cwd.
@@ -32,67 +32,65 @@ describe('validate (offline) generateFeaturesConfig()', function () {
         return record;
     };
 
-   // it('should correctly return a featuresConfig with just local features', async function () {
-
-        // TODO, rewrite
-//         const version = 'unittest';
-//         const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
-//         await mkdirpLocal(tmpFolder);
-
-//         const config: DevContainerConfig = {
-//             configFilePath: URI.from({ 'scheme': 'https' }),
-//             dockerFile: '.',
-//             features: {
-//                 first: {
-//                     'version': 'latest',
-//                     'option1': true
-//                 },
-//                 second: 'latest',
-//                 third: 'latest'
-//             },
-//         };
-
-//         const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);
-//         if (!featuresConfig) {
-//             assert.fail();
-//         }
-
-//         const localFeatureSet = (featuresConfig?.featureSets.find(set => set.sourceInformation.type === 'local-cache'));
-//         assert.exists(localFeatureSet);
-//         assert.strictEqual(localFeatureSet?.features.length, 1);
-
-//         const first = localFeatureSet?.features.find((f) => f.id === 'first');
-//         assert.exists(first);
-
-//         const second = localFeatureSet?.features.find((f) => f.id === 'second');
-//         assert.exists(second);
-
-//         const third = localFeatureSet?.features.find((f) => f.id === 'third');
-//         assert.exists(third);
-
-//         assert.isObject(first?.value);
-//         assert.isBoolean(second?.value);
-//         assert.isString(third?.value);
-
-//         // -- Test containerFeatures.ts helper functions
-
-//         // generateContainerEnvs
-// // TODO
-// //         const actualEnvs = generateContainerEnvs(featuresConfig);
-// //         const expectedEnvs = `ENV MYKEYONE=MYRESULTONE
-// // ENV MYKEYTHREE=MYRESULTHREE`;
-// //         assert.strictEqual(actualEnvs, expectedEnvs);
-    
-//         // getFeatureLayers
-//         const actualLayers = await getFeatureLayers(featuresConfig);
-//         const expectedLayers = `RUN cd /tmp/build-features/local-cache \\
-// && chmod +x ./install.sh \\
-// && ./install.sh
-
-// `;
-//         assert.strictEqual(actualLayers, expectedLayers);
-  //  });
-
-
-
-});
+    it('should correctly return a featuresConfig with just local features', async function () {
+
+        const version = 'unittest';
+        const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
+        await mkdirpLocal(tmpFolder);
+
+        const config: DevContainerConfig = {
+            configFilePath: URI.from({ 'scheme': 'https' }),
+            dockerFile: '.',
+            features: {
+                first: {
+                    'version': 'latest',
+                    'option1': true
+                },
+                third: 'latest'
+            },
+        };
+
+        const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);
+        if (!featuresConfig) {
+            assert.fail();
+        }
+
+        const localFeatureSet = (featuresConfig?.featureSets.find(set => set.sourceInformation.type === 'local-cache'));
+        assert.exists(localFeatureSet);
+        assert.strictEqual(localFeatureSet?.features.length, 3);
+
+        const first = localFeatureSet?.features.find((f) => f.id === 'first');
+        assert.exists(first);
+
+        const second = localFeatureSet?.features.find((f) => f.id === 'second');
+        assert.exists(second);
+
+        const third = localFeatureSet?.features.find((f) => f.id === 'third');
+        assert.exists(third);
+
+        assert.isObject(first?.value);
+        assert.isBoolean(second?.value);
+        assert.isString(third?.value);
+
+        // -- Test containerFeatures.ts helper functions
+
+        // generateContainerEnvs
+        // TODO
+        //         const actualEnvs = generateContainerEnvs(featuresConfig);
+        //         const expectedEnvs = `ENV MYKEYONE=MYRESULTONE
+        // ENV MYKEYTHREE=MYRESULTHREE`;
+        //         assert.strictEqual(actualEnvs, expectedEnvs);
+
+        // getFeatureLayers
+        const actualLayers = await getFeatureLayers(featuresConfig);
+        const expectedLayers = `RUN cd /tmp/build-features/local-cache \\
+&& chmod +x ./install.sh \\
+&& ./install.sh
+
+`;
+        assert.strictEqual(actualLayers, expectedLayers);
+    });
+
+
+
+});
\ No newline at end of file

From 4e0fc7e1139fd21cff2b3dbef09ce199b7706527 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Tue, 24 May 2022 16:39:08 -0700
Subject: [PATCH 16/30] Fix test.

---
 src/test/container-features/helpers.offline.test.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/container-features/helpers.offline.test.ts b/src/test/container-features/helpers.offline.test.ts
index 6d27cb70d..06f720682 100644
--- a/src/test/container-features/helpers.offline.test.ts
+++ b/src/test/container-features/helpers.offline.test.ts
@@ -56,7 +56,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
         }
         const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.features[0].id, 'octocatmyfeatureshelloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
@@ -74,7 +74,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
 
         const result = parseFeatureIdentifier(output, feature);
         assert.exists(result);
-        assert.strictEqual(result?.features[0].id, 'octocatmyfeatureshelloworld');
+        assert.strictEqual(result?.features[0].id, 'helloworld');
         assert.deepEqual(result?.sourceInformation, { type: 'github-repo', 
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 

From 4842ec52c211c887cee82033a7f98db0bf7d4bc1 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 14:47:51 -0700
Subject: [PATCH 17/30] Refactor changes and read devcontainer collections.

---
 .../containerFeaturesConfiguration.ts         | 77 +++++++++++--------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 2a2ac85e7..a099b4d4b 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -663,9 +663,13 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				await mkdirpLocal(featCachePath);
 				await cpDirectoryLocal(path.join(dstFolder, 'local-cache'), featCachePath);
 
-				const local = localFeatures.features.find(x => x.id === feature.id);
-				feature.buildArg = local?.buildArg;
-				feature.options = local?.options;
+				await parseDevContainerFeature(featureSet, feature, featCachePath);
+
+				if (featureSet.internalVersion !== '2') {
+					const local = localFeatures.features.find(x => x.id === feature.id);
+					feature.buildArg = local?.buildArg;
+					feature.options = local?.options;
+				}
 				continue;
 			}
 		
@@ -673,26 +677,13 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				params.output.write(`Detected local file path`);
 				
 				const executionPath = featureSet.sourceInformation.isRelative ? path.join(params.cwd, featureSet.sourceInformation.filePath) : featureSet.sourceInformation.filePath;
-				const jsonPath = path.join(executionPath, 'feature.json');
-
-				const jsonString: Buffer = await readLocalFile(jsonPath);
-				const featureJson = jsonc.parse(jsonString.toString());
-
-				feature.runApp = featureJson.install.app ?? '';
-				feature.runParams = featureJson.install.file ?? 'install.sh';
-				feature.containerEnv = featureJson.containerEnv;
-				feature.buildArg = featureJson.buildArg;
-				feature.options = featureJson.options;
-
-				// We only support version 2 for local features.
-				featureSet.internalVersion = '2';
 
+				await parseDevContainerFeature(featureSet, feature, executionPath);
 				await mkdirpLocal(featCachePath);
 				await cpDirectoryLocal(executionPath, featCachePath);
 				continue;
 			}
 
-			
 			const tempTarballPath = path.join(dstFolder, ASSET_NAME);
 			params.output.write(`Detected tarball`);
 			const headers = getRequestHeaders(featureSet.sourceInformation, params.env, params.output);
@@ -750,23 +741,7 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 					}
 				);
 
-				// Read version information.
-				const jsonPath = path.join(featCachePath, 'feature.json');
-
-				// TODO: load features contained in a devcontainer-collection.json for version 2
-				if (existsSync(jsonPath)) {
-					const jsonString: Buffer = await readLocalFile(jsonPath);
-					const featureJson = jsonc.parse(jsonString.toString());
-					feature.runApp = featureJson.install.app ?? '';
-					feature.runParams = featureJson.install.file ?? 'install.sh';
-					feature.containerEnv = featureJson.containerEnv;
-					featureSet.internalVersion === '2';
-					feature.buildArg = featureJson.buildArg;
-					feature.options = featureJson.options;
-				} else {
-					featureSet.internalVersion === '1';
-				}
-
+				await parseDevContainerFeature(featureSet, feature, featCachePath);
 			}
 			continue;
 		}
@@ -776,6 +751,40 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 	}
 }
 
+async function parseDevContainerFeature(featureSet: FeatureSet, feature: Feature, featCachePath: string) {
+	// Read version information.
+	const jsonPath = path.join(featCachePath, 'devcontainer-feature.json');
+	const innerPath = path.join(featCachePath, feature.id);
+	const innerJsonPath = path.join(innerPath, 'devcontainer-feature.json');
+
+	let foundPath: string | undefined;
+
+	if (existsSync(jsonPath)) {
+		foundPath = jsonPath;
+	} else if (existsSync(innerJsonPath)) {
+		foundPath = innerJsonPath;
+		feature.infoString = innerPath;
+	}
+
+	if (foundPath) {
+		const jsonString: Buffer = await readLocalFile(foundPath);
+		const featureJson = jsonc.parse(jsonString.toString());
+		feature.runApp = '';
+		feature.runParams = 'install.sh';
+		if (featureJson.install) {
+			feature.runApp = featureJson.install!.app ?? '';
+			feature.runParams = featureJson.install!.file ?? 'install.sh';
+		}
+
+		feature.containerEnv = featureJson.containerEnv;
+		featureSet.internalVersion = '2';
+		feature.buildArg = featureJson.buildArg;
+		feature.options = featureJson.options;
+	} else {
+		featureSet.internalVersion = '1';
+	}
+}
+
 export function getFeatureMainProperty(feature: Feature) {
 	return feature.options?.version ? 'version' : undefined;
 }

From ebbbc4ef9c896d4d50997aaca63bd50d2534e56c Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 15:33:06 -0700
Subject: [PATCH 18/30] Fix test.

---
 .../generateFeaturesConfig.offline.test.ts    | 40 ++++++++++---------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index fedb5b2a2..abc8bbcd0 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -6,6 +6,7 @@ import * as path from 'path';
 import { mkdirpLocal } from '../../spec-utils/pfs';
 import { DevContainerConfig } from '../../spec-configuration/configuration';
 import { URI } from 'vscode-uri';
+import { DevContainerFeature } from '../../spec-common/injectHeadless';
 
 export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
 
@@ -38,16 +39,25 @@ describe('validate (offline) generateFeaturesConfig()', function () {
         const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
         await mkdirpLocal(tmpFolder);
 
+        const features: DevContainerFeature[] = [
+            {
+                id: 'first',
+                options: {
+                    'version': 'latest'
+                },
+            },
+            {
+                id: 'second',
+                options: {
+                    'value': true
+                },
+            }
+        ];
+
         const config: DevContainerConfig = {
             configFilePath: URI.from({ 'scheme': 'https' }),
             dockerFile: '.',
-            features: {
-                first: {
-                    'version': 'latest',
-                    'option1': true
-                },
-                third: 'latest'
-            },
+            features: features
         };
 
         const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);
@@ -55,22 +65,16 @@ describe('validate (offline) generateFeaturesConfig()', function () {
             assert.fail();
         }
 
-        const localFeatureSet = (featuresConfig?.featureSets.find(set => set.sourceInformation.type === 'local-cache'));
-        assert.exists(localFeatureSet);
-        assert.strictEqual(localFeatureSet?.features.length, 3);
+        assert.strictEqual(featuresConfig?.featureSets.length, 3);
 
-        const first = localFeatureSet?.features.find((f) => f.id === 'first');
+        const first = featuresConfig.featureSets[0].features.find((f) => f.id === 'first');
         assert.exists(first);
 
-        const second = localFeatureSet?.features.find((f) => f.id === 'second');
+        const second = featuresConfig.featureSets[1].features.find((f) => f.id === 'second');
         assert.exists(second);
 
-        const third = localFeatureSet?.features.find((f) => f.id === 'third');
-        assert.exists(third);
-
-        assert.isObject(first?.value);
-        assert.isBoolean(second?.value);
-        assert.isString(third?.value);
+        assert.isString(first?.options?.version);
+        assert.isBoolean(second?.options?.value);
 
         // -- Test containerFeatures.ts helper functions
 

From 26f3f3554efcc2974c821d32e46a5085d1694c6a Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 15:36:35 -0700
Subject: [PATCH 19/30] Fix test.

---
 .../container-features/generateFeaturesConfig.offline.test.ts   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index abc8bbcd0..158f09a7d 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -65,7 +65,7 @@ describe('validate (offline) generateFeaturesConfig()', function () {
             assert.fail();
         }
 
-        assert.strictEqual(featuresConfig?.featureSets.length, 3);
+        assert.strictEqual(featuresConfig?.featureSets.length, 2);
 
         const first = featuresConfig.featureSets[0].features.find((f) => f.id === 'first');
         assert.exists(first);

From 11fb36cfa2b2b6c0258c52e7286b75a9141218d0 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 15:42:39 -0700
Subject: [PATCH 20/30] Fix test 3

---
 .../container-features/generateFeaturesConfig.offline.test.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 158f09a7d..60cd8828c 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -73,8 +73,8 @@ describe('validate (offline) generateFeaturesConfig()', function () {
         const second = featuresConfig.featureSets[1].features.find((f) => f.id === 'second');
         assert.exists(second);
 
-        assert.isString(first?.options?.version);
-        assert.isBoolean(second?.options?.value);
+        assert.isObject(first?.value);
+        assert.isObject(second?.value);
 
         // -- Test containerFeatures.ts helper functions
 

From 934ae4a082044731abb0b968c1a22571de53dbc5 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 15:47:28 -0700
Subject: [PATCH 21/30] fix test 4

---
 .../container-features/generateFeaturesConfig.offline.test.ts   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 60cd8828c..7dd1b36cc 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -87,7 +87,7 @@ describe('validate (offline) generateFeaturesConfig()', function () {
 
         // getFeatureLayers
         const actualLayers = await getFeatureLayers(featuresConfig);
-        const expectedLayers = `RUN cd /tmp/build-features/local-cache \\
+        const expectedLayers = `RUN cd /tmp/build-features/first_1\\
 && chmod +x ./install.sh \\
 && ./install.sh
 

From 63a5ed29e5866895bd2981c023cff2bfe52a8df5 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <edmundo.gonzalez@microsoft.com>
Date: Wed, 25 May 2022 15:58:09 -0700
Subject: [PATCH 22/30] fix test 5

---
 .../generateFeaturesConfig.offline.test.ts                  | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 7dd1b36cc..fab0a7a26 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -87,7 +87,11 @@ describe('validate (offline) generateFeaturesConfig()', function () {
 
         // getFeatureLayers
         const actualLayers = await getFeatureLayers(featuresConfig);
-        const expectedLayers = `RUN cd /tmp/build-features/first_1\\
+        const expectedLayers = `RUN cd /tmp/build-features/first_1 \\
+&& chmod +x ./install.sh \\
+&& ./install.sh
+
+RUN cd /tmp/build-features/second_2 \\
 && chmod +x ./install.sh \\
 && ./install.sh
 

From b304f9052231327b79dd63be3e4713a1b08701f1 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Wed, 25 May 2022 23:20:04 +0000
Subject: [PATCH 23/30] Comments from PR.

---
 package.json                                             | 2 +-
 src/spec-common/injectHeadless.ts                        | 2 +-
 src/spec-configuration/configuration.ts                  | 2 +-
 src/spec-configuration/containerFeaturesConfiguration.ts | 4 +---
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/package.json b/package.json
index 2eff458f2..3c0bb0eb2 100644
--- a/package.json
+++ b/package.json
@@ -41,6 +41,7 @@
 		"@types/js-yaml": "^4.0.5",
 		"@types/mocha": "^9.1.0",
 		"@types/node": "^16.11.7",
+		"@types/ncp": "^2.0.5",
 		"@types/pull-stream": "^3.6.2",
 		"@types/semver": "^7.3.9",
 		"@types/shell-quote": "^1.7.1",
@@ -65,7 +66,6 @@
 		"vinyl-fs": "^3.0.3"
 	},
 	"dependencies": {
-		"@types/ncp": "^2.0.5",
 		"ncp": "^2.0.0",
 		"follow-redirects": "^1.14.8",
 		"js-yaml": "^4.1.0",
diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts
index 292b0647b..5dc522a58 100644
--- a/src/spec-common/injectHeadless.ts
+++ b/src/spec-common/injectHeadless.ts
@@ -105,7 +105,7 @@ export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand'
 
 const defaultWaitFor: DevContainerConfigCommand = 'updateContentCommand';
 
-export interface DevContainerFeature{
+export interface DevContainerFeature {
 	id: string;
 	options: boolean | string | Record<string, boolean | string | undefined>;
 }
diff --git a/src/spec-configuration/configuration.ts b/src/spec-configuration/configuration.ts
index a10a7c443..4dd42dd71 100644
--- a/src/spec-configuration/configuration.ts
+++ b/src/spec-configuration/configuration.ts
@@ -26,7 +26,7 @@ export interface HostRequirements {
 	storage?: string;
 }
 
-export interface DevContainerFeature{
+export interface DevContainerFeature {
 	id: string;
 	options: boolean | string | Record<string, boolean | string | undefined>;
 }
diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index a099b4d4b..3359bf58a 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -5,14 +5,12 @@
 
 import * as jsonc from 'jsonc-parser';
 import * as path from 'path';
-//import * as semver from 'semver';
 import * as URL from 'url';
 import * as tar from 'tar';
 import { DevContainerConfig } from './configuration';
 import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal } from '../spec-utils/pfs';
 import { Log, LogLevel } from '../spec-utils/log';
 import { request } from '../spec-utils/httpRequest';
-import { DevContainerFeature } from './configuration';
 import { existsSync } from 'fs';
 
 const ASSET_NAME = 'devcontainer-features.tgz';
@@ -25,7 +23,7 @@ export interface Feature {
 	runApp?: string;
 	runParams?: string;
 	infoString?: string;
-	internalVersion?: string;
+	internalVersion?: string; // set programmatically
 	tempLocalPath?: string;
 	consecutiveId?: string;
 	install?: Record<string, string>;

From 5e29707e612554c5bf02f3468479f814c69f27b2 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Wed, 25 May 2022 23:24:00 +0000
Subject: [PATCH 24/30] Fix cleanup

---
 src/spec-configuration/containerFeaturesConfiguration.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 3359bf58a..8baf2b669 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -7,7 +7,7 @@ import * as jsonc from 'jsonc-parser';
 import * as path from 'path';
 import * as URL from 'url';
 import * as tar from 'tar';
-import { DevContainerConfig } from './configuration';
+import { DevContainerConfig, DevContainerFeature } from './configuration';
 import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal } from '../spec-utils/pfs';
 import { Log, LogLevel } from '../spec-utils/log';
 import { request } from '../spec-utils/httpRequest';

From ec8872147de9525408bd78bc7a9f2fc935e361f0 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Thu, 2 Jun 2022 17:26:59 +0000
Subject: [PATCH 25/30] Remove prefix for environment variables in features v2

---
 src/spec-node/containerFeatures.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
index de46c6715..73d1f7fa2 100644
--- a/src/spec-node/containerFeatures.ts
+++ b/src/spec-node/containerFeatures.ts
@@ -350,7 +350,7 @@ function getFeatureEnvVariables(f: Feature) {
 	} else {
 		if (values) {
 			variables.push(...Object.keys(values)
-			.map(name => `${idSafe}_${getSafeId(name)}="${values[name]}"`));
+				.map(name => `${getSafeId(name)}="${values[name]}"`));
 		}
 		if (f.buildArg) {
 			variables.push(`${f.buildArg}=${getFeatureMainValue(f)}`);

From 267ef67c1277d552313da2d517250c43f45ebd52 Mon Sep 17 00:00:00 2001
From: Josh Spicer <josh@joshspicer.com>
Date: Fri, 3 Jun 2022 14:28:43 -0400
Subject: [PATCH 26/30] support fetching v2 features from github repo (#48)

* support fetching v2 features from github repo

* update test
---
 .../containerFeaturesConfiguration.ts         | 162 +++++++++++-------
 .../helpers.offline.test.ts                   |   8 +-
 2 files changed, 106 insertions(+), 64 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 8baf2b669..322d8be88 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -13,7 +13,7 @@ import { Log, LogLevel } from '../spec-utils/log';
 import { request } from '../spec-utils/httpRequest';
 import { existsSync } from 'fs';
 
-const ASSET_NAME = 'devcontainer-features.tgz';
+const V1_ASSET_NAME = 'devcontainer-features.tgz';
 
 export interface Feature {
 	id: string;
@@ -291,17 +291,22 @@ function getRequestHeaders(sourceInformation: SourceInformation, env: NodeJS.Pro
 	return headers;
 }
 
-async function askGitHubApiForTarballUri(sourceInformation: GithubSourceInformation, headers: { 'user-agent': string; 'Authorization'?: string; 'Accept'?: string }, output: Log) {
+async function askGitHubApiForTarballUri(sourceInformation: GithubSourceInformation, feature: Feature, headers: { 'user-agent': string; 'Authorization'?: string; 'Accept'?: string }, output: Log) {
 	const options = {
 		type: 'GET',
 		url: sourceInformation.apiUri,
 		headers
 	};
+
 	const apiInfo: GitHubApiReleaseInfo = JSON.parse(((await request(options, output)).toString()));
 	if (apiInfo) {
-		const asset = apiInfo.assets.find(a => a.name === ASSET_NAME);
+		const asset =
+			apiInfo.assets.find(a => a.name === `${feature.id}.tgz`)  // v2
+			|| apiInfo.assets.find(a => a.name === V1_ASSET_NAME) // v1
+			|| undefined;
+
 		if (asset && asset.url) {
-			output.write(`Found url to fetch release artifact ${asset.name}. Asset of size ${asset.size} has been downloaded ${asset.download_count} times and was last updated at ${asset.updated_at}`);
+			output.write(`Found url to fetch release artifact '${asset.name}'. Asset of size ${asset.size} has been downloaded ${asset.download_count} times and was last updated at ${asset.updated_at}`);
 			return asset.url;
 		} else {
 			output.write('Unable to fetch release artifact URI from GitHub API', LogLevel.Error);
@@ -311,7 +316,7 @@ async function askGitHubApiForTarballUri(sourceInformation: GithubSourceInformat
 	return undefined;
 }
 
-export async function loadFeaturesJson(jsonBuffer: Buffer, output: Log): Promise<FeatureSet | undefined> {
+export async function loadFeaturesJson(jsonBuffer: Buffer, filePath: string, output: Log): Promise<FeatureSet | undefined> {
 	if (jsonBuffer.length === 0) {
 		output.write('Parsed featureSet is empty.', LogLevel.Error);
 		return undefined;
@@ -322,15 +327,16 @@ export async function loadFeaturesJson(jsonBuffer: Buffer, output: Log): Promise
 		output.write('Parsed featureSet contains no features.', LogLevel.Error);
 		return undefined;
 	}
-	output.write(`Loaded devcontainer-features.json declares ${featureSet.features.length} features and ${(!!featureSet.sourceInformation) ? 'contains' : 'does not contain'} explicit source info.`,
+	output.write(`Loaded ${filePath}, which declares ${featureSet.features.length} features and ${(!!featureSet.sourceInformation) ? 'contains' : 'does not contain'} explicit source info.`,
 		LogLevel.Trace);
 
 	return updateFromOldProperties(featureSet);
 }
 
 export async function loadFeaturesJsonFromDisk(pathToDirectory: string, output: Log): Promise<FeatureSet | undefined> {
-	const jsonBuffer: Buffer = await readLocalFile(path.join(pathToDirectory, 'devcontainer-features.json'));
-	return loadFeaturesJson(jsonBuffer, output);
+	const filePath = path.join(pathToDirectory, 'devcontainer-features.json');
+	const jsonBuffer: Buffer = await readLocalFile(filePath);
+	return loadFeaturesJson(jsonBuffer, filePath, output);
 }
 
 function updateFromOldProperties<T extends { features: (Feature & { extensions?: string[]; settings?: object; customizations?: { vscode?: { extensions?: string[]; settings?: object } } })[] }>(original: T): T {
@@ -609,7 +615,7 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 					sourceInformation : {
 					type: 'github-repo',
 					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/latest`,
-					unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download/${ASSET_NAME}`,
+						unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download`, // v1/v2 implementations append name of relevant asset
 					owner,
 					repo,
 					isLatest: true
@@ -623,7 +629,7 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 					sourceInformation : {
 					type: 'github-repo',
 					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/tags/${version}`,
-					unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${version}/${ASSET_NAME}`,
+						unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${version}`, // v1/v2 implementations append name of relevant asset
 					owner,
 					repo,
 					tag: version,
@@ -636,13 +642,15 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 }
 
 async function fetchFeatures(params: { extensionPath: string; cwd: string; output: Log; env: NodeJS.ProcessEnv }, featuresConfig: FeaturesConfig, localFeatures: FeatureSet, dstFolder: string) {
-	for(const featureSet of featuresConfig.featureSets)	{
+	for (const featureSet of featuresConfig.featureSets) {
 		try {
 			if (!featureSet || !featureSet.features || !featureSet.sourceInformation)
 			{
 				continue;
 			}
 
+			params.output.write(`* fetching feature...`, LogLevel.Trace);
+
 			if(!localFeatures)
 			{
 				continue;
@@ -682,73 +690,107 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				continue;
 			}
 
-			const tempTarballPath = path.join(dstFolder, ASSET_NAME);
 			params.output.write(`Detected tarball`);
 			const headers = getRequestHeaders(featureSet.sourceInformation, params.env, params.output);
-			let tarballUri: string | undefined = undefined;
+
+			// Ordered list of tarballUris to attempt to fetch from.
+			let tarballUris: string[] = [];
+
 			if (featureSet.sourceInformation.type === 'github-repo') {
 				params.output.write('Determining tarball URI for provided github repo.', LogLevel.Trace);
 				if (headers.Authorization && headers.Authorization !== '') {
-					params.output.write('Authenticated. Fetching from GH API.', LogLevel.Trace);
-					tarballUri = await askGitHubApiForTarballUri(featureSet.sourceInformation, headers, params.output);
+					params.output.write('GITHUB_TOKEN available. Attempting to fetch via GH API.', LogLevel.Info);
+					const authenticatedGithubTarballUri = await askGitHubApiForTarballUri(featureSet.sourceInformation, feature, headers, params.output);
+
+					if (authenticatedGithubTarballUri) {
+						tarballUris.push(authenticatedGithubTarballUri);
+					} else {
+						params.output.write('Failed to generate autenticated tarball URI for provided feature, despite a GitHub token present', LogLevel.Warning);
+					}
 					headers.Accept = 'Accept: application/octet-stream';
-				} else {
-					params.output.write('Not authenticated. Fetching from unauthenticated uri', LogLevel.Trace);
-					tarballUri = featureSet.sourceInformation.unauthenticatedUri;
 				}
+
+				// Always add the unauthenticated URIs as fallback options.
+				params.output.write('Appending unauthenticated URIs for v2 and then v1', LogLevel.Trace);
+				tarballUris.push(`${featureSet.sourceInformation.unauthenticatedUri}/${feature.id}.tgz`);
+				tarballUris.push(`${featureSet.sourceInformation.unauthenticatedUri}/${V1_ASSET_NAME}`);
+
 			} else {
-				tarballUri = featureSet.sourceInformation.tarballUri;
+				// We have a plain ol' tarball URI, since we aren't in the github-repo case.
+				tarballUris.push(featureSet.sourceInformation.tarballUri);
 			}
-			
-			if(tarballUri) {
-				const options = {
-					type: 'GET',
-					url: tarballUri,
-					headers
-				};
-				params.output.write(`Fetching tarball at ${options.url}`);
-				params.output.write(`Headers: ${JSON.stringify(options)}`, LogLevel.Trace);
-				const tarball = await request(options, params.output);
-
-				if (!tarball || tarball.length === 0) {
-					params.output.write(`Did not receive a response from tarball download URI`, LogLevel.Error);
-					// Continue loop to the next remote feature.
-					// TODO: Should be more fatal.
-					await cleanupIterationFetchAndMerge(tempTarballPath, params.output);
-					continue;
-				}
 
-				// Filter what gets emitted from the tar.extract().
-				const filter = (file: string, _: tar.FileStat) => {
-					// Don't include .dotfiles or the archive itself.
-					if (file.startsWith('./.') || file === `./${ASSET_NAME}` || file === './.') {
-						return false;
-					}
-					return true;
-				};
+			// Attempt to fetch from 'tarballUris' in order, until one succeeds.
+			for (const tarballUri of tarballUris) {
+				const didSucceed = await fetchContentsAtTarballUri(tarballUri, featCachePath, headers, dstFolder, params.output);
 
-				params.output.write(`Preparing to unarchive received tgz.`, LogLevel.Trace);
-				// Create the directory to cache this feature-set in.
-				await mkdirpLocal(featCachePath);
-				await writeLocalFile(tempTarballPath, tarball);
-				await tar.x(
-					{
-						file: tempTarballPath,
-						cwd: featCachePath,
-						filter
-					}
-				);
-
-				await parseDevContainerFeature(featureSet, feature, featCachePath);
+				if (didSucceed) {
+					params.output.write(`Succeeded fetching ${tarballUri}`, LogLevel.Trace)
+					await parseDevContainerFeature(featureSet, feature, featCachePath);
+					break;
+				}
 			}
-			continue;
+
+			const msg = `(!) Failed to fetch tarball after attempting ${tarballUris.length} possibilities.`;
+			params.output.write(msg, LogLevel.Error);
+			throw new Error(msg);
 		}
 		catch (e) {
-			params.output.write(`Exception: ${e?.Message} `, LogLevel.Trace);
+			params.output.write(`Failed to fetch feature. ${e?.Message ?? ''} `, LogLevel.Trace);
+			// TODO: Should this be more fatal?
 		}
 	}
 }
 
+async function fetchContentsAtTarballUri(tarballUri: string, featCachePath: string, headers: { 'user-agent': string; 'Authorization'?: string; 'Accept'?: string }, dstFolder: string, output: Log): Promise<boolean> {
+	const tempTarballPath = path.join(dstFolder, 'temp.tgz');
+	try {
+		const options = {
+			type: 'GET',
+			url: tarballUri,
+			headers
+		};
+		output.write(`Fetching tarball at ${options.url}`);
+		output.write(`Headers: ${JSON.stringify(options)}`, LogLevel.Trace);
+		const tarball = await request(options, output);
+
+		if (!tarball || tarball.length === 0) {
+			output.write(`Did not receive a response from tarball download URI: ${tarballUri}`, LogLevel.Trace);
+			return false;
+		}
+
+		// Filter what gets emitted from the tar.extract().
+		const filter = (file: string, _: tar.FileStat) => {
+				// Don't include .dotfiles or the archive itself.
+			if (file.startsWith('./.') || file === `./${V1_ASSET_NAME}` || file === './.') {
+				return false;
+			}
+			return true;
+		};
+
+		output.write(`Preparing to unarchive received tgz from ${tempTarballPath} -> ${featCachePath}.`, LogLevel.Trace);
+		// Create the directory to cache this feature-set in.
+		await mkdirpLocal(featCachePath);
+		await writeLocalFile(tempTarballPath, tarball);
+		await tar.x(
+			{
+				file: tempTarballPath,
+				cwd: featCachePath,
+				filter
+			}
+		);
+
+		await cleanupIterationFetchAndMerge(tempTarballPath, output);
+
+		return true;
+	} catch (e) {
+		output.write(`Caught failure when fetching from URI '${tarballUri}': ${e}`, LogLevel.Trace);
+		await cleanupIterationFetchAndMerge(tempTarballPath, output);
+		return false;
+	}
+}
+
+
 async function parseDevContainerFeature(featureSet: FeatureSet, feature: Feature, featCachePath: string) {
 	// Read version information.
 	const jsonPath = path.join(featCachePath, 'devcontainer-feature.json');
diff --git a/src/test/container-features/helpers.offline.test.ts b/src/test/container-features/helpers.offline.test.ts
index 06f720682..c33d811be 100644
--- a/src/test/container-features/helpers.offline.test.ts
+++ b/src/test/container-features/helpers.offline.test.ts
@@ -61,7 +61,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
                                                       owner: 'octocat', 
                                                       repo: 'myfeatures', 
                                                       apiUri: 'https://api.github.com/repos/octocat/myfeatures/releases/latest', 
-                                                      unauthenticatedUri: 'https://github.com/octocat/myfeatures/releases/latest/download/devcontainer-features.tgz',
+            unauthenticatedUri: 'https://github.com/octocat/myfeatures/releases/latest/download',
                                                       isLatest: true 
                                                     });
     });
@@ -80,7 +80,7 @@ describe('validate function parseRemoteFeatureToDownloadUri', function () {
                                                       repo: 'myfeatures', 
                                                       tag: 'v0.0.4', 
                                                       apiUri: 'https://api.github.com/repos/octocat/myfeatures/releases/tags/v0.0.4', 
-                                                      unauthenticatedUri: 'https://github.com/octocat/myfeatures/releases/download/v0.0.4/devcontainer-features.tgz',
+                                                      unauthenticatedUri: 'https://github.com/octocat/myfeatures/releases/download/v0.0.4',
                                                       isLatest: false 
                                                     });
     });
@@ -245,7 +245,7 @@ describe('validate function getSourceInfoString', function () {
             repo: 'mobileapp',
             isLatest: true,
             apiUri: 'https://api.github.com/repos/bob/mobileapp/releases/latest',
-            unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/latest/download/devcontainer-features.tgz'
+            unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/latest/download'
         };
         const output = getSourceInfoString(srcInfo);
         assert.include(output, 'github-bob-mobileapp-latest');
@@ -259,7 +259,7 @@ describe('validate function getSourceInfoString', function () {
             tag: 'v0.0.4',
             isLatest: false,
             apiUri: 'https://api.github.com/repos/bob/mobileapp/releases/tags/v0.0.4',
-            unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/download/v0.0.4/devcontainer-features.tgz'
+            unauthenticatedUri: 'https://github.com/bob/mobileapp/releases/download/v0.0.4'
         };
         const output = getSourceInfoString(srcInfo);
         assert.include(output, 'github-bob-mobileapp-v0.0.4');

From df7575b72f19927328c0c916e00726d477da5e36 Mon Sep 17 00:00:00 2001
From: Josh Spicer <josh@joshspicer.com>
Date: Wed, 8 Jun 2022 18:11:48 +0000
Subject: [PATCH 27/30] clean up tracing and formatting

---
 .../containerFeaturesConfiguration.ts         | 306 +++++++++---------
 1 file changed, 156 insertions(+), 150 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 322d8be88..48edd2a53 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -407,9 +407,11 @@ export async function generateFeaturesConfig(params: { extensionPath: string; cw
 	}
 
 	// Read features and get the type.
+	output.write('--- Processing User Features ----', LogLevel.Trace);
 	featuresConfig = await processUserFeatures(params.output, userFeatures, featuresConfig);
 
 	// Fetch features and get version information
+	output.write('--- Fetching User Features ----', LogLevel.Trace);
 	await fetchFeatures(params, featuresConfig, locallyCachedFeatureSet, dstFolder);
 
 	return featuresConfig;
@@ -494,151 +496,151 @@ export function parseFeatureIdentifier(output: Log, userFeature: DevContainerFea
 	//
 	//      No version can be provided, as the directory is copied 'as is' and is inherently taking the 'latest'
 	
-	output.write(`Processing feature: ${userFeature.id}`)
-			// cached feature
-			if (!userFeature.id.includes('/') && !userFeature.id.includes('\\')) {
-				output.write(`Cached feature found.`);
-
-				let feat: Feature = {
-					id: userFeature.id,
-					name: userFeature.id,
-					value: userFeature.options,
-					included: true,
-				}
+	output.write(`* Processing feature: ${userFeature.id}`);
 
-				let newFeaturesSet: FeatureSet = {
-					sourceInformation: {
-						type: 'local-cache', 
-					},
-					features: [feat],
-				};
-				
-				return newFeaturesSet;
-			}
+	// cached feature
+	if (!userFeature.id.includes('/') && !userFeature.id.includes('\\')) {
+		output.write(`Cached feature found.`);
 
-			// remote tar file
-			if (userFeature.id.startsWith('http://') || userFeature.id.startsWith('https://'))
-			{
-				output.write(`Remote tar file found.`);
-				let input = userFeature.id.replace(/\/+$/, '');
-				const featureIdDelimiter = input.lastIndexOf('#');
-				const id = input.substring(featureIdDelimiter + 1);
-
-				if (id === '' || !allowedFeatureIdRegex.test(id)) {
-					output.write(`Parse error. Specify a feature id with alphanumeric, dash, or underscore characters. Provided: ${id}.`, LogLevel.Error);
-					return undefined;
-				}
+		let feat: Feature = {
+			id: userFeature.id,
+			name: userFeature.id,
+			value: userFeature.options,
+			included: true,
+		}
 
-				const tarballUri = new URL.URL(input.substring(0, featureIdDelimiter)).toString();
-				let feat: Feature = {
-					id: id,
-					name: userFeature.id,
-					value: userFeature.options,
-					included: true,
-				}
+		let newFeaturesSet: FeatureSet = {
+			sourceInformation: {
+				type: 'local-cache',
+			},
+			features: [feat],
+		};
 
-				let newFeaturesSet: FeatureSet = {
-					sourceInformation: {
-						type: 'direct-tarball', 
-						tarballUri: tarballUri 
-					},
-					features: [feat],
-				};
+		return newFeaturesSet;
+	}
 
-				return newFeaturesSet;
-			}
+	// remote tar file
+	if (userFeature.id.startsWith('http://') || userFeature.id.startsWith('https://')) {
+		output.write(`Remote tar file found.`);
+		let input = userFeature.id.replace(/\/+$/, '');
+		const featureIdDelimiter = input.lastIndexOf('#');
+		const id = input.substring(featureIdDelimiter + 1);
 
-			// local disk
-			const userFeaturePath = path.parse(userFeature.id);
-			 // If its a valid path
-			 if (userFeature.id.startsWith('./') || userFeature.id.startsWith('../') || (userFeaturePath && path.isAbsolute(userFeature.id))) {
-			 //if (userFeaturePath && ((path.isAbsolute(userFeature.id) && existsSync(userFeature.id)) || !path.isAbsolute(userFeature.id))) {
-				output.write(`Local disk feature.`);
-				const filePath = userFeature.id;
-				const id = userFeaturePath.name;
-				const isRelative = !path.isAbsolute(userFeature.id);
-				
-				let feat: Feature = {
-					id: id,
-					name: userFeature.id,
-					value: userFeature.options,
-					included: true,
-				}
+		if (id === '' || !allowedFeatureIdRegex.test(id)) {
+			output.write(`Parse error. Specify a feature id with alphanumeric, dash, or underscore characters. Provided: ${id}.`, LogLevel.Error);
+			return undefined;
+		}
 
-				let newFeaturesSet: FeatureSet = {
-					sourceInformation: {
-						type: 'file-path', 
-						filePath, 
-						isRelative: isRelative
-					},
-					features: [feat],
-				};
+		const tarballUri = new URL.URL(input.substring(0, featureIdDelimiter)).toString();
+		let feat: Feature = {
+			id: id,
+			name: userFeature.id,
+			value: userFeature.options,
+			included: true,
+		}
 
-				return newFeaturesSet;
-			}
+		let newFeaturesSet: FeatureSet = {
+			sourceInformation: {
+				type: 'direct-tarball',
+				tarballUri: tarballUri
+			},
+			features: [feat],
+		};
 
-			output.write(`Github feature.`);
-			// Github repository source.
-			let version = 'latest';
-			let splitOnAt = userFeature.id.split('@');
-			if (splitOnAt.length > 2) {
-				output.write(`Parse error. Use the '@' symbol only to designate a version tag.`, LogLevel.Error);
-				return undefined;
-			}
-			if (splitOnAt.length === 2) {
-				output.write(`[${userFeature.id}] has version ${splitOnAt[1]}`, LogLevel.Trace);
-				version = splitOnAt[1];
-			}
+		return newFeaturesSet;
+	}
 
-			// Remaining info must be in the first part of the split.
-			const featureBlob = splitOnAt[0];
-			const splitOnSlash = featureBlob.split('/');
-			// We expect all GitHub/registry features to follow the triple slash pattern at this point
-			//  eg: <publisher>/<feature-set>/<feature>
-			if (splitOnSlash.length !== 3 || splitOnSlash.some(x => x === '') || !allowedFeatureIdRegex.test(splitOnSlash[2])) {
-				output.write(`Invalid parse for GitHub/registry feature identifier. Follow format: '<publisher>/<feature-set>/<feature>'`, LogLevel.Error);
-				return undefined;
-			}
-			const owner = splitOnSlash[0];
-			const repo = splitOnSlash[1];
-			const id = splitOnSlash[2];
-
-			let feat: Feature = {
-				id: id,
-				name: userFeature.id,
-				value: userFeature.options,
-				included: true,
-			};
-
-			if (version === 'latest') {
-				let newFeaturesSet: FeatureSet = {
-					sourceInformation : {
-					type: 'github-repo',
-					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/latest`,
-						unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download`, // v1/v2 implementations append name of relevant asset
-					owner,
-					repo,
-					isLatest: true
-					},
-					features: [feat],
-				};
-				return newFeaturesSet;
-			} else {
-				// We must have a tag, return a tarball URI for the tagged version. 
-				let newFeaturesSet: FeatureSet = {
-					sourceInformation : {
-					type: 'github-repo',
-					apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/tags/${version}`,
-						unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${version}`, // v1/v2 implementations append name of relevant asset
-					owner,
-					repo,
-					tag: version,
-					isLatest: false
-					},
-					features: [feat],
-				};
-				return newFeaturesSet;
-			}
+	// local disk
+	const userFeaturePath = path.parse(userFeature.id);
+	// If its a valid path
+	if (userFeature.id.startsWith('./') || userFeature.id.startsWith('../') || (userFeaturePath && path.isAbsolute(userFeature.id))) {
+		//if (userFeaturePath && ((path.isAbsolute(userFeature.id) && existsSync(userFeature.id)) || !path.isAbsolute(userFeature.id))) {
+		output.write(`Local disk feature.`);
+		const filePath = userFeature.id;
+		const id = userFeaturePath.name;
+		const isRelative = !path.isAbsolute(userFeature.id);
+
+		let feat: Feature = {
+			id: id,
+			name: userFeature.id,
+			value: userFeature.options,
+			included: true,
+		}
+
+		let newFeaturesSet: FeatureSet = {
+			sourceInformation: {
+				type: 'file-path',
+				filePath,
+				isRelative: isRelative
+			},
+			features: [feat],
+		};
+
+		return newFeaturesSet;
+	}
+
+	output.write(`Github feature.`);
+	// Github repository source.
+	let version = 'latest';
+	let splitOnAt = userFeature.id.split('@');
+	if (splitOnAt.length > 2) {
+		output.write(`Parse error. Use the '@' symbol only to designate a version tag.`, LogLevel.Error);
+		return undefined;
+	}
+	if (splitOnAt.length === 2) {
+		output.write(`[${userFeature.id}] has version ${splitOnAt[1]}`, LogLevel.Trace);
+		version = splitOnAt[1];
+	}
+
+	// Remaining info must be in the first part of the split.
+	const featureBlob = splitOnAt[0];
+	const splitOnSlash = featureBlob.split('/');
+	// We expect all GitHub/registry features to follow the triple slash pattern at this point
+	//  eg: <publisher>/<feature-set>/<feature>
+	if (splitOnSlash.length !== 3 || splitOnSlash.some(x => x === '') || !allowedFeatureIdRegex.test(splitOnSlash[2])) {
+		output.write(`Invalid parse for GitHub/registry feature identifier. Follow format: '<publisher>/<feature-set>/<feature>'`, LogLevel.Error);
+		return undefined;
+	}
+	const owner = splitOnSlash[0];
+	const repo = splitOnSlash[1];
+	const id = splitOnSlash[2];
+
+	let feat: Feature = {
+		id: id,
+		name: userFeature.id,
+		value: userFeature.options,
+		included: true,
+	};
+
+	if (version === 'latest') {
+		let newFeaturesSet: FeatureSet = {
+			sourceInformation: {
+				type: 'github-repo',
+				apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/latest`,
+				unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/latest/download`, // v1/v2 implementations append name of relevant asset
+				owner,
+				repo,
+				isLatest: true
+			},
+			features: [feat],
+		};
+		return newFeaturesSet;
+	} else {
+		// We must have a tag, return a tarball URI for the tagged version. 
+		let newFeaturesSet: FeatureSet = {
+			sourceInformation: {
+				type: 'github-repo',
+				apiUri: `https://api.github.com/repos/${owner}/${repo}/releases/tags/${version}`,
+				unauthenticatedUri: `https://github.com/${owner}/${repo}/releases/download/${version}`, // v1/v2 implementations append name of relevant asset
+				owner,
+				repo,
+				tag: version,
+				isLatest: false
+			},
+			features: [feat],
+		};
+		return newFeaturesSet;
+	}
 }
 
 async function fetchFeatures(params: { extensionPath: string; cwd: string; output: Log; env: NodeJS.ProcessEnv }, featuresConfig: FeaturesConfig, localFeatures: FeatureSet, dstFolder: string) {
@@ -649,10 +651,7 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				continue;
 			}
 
-			params.output.write(`* fetching feature...`, LogLevel.Trace);
-
-			if(!localFeatures)
-			{
+			if (!localFeatures) {
 				continue;
 			}
 
@@ -660,11 +659,15 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 			const consecutiveId = feature.id + '_' + getCounter();
 			// Calculate some predictable caching paths.
 			const featCachePath = path.join(dstFolder, consecutiveId);
+			const sourceInfoType = featureSet.sourceInformation?.type;
 
 			feature.infoString = featCachePath;
 			feature.consecutiveId = consecutiveId;
 
-			if(featureSet.sourceInformation?.type === 'local-cache') {
+			const featureDebugId = `${feature.consecutiveId}_${sourceInfoType}`;
+			params.output.write(`* Fetching feature: ${featureDebugId}`);
+
+			if (sourceInfoType === 'local-cache') {
 				// create copy of the local features to set the environment variables for them.
 				await mkdirpLocal(featCachePath);
 				await cpDirectoryLocal(path.join(dstFolder, 'local-cache'), featCachePath);
@@ -679,8 +682,8 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				continue;
 			}
 		
-			if(featureSet.sourceInformation?.type === 'file-path') {
-				params.output.write(`Detected local file path`);
+			if (sourceInfoType === 'file-path') {
+				params.output.write(`Detected local file path`, LogLevel.Trace);
 				
 				const executionPath = featureSet.sourceInformation.isRelative ? path.join(params.cwd, featureSet.sourceInformation.filePath) : featureSet.sourceInformation.filePath;
 
@@ -690,13 +693,13 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 				continue;
 			}
 
-			params.output.write(`Detected tarball`);
+			params.output.write(`Detected tarball`, LogLevel.Trace);
 			const headers = getRequestHeaders(featureSet.sourceInformation, params.env, params.output);
 
 			// Ordered list of tarballUris to attempt to fetch from.
 			let tarballUris: string[] = [];
 
-			if (featureSet.sourceInformation.type === 'github-repo') {
+			if (sourceInfoType === 'github-repo') {
 				params.output.write('Determining tarball URI for provided github repo.', LogLevel.Trace);
 				if (headers.Authorization && headers.Authorization !== '') {
 					params.output.write('GITHUB_TOKEN available. Attempting to fetch via GH API.', LogLevel.Info);
@@ -721,22 +724,25 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu
 			}
 
 			// Attempt to fetch from 'tarballUris' in order, until one succeeds.
+			let didSucceed: boolean = false;
 			for (const tarballUri of tarballUris) {
-				const didSucceed = await fetchContentsAtTarballUri(tarballUri, featCachePath, headers, dstFolder, params.output);
+				didSucceed = await fetchContentsAtTarballUri(tarballUri, featCachePath, headers, dstFolder, params.output);
 
 				if (didSucceed) {
-					params.output.write(`Succeeded fetching ${tarballUri}`, LogLevel.Trace)
+					params.output.write(`Succeeded fetching ${tarballUri}`, LogLevel.Trace);
 					await parseDevContainerFeature(featureSet, feature, featCachePath);
 					break;
 				}
 			}
 
-			const msg = `(!) Failed to fetch tarball after attempting ${tarballUris.length} possibilities.`;
-			params.output.write(msg, LogLevel.Error);
-			throw new Error(msg);
+			if (!didSucceed) {
+				const msg = `(!) Failed to fetch tarball for ${featureDebugId} after attempting ${tarballUris.length} possibilities.`;
+				params.output.write(msg, LogLevel.Error);
+				throw new Error(msg);
+			}
 		}
 		catch (e) {
-			params.output.write(`Failed to fetch feature. ${e?.Message ?? ''} `, LogLevel.Trace);
+			params.output.write(`(!) ERR: Failed to fetch feature: ${e?.Message ?? ''} `, LogLevel.Error);
 			// TODO: Should this be more fatal?
 		}
 	}

From 68e68b1f4a4757bc3337356a9ab3354e867adfa3 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Fri, 24 Jun 2022 11:39:17 -0700
Subject: [PATCH 28/30] Changes from the last spec proposal changes.

---
 .vscode/launch.json                           |   8 +-
 src/spec-common/injectHeadless.ts             |   7 +-
 src/spec-configuration/configuration.ts       |   9 +-
 .../containerFeaturesConfiguration.ts         |  15 ++-
 .../containerFeaturesOrder.ts                 | 108 ++++++++++++++++++
 src/spec-configuration/tsconfig.json          |   3 +
 6 files changed, 139 insertions(+), 11 deletions(-)
 create mode 100644 src/spec-configuration/containerFeaturesOrder.ts

diff --git a/.vscode/launch.json b/.vscode/launch.json
index e5639a4b7..60000face 100644
--- a/.vscode/launch.json
+++ b/.vscode/launch.json
@@ -10,7 +10,13 @@
             "name": "Launch cli - up",
             "program": "${workspaceFolder}/src/spec-node/devContainersSpecCLI.ts",
             "cwd": "${workspaceFolder}",
-            "args": ["up", "--workspace-folder", "../features-playground", "--log-level", "debug", ],
+            "args": [
+                "up",
+                "--workspace-folder",
+                "../devcontainers-features",
+                "--log-level",
+                "debug",
+            ],
             "console": "integratedTerminal",
         }
     ]
diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts
index 5dc522a58..a7a616e8b 100644
--- a/src/spec-common/injectHeadless.ts
+++ b/src/spec-common/injectHeadless.ts
@@ -105,18 +105,13 @@ export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand'
 
 const defaultWaitFor: DevContainerConfigCommand = 'updateContentCommand';
 
-export interface DevContainerFeature {
-	id: string;
-	options: boolean | string | Record<string, boolean | string | undefined>;
-}
-
 export interface CommonDevContainerConfig {
 	configFilePath?: URI;
 	remoteEnv?: Record<string, string | null>;
 	forwardPorts?: (number | string)[];
 	portsAttributes?: Record<string, PortAttributes>;
 	otherPortsAttributes?: PortAttributes;
-	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: Record<string, string | boolean | Record<string, string | boolean>>;
 	onCreateCommand?: string | string[];
 	updateContentCommand?: string | string[];
 	postCreateCommand?: string | string[];
diff --git a/src/spec-configuration/configuration.ts b/src/spec-configuration/configuration.ts
index a5c5b38b9..2271b9c38 100644
--- a/src/spec-configuration/configuration.ts
+++ b/src/spec-configuration/configuration.ts
@@ -59,7 +59,8 @@ export interface DevContainerFromImageConfig {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	overrideFeatureInstallOrder?: string[];
 	hostRequirements?: HostRequirements;
 }
 
@@ -90,7 +91,8 @@ export type DevContainerFromDockerfileConfig = {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	overrideFeatureInstallOrder?: string[];
 	hostRequirements?: HostRequirements;
 } & (
 		{
@@ -137,7 +139,8 @@ export interface DevContainerFromDockerComposeConfig {
 	remoteUser?: string;
 	updateRemoteUserUID?: boolean;
 	userEnvProbe?: UserEnvProbe;
-	features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>;
+	features?: Record<string, string | boolean | Record<string, string | boolean>>;
+	overrideFeatureInstallOrder?: string[];
 	hostRequirements?: HostRequirements;
 }
 
diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts
index 48edd2a53..7f4c95fd8 100644
--- a/src/spec-configuration/containerFeaturesConfiguration.ts
+++ b/src/spec-configuration/containerFeaturesConfiguration.ts
@@ -7,11 +7,13 @@ import * as jsonc from 'jsonc-parser';
 import * as path from 'path';
 import * as URL from 'url';
 import * as tar from 'tar';
+import { existsSync } from 'fs';
 import { DevContainerConfig, DevContainerFeature } from './configuration';
 import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal } from '../spec-utils/pfs';
 import { Log, LogLevel } from '../spec-utils/log';
 import { request } from '../spec-utils/httpRequest';
-import { existsSync } from 'fs';
+import { computeFeatureInstallationOrder } from './containerFeaturesOrder';
+
 
 const V1_ASSET_NAME = 'devcontainer-features.tgz';
 
@@ -38,6 +40,7 @@ export interface Feature {
 	capAdd?: string[];
 	securityOpt?: string[];
 	entrypoint?: string;
+	installAfter?: string[];
 	include?: string[];
 	exclude?: string[];
 	value: boolean | string | Record<string, boolean | string | undefined>; // set programmatically
@@ -414,6 +417,15 @@ export async function generateFeaturesConfig(params: { extensionPath: string; cw
 	output.write('--- Fetching User Features ----', LogLevel.Trace);
 	await fetchFeatures(params, featuresConfig, locallyCachedFeatureSet, dstFolder);
 
+	const ordererdFeatures = computeFeatureInstallationOrder(config, featuresConfig.featureSets);
+
+	output.write('--- Computed order ----', LogLevel.Trace);
+	for (const feature of ordererdFeatures) {
+		output.write(`${feature.features[0].id}`, LogLevel.Trace);
+	}
+
+	featuresConfig.featureSets = ordererdFeatures;
+
 	return featuresConfig;
 }
 
@@ -826,6 +838,7 @@ async function parseDevContainerFeature(featureSet: FeatureSet, feature: Feature
 		featureSet.internalVersion = '2';
 		feature.buildArg = featureJson.buildArg;
 		feature.options = featureJson.options;
+		feature.installAfter = featureJson.installAfter;
 	} else {
 		featureSet.internalVersion = '1';
 	}
diff --git a/src/spec-configuration/containerFeaturesOrder.ts b/src/spec-configuration/containerFeaturesOrder.ts
new file mode 100644
index 000000000..3eaebce6d
--- /dev/null
+++ b/src/spec-configuration/containerFeaturesOrder.ts
@@ -0,0 +1,108 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+
+import { ContainerError } from '../spec-common/errors';
+import { FeatureSet } from '../spec-configuration/containerFeaturesConfiguration';
+import { DevContainerConfig } from './configuration';
+
+interface FeatureNode {
+    feature: FeatureSet;
+    before: Set<FeatureNode>;
+    after: Set<FeatureNode>;
+}
+
+export function computeFeatureInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
+
+    if (!config.overrideFeatureInstallOrder) {
+        return computeInstallationOrder(features);
+    }
+    else {
+        return computeOverrideInstallationOrder(config, features);
+    }
+}
+
+function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
+    const automaticOrder = computeInstallationOrder(features);
+
+    const orderedFeatures = [];
+
+    for (const featureId of config.overrideFeatureInstallOrder!) {
+        const feature = automaticOrder.find(feature => feature.features[0].name === featureId);
+        if (!feature) {
+            throw new ContainerError({ description: `Feature ${featureId} not found` });
+        }
+        orderedFeatures.push(feature);
+        features.splice(features.indexOf(feature), 1);
+    }
+
+    return orderedFeatures.concat(features);
+}
+
+function computeInstallationOrder(features: FeatureSet[]) {
+    const nodesById = features.map<FeatureNode>(feature => ({
+        feature,
+        before: new Set(),
+        after: new Set(),
+    })).reduce((map, feature) => map.set(feature.feature.features[0].id, feature), new Map<string, FeatureNode>());
+
+    const nodes = [...nodesById.values()];
+    for (const later of nodes) {
+        for (const firstId of later.feature.features[0].installAfter || []) {
+            const first = nodesById.get(firstId);
+            // soft dependencies
+            if (first) {
+                later.after.add(first);
+                first.before.add(later);
+            }
+        }
+    }
+
+    const { roots, islands } = nodes.reduce((prev, node) => {
+        if (node.after.size === 0) {
+            if (node.before.size === 0) {
+                prev.islands.push(node);
+            } else {
+                prev.roots.push(node);
+            }
+        }
+        return prev;
+    }, { roots: [] as FeatureNode[], islands: [] as FeatureNode[] });
+
+    const orderedFeatures = [];
+    let current = roots;
+    while (current.length) {
+        const next = [];
+        for (const first of current) {
+            for (const later of first.before) {
+                later.after.delete(first);
+                if (later.after.size === 0) {
+                    next.push(later);
+                }
+            }
+        }
+        orderedFeatures.push(
+            ...current.map(node => node.feature)
+                .sort((a, b) => a.features[0].id.localeCompare(b.features[0].id)) // stable order
+        );
+        current = next;
+    }
+
+    orderedFeatures.push(
+        ...islands.map(node => node.feature)
+            .sort((a, b) => a.features[0].id.localeCompare(b.features[0].id)) // stable order
+    );
+
+    const missing = new Set(nodesById.keys());
+    for (const feature of orderedFeatures) {
+        missing.delete(feature.features[0].id);
+    }
+
+    if (missing.size !== 0) {
+        throw new ContainerError({ description: `Features declare cyclic dependency: ${[...missing].join(', ')}` });
+    }
+
+    return orderedFeatures;
+}
\ No newline at end of file
diff --git a/src/spec-configuration/tsconfig.json b/src/spec-configuration/tsconfig.json
index eff319378..2b22c3542 100644
--- a/src/spec-configuration/tsconfig.json
+++ b/src/spec-configuration/tsconfig.json
@@ -3,6 +3,9 @@
 	"references": [
 		{
 			"path": "../spec-utils"
+		},
+		{
+			"path": "../spec-common"
 		}
 	]
 }
\ No newline at end of file

From 39a1b222373f1276ef4447e6f19a7d8cb775db10 Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Mon, 27 Jun 2022 14:09:49 -0700
Subject: [PATCH 29/30] Comments and fixes from PR.

---
 src/spec-configuration/containerFeaturesOrder.ts         | 9 +++++----
 .../generateFeaturesConfig.offline.test.ts               | 3 +--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/spec-configuration/containerFeaturesOrder.ts b/src/spec-configuration/containerFeaturesOrder.ts
index 3eaebce6d..ac0bbf149 100644
--- a/src/spec-configuration/containerFeaturesOrder.ts
+++ b/src/spec-configuration/containerFeaturesOrder.ts
@@ -16,19 +16,20 @@ interface FeatureNode {
 
 export function computeFeatureInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
 
-    if (!config.overrideFeatureInstallOrder) {
-        return computeInstallationOrder(features);
+    if (config.overrideFeatureInstallOrder) {
+        return computeOverrideInstallationOrder(config, features);
     }
     else {
-        return computeOverrideInstallationOrder(config, features);
+        return computeInstallationOrder(features);
     }
 }
 
 function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
+    // Starts with the automatic installation order.
     const automaticOrder = computeInstallationOrder(features);
 
+    // Moves to the beginning the features that are explicitly configured.
     const orderedFeatures = [];
-
     for (const featureId of config.overrideFeatureInstallOrder!) {
         const feature = automaticOrder.find(feature => feature.features[0].name === featureId);
         if (!feature) {
diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index fab0a7a26..0296e1866 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -4,9 +4,8 @@ import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';
 import * as os from 'os';
 import * as path from 'path';
 import { mkdirpLocal } from '../../spec-utils/pfs';
-import { DevContainerConfig } from '../../spec-configuration/configuration';
+import { DevContainerConfig, DevContainerFeature } from '../../spec-configuration/configuration';
 import { URI } from 'vscode-uri';
-import { DevContainerFeature } from '../../spec-common/injectHeadless';
 
 export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
 

From e92dd247f1b004d0f879afe011ab15e57182ad5f Mon Sep 17 00:00:00 2001
From: Edmundo Gonzalez <51725820+edgonmsft@users.noreply.github.com>
Date: Mon, 27 Jun 2022 14:37:12 -0700
Subject: [PATCH 30/30] Unit test fixes.

---
 .../containerFeaturesOrder.ts                 |   6 +-
 .../containerFeaturesOrder.offline.test.ts    | 113 ++++++++++++++++++
 .../generateFeaturesConfig.offline.test.ts    |  25 ++--
 3 files changed, 126 insertions(+), 18 deletions(-)
 create mode 100644 src/test/container-features/containerFeaturesOrder.offline.test.ts

diff --git a/src/spec-configuration/containerFeaturesOrder.ts b/src/spec-configuration/containerFeaturesOrder.ts
index ac0bbf149..d143d5dd7 100644
--- a/src/spec-configuration/containerFeaturesOrder.ts
+++ b/src/spec-configuration/containerFeaturesOrder.ts
@@ -24,7 +24,8 @@ export function computeFeatureInstallationOrder(config: DevContainerConfig, feat
     }
 }
 
-function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
+// Exported for unit tests.
+export function computeOverrideInstallationOrder(config: DevContainerConfig, features: FeatureSet[]) {
     // Starts with the automatic installation order.
     const automaticOrder = computeInstallationOrder(features);
 
@@ -42,7 +43,8 @@ function computeOverrideInstallationOrder(config: DevContainerConfig, features:
     return orderedFeatures.concat(features);
 }
 
-function computeInstallationOrder(features: FeatureSet[]) {
+// Exported for unit tests.
+export function computeInstallationOrder(features: FeatureSet[]) {
     const nodesById = features.map<FeatureNode>(feature => ({
         feature,
         before: new Set(),
diff --git a/src/test/container-features/containerFeaturesOrder.offline.test.ts b/src/test/container-features/containerFeaturesOrder.offline.test.ts
new file mode 100644
index 000000000..9e99f7dda
--- /dev/null
+++ b/src/test/container-features/containerFeaturesOrder.offline.test.ts
@@ -0,0 +1,113 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { ContainerError } from '../../spec-common/errors';
+import { FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration';
+import { computeInstallationOrder, computeOverrideInstallationOrder } from '../../spec-configuration/containerFeaturesOrder';
+import { URI } from 'vscode-uri';
+
+describe('Container features install order', () => {
+
+    it('has stable order among independent features', () => {
+        assert.deepEqual(
+            computeInstallationOrder([
+                installAfter('C'),
+                installAfter('A'),
+                installAfter('B'),
+            ]).map(f => f.features[0].id),
+            ['A', 'B', 'C']
+        );
+    });
+
+    it('orders "installAfter" first in breadth-first order (tree)', () => {
+        assert.deepEqual(
+            computeInstallationOrder([
+                installAfter('A', 'B'),
+                installAfter('B', 'C'),
+                installAfter('C'),
+                installAfter('D', 'E'),
+                installAfter('E', 'C'),
+            ]).map(f => f.features[0].id),
+            ['C', 'B', 'E', 'A', 'D']
+        );
+    });
+
+    it('orders "installAfter" first in breadth-first order (DAG)', () => {
+        assert.deepEqual(
+            computeInstallationOrder([
+                installAfter('A', 'B', 'C'),
+                installAfter('B', 'C'),
+                installAfter('C'),
+                installAfter('D', 'C'),
+            ]).map(f => f.features[0].id),
+            ['C', 'B', 'D', 'A']
+        );
+    });
+
+    it('treats "installAfter" is a soft dependency', () => {
+        assert.deepEqual(
+            computeInstallationOrder([
+                installAfter('A', 'B', 'C'),
+                installAfter('B'),
+            ]).map(f => f.features[0].id),
+            ['B', 'A']
+        );
+    });
+
+    it('orders independent features last', () => {
+        assert.deepEqual(
+            computeInstallationOrder([
+                installAfter('A'),
+                installAfter('B', 'C'),
+                installAfter('C'),
+            ]).map(f => f.features[0].id),
+            ['C', 'B', 'A']
+        );
+    });
+
+    it('detects cycles', () => {
+        try {
+            computeInstallationOrder([
+                installAfter('A', 'B'),
+                installAfter('B'),
+                installAfter('C', 'D'),
+                installAfter('D', 'C'),
+            ]);
+            assert.fail('Cyclic dependency not detected.');
+        } catch (err) {
+            assert.ok(err instanceof ContainerError);
+            assert.ok(err.message.indexOf('cyclic'));
+        }
+    });
+
+    it('respects OverrideConfig', () => {
+        assert.deepEqual(
+            computeOverrideInstallationOrder(
+                { image: 'ubuntu', configFilePath: URI.from({ 'scheme': 'https' }), overrideFeatureInstallOrder: ['A', 'B', 'C'] },
+                [
+                    installAfter('A', 'C'),
+                    installAfter('B', 'C'),
+                    installAfter('C', 'D'),
+                ]).map(f => f.features[0].id),
+            ['A', 'B', 'C']
+        );
+    });
+
+    function installAfter(id: string, ...installAfter: string[]): FeatureSet {
+        return {
+            sourceInformation: {
+                type: 'local-cache',
+            },
+            features: [{
+                id,
+                name: id,
+                installAfter,
+                value: true,
+                included: true,
+            }],
+        };
+    }
+});
\ No newline at end of file
diff --git a/src/test/container-features/generateFeaturesConfig.offline.test.ts b/src/test/container-features/generateFeaturesConfig.offline.test.ts
index 0296e1866..bd3c3847e 100644
--- a/src/test/container-features/generateFeaturesConfig.offline.test.ts
+++ b/src/test/container-features/generateFeaturesConfig.offline.test.ts
@@ -4,7 +4,7 @@ import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';
 import * as os from 'os';
 import * as path from 'path';
 import { mkdirpLocal } from '../../spec-utils/pfs';
-import { DevContainerConfig, DevContainerFeature } from '../../spec-configuration/configuration';
+import { DevContainerConfig } from '../../spec-configuration/configuration';
 import { URI } from 'vscode-uri';
 
 export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
@@ -38,25 +38,18 @@ describe('validate (offline) generateFeaturesConfig()', function () {
         const tmpFolder: string = path.join(os.tmpdir(), 'vsch', 'container-features', `${version}-${Date.now()}`);
         await mkdirpLocal(tmpFolder);
 
-        const features: DevContainerFeature[] = [
-            {
-                id: 'first',
-                options: {
-                    'version': 'latest'
-                },
-            },
-            {
-                id: 'second',
-                options: {
-                    'value': true
-                },
-            }
-        ];
 
         const config: DevContainerConfig = {
             configFilePath: URI.from({ 'scheme': 'https' }),
             dockerFile: '.',
-            features: features
+            features: {
+                first: {
+                    'version': 'latest'
+                },
+                second: {
+                    'value': true
+                },
+            },
         };
 
         const featuresConfig = await generateFeaturesConfig(params, tmpFolder, config, labels, localFeaturesFolder);