feat: Add --filename option to web-ext build command (#1900)

- Supports template filename (e.g. --filename "{applications.gecko.id}-v{version}.zip")
- Fixes #1335

src/cmd/build.js

@@ -24,6 +24,7 @@ import type {ExtensionManifest} from '../util/manifest';
 import type {FileFilterCreatorFn} from '../util/file-filter';
 
 const log = createLogger(__filename);
+const DEFAULT_FILENAME_TEMPLATE = '{name}-{version}.zip';
 
 
 export function safeFileName(name: string): string {
@@ -43,7 +44,8 @@ export type PackageCreatorParams = {|
   fileFilter: FileFilter,
   artifactsDir: string,
   overwriteDest: boolean,
-  showReadyMessage: boolean
+  showReadyMessage: boolean,
+  filename?: string,
 |};
 
 export type LocalizedNameParams = {|
@@ -104,6 +106,55 @@ export async function getDefaultLocalizedName(
   return Promise.resolve(extensionName);
 }
 
+// https://stackoverflow.com/a/22129960
+export function getStringPropertyValue(
+  prop: string,
+  obj: Object,
+): string {
+  const properties = prop.split('.');
+  const value = properties.reduce((prev, curr) => prev && prev[curr], obj);
+  if (!['string', 'number'].includes(typeof value)) {
+    throw new UsageError(
+      `Manifest key "${prop}" is missing or has an invalid type: ${value}`
+    );
+  }
+  const stringValue = `${value}`;
+  if (!stringValue.length) {
+    throw new UsageError(`Manifest key "${prop}" value is an empty string`);
+  }
+  return stringValue;
+}
+
+function getPackageNameFromTemplate(
+  filenameTemplate: string,
+  manifestData: ExtensionManifest
+): string {
+  const packageName = filenameTemplate.replace(
+    /{([A-Za-z0-9._]+?)}/g,
+    (match, manifestProperty) => {
+      return getStringPropertyValue(manifestProperty, manifestData);
+    }
+  );
+
+  // Validate the resulting packageName string, after interpolating the manifest property
+  // specified in the template string.
+  const parsed = path.parse(packageName);
+  if (parsed.dir) {
+    throw new UsageError(
+      `Invalid filename template "${filenameTemplate}". ` +
+      `Filename "${packageName}" should not contain a path`
+    );
+  }
+  if (!['.zip', '.xpi'].includes(parsed.ext)) {
+    throw new UsageError(
+      `Invalid filename template "${filenameTemplate}". ` +
+      `Filename "${packageName}" should have a zip or xpi extension`
+    );
+  }
+
+  return packageName;
+}
+
 export type PackageCreatorFn =
     (params: PackageCreatorParams) => Promise<ExtensionBuildResult>;
 
@@ -115,6 +166,7 @@ export async function defaultPackageCreator(
     artifactsDir,
     overwriteDest,
     showReadyMessage,
+    filename = DEFAULT_FILENAME_TEMPLATE,
   }: PackageCreatorParams,
   {
     eventToPromise = defaultEventToPromise,
@@ -132,7 +184,7 @@ export async function defaultPackageCreator(
     filter: (...args) => fileFilter.wantFile(...args),
   });
 
-  let extensionName: string = manifestData.name;
+  let filenameTemplate = filename;
 
   let {default_locale} = manifestData;
   if (default_locale) {
@@ -142,12 +194,16 @@ export async function defaultPackageCreator(
       default_locale, 'messages.json'
     );
     log.debug('Manifest declared default_locale, localizing extension name');
-    extensionName = await getDefaultLocalizedName({
+    const extensionName = await getDefaultLocalizedName({
       messageFile, manifestData,
     });
+    // allow for a localized `{name}`, without mutating `manifestData`
+    filenameTemplate = filenameTemplate.replace(/{name}/g, extensionName);
   }
+
   const packageName = safeFileName(
-    `${extensionName}-${manifestData.version}.zip`);
+    getPackageNameFromTemplate(filenameTemplate, manifestData)
+  );
   const extensionPath = path.join(artifactsDir, packageName);
 
   // Added 'wx' flags to avoid overwriting of existing package.
@@ -187,6 +243,7 @@ export type BuildCmdParams = {|
   asNeeded?: boolean,
   overwriteDest?: boolean,
   ignoreFiles?: Array<string>,
+  filename?: string,
 |};
 
 export type BuildCmdOptions = {|
@@ -206,6 +263,7 @@ export default async function build(
     asNeeded = false,
     overwriteDest = false,
     ignoreFiles = [],
+    filename = DEFAULT_FILENAME_TEMPLATE,
   }: BuildCmdParams,
   {
     manifestData,
@@ -231,6 +289,7 @@ export default async function build(
     artifactsDir,
     overwriteDest,
     showReadyMessage,
+    filename,
   });
 
   await prepareArtifactsDir(artifactsDir);

src/program.js

@@ -342,6 +342,15 @@ type MainParams = {
   runOptions?: Object,
 }
 
+export function throwUsageErrorIfArray(errorMessage: string): any {
+  return (value: any): any => {
+    if (Array.isArray(value)) {
+      throw new UsageError(errorMessage);
+    }
+    return value;
+  };
+}
+
 export function main(
   absolutePackageDir: string,
   {
@@ -429,6 +438,18 @@ Example: $0 --help run.
       default: true,
       type: 'boolean',
     },
+    'filename': {
+      alias: 'n',
+      describe: 'Name of the created extension package file.',
+      default: undefined,
+      normalize: false,
+      demandOption: false,
+      requiresArg: true,
+      type: 'string',
+      coerce: throwUsageErrorIfArray(
+        'Multiple --filename/-n option are not allowed'
+      ),
+    },
   });
 
   program

tests/functional/test.cli.build.js

@@ -1,5 +1,6 @@
 /* @flow */
 import {describe, it} from 'mocha';
+import {assert} from 'chai';
 
 import {
   minimalAddonPath, withTempAddonDir, execWebExt, reportCommandErrors,
@@ -23,4 +24,15 @@ describe('web-ext build', () => {
        });
      })
   );
+
+  it('throws an error on multiple -n',
+     () => withTempAddonDir({addonPath: minimalAddonPath}, (srcDir, tmpDir) => {
+       const argv = ['build', '-n', 'foo', '-n', 'bar'];
+       const cmd = execWebExt(argv, {cwd: tmpDir});
+       return cmd.waitForExit.then(({exitCode, stderr}) => {
+         assert.notEqual(exitCode, 0);
+         assert.match(stderr, /Multiple --filename\/-n option are not allowed/);
+       });
+     })
+  );
 });

