Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Publish types-registry to npm AND github package registry #661

Merged
merged 8 commits into from
Oct 2, 2019
Merged

Publish types-registry to npm AND github package registry #661

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e8d00b6
Fix non-dry actions happening during dry run
sandersn Oct 1, 2019
4606de1
github publish works (with many faked/bad changes)
sandersn Oct 1, 2019
f83cacf
Dual publish
sandersn Oct 2, 2019
524daf0
Update jsdoc for secrets
sandersn Oct 2, 2019
f9bb495
Publish to @definitelytyped instead
sandersn Oct 2, 2019
c5dacb2
Publish to DefinitelyTyped not TestTypePublishing
sandersn Oct 2, 2019
cac9932
Fix lint
sandersn Oct 2, 2019
36543e3
Compute time-to-update once and ignore errors from github publication
sandersn Oct 2, 2019
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
34 changes: 20 additions & 14 deletions src/lib/npm-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ import { createTgz } from "../util/tgz";
import { identity, joinPaths, mapToRecord, recordToMap } from "../util/util";

import { getSecret, Secret } from "./secrets";
import { npmApi, npmRegistry, npmRegistryHostName } from "./settings";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint


function packageUrl(packageName: string): string {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function was only used once so I inlined it

return resolveUrl(npmRegistry, packageName);
}
import { githubRegistry, npmApi, npmRegistry, npmRegistryHostName } from "./settings";

const cacheFile = joinPaths(__dirname, "..", "..", "cache", "npmInfo.json");

Expand Down Expand Up @@ -141,12 +137,15 @@ function splitToFixedSizeGroups(names: ReadonlyArray<string>, chunkSize: number)
}

