Skip to content

Support stripping a prefix from all file paths#231

Merged
tschneidereit merged 1 commit intobytecodealliance:mainfrom
tschneidereit:no-strip-base
Mar 20, 2025
Merged

Support stripping a prefix from all file paths#231
tschneidereit merged 1 commit intobytecodealliance:mainfrom
tschneidereit:no-strip-base

Conversation

@tschneidereit
Copy link
Member

@tschneidereit tschneidereit commented Mar 19, 2025

This introduces the CLI option --strip-path-prefix prefix and strips the provided prefix from all paths as seen by content, in stack traces, and the debugger.

Previously, we unilaterally stripped the directory part of the top-level script's path instead. This new approach is better in two ways:

  1. it works for files that are in ancestor or sibling directories to the top-level script
  2. it enables using an ancestor directory as the root

The second of these advantages can be seen in the diffs to the test expectations files: the stacks for those are more informative, because they now include the directory of the actual test, since we're only stripping the [StarlingMonkey root]/tests part of the path.

@guybedford
Copy link
Contributor

Could we define a concept of base path? Or can we run Wizer such that the project root is /? It's always unfortunate to spill local paths into production systems IMO.

This introduces the CLI option `--strip-path-prefix prefix` and strips the provided `prefix` from all paths as seen by content, in stack traces, and the debugger.

Previously, we unilaterally stripped the directory part of the top-level script's path instead. This new approach is better in two ways:
1. it works for files that are in ancestor or sibling directories to the top-level script
2. it enables using an ancestor directory as the root

The second of these advantages can be seen in the diffs to the test expectations files: the stacks for those are more informative, because they now include the directory of the actual test, since we're only stripping the `[StarlingMonkey root]/tests` part of the path.
@tschneidereit
Copy link
Member Author

@guybedford agreed that that is much better, so I did that in this new version of this PR. Turns out without that kind of approach it was hard to make the e2e tests pass again anyway :)

@tschneidereit tschneidereit changed the title Don't strip the top-level module's dirname from file names Support stripping a prefix from all file paths Mar 20, 2025
@tschneidereit tschneidereit merged commit 30756b2 into bytecodealliance:main Mar 20, 2025
5 checks passed
@tschneidereit tschneidereit deleted the no-strip-base branch March 20, 2025 16:45
tschneidereit added a commit to tschneidereit/ComponentizeJS that referenced this pull request Mar 21, 2025
tschneidereit added a commit to tschneidereit/ComponentizeJS that referenced this pull request Mar 21, 2025
guybedford pushed a commit to bytecodealliance/ComponentizeJS that referenced this pull request Mar 21, 2025
* Update to latest StarlingMonkey

Pulls in bytecodealliance/StarlingMonkey#231

* Strip path prefix

Since for weval we can't map the build dir to something else, we instruct StarlingMonkey to strip the path prefix instead, achieving stack traces that don't depend on source location.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants