Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deploy): ensure import map or config file is included in the manifest #326

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ jobs:
os: [macOS-latest, windows-latest, ubuntu-22.04-xl]

steps:
# Some test cases are sensitive to line endings. Disable autocrlf on
# Windows to ensure consistent behavior.
- name: Disable autocrlf
if: runner.os == 'Windows'
run: git config --global core.autocrlf false

- name: Setup repo
uses: actions/checkout@v3

Expand Down
14 changes: 11 additions & 3 deletions action/deps.js
Original file line number Diff line number Diff line change
Expand Up @@ -4354,7 +4354,15 @@ function include(path, include, exclude) {
}
return true;
}
async function walk(cwd, dir, files, options) {
async function walk(cwd, dir, options) {
const hashPathMap = new Map();
const manifestEntries = await walkInner(cwd, dir, hashPathMap, options);
return {
manifestEntries,
hashPathMap
};
}
async function walkInner(cwd, dir, hashPathMap, options) {
const entries = {};
for await (const file of Deno.readDir(dir)){
const path = join2(dir, file.name);
Expand All @@ -4371,12 +4379,12 @@ async function walk(cwd, dir, files, options) {
gitSha1,
size: data.byteLength
};
files.set(gitSha1, path);
hashPathMap.set(gitSha1, path);
} else if (file.isDirectory) {
if (relative === "/.git") continue;
entry = {
kind: "directory",
entries: await walk(cwd, path, files, options)
entries: await walkInner(cwd, path, hashPathMap, options)
};
} else if (file.isSymlink) {
const target = await Deno.readLink(path);
Expand Down
13 changes: 8 additions & 5 deletions action/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,14 @@ async function main() {
if (!includes.some((i) => i.includes("node_modules"))) {
excludes.push("**/node_modules");
}
const assets = new Map();
const entries = await walk(cwd, cwd, assets, {
include: includes.map(convertPatternToRegExp),
exclude: excludes.map(convertPatternToRegExp),
});
const { manifestEntries: entries, hashPathMap: assets } = await walk(
cwd,
cwd,
{
include: includes.map(convertPatternToRegExp),
exclude: excludes.map(convertPatternToRegExp),
},
);
core.debug(`Discovered ${assets.size} assets`);

const api = new API(`GitHubOIDC ${token}`, ORIGIN);
Expand Down
44 changes: 40 additions & 4 deletions src/subcommands/deploy.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
// Copyright 2021 Deno Land Inc. All rights reserved. MIT license.

import { fromFileUrl, type Spinner } from "../../deps.ts";
import { fromFileUrl, relative, type Spinner, yellow } from "../../deps.ts";
import { envVarsFromArgs } from "../utils/env_vars.ts";
import { wait } from "../utils/spinner.ts";
import configFile from "../config_file.ts";
import { error } from "../error.ts";
import { API, APIError, endpoint } from "../utils/api.ts";
import type { ManifestEntry } from "../utils/api_types.ts";
import { parseEntrypoint } from "../utils/entrypoint.ts";
import { convertPatternToRegExp, walk } from "../utils/walk.ts";
import {
containsEntryInManifest,
convertPatternToRegExp,
walk,
} from "../utils/manifest.ts";
import TokenProvisioner from "../utils/access_token.ts";
import type { Args as RawArgs } from "../args.ts";
import organization from "../utils/organization.ts";
Expand Down Expand Up @@ -273,14 +277,46 @@ async function deploy(opts: DeployOpts): Promise<void> {
if (opts.static) {
wait("").start().info(`Uploading all files from the current dir (${cwd})`);
const assetSpinner = wait("Finding static assets...").start();
const assets = new Map<string, string>();
const include = opts.include.map(convertPatternToRegExp);
const exclude = opts.exclude.map(convertPatternToRegExp);
const entries = await walk(cwd, cwd, assets, { include, exclude });
const { manifestEntries: entries, hashPathMap: assets } = await walk(
cwd,
cwd,
{ include, exclude },
);
assetSpinner.succeed(
`Found ${assets.size} asset${assets.size === 1 ? "" : "s"}.`,
);

// If the import map is specified but not in the manifest, error out.
if (
opts.importMapUrl !== null &&
!containsEntryInManifest(
entries,
relative(cwd, fromFileUrl(opts.importMapUrl)),
)
) {
error(
`Import map ${opts.importMapUrl} not found in the assets to be uploaded. Please check --include and --exclude options to make sure the import map is included.`,
);
}

// If the config file is present but not in the manifest, show a warning
// that any import map settings in the config file will not be used.
if (
opts.importMapUrl === null && opts.config !== null &&
!containsEntryInManifest(
entries,
relative(cwd, opts.config),
)
) {
wait("").start().warn(
yellow(
`Config file ${opts.config} not found in the assets to be uploaded; any import map settings in the config file will not be applied during deployment. If this is not your intention, please check --include and --exclude options to make sure the config file is included.`,
),
);
}

uploadSpinner = wait("Determining assets to upload...").start();
const neededHashes = await api.projectNegotiateAssets(project.id, {
entries,
Expand Down
63 changes: 60 additions & 3 deletions src/utils/walk.ts → src/utils/manifest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { globToRegExp, isGlob, join, normalize } from "../../deps.ts";
import { unreachable } from "../../tests/deps.ts";
import type { ManifestEntry } from "./api_types.ts";

/** Calculate git object hash, like `git hash-object` does. */
Expand Down Expand Up @@ -38,7 +39,25 @@ function include(
export async function walk(
cwd: string,
dir: string,
files: Map<string, string>,
options: { include: RegExp[]; exclude: RegExp[] },
): Promise<
{
manifestEntries: Record<string, ManifestEntry>;
hashPathMap: Map<string, string>;
}
> {
const hashPathMap = new Map<string, string>();
const manifestEntries = await walkInner(cwd, dir, hashPathMap, options);
return {
manifestEntries,
hashPathMap,
};
}

async function walkInner(
cwd: string,
dir: string,
hashPathMap: Map<string, string>,
options: { include: RegExp[]; exclude: RegExp[] },
): Promise<Record<string, ManifestEntry>> {
const entries: Record<string, ManifestEntry> = {};
Expand All @@ -65,12 +84,12 @@ export async function walk(
gitSha1,
size: data.byteLength,
};
files.set(gitSha1, path);
hashPathMap.set(gitSha1, path);
} else if (file.isDirectory) {
if (relative === "/.git") continue;
entry = {
kind: "directory",
entries: await walk(cwd, path, files, options),
entries: await walkInner(cwd, path, hashPathMap, options),
};
} else if (file.isSymlink) {
const target = await Deno.readLink(path);
Expand Down Expand Up @@ -98,3 +117,41 @@ export function convertPatternToRegExp(pattern: string): RegExp {
? new RegExp(globToRegExp(normalize(pattern)).toString().slice(1, -2))
: new RegExp(`^${normalize(pattern)}`);
}

/**
* Determines if the manifest contains the entry at the given relative path.
*
* @param manifestEntries manifest entries to search
* @param entryRelativePathToLookup a relative path to look up in the manifest
* @returns `true` if the manifest contains the entry at the given relative path
*/
export function containsEntryInManifest(
manifestEntries: Record<string, ManifestEntry>,
entryRelativePathToLookup: string,
): boolean {
for (const [entryName, entry] of Object.entries(manifestEntries)) {
switch (entry.kind) {
case "file":
case "symlink": {
if (entryName === entryRelativePathToLookup) {
return true;
}
break;
}
case "directory": {
if (!entryRelativePathToLookup.startsWith(entryName)) {
break;
}

const relativePath = entryRelativePathToLookup.slice(
entryName.length + 1,
);
return containsEntryInManifest(entry.entries, relativePath);
}
default:
unreachable();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: replace this with assertNever once this is released
denoland/std#5690

}
}

return false;
}
Loading
Loading