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: copy LICENSE file from monorepo root on pack #6366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
24 changes: 24 additions & 0 deletions .yarn/versions/c2ad8ad7.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
releases:
"@yarnpkg/cli": minor
"@yarnpkg/plugin-pack": major

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- "@yarnpkg/builder"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@ import {xfs, npath} from '@yarnpkg/fslib';
import {fs as fsUtils, misc} from 'pkg-tests-core';
import tar from 'tar';

async function genPackList(run) {
const {stdout} = await run(`pack`, `--dry-run`, `--json`);

function parseJsonStream(stdout) {
return misc.parseJsonStream(stdout).map(entry => {
return entry.location;
}).filter(location => !!location);
}

async function genPackList(run) {
const {stdout} = await run(`pack`, `--dry-run`, `--json`);

return parseJsonStream(stdout);
}

describe(`Commands`, () => {
describe(`pack`, () => {
test(
Expand Down Expand Up @@ -890,5 +894,49 @@ describe(`Commands`, () => {
expect(originalManifest.devDependencies[dependency]).toBe(`workspace:*`);
}),
);

test(
`pack packages with workspace root LICENSE if no own LICENSE is present`,
makeTemporaryMonorepoEnv({
name: `monorepo`,
private: true,
workspaces: [
`packages/*`,
],
}, {
[`packages/foo`]: {
name: `foo`,
version: `1.0.0`,
},
[`packages/bar`]: {
name: `bar`,
version: `1.0.0`,
},
}, async ({path, run}) => {
await run(`install`);

await xfs.writeFilePromise(npath.toPortablePath(`${path}/LICENSE`), `# title\n`);
await xfs.writeFilePromise(npath.toPortablePath(`${path}/LICENSE.md`), `# title\n`);

const {stdout: fooStdout} = await run(`workspace`, `foo`, `pack`, `--dry-run`, `--json`);
const fooPackList = parseJsonStream(fooStdout);

expect(fooPackList).toEqual([
`package.json`,
`LICENSE`,
`LICENSE.md`,
]);

await xfs.writeFilePromise(npath.toPortablePath(`${path}/packages/bar/LICENSE.txt`), `# title\n`);

const {stdout: barStdout} = await run(`workspace`, `bar`, `pack`, `--dry-run`, `--json`);
const barPackList = parseJsonStream(barStdout);

expect(barPackList).toEqual([
`LICENSE.txt`,
`package.json`,
]);
}),
);
});
});
2 changes: 1 addition & 1 deletion packages/plugin-npm-cli/sources/commands/npm/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export default class NpmPublishCommand extends BaseCommand {
await packUtils.prepareForPack(workspace, {report}, async () => {
const files = await packUtils.genPackList(workspace);

for (const file of files)
for (const {file} of files)
report.reportInfo(null, file);

const pack = await packUtils.genPackStream(workspace, files);
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-pack/sources/commands/pack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export default class PackCommand extends BaseCommand {

const files = await packUtils.genPackList(workspace);

for (const file of files) {
for (const {file} of files) {
report.reportInfo(null, npath.fromPortablePath(file));
report.reportJson({location: npath.fromPortablePath(file)});
}
Expand Down
37 changes: 31 additions & 6 deletions packages/plugin-pack/sources/packUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {createGzip} fr

import {Hooks} from './';

const LICENSE_PATTERN = /licen[cs]e(\.[a-zA-Z0-9]+)?/i;

const NEVER_IGNORE = [
`/package.json`,

Expand Down Expand Up @@ -67,9 +69,10 @@ export async function prepareForPack(workspace: Workspace, {report}: {report: Re
}
}

export async function genPackStream(workspace: Workspace, files?: Array<PortablePath>) {
if (typeof files === `undefined`)
files = await genPackList(workspace);
export async function genPackStream(workspace: Workspace, files?: Array<PortablePath | {file: PortablePath, source: PortablePath}>) {
const normalizedFiles = files?.map(file => typeof file === `string`
? {file, source: ppath.resolve(workspace.cwd, file)}
Comment on lines +73 to +74
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest to make source a string parameter (and to read the content of the license file outside of the genPackStream function); this way it gives us a way to generate files within the archive that don't necessarily exist on disk (for example if we wanted to generate the LICENSE file from the SPDX code).

Copy link
Author

@tien tien Jul 31, 2024

Choose a reason for hiding this comment

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

I believe source is already is a string because PortablePath is in fact a string. I'm not too aware of the differences between the 2 though.

Also because I'm not too familiar with how gzip and tar works. Can give me further guidance on where the inclusion of LICENSE files should happen instead? Are you suggesting that genPackStream should also accept something like { file: PortablePath, content: string | Buffer }?

: file) ?? await genPackList(workspace);

const executableFiles = new Set<PortablePath>();
for (const value of workspace.manifest.publishConfig?.executableFiles ?? new Set())
Expand All @@ -80,10 +83,9 @@ export async function genPackStream(workspace: Workspace, files?: Array<Portable
const pack = tar.pack();

process.nextTick(async () => {
for (const fileRequest of files!) {
for (const {file: fileRequest, source} of normalizedFiles!) {
const file = ppath.normalize(fileRequest);

const source = ppath.resolve(workspace.cwd, file);
const dest = ppath.join(`package`, file);

const stat = await xfs.lstatPromise(source);
Expand Down Expand Up @@ -239,11 +241,34 @@ export async function genPackList(workspace: Workspace) {
}
}

return await walk(workspace.cwd, {
const paths = await walk(workspace.cwd, {
hasExplicitFileList,
globalList,
ignoreList,
});

const files = paths.map(path => ({file: path, source: ppath.resolve(workspace.cwd, path)}));

if (workspace.cwd !== project.cwd) {
const workspaceHasLicense = paths.some(path => path.match(LICENSE_PATTERN));
Copy link
Member

Choose a reason for hiding this comment

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

Hardcoding something about LICENSE files in this function feels a little too specific. Perhaps instead the files set via the files parameter should all be case insensitive?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @arcanis the regex is case-insensitive. Also, in this case, it's a little hard to reduce specificity because what we are doing is so specific :p. Would love any suggestion for alternatives.


if (!workspaceHasLicense) {
const projectDir = await xfs.readdirPromise(project.cwd);

const licenses = await Promise.all(projectDir
.filter(path => path.match(LICENSE_PATTERN))
.map(async path => [path, await xfs.lstatPromise(ppath.join(project.cwd, path))] as const))
.then(result => result
.filter(([_, stat]) => stat.isFile())
.map(([file]) => file));

for (const file of licenses) {
files.push({file, source: ppath.join(project.cwd, file)});
}
}
}

return files;
}

async function walk(initialCwd: PortablePath, {hasExplicitFileList, globalList, ignoreList}: {hasExplicitFileList: boolean, globalList: IgnoreList, ignoreList: IgnoreList}) {
Expand Down
Loading