Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove --id flag on web-ext sign #3126

Merged
merged 2 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 12 additions & 30 deletions src/cmd/sign.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export default function sign(
apiProxy,
apiSecret,
artifactsDir,
id,
ignoreFiles = [],
sourceDir,
timeout,
Expand Down Expand Up @@ -63,42 +62,25 @@ export default function sign(
getIdFromFile(savedIdPath),
]);

const manifestId = getManifestId(manifestData);

if (id && !manifestId) {
throw new UsageError(
`Cannot set custom ID ${id} - The add-on ID must be specified in the manifest.json file.`,
);
}
if (idFromSourceDir && !manifestId) {
const id = getManifestId(manifestData);
if (idFromSourceDir && !id) {
willdurand marked this conversation as resolved.
Show resolved Hide resolved
throw new UsageError(
'Cannot use previously auto-generated extension ID ' +
`${idFromSourceDir} - This add-on ID must be specified in the manifest.json file.`,
);
}
if (id && manifestId) {
throw new UsageError(
`Cannot set custom ID ${id} because manifest.json ` +
`already defines ID ${manifestId}`,
`${idFromSourceDir} - This extension ID must be specified in the manifest.json file.`,
);
}
if (id) {
log.info(`Using custom ID declared as --id=${id}`);
}

if (manifestId) {
id = manifestId;
}
if (!id) {
// We only auto-generate add-on IDs for MV2 add-ons on AMO.
if (manifestData.manifest_version !== 2) {
throw new UsageError(
'An extension ID must be specified in the manifest.json file.',
);
}

if (!id && idFromSourceDir) {
log.info(
`Using previously auto-generated extension ID: ${idFromSourceDir}`,
log.warn(
'No extension ID specified (it will be auto-generated the first time)',
);
id = idFromSourceDir;
}

if (!id) {
log.warn('No extension ID specified (it will be auto-generated)');
}

if (!channel) {
Expand Down
7 changes: 0 additions & 7 deletions src/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,13 +542,6 @@ Example: $0 --help run.
demandOption: false,
type: 'string',
},
id: {
describe:
'A custom ID for the extension. This has no effect if the ' +
'extension already declares an explicit ID in its manifest.',
demandOption: false,
type: 'string',
},
timeout: {
describe: 'Number of milliseconds to wait before giving up',
type: 'number',
Expand Down
80 changes: 2 additions & 78 deletions tests/unit/test-cmd/test.sign.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import completeSignCommand, {
extensionIdFile,
getIdFromFile,
} from '../../../src/cmd/sign.js';
import { basicManifest, manifestWithoutApps, fixturePath } from '../helpers.js';
import { basicManifest, fixturePath } from '../helpers.js';

describe('sign', () => {
function getStubs() {
Expand Down Expand Up @@ -115,82 +115,6 @@ describe('sign', () => {
},
));

it("doesn't allow a custom ID when no ID in manifest.json with submission api", () =>
withTempDir(async (tmpDir) => {
const customId = 'some-custom-id';
const stubs = getStubs();
const promiseSigned = sign(tmpDir, stubs, {
extraArgs: {
id: customId,
},
extraOptions: {
preValidatedManifest: manifestWithoutApps,
},
});
await assert.isRejected(promiseSigned, UsageError);
await assert.isRejected(
promiseSigned,
/Cannot set custom ID some-custom-id/,
);
await assert.isRejected(
promiseSigned,
/The add-on ID must be specified in the manifest.json file/,
);
}));

it("doesn't allow ID file when no ID in manifest.json with submission api", () =>
withTempDir(async (tmpDir) => {
const sourceDir = path.join(tmpDir.path(), 'source-dir');
const idFile = path.join(sourceDir, extensionIdFile);
const stubs = getStubs();
await fs.mkdir(sourceDir);
await saveIdToFile(idFile, 'some-other-id');
// Now, make a signing call with a custom ID.
const promiseSigned = sign(tmpDir, stubs, {
extraArgs: {
sourceDir,
},
extraOptions: {
preValidatedManifest: manifestWithoutApps,
},
});

await assert.isRejected(promiseSigned, UsageError);
await assert.isRejected(
promiseSigned,
/Cannot use previously auto-generated extension ID/,
);
await assert.isRejected(promiseSigned, /some-other-id - /);
await assert.isRejected(
promiseSigned,
/This add-on ID must be specified in the manifest.json file/,
);
}));

it('disallows a custom ID when manifest.json has ID', () =>
withTempDir(async (tmpDir) => {
const customId = 'some-custom-id';
const stubs = getStubs();
const signPromise = sign(tmpDir, stubs, {
extraArgs: {
id: customId,
},
extraOptions: {
// This manifest has an ID in it.
preValidatedManifest: basicManifest,
},
});
await assert.isRejected(signPromise, UsageError);
await assert.isRejected(
signPromise,
/Cannot set custom ID some-custom-id/,
);
await assert.isRejected(
signPromise,
/manifest\.json already defines ID basic-manifest@web-ext-test-suite/,
);
}));

it('requires a channel for submission API', () =>
withTempDir(async (tmpDir) => {
const stubs = getStubs();
Expand All @@ -199,7 +123,7 @@ describe('sign', () => {
channel: '',
},
extraOptions: {
preValidatedManifest: manifestWithoutApps,
preValidatedManifest: basicManifest,
},
});
await assert.isRejected(signPromise, UsageError);
Expand Down