Skip to content

fix(ext/node): process.argv[0] is equivalent to execPath#28915

Open
littledivy wants to merge 16 commits intodenoland:mainfrom
littledivy:process_argv_0
Open

fix(ext/node): process.argv[0] is equivalent to execPath#28915
littledivy wants to merge 16 commits intodenoland:mainfrom
littledivy:process_argv_0

Conversation

@littledivy
Copy link
Member

@littledivy littledivy commented Apr 15, 2025

Fixes #28889

marvinhagemeister
marvinhagemeister approved these changes Apr 15, 2025
View reviewed changes
Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

LGTM

kt3k
kt3k previously approved these changes May 21, 2025
View reviewed changes
@Tango992
Copy link
Contributor

Tango992 commented Oct 17, 2025

I think we could redo the CI and merge this one

@kt3k kt3k dismissed their stale review October 20, 2025 01:58

Looks like 2 cases consistently failing?

specs::npm::deno_run_cowthink
specs::task::byonm::deno_json
@kt3k
Copy link
Contributor

kt3k commented Oct 20, 2025 •
edited
Loading

@Tango992 Can you take a look at what's happening on the 2 cases above?

@bartlomieju
Copy link
Member

bartlomieju commented Feb 21, 2026

We can't make specs::npm::deno_run_cowthink work correctly, the code in cowsay that decides which entrypoint to use is:

function say() {
  const module = require('./index');
  const think = /think$/.test(argv['$0']) || argv.think;

  console.log(think ? module.think(argv) : module.say(argv));
}

argv['$0'] comes from yargs here:

    // ignore the node bin, specify this in your
    // bin file with #!/usr/bin/env node
    let default$0;
    if (/\b(node|iojs|electron)(\.exe)?$/.test(process.argv[0])) {
        default$0 = process.argv.slice(1, 2);
    }
    else {
        default$0 = process.argv.slice(0, 1);
    }

And it literally hardcodes regex to check if it's node/iojs/electron.

bartlomieju and others added 4 commits February 21, 2026 14:36
@bartlomieju
explainer
d31305c
@bartlomieju
fix test
e9dfe16
@bartlomieju
fix?
ec7840f
@bartlomieju @claude
fix: use separate expected output for compiled cowthink test
534559e 
The compiled binary is named "cowthink" so argv[0] contains "cowthink"
and yargs correctly produces cowthink output, unlike `deno run` where
argv[0] is the deno executable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bartlomieju bartlomieju mentioned this pull request Mar 2, 2026
Merged
3 tasks
@bartlomieju bartlomieju mentioned this pull request Mar 25, 2026
Merged
2 tasks
bartlomieju and others added 3 commits March 27, 2026 08:55
@bartlomieju
Merge branch 'main' into process_argv_0
c5a4e87
@bartlomieju
fix
90181e7
@bartlomieju @claude
fix argv[0] initialization order and remove broken cowthink tests
62be9ca 
argv[0] was set to `String(execPath)` before `execPath` was initialized,
resulting in an empty string. Use `Deno.execPath()` directly instead.

Remove cowthink/cowsay tests that can't work correctly because yargs
hardcodes a regex for node/iojs/electron in process.argv[0].
Add a direct process_argv0 spec test instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

@kt3k kt3k kt3k left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Node compat: process.argv[0] does not use execPath

6 participants

@littledivy @Tango992 @kt3k @bartlomieju @marvinhagemeister @dsherret