-
Notifications
You must be signed in to change notification settings - Fork 219
Publish old versions without touching latest tag #472
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
base: main
Are you sure you want to change the base?
Changes from all commits
e1fa9d3
ec5ba4d
9a6415b
f94fd36
e941dd1
6d27c3b
a6061ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
"@definitelytyped/publisher": patch | ||
"@definitelytyped/retag": patch | ||
"@definitelytyped/utils": patch | ||
--- | ||
|
||
Publish old versions without touching latest tag |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,46 @@ | ||
import assert = require("assert"); | ||
import { Logger, joinPaths, readFileAndWarn, NpmPublishClient } from "@definitelytyped/utils"; | ||
import { Logger, NpmPublishClient } from "@definitelytyped/utils"; | ||
import { NotNeededPackage, AnyPackage } from "@definitelytyped/definitions-parser"; | ||
import { updateTypeScriptVersionTags, updateLatestTag } from "@definitelytyped/retag"; | ||
import { updateTypeScriptVersionTags } from "@definitelytyped/retag"; | ||
import * as libpub from "libnpmpublish"; | ||
import * as pacote from "pacote"; | ||
import { ChangedTyping } from "./versions"; | ||
import { outputDirectory } from "../util/util"; | ||
|
||
export async function publishTypingsPackage( | ||
client: NpmPublishClient, | ||
changedTyping: ChangedTyping, | ||
token: string, | ||
dry: boolean, | ||
log: Logger, | ||
): Promise<void> { | ||
const { pkg, version, latestVersion } = changedTyping; | ||
await common(client, pkg, log, dry); | ||
const { pkg, version } = changedTyping; | ||
await common(pkg, token, dry); | ||
if (pkg.isLatest) { | ||
await updateTypeScriptVersionTags(pkg, version, client, log, dry); | ||
} | ||
assert((latestVersion === undefined) === pkg.isLatest); | ||
if (latestVersion !== undefined) { | ||
// If this is an older version of the package, we still update tags for the *latest*. | ||
// NPM will update "latest" even if we are publishing an older version of a package (https://github.com/npm/npm/issues/6778), | ||
// so we must undo that by re-tagging latest. | ||
await updateLatestTag(pkg.name, latestVersion, client, log, dry); | ||
} | ||
} | ||
|
||
export async function publishNotNeededPackage( | ||
client: NpmPublishClient, | ||
pkg: NotNeededPackage, | ||
token: string, | ||
dry: boolean, | ||
log: Logger, | ||
): Promise<void> { | ||
log(`Deprecating ${pkg.name}`); | ||
await common(client, pkg, log, dry); | ||
await common(pkg, token, dry); | ||
} | ||
|
||
async function common(client: NpmPublishClient, pkg: AnyPackage, log: Logger, dry: boolean): Promise<void> { | ||
async function common(pkg: AnyPackage, token: string, dry: boolean): Promise<void> { | ||
const packageDir = outputDirectory(pkg); | ||
const packageJson = await readFileAndWarn("generate", joinPaths(packageDir, "package.json")); | ||
await client.publish(packageDir, packageJson, dry, log); | ||
const manifest = await pacote.manifest(packageDir).catch((reason) => { | ||
throw reason.code === "ENOENT" ? new Error("Run generate first!", { cause: reason }) : reason; | ||
}); | ||
const tarData = await pacote.tarball(packageDir); | ||
// Make sure we never assign the latest tag to an old version. | ||
if (!dry) | ||
await libpub.publish(manifest ? { ...manifest, bundledDependencies: undefined } : manifest, tarData, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why might manifest be undefined? Should we publish if it's missing? I noticed this because I'd prefer to pull out a |
||
defaultTag: pkg.isLatest ? "latest" : "", | ||
access: "public", | ||
token, | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,12 +46,13 @@ export default async function publishPackages( | |
log("=== Publishing packages ==="); | ||
} | ||
|
||
const client = await NpmPublishClient.create(await getSecret(Secret.NPM_TYPES_TOKEN), undefined); | ||
const token = await getSecret(Secret.NPM_TYPES_TOKEN); | ||
const client = new NpmPublishClient(token); | ||
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. splitting this statement seems unnecessary |
||
|
||
for (const cp of changedPackages.changedTypings) { | ||
log(`Publishing ${cp.pkg.desc}...`); | ||
|
||
await publishTypingsPackage(client, cp, dry, log); | ||
await publishTypingsPackage(client, cp, token, dry, log); | ||
|
||
const commits = (await queryGithub( | ||
`repos/DefinitelyTyped/DefinitelyTyped/commits?path=types%2f${cp.pkg.subDirectoryPath}`, | ||
|
@@ -116,7 +117,7 @@ export default async function publishPackages( | |
|
||
for (const n of changedPackages.changedNotNeededPackages) { | ||
const target = await skipBadPublishes(n, log); | ||
await publishNotNeededPackage(client, target, dry, log); | ||
await publishNotNeededPackage(target, token, dry, log); | ||
} | ||
|
||
await writeLog("publishing.md", logResult()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { NotNeededPackage, TypingsData, createMockDT } from "@definitelytyped/definitions-parser"; | ||
import * as libpub from "libnpmpublish"; | ||
import { publishNotNeededPackage, publishTypingsPackage } from "../src/lib/package-publisher"; | ||
|
||
jest.mock("libnpmpublish"); | ||
jest.mock("pacote", () => ({ async manifest() {}, async tarball() {} })); | ||
|
||
const client = { async tag() {} }; | ||
|
||
const dt = createMockDT(); | ||
|
||
const latestVersion = { | ||
pkg: new TypingsData(dt.fs, { header: { name: "@types/some-package" } } as never, /*isLatest*/ true), | ||
version: "1.2.3", | ||
}; | ||
const oldVersion = { | ||
pkg: new TypingsData(dt.fs, { header: { name: "@types/some-package" } } as never, /*isLatest*/ false), | ||
version: "1.2.3", | ||
}; | ||
const notNeeded = new NotNeededPackage("not-needed", "not-needed", "1.2.3"); | ||
|
||
function log() {} | ||
|
||
test("Latest version gets latest tag", async () => { | ||
await publishTypingsPackage(client as never, latestVersion, /*token*/ "", /*dry*/ false, log); | ||
expect(libpub.publish).toHaveBeenLastCalledWith(/*manifest*/ undefined, /*tarData*/ undefined, { | ||
defaultTag: "latest", | ||
access: "public", | ||
token: "", | ||
}); | ||
}); | ||
|
||
test("Publishing old versions doesn't change the latest tag", async () => { | ||
await publishTypingsPackage(client as never, oldVersion, /*token*/ "", /*dry*/ false, log); | ||
expect(libpub.publish).toHaveBeenLastCalledWith(/*manifest*/ undefined, /*tarData*/ undefined, { | ||
defaultTag: "", | ||
access: "public", | ||
token: "", | ||
}); | ||
}); | ||
|
||
test("Not-needed package gets latest tag", async () => { | ||
await publishNotNeededPackage(notNeeded, /*token*/ "", /*dry*/ false, log); | ||
expect(libpub.publish).toHaveBeenLastCalledWith(/*manifest*/ undefined, /*tarData*/ undefined, { | ||
defaultTag: "latest", | ||
access: "public", | ||
token: "", | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
logs | ||
test/data/pack.tgz |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,13 +25,11 @@ | |
"@types/node": "^16.18.61", | ||
"charm": "^1.0.2", | ||
"minimatch": "^9.0.3", | ||
"tar": "^6.2.0", | ||
"tar-stream": "^3.1.6", | ||
"which": "^4.0.0" | ||
}, | ||
"devDependencies": { | ||
"@types/charm": "^1.0.6", | ||
"@types/tar": "^6.1.9", | ||
"@types/tar-stream": "^3.1.3", | ||
"@types/which": "^3.0.2", | ||
"@qiwi/npm-types": "^1.0.3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wait, wasn't this PR supposed to get rid of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one step, I have more changes I was building on this PR; probably I'll send a new one instead that includes this one. |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this change would let us stop depending on npm-registry-publish, but it just adds the new dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's half a change; this PR removed some of the registry dep but not all.