`pulsar -p` behaves differently from `ppm`

[savetheclocktower]

Thanks in advance for your bug report!

• [X] Have you reproduced issue in safe mode?
• [X] Have you used the debugging guide to try to resolve the issue?
• [X] Have you checked our FAQs to make sure your question isn't answered there?
• [X] Have you checked to make sure your issue does not already exist?
• [X] Have you checked you are on the latest release of Pulsar?

What happened?

I tend not to use pulsar -p much because of muscle memory. But while investigating and documenting Windows build toolchain stuff for Pulsar, I realized that pulsar -p doesn't behave like ppm in a couple of crucial ways:

pulsar -p --version

Running pulsar -p --version should produce the same output as ppm --version — but instead it produces the same output as pulsar --version. This appears to happen on all platforms.

Windows issues

This might even warrant a separate issue, but I ran into several problems that seem to be Windows-specific:

• Invocations of pulsar (in PowerShell and cmd.exe) did not wait for output before showing me a new prompt. The output typically arrived later, after the prompt. This applied to pulsar --version and pulsar --help.
• Bare invocations of pulsar (with no second argument) or pulsar . (or any other path argument) worked as expected, but put a bunch of logging statements into the terminal. As above, the new prompt did not wait, so this output was dumped into an active terminal session that the user might've been doing something else with. The --wait flag didn't affect this.
• I was unable to get any Windows commands invoked with pulsar -p to do anything, much less match their equivalent output with ppm. The --version issue described above was present, but otherwise: running pulsar -p X, where X is any valid command, produced only an empty newline in my STDOUT. At first I thought it was hanging, but then I realized that I could run another command immediately.

---

Just as a sanity check, I verified that pulsar -p works as I'd expect on macOS, except for the --version bug described above.

Pulsar version

1.118

Which OS does this happen on?

🪟 Windows

OS details

Windows 10 (with cmd.exe and Windows Terminal)

Which CPU architecture are you running this on?

x86_64/AMD64

What steps are needed to reproduce this?

1. Add pulsar to your path, either by the option in Settings or manually.
2. Run pulsar -p X, where X is any valid command.
3. Observe that nothing happens.

Additional Information:

No response

[savetheclocktower]

After some investigation, here's what I think:

• It looks like yargs has a shortcut method for --version that pre-empts later logic and simply returns specific output if --version is present in the arguments list, no matter what other arguments are present. This should be easy to fix.
• The issues on Windows are twofold:

  • First, I'm silly; the script does the right thing, and I don't. The script didn't work for me, so I responded by adding C:\Program Files\Pulsar\ to my PATH when I should've added C:\Program Files\Pulsar\resources\ instead. That's where pulsar.cmd resides. That script does seem to wait instead of returning to the prompt too early.

    Since case-sensitivity isn't a thing on Windows, the presence of a Pulsar.exe in the root installation directory means that this sort of issue would be hard to diagnose if a user did the same thing I did; maybe we can try to detect it and let the user know.

  • Second: even when I use pulsar.cmd, I run into an issue where the output of ppm is not shown. Despite the {stdio: 'inherit'} present on this line, nothing is shown before we return to the prompt.

    I confirmed that the command was running by changing it to {stdio: 'pipe'}, capturing the stdout, then logging it; that's one possible workaround, but we'd lose the output streaming and we'd run into character encoding issues. (Run ppm list and note the characters we use to imply a tree-like structure; somehow an unwanted encoding conversion takes place with this workaround and we'd have to fix it.)

It might be worth trying to clean up this code path a bit. The --package/-p feature happens entirely inside of the parseCommandLine function; if we moved it to start.js, we'd have a bit more control here. I think one proper fix would be to change from spawnSync to spawn, which is hard to do while this code lives in its current location.

[savetheclocktower]

An update:

