Page MenuHomePhabricator

[desktop] Have `preinstall` fail silently if not on Windows
AbandonedPublic

Authored by atul on Apr 27 2023, 12:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 29 2024, 2:04 AM
Unknown Object (File)
Mar 27 2024, 10:25 PM
Unknown Object (File)
Mar 27 2024, 10:24 PM
Unknown Object (File)
Mar 27 2024, 10:11 PM
Unknown Object (File)
Feb 23 2024, 8:50 PM
Unknown Object (File)
Feb 23 2024, 8:50 PM
Unknown Object (File)
Feb 23 2024, 8:49 PM
Unknown Object (File)
Feb 21 2024, 10:00 AM
Subscribers

Details

Summary

Saw this in yarn cleaninstall logs:

5e9d85.png (310×2 px, 271 KB)

Doesn't make sense to exit 1 if not on Windows because it's not an error. Doesn't really matter, but quick fix.

Test Plan

Run yarn cleaninstall and CMD-F for Exit code: 1 and found no instances.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Apr 27 2023, 12:29 PM
atul edited the summary of this revision. (Show Details)

I think this was intentional, but I forget why

michal requested changes to this revision.Apr 28 2023, 4:13 AM

I've just tried this code and during yarn cleaninstall I'm getting an error with node-gyp rebuild (which makes sense because it tries to use things from nuget dependencies which aren't downloaded on macOS). @atul are you also getting this error?

Either command fail is "fine" because this whole package is marked as optional so if anything fails during installation it won't be included on macOS (which is the behavior we want). Personally, I think that exiting early with exit 1 is cleaner than letting node-gyp fail with an arcane error.

The best solution would be to make a dummy node-gyp config for this package for macOS so that it can be built correctly, but I had some problems with that and didn't have much time. I would like to handle this during the bugbash.

This revision now requires changes to proceed.Apr 28 2023, 4:13 AM

Not sure it really needs to be prioritized