Fix pulsar --package usage when APM_PATH is not set#1388
Open
savetheclocktower wants to merge 3 commits intomasterfrom
Open
Fix pulsar --package usage when APM_PATH is not set#1388savetheclocktower wants to merge 3 commits intomasterfrom
pulsar --package usage when APM_PATH is not set#1388savetheclocktower wants to merge 3 commits intomasterfrom
Conversation
5 tasks
Co-authored-by: confused_techie <dev@lhbasics.com>
Contributor
Author
|
I think these CI failures are legit. I'll investigate when I get the time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A user of
action-pulsar-dependencyreported this one. Note the output on the “Setup Pulsar Editor” step of this CI job: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 --packageas 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 runppm. 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 onPackageManager, and that method in turn calls a method defined on theatomglobal to figure out the right release channel. But theatomglobal isn't set up at this point!The fix is easy:
atom.getReleaseChanneljust aliases a function that's defined elsewhere. We can import the original function directly instead of assumingatomwill be defined.Why did it get this far?
We have main process code that envisions handling
--packagedirectly… but our wrapper scripts also look for-pand--packageand try to delegate toppmbefore 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
PATHinaction-pulsar-dependency. We want the directory that containspulsar.shandpulsar.cmdto exist on the path so that a bare invocation ofpulsarin a DOS/PowerShell session hitspulsar.cmdinstead ofpulsar.exe. So we should fix that inaction-pulsar-dependency.But this is why we still have code to handle
--packagein 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.exeon Windows: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.