Page MenuHomePhabricator

[native] Make native build on Windows
ClosedPublic

Authored by michal on Jan 2 2023, 10:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:44 AM
Unknown Object (File)
Sun, Apr 21, 12:40 AM
Unknown Object (File)
Apr 3 2024, 2:33 AM
Subscribers

Details

Reviewers
atul
varun
jon
tomek
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMM1d332ea54def: [native] Make native build on Windows
Summary

Part of ENG-2457.
We want to be able to build the desktop app on Windows but currently, the postinstall script of native workspace doesn't work on Windows:

  • it uses the bash syntax (and on windows postinstall runs under cmd.exe)
  • there are some problems with paths when it comes to patch-package because of some symlink weirdness in node_modules

This diff moves the postinstall logic into a separate script file and runs it with bash explicitly. If we are on windows we skip everything (we are only going to be building the desktop app).
This will work on windows provided there's a bash executable in PATH, thankfully that's true for windows github runners (bash is included in the git for windows).

Additionally, we skip pod install if we aren't on macOS instead of using || true which hides errors.

Test Plan
  • yarn cleaninstall on macOS laptop works
  • CI on phab works
  • yarn cleaninstall on Windows laptop and on GitHub Windows Runner (in a forked repo) works

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 2 2023, 10:58 AM
Harbormaster failed remote builds in B15029: Diff 20507!

Update keyserver dockerfile

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 3 2023, 2:01 AM
Harbormaster failed remote builds in B15030: Diff 20508!
michal requested review of this revision.Jan 3 2023, 2:27 AM
jon requested changes to this revision.Jan 3 2023, 7:56 AM
jon added inline comments.
native/postinstall.sh
2 ↗(On Diff #20509)

I generally like using these defaults.

u - Error if variable is undefined
o pipefail - Error if part of a pipe errors (e.g. some command which fails | cat)

18–21 ↗(On Diff #20509)

If we add set -E (which exits on subshell errors), we can do.

This also avoids changing the directory in case a command fails

This revision now requires changes to proceed.Jan 3 2023, 7:56 AM
.dockerignore
18 ↗(On Diff #20509)

Good work figuring this out... the error message is pretty confusing

native/postinstall.sh
1 ↗(On Diff #20509)

Add a newline here

Fix issues with postinstall.sh

This revision is now accepted and ready to land.Jan 4 2023, 9:14 AM
michal added a reviewer: Restricted Owners Package.Jan 5 2023, 2:55 AM
This revision now requires review to proceed.Jan 5 2023, 2:55 AM
This revision is now accepted and ready to land.Jan 5 2023, 7:01 AM