• #1063 is the best fix for this issue on Windows; it bypasses Pulsar.exe altogether.
• #1054 improves the situation in all cases where the main Pulsar executable handles -p/--package. That situation isn't ideal — because of character encoding issues well explored in #1054, and because it's wasteful to launch an Electron app just to launch a different CLI — but it's a good fallback.
• On macOS and Linux, pulsar.sh should get an update so that it, too, can intercept -p/--package and route them to ppm without launching Pulsar itself.
• The .tar.gz distribution of Pulsar is not envisioned at all in pulsar.sh; if that script sees that it's running on Linux, it essentially hard-codes an install location. ~~This should be improved in its own ticket.~~ (EDIT: #1066 addresses this.)
• I'll keep investigating this, but I strongly suspect the AppImage distribution of Pulsar, when invoked from a terminal, will invoke the main executable. This is like if you tried to do (e.g.) Pulsar.exe --version on Windows; it'll mostly work, but we want that stuff to go through the script first so that we can support things like --wait better. (EDIT: #1069 addresses this, but it's based on #1066, which should therefore land first.)

So I'll probably end up working on two other tasks:

• Changing pulsar.sh so that it can infer its installation location based on the working directory of pulsar.sh itself; this would allow someone who installed the tar.gz version to symlink pulsar.sh wherever they liked and have it just work.
• Investigating how to build the AppImage version so that executing it from the terminal ends up executing the inner pulsar.sh file instead of the main executable. This is feasible, according to the docs, but electron-builder might not make it easy. Once that's done, pulsar.sh should be able to infer its location (and the locations of pulsar and ppm) based on its working directory, but I'm sure there will be wrinkles.

[savetheclocktower]

I'm digging into the AppImage stuff.

The AppImage spec envisions that you might want to set an arbitrary script as the entry point for an AppImage. It's called AppRun; you can make it a shell script, an executable, or even a symlink. Since electron-builder generates AppImages for us largely automatically, it must know how to make an AppRun script.

It took me quite a while just to be able to find where this is happening: in a dependency of electron-builder’s called app-builder. This script is where the magic appears to happen; it does a number of things, but in its function it isn't too different from what pulsar.sh already does.

The best solution would be to have a way to customize this script, or even swap in a new one entirely, but that's not an official feature. It's been requested in several issues that I've seen, but never with any sort of official response.

Still, I'm pretty sure we could do it somehow as part of the build process. It'd probably involve something ugly like extracting the AppImage, editing a file, then compressing it again… but I doubt we're the first people who've needed to do that.

The status quo of AppImage is presumably one in which invoking Pulsar.AppImage from the terminal is mostly but not exactly the same as invoking pulsar when it's symlinked to pulsar.sh. But the documentation page on installing terminal commands gets so much simpler if those two things are made identical; it means we can just tell people to alias pulsar to Pulsar.AppImage and ppm to Pulsar.AppImage -p.

[savetheclocktower]

The AppImage docs have this section about extracting an AppImage, and it's just what we need. It would allow us to edit the AppRun.sh script and then use appimagetool to turn the extracted AppImage back into an AppImage.

[savetheclocktower]

OK, I got an AppImage version of Pulsar to run pulsar.sh instead of the pulsar executable. These were the steps I used:

• Downloaded the latest regular release in AppImage format to Linux-Pulsar-1.119.0.AppImage

• Made it executable with chmod +x Linux-Pulsar-1.119.0.AppImage

• Extracted it by running Linux-Pulsar-1.119.0.AppImage --appimage-extract

• The AppImage extracted itself to a folder called squashfs-root; I renamed this to Pulsar.AppDir

• cd Pulsar.AppDir && nano AppRun

• The AppRun script is basically this one; there was a line that said

  BIN="$APPDIR/pulsar"
  

• I edited the line to read

  BIN="$APPDIR/resources/pulsar.sh"
  

• I then opened resources/pulsar.sh and changed it to be identical to what’s in #1066

• To repackage the directory, I downloaded appimagetool from here

• I was then able to do ARCH=x86_x64 appimagetool-x86_64.AppImage Pulsar.AppRun (it wanted an ARCH flag for some reason), which eventually produced a Pulsar-x86_64.AppImage file.

And I was done. All the following worked as I expected them to:

• ./Pulsar-x86_64.AppImage --version (showed the Pulsar version(s))
• ./Pulsar-x86_64.AppImage --package --version (showed the PPM version(s))
• ./Pulsar-x86_64.AppImage --wait foo.txt (waited as I edited foo.txt, then exited once I saved and closed foo.txt)
• ./Pulsar-x86_64.AppImage --package install inline-autocomplete-textmate (installed a package successfully)
• ./Pulsar-x86_64.AppImage --package install --check (failed with a cryptic node-gyp error, but the same one I got when I ran this command against the original AppImage before I modified it)

I’m writing this down just so I remember what to do later. #1066 is a prerequisite, but then once it’s landed, I’ll make another PR that adds a couple steps to the build job for Linux. A self-contained version of appimagetool can apparently always be downloaded from GitHub, as illustrated by this heading in its README. So it’s just a matter of getting the right one for a given architecture, noting the original filename, extracting the AppImage, making a one-line change, then rebuilding the AppImage at the original filename.

It’d be great if electron-builder allowed us to do this, but it seems not to. I haven’t ruled out other approaches yet, but this is a low-risk strategy we can use to fix AppImage builds without affecting other Linux builds.

[savetheclocktower]

Three PRs later, this is finally closed! Thanks to those who helped get it across the finish line.