tests/unit/test-cmd/test.build.js

@@ -10,6 +10,7 @@ import defaultEventToPromise from 'event-to-promise';
 import build, {
   safeFileName,
   getDefaultLocalizedName,
+  getStringPropertyValue,
   defaultPackageCreator,
 } from '../../../src/cmd/build';
 import {FileFilter} from '../../../src/util/file-filter';
@@ -72,6 +73,40 @@ describe('build', () => {
     });
   });
 
+  it('throws on missing manifest properties in filename template', () => {
+    return withTempDir(
+      (tmpDir) =>
+        build({
+          sourceDir: fixturePath('minimal-web-ext'),
+          artifactsDir: tmpDir.path(),
+          filename: '{missingKey}-{version}.xpi',
+        })
+          .then(makeSureItFails())
+          .catch((error) => {
+            log.info(error);
+            assert.instanceOf(error, UsageError);
+            assert.match(
+              error.message,
+              /Manifest key "missingKey" is missing/);
+          })
+    );
+  });
+
+  it('gives the correct custom name to an extension', () => {
+    return withTempDir(
+      (tmpDir) =>
+        build({
+          sourceDir: fixturePath('minimal-web-ext'),
+          artifactsDir: tmpDir.path(),
+          filename: '{applications.gecko.id}-{version}.xpi',
+        })
+          .then((buildResult) => {
+            assert.match(path.basename(buildResult.extensionPath),
+                         /minimal-example_web-ext-test-suite-1.0\.xpi$/);
+          })
+    );
+  });
+
   it('gives the correct name to a localized extension', () => {
     return withTempDir(
       (tmpDir) =>
@@ -86,6 +121,44 @@ describe('build', () => {
     );
   });
 
+  it('throws an error if the filename contains a path', () => {
+    return withTempDir(
+      (tmpDir) =>
+        build({
+          sourceDir: fixturePath('minimal-web-ext'),
+          artifactsDir: tmpDir.path(),
+          filename: 'foo/{version}.xpi',
+        })
+          .then(makeSureItFails())
+          .catch((error) => {
+            log.info(error);
+            assert.instanceOf(error, UsageError);
+            assert.match(
+              error.message,
+              /Filename "foo\/1.0.xpi" should not contain a path/);
+          })
+    );
+  });
+
+  it('throws an error if the filename doesn\'t end in .xpi or .zip', () => {
+    return withTempDir(
+      (tmpDir) =>
+        build({
+          sourceDir: fixturePath('minimal-web-ext'),
+          artifactsDir: tmpDir.path(),
+          filename: '{version}.unknown-ext',
+        })
+          .then(makeSureItFails())
+          .catch((error) => {
+            log.info(error);
+            assert.instanceOf(error, UsageError);
+            assert.match(
+              error.message,
+              /Filename "1.0.unknown-ext" should have a zip or xpi extension/);
+          })
+    );
+  });
+
   it('accept a dash in the default_locale field', () => {
     return withTempDir(
       (tmpDir) =>
@@ -509,4 +582,17 @@ describe('build', () => {
 
   });
 
+  describe('getStringPropertyValue', () => {
+
+    it('accepts the value 0', () => {
+      assert.equal(getStringPropertyValue('foo', {foo: 0}), '0');
+    });
+
+    it('throws an error if string value is empty', () => {
+      assert.throws(() => getStringPropertyValue('foo', {foo: ''}),
+                    UsageError, /Manifest key "foo" value is an empty string/);
+    });
+
+  });
+
 });

tests/unit/test.program.js

@@ -8,7 +8,12 @@ import sinon, {spy} from 'sinon';
 import {assert} from 'chai';
 
 import {applyConfigToArgv} from '../../src/config';
-import {defaultVersionGetter, main, Program} from '../../src/program';
+import {
+  defaultVersionGetter,
+  main,
+  Program,
+  throwUsageErrorIfArray,
+} from '../../src/program';
 import commands from '../../src/cmd';
 import {
   onlyInstancesOf,
@@ -858,3 +863,12 @@ describe('program.defaultVersionGetter', () => {
                  commit);
   });
 });
+
+describe('program.throwUsageErrorIfArray', () => {
+  const errorMessage = 'This is the expected error message';
+  const innerFn = throwUsageErrorIfArray(errorMessage);
+
+  it('throws UsageError on array', () => {
+    assert.throws(() => innerFn(['foo', 'bar']), UsageError, errorMessage);
+  });
+});