Add 'ppm' bins to complement existing 'apm' bins #80
Conversation
Makes it easier to integrate ppm in various contexts, such as on the user's PATH to be run from the CLI, without having to run it as 'apm'. Leaving the old 'apm' and 'apm.cmd' bins in case these are useful for troubleshooting during the transition to 'ppm'-named bins, or in case some edge case requires them. They're two small text files, I think we can afford to keep ones with either name, at least for now, if not forever. We'll see what's best.
We just added a 'ppm' bin, better treat it the same as the 'apm' bin. Apparently the file permissions for these binaries get easily lost in translation when synced between Linux and Windows and back... So, we explicitly set correct file permissions for the bins during the postinstall script on Windows. Otherwise, the published bins are somewhat broken (not easily runnable) for Linux users when published from a Windows machine to the npm package registry and then downloaded from there onto a Linux machine. (Pulsar team doesn't publish ppm to the npm package registry, but you never know when we might want to start doing that, like they used to do with apm?) (We might as well keep this "legacy code" functional, ensuring parity for all relevant files it pertains to, unless we're going to outright delete said "legacy code" entirely...)
There was a problem hiding this comment.
Super excited to see this!
Confirmed that these are completely identical with ./bin/apm and ./bin/apm.cmd respectively. As well as the change in ./script/postinstall.js has the exact same behavior applied to these others.
So even without testing behavior, if apm works (which obviously it does) then these are really almost sure fire to work as well.
Plus the want of having this change finally implemented is exciting, so wanting to fast track this.
Otherwise, like you mentioned, I see no harm in keeping the other files around still, they don't do any harm, and since Windows users have had to use apm for months, it makes sense to leave a grace period. Even if eventually I'd like to see those other files removed, I don't think it's harmful if that takes a while, or even only happens once PPM is moved into the core.
Thanks a ton for getting to this, lets get it merged!
|
Restating a key point in case that wall of text above was too long: I opted for adding new bins ( I figure this is the lower-risk way to get the I still expect us to stop symlinking to the |
|
Codacy wants me to change code style in the middle of an existing JS file by adding a semicolon... 🤦 Suggested diff: fs.chmodSync(path.join(__dirname, '..', 'bin', 'apm'), 0o755)
- fs.chmodSync(path.join(__dirname, '..', 'bin', 'ppm'), 0o755)
+ fs.chmodSync(path.join(__dirname, '..', 'bin', 'ppm'), 0o755);
fs.chmodSync(path.join(__dirname, '..', 'bin', 'npm'), 0o755)(I'm not gonna do that, sorry Codacy.) |
I think the PR title pretty much says what's going on here, but my extended thoughts below for anyone curious
Makes it easier to integrate ppm in various contexts, such as on the user's PATH to be run from the CLI, without having to run it as 'apm'.
Notably, this should make it easier to get a
ppm.cmdcommand on users' PATHs on Windows, based on the method we've been using so far, which thus far has ended up giving users anapm.cmdcommand on Windows...Aside/commentary: It also subjectively lets us do something less "jank" feeling than for
ppmto be a symlink toapmon Unix-likes (Linux + macOS)... We can doppm-->ppminstead now... But that hasn't been an issue thus far, honestly. I don't worry myself over that one, truthfully. It's just been an itch at the back of my neck to finally do this, I suppose, and the Windows integration gave me a real reason to finally do so.Leaving the old 'apm' and 'apm.cmd' bins in case these are useful for troubleshooting during the transition to 'ppm'-named bins, or in case some edge case requires them.
(They're two small text files, I think we can afford to keep ones with either name, at least for now, if not forever. We'll see what's best.)
And less importantly, but relatedly...
Also adjusted a script that sets the correct file permissions on Windows, although this is really only relevant for sharing the files in archives (tarballs definitely, not sure if it affects zip files) -- notably when uploading the (packed, tarballed) package to the npm registry.
We do not publish ppm to the npm package registry, unlike what they used to do with apm. So, this doesn't really apply to us. Still, I wanted there to be parity / no bit-rot in this "legacy code" in case we change our minds and start publishing ppm to the npm package registry again like they did with apm. (Or else we can delete said code, on way or the other. I vote for keeping it, since it's not doing any harm, IMO... Less churn = less unexpected issues. Low stakes, might as well keep it, again IMO).