Page MenuHomePhabricator

[desktop] Display a warning when trying to build macos universal
ClosedPublic

Authored by michal on Dec 14 2022, 10:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 5:49 AM
Unknown Object (File)
Thu, Nov 7, 5:48 AM
Unknown Object (File)
Tue, Nov 5, 7:25 AM
Unknown Object (File)
Tue, Nov 5, 7:25 AM
Unknown Object (File)
Tue, Nov 5, 7:25 AM
Unknown Object (File)
Sun, Nov 3, 1:57 AM
Unknown Object (File)
Sun, Nov 3, 1:57 AM
Unknown Object (File)
Sun, Nov 3, 1:57 AM
Subscribers

Details

Summary

More context in ENG-2214. There's an unresolved bug when you are trying to build a universal macos (arm + x64 in one app) version of the app.
It only happens if you already have built the app for one of these versions. This diff adds a warning message that tells you to remove these files if they exist. It also adds a helper script for removing the build files.

Test Plan
  • Build the x64 or arm version
  • Try building the universal version

The warning should be displayed

  • Remove the files and try building the universal version. It should succeed.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Looks good, but does Windows still need rimraf (from this package) instead of rm -rf? Or do a modern Windows environment handle rm -rf well?

This revision is now accepted and ready to land.Dec 14 2022, 11:03 AM
desktop/package.json
15

Would it be a bad idea to just add yarn clean-build here?

17

Might be better to only define it once

Looks good, but does Windows still need rimraf (from this package) instead of rm -rf? Or do a modern Windows environment handle rm -rf well?

@michal should be pretty easy to set up a Windows agent on GitHub Actions to give this a shot/add to CI. There's plenty of reference material in our repo that can probably be cut/paste

Looks good, but does Windows still need rimraf (from this package) instead of rm -rf? Or do a modern Windows environment handle rm -rf well?

It doesn't handle the -rf unfortunately, but I'm not sure if it's worth it thinking about now. This fix is only required for the macOS builds and there are a lot of other things that won't work on windows (like using && in commands or ENV=dev). If we want all the scripts in desktop to work on windows we should probably use some more general solution (maybe something like this? but I'm not sure about it).

desktop/package.json
15

It's going to clean all of the build files so it might be unexpected if someone tests windows and macos versions at the same time, or maybe tests some code and renames one of the folders to have another executable for comparison.

Use yarn clean-build in yarn clean