export class NpmPublishClient {
static async create(config?: RegClient.Config): Promise<NpmPublishClient> {
const token = await getSecret(Secret.NPM_TOKEN);
return new this(new RegClient(config), { token });
static async create(config?: RegClient.Config, registryName: "github" | "npm" = "npm"): Promise<NpmPublishClient> {
if (registryName === "github") {
return new this(new RegClient(config), { token: await getSecret(Secret.GITHUB_PUBLISH_ACCESS_TOKEN) }, githubRegistry);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the secret github-publish-access-token on azure. It's using a token from my account for now, but after this merges we should give typescript-bot a publish token. Unfortunately, these tokens currently require full repo access, which I would prefer to not give to types-publisher.

} else {
return new this(new RegClient(config), { token: await getSecret(Secret.NPM_TOKEN) }, npmRegistry);
}
}

private constructor(private readonly client: RegClient, private readonly auth: RegClient.Credentials) {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readonly seems superfluous for a 20-line class.

private constructor(private client: RegClient, private auth: RegClient.Credentials, private registry: string) {}

async publish(publishedDirectory: string, packageJson: {}, dry: boolean, log: Logger): Promise<void> {
const readme = await readFile(joinPaths(publishedDirectory, "README.md"));
Expand All @@ -155,20 +154,27 @@ export class NpmPublishClient {
const body = createTgz(publishedDirectory, reject);
const metadata = { readme, ...packageJson };
if (dry) {
log("(dry) Skip publish of " + publishedDirectory);
log(`(dry) Skip publish of ${publishedDirectory} to ${this.registry}`);
}
resolve(dry ? undefined : promisifyVoid(cb => {
this.client.publish(npmRegistry, { access: "public", auth: this.auth, metadata, body }, cb);
this.client.publish(this.registry, { access: "public", auth: this.auth, metadata, body }, cb);
}));
});
}

tag(packageName: string, version: string, tag: string): Promise<void> {
return promisifyVoid(cb => { this.client.tag(packageUrl(packageName), { version, tag, auth: this.auth }, cb); });
tag(packageName: string, version: string, distTag: string, dry: boolean, log: Logger): Promise<void> {
return promisifyVoid(cb => {
if (dry) {
log(`(dry) Skip tag of ${packageName}@${distTag} as ${version}`);
cb(undefined);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a less clunky way to write this? I haven't written much promisified code, so I fell back to my base knowledge of the monads+continuations. (It also made me want to rename cb to k or return_, but I refrained.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not familiar with promisifyVoid, but you could move if (dry) { ... } outside the closure and return Promise.resolve() instead of cb(undefined)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a utility function lower in the file, but it doesn't seem that useful.

I'll use return Promise.resolve().

} else {
this.client.distTags.add(this.registry, { package: packageName, version, distTag, auth: this.auth }, cb);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched from tag to distTags.add because it allows me to separately specify the registry URL.

}
});
}

deprecate(packageName: string, version: string, message: string): Promise<void> {
const url = packageUrl(packageName.replace("/", "%2f"));
const url = resolveUrl(npmRegistry, packageName.replace("/", "%2f"));
const params = {
message,
version,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/package-publisher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export async function updateTypeScriptVersionTags(
log("(dry) Skip tag");
} else {
for (const tagName of tags) {
await client.tag(pkg.fullEscapedNpmName, version, tagName);
await client.tag(pkg.fullEscapedNpmName, version, tagName, dry, log);
}
}
}
Expand All @@ -66,6 +66,6 @@ export async function updateLatestTag(
if (dry) {
log(" (dry) Skip move \"latest\" back to newest version");
} else {
await client.tag(fullEscapedNpmName, version, "latest");
await client.tag(fullEscapedNpmName, version, "latest", dry, log);
}
}
5 changes: 5 additions & 0 deletions src/lib/secrets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export enum Secret {
* We only need one token in existence, so delete old tokens at: https://www.npmjs.com/settings/tokens
*/
NPM_TOKEN,
/**
* Token used to publish packages to Github.
* This *could* be the same as GITHUB_ACCESS_TOKEN, but I think it's better if they remain separate.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure about this. It might be OK to have one token which can

  1. publish
  2. search for issues
  3. comment on issues

I'll have to think about security a bit before deciding. I think it's fine to start with them separate.

*/
GITHUB_PUBLISH_ACCESS_TOKEN,
}

export const allSecrets: Secret[] = mapDefined(Object.keys(Secret), key => {
Expand Down
2 changes: 2 additions & 0 deletions src/lib/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { join as joinPaths } from "path";

/** URL of the NPM registry to upload to. */
export const npmRegistryHostName = "registry.npmjs.org";
export const githubRegistryHostName = "npm.pkg.github.com";
export const npmRegistry = `https://${npmRegistryHostName}/`;
export const githubRegistry = `https://${githubRegistryHostName}/`;
export const npmApi = "api.npmjs.org";
/** Note: this is 'types' and not '@types' */
export const scopeName = "types";
Expand Down
115 changes: 65 additions & 50 deletions src/publish-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import * as yargs from "yargs";

import { FS, getDefinitelyTyped } from "./get-definitely-typed";
import { Options } from "./lib/common";
import { withNpmCache, CachedNpmInfoClient, NpmPublishClient, UncachedNpmInfoClient } from "./lib/npm-client";
import { CachedNpmInfoClient, NpmPublishClient, UncachedNpmInfoClient, withNpmCache } from "./lib/npm-client";
import { AllPackages, NotNeededPackage, readNotNeededPackages, TypingsData } from "./lib/packages";
import { outputDirPath, validateOutputPath } from "./lib/settings";
import { Semver } from "./lib/versions";
import { npmInstallFlags, readJson, sleep, writeFile, writeJson } from "./util/io";
import { logger, Logger, loggerWithErrors, writeLog } from "./util/logging";
import { assertDefined, best, computeHash, execAndThrowErrors, joinPaths, logUncaughtErrors, mapDefined } from "./util/util";

const packageName = "types-registry";
const registryOutputPath = joinPaths(outputDirPath, packageName);
const typesRegistry = "types-registry";
const registryOutputPath = joinPaths(outputDirPath, typesRegistry);
const readme =
`This package contains a listing of all packages published to the @types scope on NPM.
Generated by [types-publisher](https://github.com/Microsoft/types-publisher).`;
Expand All @@ -30,46 +30,52 @@ export default async function publishRegistry(dt: FS, allPackages: AllPackages,
const [log, logResult] = logger();
log("=== Publishing types-registry ===");

const { version: oldVersion, highestSemverVersion, contentHash: oldContentHash, lastModified } =
await fetchAndProcessNpmInfo(packageName, client);
const { npmVersion, highestSemverVersion, npmContentHash, lastModified } =
await fetchAndProcessNpmInfo(typesRegistry, client);
assert.strictEqual(npmVersion.major, 0);
assert.strictEqual(npmVersion.minor, 1);

// Don't include not-needed packages in the registry.
const registryJsonData = await withNpmCache(client, cachedClient => generateRegistry(allPackages.allLatestTypings(), cachedClient));
const registry = JSON.stringify(registryJsonData);
const newContentHash = computeHash(registry);
const newVersion = `0.1.${npmVersion.patch + 1}`;

assert.strictEqual(oldVersion.major, 0);
assert.strictEqual(oldVersion.minor, 1);
const newVersion = `0.1.${oldVersion.patch + 1}`;
const packageJson = generatePackageJson(newVersion, newContentHash);
await generate(registry, packageJson);

const publishClient = () => NpmPublishClient.create({ defaultTag: "next" });
if (!highestSemverVersion.equals(oldVersion)) {
// There was an error in the last publish and types-registry wasn't validated.
// This may have just been due to a timeout, so test if types-registry@next is a subset of the one we're about to publish.
// If so, we should just update it to "latest" now.
log("Old version of types-registry was never tagged latest, so updating");
await validateIsSubset(await readNotNeededPackages(dt), log);
await (await publishClient()).tag(packageName, highestSemverVersion.versionString, "latest");
} else if (oldContentHash !== newContentHash && isAWeekAfter(lastModified)) {
log("New packages have been added, so publishing a new registry.");
await publish(await publishClient(), packageJson, newVersion, dry, log);
} else {
const reason = oldContentHash === newContentHash ? "Was modified less than a week ago" : "No new packages published";
log(`${reason}, so no need to publish new registry.`);
// Just making sure...
await validate(log);
await publishToRegistry("github");
await publishToRegistry("npm");
await writeLog("publish-registry.md", logResult());

async function publishToRegistry(registryName: "github" | "npm") {
const packageName = registryName === "github" ? "@definitelytyped/" + typesRegistry : typesRegistry;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line, plus the calls to publishToRegistry, are almost all the new code except for registryName in generateJson. Everything else is renames or reordering.

const packageJson = generatePackageJson(packageName, registryName, newVersion, newContentHash);
await generate(registry, packageJson);

const publishClient = () => NpmPublishClient.create({ defaultTag: "next" }, registryName);
if (!highestSemverVersion.equals(npmVersion)) {
// There was an error in the last publish and types-registry wasn't validated.
// This may have just been due to a timeout, so test if types-registry@next is a subset of the one we're about to publish.
// If so, we should just update it to "latest" now.
log("Old version of types-registry was never tagged latest, so updating");
await validateIsSubset(await readNotNeededPackages(dt), log);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this throw? An error here (or anywhere in this function) publishing to GitHub will stop us from attempting a publish to npm. Not sure whether that’s intentional or not. If the goal is to keep them in sync, that seems good. If the goal is to try publishing to GitHub without effect on current npm infrastructure, that seems bad.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! It does. The intention is to avoid publishing whenever anything goes wrong even if we're not quite sure why, since an old types-registry is way less bad than a new wrong one.

I don't think it's a problem here, but I'll need to inspect the rest of the code.

(I inspected a bunch of code and concluded that there WERE a few problems that got complicated to fix)

I think the safest thing here is to make sure, as long as we're publishing to two places, that github never gets ahead of npm. It's fine if github misses some updates compared to npm, but not the other way around since we always check against npm. That also means, unfortunately, that github needs to update first so that validateIsSubset checks against the current version of npm, not current+1.

I think the right fix is to surround await publishToRegistry('github') with try { ... } catch { }, so that if github fails, npm tries as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my hunch (that npm should still try to publish even if GitHub fails), but wanted to double check.

await (await publishClient()).tag(packageName, highestSemverVersion.versionString, "latest", dry, log);
} else if (npmContentHash !== newContentHash && isTenMinutesAfter(lastModified)) {
log("New packages have been added, so publishing a new registry.");
await publish(await publishClient(), packageName, packageJson, newVersion, dry, log);
} else {
const reason = npmContentHash === newContentHash ? "No new packages published" : "Was modified less than a week ago";
log(`${reason}, so no need to publish new registry.`);
// Just making sure...
await validate(log);
}
}

await writeLog("publish-registry.md", logResult());
}

const millisecondsPerDay = 1000 * 60 * 60 * 24;
function isAWeekAfter(time: Date): boolean {
const millisecondsPerMinute = 1000 * 60;
function isTenMinutesAfter(time: Date): boolean {
const diff = Date.now() - time.getTime();
const days = diff / millisecondsPerDay;
return days > 7;
const minutes = diff / millisecondsPerMinute;
return minutes > 10;
}

async function generate(registry: string, packageJson: {}): Promise<void> {
Expand All @@ -91,13 +97,18 @@ async function generate(registry: string, packageJson: {}): Promise<void> {
}
}

async function publish(client: NpmPublishClient, packageJson: {}, version: string, dry: boolean, log: Logger): Promise<void> {
async function publish(client: NpmPublishClient, packageName: string, packageJson: {}, version: string, dry: boolean, log: Logger): Promise<void> {
await client.publish(registryOutputPath, packageJson, dry, log);
// Sleep for 60 seconds to let NPM update.
await sleep(60);
if (dry) {
log("(dry) Skipping 60 second sleep...");
} else {
log("Sleeping for 60 seconds ...");
await sleep(60);
}
// Don't set it as "latest" until *after* it's been validated.
await validate(log);
await client.tag(packageName, version, "latest");
await client.tag(packageName, version, "latest", dry, log);
}

async function installForValidate(log: Logger): Promise<void> {
Expand All @@ -112,7 +123,7 @@ async function installForValidate(log: Logger): Promise<void> {
});

const npmPath = joinPaths(__dirname, "..", "node_modules", "npm", "bin", "npm-cli.js");
const cmd = `node ${npmPath} install types-registry@next ${npmInstallFlags}`
const cmd = `node ${npmPath} install types-registry@next ${npmInstallFlags}`;
log(cmd);
const err = (await execAndThrowErrors(cmd, validateOutputPath)).trim();
if (err) {
Expand All @@ -125,9 +136,9 @@ const validateTypesRegistryPath = joinPaths(validateOutputPath, "node_modules",
async function validate(log: Logger): Promise<void> {
await installForValidate(log);
const output = joinPaths(registryOutputPath, "index.json");
const node_modules = joinPaths(validateTypesRegistryPath, "index.json");
log(`Checking that ${output} is newer than ${node_modules}`);
assertJsonNewer(await readJson(output), await readJson(node_modules), log);
const nodeModules = joinPaths(validateTypesRegistryPath, "index.json");
log(`Checking that ${output} is newer than ${nodeModules}`);
assertJsonNewer(await readJson(output), await readJson(nodeModules), log);
}

async function validateIsSubset(notNeeded: ReadonlyArray<NotNeededPackage>, log: Logger): Promise<void> {
Expand Down Expand Up @@ -169,14 +180,16 @@ function assertJsonNewer(newer: { [s: string]: any }, older: { [s: string]: any
}
}

function generatePackageJson(version: string, typesPublisherContentHash: string): object {
return {
name: packageName,
function generatePackageJson(name: string, registryName: "github" | "npm", version: string, typesPublisherContentHash: string): object {
const json = {
name,
version,
description: "A registry of TypeScript declaration file packages published within the @types scope.",
repository: {
type: "git",
url: "https://github.com/Microsoft/types-publisher.git",
url: registryName === "github"
? "https://github.com/DefinitelyTyped/DefinitelyTyped.git"
: "https://github.com/Microsoft/types-publisher.git",
},
keywords: [
"TypeScript",
Expand All @@ -189,6 +202,10 @@ function generatePackageJson(version: string, typesPublisherContentHash: string)
license: "MIT",
typesPublisherContentHash,
};
if (registryName === "github") {
(json as any).publishConfig = { registry: "https://npm.pkg.github.com/" };
}
return json;
}

interface Registry {
Expand Down Expand Up @@ -225,23 +242,21 @@ async function generateRegistry(typings: ReadonlyArray<TypingsData>, client: Cac
}

interface ProcessedNpmInfo {
readonly version: Semver;
readonly npmVersion: Semver;
readonly highestSemverVersion: Semver;
readonly contentHash: string;
readonly npmContentHash: string;
readonly lastModified: Date;
}

async function fetchAndProcessNpmInfo(escapedPackageName: string, client: UncachedNpmInfoClient): Promise<ProcessedNpmInfo> {
const info = assertDefined(await client.fetchNpmInfo(escapedPackageName));
const version = Semver.parse(assertDefined(info.distTags.get("latest")));
const npmVersion = Semver.parse(assertDefined(info.distTags.get("latest")));
const { distTags, versions, time } = info;
const highestSemverVersion = getLatestVersion(versions.keys());
assert.strictEqual(highestSemverVersion.versionString, distTags.get("next"));
const contentHash = versions.get(version.versionString)!.typesPublisherContentHash || "";
return { version, highestSemverVersion, contentHash, lastModified: new Date(time.get("modified")!) };
const npmContentHash = versions.get(npmVersion.versionString)!.typesPublisherContentHash || "";
return { npmVersion, highestSemverVersion, npmContentHash, lastModified: new Date(time.get("modified")!) };
}

function getLatestVersion(versions: Iterable<string>): Semver {
return best(mapDefined(versions, v => Semver.tryParse(v)), (a, b) => a.greaterThan(b))!;
}

9 changes: 6 additions & 3 deletions src/types/npm-registry-client.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ declare class RegClient {
constructor(config?: RegClient.Config);
request(uri: string, params: RegClient.RequestParams, cb: (error: Error, data: unknown, json: unknown, response: unknown) => void): void;
publish(uri: string, params: RegClient.PublishParams, cb: (error: Error) => void): void;
tag(uri: string, params: RegClient.TagParams, cb: (error: Error) => void): void;
deprecate(uri: string, params: RegClient.DeprecateParams, cb: (error: Error, data: unknown, raw: string, response: unknown) => void): void;
distTags: {
add(uri: string, params: RegClient.AddTagParams, cb: (error: Error) => void): void;
}
}

declare namespace RegClient {
Expand All @@ -21,9 +23,10 @@ declare namespace RegClient {
body: NodeJS.ReadableStream;
auth: Credentials;
}
interface TagParams {
interface AddTagParams {
package: string;
version: string;
tag: string;
distTag: string;
auth: Credentials;
}
interface DeprecateParams {
Expand Down