Docker: fix Windows builds (fix #997), line endings, switch to Alpine (PR #1122)

- Docker builds for Windows are fixed (fixes #997)
- Switched over to use Alpine (as was indicated as desired in https://github.com/nativefier/nativefier/issues/375#issuecomment-304247033) - which may mean #375 is fixed as well.
- Fixed bug where Docker has the wrong line endings when copying from a Windows host
- Fixed the invalid `arm` arch to `armv7l`
- Add `npm t` to the docker build to ensure tests pass before we start trying to do builds
- Add a message to help the user when trying to build Mac apps on Windows as a non-Admin (currently an unhelpful exception)

Co-authored-by: Ronan Jouchet <ronan@jouchet.fr>

.dockerignore

@@ -41,6 +41,7 @@ build/Release
 # Dependency directory
 # https://docs.npmjs.com/misc/faq#should-i-check-my-node-modules-folder-into-git
 node_modules
+app/node_modules
 
 # IntelliJ project files
 .idea

Dockerfile

@@ -1,34 +1,46 @@
-FROM node:12-stretch
-LABEL description="Debian image to build nativefier apps"
+FROM node:12-alpine
+LABEL description="Alpine image to build Nativefier apps"
 
-# Get wine32, not 64, to work around binary incompatibility with rcedit.
-# https://github.com/nativefier/nativefier/issues/375#issuecomment-304247033
-# Forced us to use Debian rather than Alpine, which doesn't do multiarch.
-RUN dpkg --add-architecture i386
 
 # Install dependencies
-RUN apt-get update \
-    && apt-get --yes install wine32 imagemagick \
-    && apt-get clean && rm -rf /var/lib/apt/lists/*
+RUN apk update \
+    && apk add bash wine imagemagick dos2unix \
+    && rm -rf /var/cache/apk/*
+
+WORKDIR /nativefier
 
 # Add sources
-COPY . /nativefier
+COPY . .
+
+# Fix line endings that may have gotten mangled in Windows
+RUN find ./icon-scripts ./src ./app -type f -print0 | xargs -0 dos2unix
 
 # Build nativefier and link globally
 WORKDIR /nativefier/app
 RUN npm install
 WORKDIR /nativefier
-RUN npm install && npm run build && npm link
+
+# Install (note that we had to manually install in `app` before, as `prepare` won't run as root)
+# Also, running tests, to ensure we don't Docker build & publish broken stuff
+RUN npm install && npm run build && npm test && npm link
+
+# Cleanup test artifacts
+RUN rm -rf /tmp/nativefier*
 
 # Use 1000 as default user not root
 USER 1000
 
-# Run a {lin,mac,win} build: 1. to check installation was sucessful,
-# 2. to cache electron distributables and avoid downloads at runtime.
+# Run a {lin,mac,win} build
+# 1. to check installation was sucessful
+# 2. to cache electron distributables and avoid downloads at runtime
 RUN nativefier https://github.com/nativefier/nativefier /tmp/nativefier \
     && nativefier -p osx https://github.com/nativefier/nativefier /tmp/nativefier \
-    && nativefier -p windows https://github.com/nativefier/nativefier /tmp/nativefier \
-    && rm -rf /tmp/nativefier
+    && nativefier -p windows https://github.com/nativefier/nativefier /tmp/nativefier
+
+
+RUN echo Generated Electron cache size: $(du -sh ~/.cache/electron) \
+    && rm -rf /tmp/nativefier \
+    && echo Final image size: $(du -sh / 2>/dev/null)
 
 ENTRYPOINT ["nativefier"]
 CMD ["--help"]

docs/api.md

@@ -70,12 +70,12 @@
   - [[browserwindow-options]](#browserwindow-options)
   - [[darwin-dark-mode-support]](#darwin-dark-mode-support)
   - [[background-color]](#background-color)
-  - [[disable-old-build-warning-yesiknowitisinsecure]](#disable-old-build-warning-yesiknowitisinsecure)
 - [Programmatic API](#programmatic-api)
   - [Addition packaging options for Windows](#addition-packaging-options-for-windows)
     - [[version-string]](#version-string)
     - [[win32metadata]](#win32metadata)
-      - [Programmatic API](#programmatic-api)
+      - [Programmatic API](#programmatic-api-1)
+    - [[disable-old-build-warning-yesiknowitisinsecure]](#disable-old-build-warning-yesiknowitisinsecure)
 
 ## Packaging Squirrel-based installers
 
@@ -129,7 +129,13 @@ The name of the application, which will affect strings in titles and the icon.
 -p, --platform <value>
 ```
 
-Automatically determined based on the current OS. Can be overwritten by specifying either `linux`, `windows`, `osx` or `mas` for a Mac App Store specific build.
+- Default: current operating system.
+    - To test your default platform you can run
+      ```
+      node -p "process.platform"
+      ```
+      (See https://nodejs.org/api/os.html#os_os_platform)
+- Can be overwritten by specifying either `linux`, `windows`, `osx` or `mas` for a Mac App Store specific build.
 
 The alternative values `win32` (for Windows) or `darwin`, `mac` (for macOS) can also be used.
 
@@ -141,8 +147,14 @@ The alternative values `win32` (for Windows) or `darwin`, `mac` (for macOS) can
 
 The processor architecture to target when building.
 
-- Automatically set to the build-time machine architecture...
-- ... or can be overridden by specifying one of: `x64`, `arm`, `arm64`, `ia32`.
+- Default: the architecture of the installed version of node (usually the architecture of the build-time machine).
+    - To test your default architecture you can run
+      ```
+      node -p "process.arch"
+      ```
+      (See https://nodejs.org/api/os.html#os_os_arch)
+    - Please note: On M1 Macs, unless an arm64 version of brew is used to install nodejs, the version installed will be an `x64` version run through Rosetta, and will result in an `x64` app being generated. If this is not desired, either specify `-a arm64` to build for M1, or re-install node with an arm64 version of brew. See https://github.com/nativefier/nativefier/issues/1089
+- Can be overridden by specifying one of: `ia32`, `x64`, `armv7l`, `arm64`.
 
 #### [app-copyright]
 
@@ -858,12 +870,6 @@ Example:
 nativefier <your-geolocation-enabled-website> --win32metadata '{"ProductName": "Your Product Name", "InternalName", "Your Internal Name", "FileDescription": "Your File Description"}'
 ```
 
-#### [disable-old-build-warning-yesiknowitisinsecure]
-
-Disables the warning shown when opening a Nativefier app made a long time ago, using an old and probably insecure Electron. Nativefier uses the Chrome browser (through Electron), and remaining on an old version is A. performance sub-optimal and B. dangerous.
-
-However, there are legitimate use cases to disable such a warning. For example, if you are using Nativefier to ship a kiosk app exposing an internal site (over which you have control). Under those circumstances, it is reasonable to disable this warning that you definitely don't want end-users to see.
-
 ##### Programmatic API
 
 _Object_
@@ -893,4 +899,10 @@ var options = {
 };
 ```
 
+#### [disable-old-build-warning-yesiknowitisinsecure]
+
+Disables the warning shown when opening a Nativefier app made a long time ago, using an old and probably insecure Electron. Nativefier uses the Chrome browser (through Electron), and remaining on an old version is A. performance sub-optimal and B. dangerous.
+
+However, there are legitimate use cases to disable such a warning. For example, if you are using Nativefier to ship a kiosk app exposing an internal site (over which you have control). Under those circumstances, it is reasonable to disable this warning that you definitely don't want end-users to see.
+
 More description about the options for `nativefier` can be found at the above [section](#command-line).

src/build/buildNativefierApp.ts

@@ -5,11 +5,16 @@ import * as electronPackager from 'electron-packager';
 import * as hasbin from 'hasbin';
 import * as log from 'loglevel';
 
-import { isWindows, getTempDir, copyFileOrDir } from '../helpers/helpers';
+import { convertIconIfNecessary } from './buildIcon';
+import {
+  copyFileOrDir,
+  getTempDir,
+  isWindows,
+  isWindowsAdmin,
+} from '../helpers/helpers';
+import { AppOptions, NativefierOptions } from '../options/model';
 import { getOptions } from '../options/optionsMain';
 import { prepareElectronApp } from './prepareElectronApp';
-import { convertIconIfNecessary } from './buildIcon';
-import { AppOptions, NativefierOptions } from '../options/model';
 
 const OPTIONS_REQUIRING_WINDOWS_FOR_WINDOWS_BUILD = [
   'icon',
@@ -106,6 +111,22 @@ export async function buildNativefierApp(
   log.info('Processing options...');
   const options = await getOptions(rawOptions);
 
+  if (options.packager.platform === 'darwin' && isWindows()) {
+    // electron-packager has to extract the desired electron package for the target platform.
+    // For a target platform of Mac, this zip file contains symlinks. And on Windows, extracting
+    // files that are symlinks need Admin permissions. So we'll check if the user is an admin, and
+    // fail early if not.
+    // For reference
+    // https://github.com/electron/electron-packager/issues/933
+    // https://github.com/electron/electron-packager/issues/1194
+    // https://github.com/electron/electron/issues/11094
+    if (!isWindowsAdmin()) {
+      throw new Error(
+        'Building an app with a target platform of Mac on a Windows machine requires admin priveleges to perform. Please rerun this command in an admin command prompt.',
+      );
+    }
+  }
+
   log.info('\nPreparing Electron app...');
   const tmpPath = getTempDir('app', 0o755);
   await prepareElectronApp(options.packager.dir, tmpPath, options);
@@ -135,7 +156,7 @@ export async function buildNativefierApp(
       osRunHelp = `the app bundle.`;
     }
     log.info(
-      `App built to ${appPath} , move it wherever it makes sense for you and run ${osRunHelp}`,
+      `App built to ${appPath}, move to wherever it makes sense for you and run ${osRunHelp}`,
     );
   }
   return appPath;

src/cli.ts

@@ -1,13 +1,14 @@
 #!/usr/bin/env node
 import 'source-map-support/register';
 
-import * as commander from 'commander';
 import * as dns from 'dns';
+
+import * as commander from 'commander';
 import * as log from 'loglevel';
 
+import { isArgFormatInvalid, isWindows } from './helpers/helpers';
+import { supportedArchs, supportedPlatforms } from './infer/inferOs';
 import { buildNativefierApp } from './main';
-import { isArgFormatInvalid } from './helpers/helpers';
-import { isWindows } from './helpers/helpers';
 
 // package.json is `require`d to let tsc strip the `src` folder by determining
 // baseUrl=src. A static import would prevent that and cause an ugly extra "src" folder
@@ -102,8 +103,14 @@ if (require.main === module) {
       positionalOptions.out = outputDirectory;
     })
     .option('-n, --name <value>', 'app name')
-    .option('-p, --platform <value>', "'mac', 'mas', 'linux' or 'windows'")
-    .option('-a, --arch <value>', "'ia32' or 'x64' or 'arm' or 'arm64'")
+    .addOption(
+      new commander.Option('-p, --platform <value>').choices(
+        supportedPlatforms,
+      ),
+    )
+    .addOption(
+      new commander.Option('-a, --arch <value>').choices(supportedArchs),
+    )
     .option(
       '--app-version <value>',
       '(macOS, windows only) the version of the app. Maps to the `ProductVersion` metadata property on Windows, and `CFBundleShortVersionString` on macOS.',

src/helpers/helpers.ts

@@ -1,11 +1,13 @@
+import { spawnSync } from 'child_process';
 import * as os from 'os';
 import * as path from 'path';
 
 import axios from 'axios';
 import * as hasbin from 'hasbin';
-import { ncp } from 'ncp';
 import * as log from 'loglevel';
+import { ncp } from 'ncp';
 import * as tmp from 'tmp';
+
 tmp.setGracefulCleanup(); // cleanup temp dirs even when an uncaught exception occurs
 
 const now = new Date();
@@ -24,6 +26,16 @@ export function isWindows(): boolean {
   return os.platform() === 'win32';
 }
 
+export function isWindowsAdmin(): boolean {
+  if (process.platform !== 'win32') {
+    return false;
+  }
+
+  // https://stackoverflow.com/questions/4051883/batch-script-how-to-check-for-admin-rights
+  // https://stackoverflow.com/questions/57009374/check-admin-or-non-admin-users-in-nodejs-or-javascript
+  return spawnSync('fltmc').status === 0;
+}
+
 /**
  * Create a temp directory with a debug-friendly name, and return its path.
  * Will be automatically deleted on exit.

src/infer/inferOs.ts

@@ -1,6 +1,22 @@
 import * as os from 'os';
+
 import * as log from 'loglevel';
 
+// Ideally we'd get this list directly from electron-packager, but it's not
+// accessible in the package without importing its private js files, which felt
+// dirty. So if those change, we'll update these as well.
+// https://electron.github.io/electron-packager/master/interfaces/electronpackager.options.html#platform
+// https://electron.github.io/electron-packager/master/interfaces/electronpackager.options.html#arch
+export const supportedArchs = ['ia32', 'x64', 'armv7l', 'arm64'];
+export const supportedPlatforms = [
+  'darwin',
+  'linux',
+  'mac',
+  'mas',
+  'osx',
+  'windows',
+];
+
 export function inferPlatform(): string {
   const platform = os.platform();
   if (
@@ -19,7 +35,7 @@ export function inferPlatform(): string {
 
 export function inferArch(): string {
   const arch = os.arch();
-  if (arch !== 'ia32' && arch !== 'x64' && arch !== 'arm' && arch !== 'arm64') {
+  if (!supportedArchs.includes(arch)) {
     throw new Error(`Incompatible architecture ${arch} detected`);
   }
   log.debug('Inferred arch', arch);