Skip to content

Fix pulsar --package usage when APM_PATH is not set#1388

Open
savetheclocktower wants to merge 3 commits intomasterfrom
fix-pulsar-p-cli
Open

Fix pulsar --package usage when APM_PATH is not set#1388
savetheclocktower wants to merge 3 commits intomasterfrom
fix-pulsar-p-cli

Conversation

@savetheclocktower
Copy link
Contributor

A user of action-pulsar-dependency reported this one. Note the output on the “Setup Pulsar Editor” step of this CI job:

Pulsar  : 1.130.2025120304
Electron: 30.0.9
Chrome  : 124.0.6367.233
Node    : 20.11.1
atom is not defined
ReferenceError: atom is not defined
    at PackageManager.getCommandName (C:\Users\runneradmin\AppData\Local\Programs\Pulsar\resources\app.asar\src\package-manager.js:182:26)
    at PackageManager.possibleApmPaths (C:\Users\runneradmin\AppData\Local\Programs\Pulsar\resources\app.asar\src\package-manager.js:192:30)
    at parseCommandLine (C:\Users\runneradmin\AppData\Local\Programs\Pulsar\resources\app.asar\src\main-process\parse-command-line.js:157:36)
    at start (C:\Users\runneradmin\AppData\Local\Programs\Pulsar\resources\app.asar\src\main-process\start.js:40:16)
    at Object.<anonymous> (C:\Users\runneradmin\AppData\Local\Programs\Pulsar\resources\app.asar\src\main-process\main.js:66:1)
    at Module._compile (node:internal/modules/cjs/loader:1391:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1451:10)
    at Module.load (node:internal/modules/cjs/loader:1214:32)
    at Module._load (node:internal/modules/cjs/loader:1030:12)
    at c._load (node:electron/js2c/node_init:2:13672)

That's an error after running pulsar --package list. And what a thoughtful and insightful stack trace we have!

The cause (one of them, at least)

We parse pulsar --package as early as possible in order to minimize the amount of time we spend bootstrapping the editor in scenarios where the user is actually trying to run ppm. But in order to know the name of the binary we're meant to run (depends on the release channel!), we call a static method on PackageManager, and that method in turn calls a method defined on the atom global to figure out the right release channel. But the atom global isn't set up at this point!

The fix is easy: atom.getReleaseChannel just aliases a function that's defined elsewhere. We can import the original function directly instead of assuming atom will be defined.

Why did it get this far?

We have main process code that envisions handling --package directly… but our wrapper scripts also look for -p and --package and try to delegate to ppm before we run the main executable. So why are we hitting this code path in the first place?

I think the wrong directory is being added to the PATH in action-pulsar-dependency. We want the directory that contains pulsar.sh and pulsar.cmd to exist on the path so that a bare invocation of pulsar in a DOS/PowerShell session hits pulsar.cmd instead of pulsar.exe. So we should fix that in action-pulsar-dependency.

But this is why we still have code to handle --package in Pulsar itself — it's a useful fallback, and this is still a bug. Just makes it a bit hard to verify!

Verification

On that note, here's how you can verify this fix.

First, to try to reproduce the bug: you can directly invoke the Pulsar binary — explicitly referencing the full path to Pulsar.exe on Windows:

C:\Path\To\Pulsar.exe --package list

That will probably trigger this bug.

If so, then the built version generated by this PR should have a fix. Once you install it, the same command ought to work just fine.

Co-authored-by: confused_techie <dev@lhbasics.com>
@savetheclocktower
Copy link
Contributor Author

I think these CI failures are legit. I'll investigate when I get the time.

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

Comments