pulsar-edit / ppm

Pulsar Package Manager
MIT License
35 stars 13 forks source link

Fix installing with yarn on Windows #58

Closed DeeDeeG closed 1 year ago

DeeDeeG commented 1 year ago

In short, this fixes yarn install for this repo on Windows

(Without breaking npm install for this repo, either.)

This PR fixes the scenario where you want to install this repo's dependencies with Yarn on Windows. (Should allow yarn build:apm to actually work on Windows over at the Pulsar core repo!) It does so by working around some differences with how Yarn interacts with Windows' cmd.exe command line interpreter and batch scripts, compared to how npm interacts with cmd.exe and batch scripts.

For technical explanation of each change, see the commit messages. Each change is no more than one line (and no more than 6 characters, at that!), other than whitespace. So do consider looking at the diff first, then referring to the commit messages if/only if you have further questions. 100% of the changes are found in two .cmd batch script files.

That's the technical info, the rest of this PR body is commentary/not much more than a rant, but it gives context. Other than the footnote about npm major versions, which contains technical information I learned more about while preparing this PR, but which is a tangent and arguably off-topic. Feel free to skip reading the rest of this PR body if you like, since it shouldn't be technically relevant to reviewing the changes in this PR.

Background, effort, and why's cmd.exe gotta be this way?

This was surprisingly involved, as I wanted to know what I was doing with these fixes, but IMO cmd.exe's documentation is loosely worded, sparsely provided, scattered to the many corners of the internet, describes a generally janky/weird command execution environment / scripting language, and feels like a bunch of loosely cobbled together conventions accreted into a de-facto "don't touch it, it'll break someone's scripts" ball of stuff more than it feels like a (well or reasonably or even "at all") designed scripting and command execution environment.

CMD.exe's behavior feels more like happenstance, magic and luck (or misfortune, depending on your perspective) than a well-thought out scripting environment. IMO.

I'm not a fan.

Idea: Maybe we move everything to JS scripts where possible??? But more than a little bit out of scope for this PR.

What did I do about it in this PR?

Anyway, this has not only a few tweaks to the .cmd scripts, but a detailed explanation of the rationale of why I made these tweaks. (See the commit message for each effectively one-line change for the detailed explanation).

I don't like changing small, obscure things because "eh, works for me on my machine", I want to do it because it's the right solution. Otherwise, the fix always feels like a temporary band-aid, and I spend all my time waiting for it to spring a leak or turn out to be woefully problematic on somebody's machine or with somebody's use-case. I want it done once, and right. Doing it the "works on my machine, don't think too hard about why" way always feels like how we got here, not like a way out from here.

So.

Here it is. This makes the postinstall scripts behave under yarn install and npm install* alike.

* Tested and working with npm 6.x only.

Compatibility note for npm major versions (kind of a tangent/off-topic, click to expand if you want): If/when we migrate to newer npm, we should do so once and never look back, since the major versions do not forward-and-backward interop well with each-other, though one major npm version range is mostly compatible with itself. Lockfile handling is different enough between the different major npm versions to be a major headache for this repo. Particularly, the `jasmine-node` github URL gets saved in package-lock.json in one way with npm 8 or 9, that eventually breaks npm 6. npm 6 tries to reformat this github URL to suit its own way of doing things, but after it modifies and saves package-lock.json, npm 6 cannot handle what it wrote out, and it will error if you try to `npm install` with no `node_modules` folder populated (a fresh install). Either we need to do some R&D on how to get that to stop happening, or interop between npm 6 and npm 8 or 9 will be a real nuisance, enough of a hassle to want to strongly avoid IMO. So we should stay on npm 6 for now, IMO, until we can make the jump to npm 8 or 9, and then not go back to npm 6. npm 6.x is kind of sort of going end-of-life at the end of April 2023, when NodeJS 14 stops being supported, (https://github.com/nodejs/Release/#release-schedule), so we should look into this soon, IMO. (npm 6 is already sort of not supported, but it gets attention due to shipping with NodeJS LTS 14, support for which ends May 2023.) But upgrading npm is way out of scope for this PR, again.
DeeDeeG commented 1 year ago

@confused-Techie I think you were wondering why yarn install in this repo or yarn build:apm in Pulsar core repo didn't work on Windows.

This PR fixes it, and my notes (commit messages) hopefully shed some light on what was going on if you want to know those sorts of answers as well.

So I figured I would tag you and let you know about this PR addressing the issue.

Best Regards, - DeeDeeG