Skip to content

Commit

Permalink
Merge pull request #1018 from ckeditor/i/17191-pacote
Browse files Browse the repository at this point in the history 
Other (release-tools): We do not spawn an npm process to download a package manifest from the npm registry. Instead, we send an HTTP request using the `pacote` package. Closes ckeditor/ckeditor5#17191.
  • Loading branch information
@pomek
pomek authored Oct 7, 2024
2 parents 4cc0ea6 + 4f5c7e8 commit a155390
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 238 deletions.
14 changes: 5 additions & 9 deletions packages/ckeditor5-dev-release-tools/lib/tasks/verifypackagespublishedcorrectly.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,12 @@ export default async function verifyPackagesPublishedCorrectly( options ) {
for ( const packageToVerify of packagesToVerify ) {
const packageJson = await fs.readJson( upath.join( packageToVerify, 'package.json' ) );

try {
const packageWasUploadedCorrectly = !await checkVersionAvailability( version, packageJson.name );

if ( packageWasUploadedCorrectly ) {
await fs.remove( packageToVerify );
} else {
errors.push( packageJson.name );
}
} catch {
const isPackageVersionAvailable = await checkVersionAvailability( version, packageJson.name );

if ( isPackageVersionAvailable ) {
errors.push( packageJson.name );
} else {
await fs.remove( packageToVerify );
}
}

Expand Down
28 changes: 6 additions & 22 deletions packages/ckeditor5-dev-release-tools/lib/utils/checkversionavailability.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,25 @@
* For licensing, see LICENSE.md.
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
import pacote from 'pacote';

/**
* Checks if the provided version for the package exists in the npm registry.
*
* Returns a promise that resolves to `true` if the provided version does not exist or resolves the promise to `false` otherwise.
* If the `npm show` command exits with an error, it is re-thrown.
*
* @param {string} version
* @param {string} packageName
* @returns {Promise}
*/
export default async function checkVersionAvailability( version, packageName ) {
const command = `npm show ${ shellEscape( [ packageName ] ) }@${ shellEscape( [ version ] ) } version`;

return tools.shExec( command, { verbosity: 'silent', async: true } )
.then( result => {
// Explicit check for npm < 8.13.0, which does not return anything (an empty result) and it exits with a zero status code when
// the version for the provided package does not exist in the npm registry.
if ( !result ) {
return true;
}

// Provided version exists in the npm registry.
return pacote.manifest( `${ packageName }@${ version }` )
.then( () => {
// If `pacote.manifest` resolves, a package with the given version exists.
return false;
} )
.catch( error => {
// All errors except the "E404" are re-thrown.
if ( !error.toString().includes( 'code E404' ) ) {
throw error;
}

// Npm >= 8.13.0 throws an "E404" error if a version does not exist.
// Npm < 8.13.0 should never reach this line.
.catch( () => {
// When throws, the package does not exist.
return true;
} );
}
10 changes: 4 additions & 6 deletions packages/ckeditor5-dev-release-tools/lib/utils/isversionpublishablefortag.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
* For licensing, see LICENSE.md.
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import semver from 'semver';
import shellEscape from 'shell-escape';
import pacote from 'pacote';

/**
* This util aims to verify if the given `packageName` can be published with the given `version` on the `npmTag`.
Expand All @@ -16,10 +15,9 @@ import shellEscape from 'shell-escape';
* @returns {Promise.<boolean>}
*/
export default async function isVersionPublishableForTag( packageName, version, npmTag ) {
const command = `npm view ${ shellEscape( [ packageName ] ) }@${ shellEscape( [ npmTag ] ) } version --silent`;
const npmVersion = await tools.shExec( command, { async: true, verbosity: 'silent' } )
.then( value => value.trim() )
// An `npmTag` does not exist.
const npmVersion = await pacote.manifest( `${ packageName }@${ npmTag }` )
.then( ( { version } ) => version )
// An `npmTag` does not exist, or it's a first release of a package.
.catch( () => null );

if ( npmVersion && semver.lte( version, npmVersion ) ) {
Expand Down
5 changes: 3 additions & 2 deletions packages/ckeditor5-dev-release-tools/lib/utils/versions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { tools } from '@ckeditor/ckeditor5-dev-utils';
import pacote from 'pacote';
import getChangelog from './getchangelog.js';
import getPackageJson from './getpackagejson.js';

Expand Down Expand Up @@ -37,9 +38,9 @@ export function getLastFromChangelog( cwd = process.cwd() ) {
export function getLastPreRelease( releaseIdentifier, cwd = process.cwd() ) {
const packageName = getPackageJson( cwd ).name;

return tools.shExec( `npm view ${ packageName } versions --json`, { verbosity: 'silent', async: true } )
return pacote.packument( packageName )
.then( result => {
const lastVersion = JSON.parse( result )
const lastVersion = Object.keys( result.versions )
.filter( version => version.startsWith( releaseIdentifier ) )
.sort( ( a, b ) => a.localeCompare( b, undefined, { numeric: true } ) )
.pop();
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-dev-release-tools/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"inquirer": "^11.0.0",
"lodash-es": "^4.17.21",
"minimatch": "^9.0.0",
"pacote": "^19.0.0",
"semver": "^7.6.3",
"shell-escape": "^0.2.0",
"upath": "^2.0.1"
Expand Down
2 changes: 0 additions & 2 deletions packages/ckeditor5-dev-release-tools/tests/tasks/push.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
Expand Down
2 changes: 0 additions & 2 deletions packages/ckeditor5-dev-release-tools/tests/tasks/reassignnpmtags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';

import columns from 'cli-columns';
Expand Down
2 changes: 0 additions & 2 deletions packages/ckeditor5-dev-release-tools/tests/tasks/updateversions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { glob } from 'glob';
import upath from 'upath';
Expand Down
24 changes: 1 addition & 23 deletions packages/ckeditor5-dev-release-tools/tests/tasks/verifypackagespublishedcorrectly.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

'use strict';

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { glob } from 'glob';
import fs from 'fs-extra';
Expand Down Expand Up @@ -34,7 +32,7 @@ describe( 'verifyPackagesPublishedCorrectly()', () => {
expect( vi.mocked( checkVersionAvailability ) ).not.toHaveBeenCalled();
} );

it( 'should verify packages and remove them from the release directory on "npm show" command success', async () => {
it( 'should verify packages and remove them from the release directory on if their version are already taken', async () => {
vi.mocked( glob ).mockResolvedValue( [ 'package1', 'package2' ] );
vi.mocked( fs ).readJson
.mockResolvedValueOnce( { name: '@namespace/package1' } )
Expand Down Expand Up @@ -72,24 +70,4 @@ describe( 'verifyPackagesPublishedCorrectly()', () => {

expect( vi.mocked( fs ).remove ).toHaveBeenCalledExactlyOnceWith( 'package2' );
} );

it( 'should not remove package from release directory when checking version on npm throws error', async () => {
vi.mocked( glob ).mockResolvedValue( [ 'package1', 'package2' ] );
vi.mocked( fs ).readJson
.mockResolvedValueOnce( { name: '@namespace/package1' } )
.mockResolvedValueOnce( { name: '@namespace/package2' } );
vi.mocked( checkVersionAvailability )
.mockRejectedValueOnce( )
.mockResolvedValueOnce( false );

const packagesDirectory = '/workspace/ckeditor5/release/npm';
const version = 'latest';
const onSuccess = vi.fn();

await expect( verifyPackagesPublishedCorrectly( { packagesDirectory, version, onSuccess } ) )
.rejects
.toThrow( 'Packages that were uploaded incorrectly, and need manual verification:\n@namespace/package1' );

expect( vi.mocked( fs ).remove ).toHaveBeenCalledExactlyOnceWith( 'package2' );
} );
} );
54 changes: 8 additions & 46 deletions packages/ckeditor5-dev-release-tools/tests/utils/checkversionavailability.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,61 +3,23 @@
* For licensing, see LICENSE.md.
*/

import { beforeEach, describe, expect, it, vi } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import shellEscape from 'shell-escape';
import { describe, expect, it, vi } from 'vitest';
import pacote from 'pacote';
import checkVersionAvailability from '../../lib/utils/checkversionavailability.js';

vi.mock( 'shell-escape' );
vi.mock( '@ckeditor/ckeditor5-dev-utils' );
vi.mock( 'pacote' );

describe( 'checkVersionAvailability()', () => {
beforeEach( () => {
vi.mocked( shellEscape ).mockImplementation( v => v[ 0 ] );
} );

it( 'should resolve to true if version does not exist (npm >= 8.13.0 && npm < 10.0.0)', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm ERR! code E404' ) );

const result = await checkVersionAvailability( '1.0.1', 'stub-package' );

expect( vi.mocked( tools ).shExec ).toHaveBeenCalledExactlyOnceWith( 'npm show [email protected] version', expect.any( Object ) );
expect( result ).toBe( true );
} );

it( 'should resolve to true if version does not exist (npm >= 10.0.0)', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm error code E404' ) );

const result = await checkVersionAvailability( '1.0.1', 'stub-package' );
expect( vi.mocked( tools ).shExec ).toHaveBeenCalledExactlyOnceWith( 'npm show [email protected] version', expect.any( Object ) );
expect( result ).toBe( true );
} );

it( 'should resolve to true if version does not exist (npm < 8.13.0)', async () => {
vi.mocked( tools ).shExec.mockResolvedValue( '' );
it( 'should resolve to true if version does not exist', async () => {
vi.mocked( pacote.manifest ).mockRejectedValue( 'E404' );

await expect( checkVersionAvailability( '1.0.1', 'stub-package' ) ).resolves.toBe( true );
} );

expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( '[email protected]' );
} );
it( 'should resolve to false if version exists', async () => {
vi.mocked( tools ).shExec.mockResolvedValue( '1.0.1' );
pacote.manifest.mockResolvedValue( '1.0.1' );

await expect( checkVersionAvailability( '1.0.1', 'stub-package' ) ).resolves.toBe( false );
} );

it( 'should re-throw an error if unknown error occurred', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'Unknown error.' ) );

await expect( checkVersionAvailability( '1.0.1', 'stub-package' ) )
.rejects.toThrow( 'Unknown error.' );
} );

it( 'should escape arguments passed to a shell command', async () => {
vi.mocked( tools ).shExec.mockRejectedValue( new Error( 'npm ERR! code E404' ) );

await checkVersionAvailability( '1.0.1', 'stub-package' );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledTimes( 2 );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledWith( [ 'stub-package' ] );
expect( vi.mocked( shellEscape ) ).toHaveBeenCalledWith( [ '1.0.1' ] );
} );
} );
58 changes: 20 additions & 38 deletions packages/ckeditor5-dev-release-tools/tests/utils/isversionpublishablefortag.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,67 +3,49 @@
* For licensing, see LICENSE.md.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { tools } from '@ckeditor/ckeditor5-dev-utils';
import { describe, expect, it, vi } from 'vitest';
import pacote from 'pacote';
import semver from 'semver';
import shellEscape from 'shell-escape';

import isVersionPublishableForTag from '../../lib/utils/isversionpublishablefortag.js';

vi.mock( '@ckeditor/ckeditor5-dev-utils' );
vi.mock( 'pacote' );
vi.mock( 'semver' );
vi.mock( 'shell-escape' );

describe( 'isVersionPublishableForTag()', () => {
beforeEach( () => {
vi.mocked( shellEscape ).mockImplementation( v => v[ 0 ] );
} );

it( 'should return true if given version is available', async () => {
vi.mocked( semver.lte ).mockReturnValue( false );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );
it( 'should return false if given version is not available', async () => {
vi.mocked( semver.lte ).mockReturnValue( true );
vi.mocked( pacote.manifest ).mockResolvedValue( ( {
version: '1.0.0'
} ) );

const result = await isVersionPublishableForTag( 'package-name', '1.0.1', 'latest' );
const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'latest' );

expect( result ).to.equal( true );
expect( semver.lte ).toHaveBeenCalledTimes( 1 );
expect( semver.lte ).toHaveBeenCalledWith( '1.0.1', '1.0.0' );
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@latest version --silent', expect.anything() );
expect( result ).to.equal( false );
expect( semver.lte ).toHaveBeenCalledExactlyOnceWith( '1.0.0', '1.0.0' );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@latest' );
} );

it( 'should return false if given version is not available', async () => {
it( 'should return false if given version is not higher than the latest published', async () => {
vi.mocked( semver.lte ).mockReturnValue( true );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );

vi.mocked( pacote.manifest ).mockResolvedValue( ( {
version: '1.0.1'
} ) );

const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'latest' );

expect( result ).to.equal( false );
expect( semver.lte ).toHaveBeenCalledTimes( 1 );
expect( semver.lte ).toHaveBeenCalledWith( '1.0.0', '1.0.0' );
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@latest version --silent', expect.anything() );
expect( semver.lte ).toHaveBeenCalledExactlyOnceWith( '1.0.0', '1.0.1' );
} );

it( 'should return true if given npm tag is not published yet', async () => {
vi.mocked( tools.shExec ).mockRejectedValue( 'E404' );
vi.mocked( pacote.manifest ).mockRejectedValue( 'E404' );

const result = await isVersionPublishableForTag( 'package-name', '1.0.0', 'alpha' );

expect( result ).to.equal( true );
expect( semver.lte ).not.toHaveBeenCalled();
expect( tools.shExec ).toHaveBeenCalledTimes( 1 );
expect( tools.shExec ).toHaveBeenCalledWith( 'npm view package-name@alpha version --silent', expect.anything() );
} );

it( 'should escape arguments passed to a shell command', async () => {
vi.mocked( semver.lte ).mockReturnValue( false );
vi.mocked( tools.shExec ).mockResolvedValue( '1.0.0\n' );

await isVersionPublishableForTag( 'package-name', '1.0.0', 'alpha' );

expect( shellEscape ).toHaveBeenCalledTimes( 2 );
expect( shellEscape ).toHaveBeenNthCalledWith( 1, [ 'package-name' ] );
expect( shellEscape ).toHaveBeenNthCalledWith( 2, [ 'alpha' ] );
expect( pacote.manifest ).toHaveBeenCalledExactlyOnceWith( 'package-name@alpha' );
} );
} );
Loading

0 comments on commit a155390

Please sign in to comment.