Skip to content

Commit

Permalink
fix(compile): certain jsr specifiers sometimes can't load (#23567)
Browse files Browse the repository at this point in the history
When returning a jsr specifier for resolve it seems like deno core does
not work properly and hangs.

Closes #23551
Closes #23139
  • Loading branch information
dsherret committed Apr 27, 2024
1 parent e0f8492 commit 651e3e9
Show file tree
Hide file tree
Showing 22 changed files with 429 additions and 4 deletions.
4 changes: 3 additions & 1 deletion cli/standalone/binary.rs
Expand Up @@ -488,7 +488,9 @@ impl<'a> DenoCompileBinaryWriter<'a> {
// Phase 2 of the 'min sized' deno compile RFC talks
// about adding this as a flag.
if let Some(path) = std::env::var_os("DENORT_BIN") {
return Ok(std::fs::read(path)?);
return std::fs::read(&path).with_context(|| {
format!("Could not find denort at '{}'", path.to_string_lossy())
});
}

let target = compile_flags.resolve_target();
Expand Down
7 changes: 7 additions & 0 deletions cli/standalone/mod.rs
Expand Up @@ -150,6 +150,13 @@ impl ModuleLoader for EmbeddedModuleLoader {
Some(resolved) => resolved,
None => deno_core::resolve_import(specifier, referrer.as_str())?,
};

if specifier.scheme() == "jsr" {
if let Some(module) = self.shared.eszip.get_module(specifier.as_str()) {
return Ok(ModuleSpecifier::parse(&module.specifier).unwrap());
}
}

self
.shared
.node_resolver
Expand Down
1 change: 1 addition & 0 deletions tests/specs/README.md
Expand Up @@ -78,6 +78,7 @@ a "steps" array.
- `output` - Path to use to assert the output.
- `cleanDenoDir` (boolean) - Whether to empty the deno_dir before running the
step.
- `if` (`"windows"`, `"linux"`, `"mac"`, `"unix"`) - Whether to run this step.
- `exitCode` (number) - Expected exit code.

### Auto-complete
Expand Down
22 changes: 22 additions & 0 deletions tests/specs/compile/jsr_with_deps/__test__.jsonc
@@ -0,0 +1,22 @@
{
"tempDir": true,
"steps": [{
"if": "unix",
"args": "compile --output main main.ts",
"output": "[WILDCARD]"
}, {
"if": "unix",
"commandName": "./main",
"args": [],
"output": "main.out"
}, {
"if": "windows",
"args": "compile --output main.exe main.ts",
"output": "[WILDCARD]"
}, {
"if": "windows",
"commandName": "./main.exe",
"args": [],
"output": "main.out"
}]
}
2 changes: 2 additions & 0 deletions tests/specs/compile/jsr_with_deps/main.out
@@ -0,0 +1,2 @@
[Function: join]
http://[WILDLINE]/@std/url/0.220.1/join.ts
8 changes: 8 additions & 0 deletions tests/specs/compile/jsr_with_deps/main.ts
@@ -0,0 +1,8 @@
// this was previously hanging in deno compile and wouldn't work
import { join } from "jsr:@std/url@0.220/join";
import "jsr:@std/url@0.220/normalize";

console.log(join);

// ensure import.meta.resolve works in compile for jsr specifiers
console.log(import.meta.resolve("jsr:@std/url@0.220/join"));
25 changes: 22 additions & 3 deletions tests/specs/mod.rs
Expand Up @@ -73,6 +73,8 @@ struct StepMetaData {
pub clean_deno_dir: bool,
pub args: VecOrString,
pub cwd: Option<String>,
#[serde(rename = "if")]
pub if_cond: Option<String>,
pub command_name: Option<String>,
#[serde(default)]
pub envs: HashMap<String, String>,
Expand Down Expand Up @@ -152,8 +154,11 @@ fn run_test(test: &CollectedTest, diagnostic_logger: Rc<RefCell<Vec<u8>>>) {
// todo(dsherret): add bases in the future as needed
Some(base) => panic!("Unknown test base: {}", base),
None => {
// by default add npm and jsr env vars
builder = builder.add_jsr_env_vars().add_npm_env_vars();
// by default add all these
builder = builder
.add_jsr_env_vars()
.add_npm_env_vars()
.add_compile_env_vars();
}
}

Expand All @@ -167,7 +172,7 @@ fn run_test(test: &CollectedTest, diagnostic_logger: Rc<RefCell<Vec<u8>>>) {
cwd.copy_to_recursive_with_exclusions(temp_dir, &assertion_paths);
}

for step in &metadata.steps {
for step in metadata.steps.iter().filter(|s| should_run_step(s)) {
if step.clean_deno_dir {
context.deno_dir().path().remove_dir_all();
}
Expand Down Expand Up @@ -198,6 +203,20 @@ fn run_test(test: &CollectedTest, diagnostic_logger: Rc<RefCell<Vec<u8>>>) {
}
}

fn should_run_step(step: &StepMetaData) -> bool {
if let Some(cond) = &step.if_cond {
match cond.as_str() {
"windows" => cfg!(windows),
"unix" => cfg!(unix),
"mac" => cfg!(target_os = "macos"),
"linux" => cfg!(target_os = "linux"),
value => panic!("Unknown if condition: {}", value),
}
} else {
true
}
}

fn resolve_test_and_assertion_files(
dir: &PathRef,
metadata: &MultiTestMetaData,
Expand Down
9 changes: 9 additions & 0 deletions tests/specs/schema.json
Expand Up @@ -36,6 +36,15 @@
"type": "string"
}
},
"if": {
"type": "string",
"examples": [
"mac",
"linux",
"windows",
"unix"
]
},
"output": {
"type": "string"
},
Expand Down
@@ -0,0 +1,10 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// Copyright the Browserify authors. MIT License.

export function assertPath(path?: string) {
if (typeof path !== "string") {
throw new TypeError(
`Path must be a string. Received ${JSON.stringify(path)}`,
);
}
}
49 changes: 49 additions & 0 deletions tests/testdata/jsr/registry/@std/path/0.220.1/_common/constants.ts
@@ -0,0 +1,49 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// Copyright the Browserify authors. MIT License.
// Ported from https://github.com/browserify/path-browserify/
// This module is browser compatible.

// Alphabet chars.
export const CHAR_UPPERCASE_A = 65; /* A */
export const CHAR_LOWERCASE_A = 97; /* a */
export const CHAR_UPPERCASE_Z = 90; /* Z */
export const CHAR_LOWERCASE_Z = 122; /* z */

// Non-alphabetic chars.
export const CHAR_DOT = 46; /* . */
export const CHAR_FORWARD_SLASH = 47; /* / */
export const CHAR_BACKWARD_SLASH = 92; /* \ */
export const CHAR_VERTICAL_LINE = 124; /* | */
export const CHAR_COLON = 58; /* : */
export const CHAR_QUESTION_MARK = 63; /* ? */
export const CHAR_UNDERSCORE = 95; /* _ */
export const CHAR_LINE_FEED = 10; /* \n */
export const CHAR_CARRIAGE_RETURN = 13; /* \r */
export const CHAR_TAB = 9; /* \t */
export const CHAR_FORM_FEED = 12; /* \f */
export const CHAR_EXCLAMATION_MARK = 33; /* ! */
export const CHAR_HASH = 35; /* # */
export const CHAR_SPACE = 32; /* */
export const CHAR_NO_BREAK_SPACE = 160; /* \u00A0 */
export const CHAR_ZERO_WIDTH_NOBREAK_SPACE = 65279; /* \uFEFF */
export const CHAR_LEFT_SQUARE_BRACKET = 91; /* [ */
export const CHAR_RIGHT_SQUARE_BRACKET = 93; /* ] */
export const CHAR_LEFT_ANGLE_BRACKET = 60; /* < */
export const CHAR_RIGHT_ANGLE_BRACKET = 62; /* > */
export const CHAR_LEFT_CURLY_BRACKET = 123; /* { */
export const CHAR_RIGHT_CURLY_BRACKET = 125; /* } */
export const CHAR_HYPHEN_MINUS = 45; /* - */
export const CHAR_PLUS = 43; /* + */
export const CHAR_DOUBLE_QUOTE = 34; /* " */
export const CHAR_SINGLE_QUOTE = 39; /* ' */
export const CHAR_PERCENT = 37; /* % */
export const CHAR_SEMICOLON = 59; /* ; */
export const CHAR_CIRCUMFLEX_ACCENT = 94; /* ^ */
export const CHAR_GRAVE_ACCENT = 96; /* ` */
export const CHAR_AT = 64; /* @ */
export const CHAR_AMPERSAND = 38; /* & */
export const CHAR_EQUAL = 61; /* = */

// Digits
export const CHAR_0 = 48; /* 0 */
export const CHAR_9 = 57; /* 9 */
@@ -0,0 +1,9 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// This module is browser compatible.

import { assertPath } from "./assert_path.ts";

export function assertArg(path: string) {
assertPath(path);
if (path.length === 0) return ".";
}
@@ -0,0 +1,74 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// Copyright the Browserify authors. MIT License.
// Ported from https://github.com/browserify/path-browserify/
// This module is browser compatible.

import { CHAR_DOT, CHAR_FORWARD_SLASH } from "./constants.ts";

// Resolves . and .. elements in a path with directory names
export function normalizeString(
path: string,
allowAboveRoot: boolean,
separator: string,
isPathSeparator: (code: number) => boolean,
): string {
let res = "";
let lastSegmentLength = 0;
let lastSlash = -1;
let dots = 0;
let code: number | undefined;
for (let i = 0; i <= path.length; ++i) {
if (i < path.length) code = path.charCodeAt(i);
else if (isPathSeparator(code!)) break;
else code = CHAR_FORWARD_SLASH;

if (isPathSeparator(code!)) {
if (lastSlash === i - 1 || dots === 1) {
// NOOP
} else if (lastSlash !== i - 1 && dots === 2) {
if (
res.length < 2 ||
lastSegmentLength !== 2 ||
res.charCodeAt(res.length - 1) !== CHAR_DOT ||
res.charCodeAt(res.length - 2) !== CHAR_DOT
) {
if (res.length > 2) {
const lastSlashIndex = res.lastIndexOf(separator);
if (lastSlashIndex === -1) {
res = "";
lastSegmentLength = 0;
} else {
res = res.slice(0, lastSlashIndex);
lastSegmentLength = res.length - 1 - res.lastIndexOf(separator);
}
lastSlash = i;
dots = 0;
continue;
} else if (res.length === 2 || res.length === 1) {
res = "";
lastSegmentLength = 0;
lastSlash = i;
dots = 0;
continue;
}
}
if (allowAboveRoot) {
if (res.length > 0) res += `${separator}..`;
else res = "..";
lastSegmentLength = 2;
}
} else {
if (res.length > 0) res += separator + path.slice(lastSlash + 1, i);
else res = path.slice(lastSlash + 1, i);
lastSegmentLength = i - lastSlash - 1;
}
lastSlash = i;
dots = 0;
} else if (code === CHAR_DOT && dots !== -1) {
++dots;
} else {
dots = -1;
}
}
return res;
}
10 changes: 10 additions & 0 deletions tests/testdata/jsr/registry/@std/path/0.220.1/posix/_util.ts
@@ -0,0 +1,10 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// Copyright the Browserify authors. MIT License.
// Ported from https://github.com/browserify/path-browserify/
// This module is browser compatible.

import { CHAR_FORWARD_SLASH } from "../_common/constants.ts";

export function isPosixPathSeparator(code: number): boolean {
return code === CHAR_FORWARD_SLASH;
}
25 changes: 25 additions & 0 deletions tests/testdata/jsr/registry/@std/path/0.220.1/posix/join.ts
@@ -0,0 +1,25 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// This module is browser compatible.

import { assertPath } from "../_common/assert_path.ts";
import { normalize } from "./normalize.ts";

/**
* Join all given a sequence of `paths`,then normalizes the resulting path.
* @param paths to be joined and normalized
*/
export function join(...paths: string[]): string {
if (paths.length === 0) return ".";

let joined: string | undefined;
for (let i = 0; i < paths.length; ++i) {
const path = paths[i]!;
assertPath(path);
if (path.length > 0) {
if (!joined) joined = path;
else joined += `/${path}`;
}
}
if (!joined) return ".";
return normalize(joined);
}
30 changes: 30 additions & 0 deletions tests/testdata/jsr/registry/@std/path/0.220.1/posix/normalize.ts
@@ -0,0 +1,30 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
// This module is browser compatible.

import { assertArg } from "../_common/normalize.ts";
import { normalizeString } from "../_common/normalize_string.ts";
import { isPosixPathSeparator } from "./_util.ts";

/**
* Normalize the `path`, resolving `'..'` and `'.'` segments.
* Note that resolving these segments does not necessarily mean that all will be eliminated.
* A `'..'` at the top-level will be preserved, and an empty path is canonically `'.'`.
* @param path to be normalized
*/
export function normalize(path: string): string {
assertArg(path);

const isAbsolute = isPosixPathSeparator(path.charCodeAt(0));
const trailingSeparator = isPosixPathSeparator(
path.charCodeAt(path.length - 1),
);

// Normalize the path
path = normalizeString(path, !isAbsolute, "/", isPosixPathSeparator);

if (path.length === 0 && !isAbsolute) path = ".";
if (path.length > 0 && trailingSeparator) path += "/";

if (isAbsolute) return `/${path}`;
return path;
}

0 comments on commit 651e3e9

Please sign in to comment.