Skip to content

Commit

Permalink
Fix linked deps resolution (opensearch-project#4342)
Browse files Browse the repository at this point in the history
Signed-off-by: Miki <[email protected]>
  • Loading branch information
AMoo-Miki authored Jun 23, 2023
1 parent 2e10b22 commit 931b915
Show file tree
Hide file tree
Showing 13 changed files with 4,457 additions and 4,232 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Hide any output from use_node checking for Node compatibility ([#4237](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4237))
- Add category option within groups for context menus ([#4144](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4144))
- [Saved Object Service] Add Repository Factory Provider ([#4149](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4149))
- [@osd/pm] Fix `file:`-linked dependencies' resolution to improve ability to test with local packages ([#4342](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4342))
- [Multiple DataSource] Backend support for adding sample data ([#4268](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4268))

### 🐛 Bug Fixes
Expand Down
8,447 changes: 4,218 additions & 4,229 deletions packages/osd-pm/dist/index.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "opensearch-dashboards",
"version": "1.0.0",
"private": "true",
"dependencies": {
"foo": "1.0.0",
"@scoped/baz": "file:../other-plugins/baz"
},
"workspaces": {
"packages": [
"packages/*"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "bar",
"version": "1.0.0",
"dependencies": {
"foo": "1.0.0",
"@scoped/baz": "file:../../../other-plugins/baz"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "foo",
"version": "1.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"@scoped/baz@file:../other-plugins/baz":
version "1.0.0"
40 changes: 40 additions & 0 deletions packages/osd-pm/src/utils/yarn_lock.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { resolve } from 'path';
import { OpenSearchDashboards } from './opensearch_dashboards';
import { readYarnLock } from './yarn_lock';

const rootPath = resolve(`${__dirname}/__fixtures__/opensearch-dashboards-dev`);

const linkedDepmarker = 'file:';
const linkedDepmarkerLength = linkedDepmarker.length;

describe('#readYarnLock', () => {
it('includes an entry with the absolute path to a linked dependency', async () => {
const devProject = await OpenSearchDashboards.loadFrom(rootPath);
const { allDependencies } = devProject.getProject('opensearch-dashboards');
const expectedObject = Object.keys(allDependencies).reduce(
(accumulator: { [key: string]: any }, key) => {
if (allDependencies[key].startsWith('file:')) {
accumulator[
key +
'@file:' +
resolve(
devProject.getAbsolute(),
allDependencies[key].substring(linkedDepmarkerLength)
)
] = expect.any(Object);
}
return accumulator;
},
{}
);

const result = await readYarnLock(devProject);

expect(result).toMatchObject(expectedObject);
});
});
31 changes: 30 additions & 1 deletion packages/osd-pm/src/utils/yarn_lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

// @ts-expect-error published types are worthless
import { parse as parseLockfile } from '@yarnpkg/lockfile';
import { standardize } from '@osd/cross-platform';
import { resolve, isAbsolute } from 'path';

import { readFile } from '../utils/fs';
import { OpenSearchDashboards } from '../utils/opensearch_dashboards';
Expand Down Expand Up @@ -62,7 +64,7 @@ export async function readYarnLock(osd: OpenSearchDashboards): Promise<YarnLock>
const yarnLock = parseLockfile(contents);

if (yarnLock.type === 'success') {
return yarnLock.object;
return fixFileLinks(yarnLock.object, osd.getAbsolute());
}

throw new Error('unable to read yarn.lock file, please run `yarn osd bootstrap`');
Expand All @@ -75,6 +77,33 @@ export async function readYarnLock(osd: OpenSearchDashboards): Promise<YarnLock>
return {};
}

/**
* Converts relative `file:` paths to absolute paths
* Yarn parsing method converts all file URIs to relative paths and this
* breaks the single-version requirement as dependencies to the same path
* would differ in their URIs across OSD and packages.
*/
function fixFileLinks(yarnLock: YarnLock, projectRoot: string): YarnLock {
const fileLinkDelimiter = '@file:';

const linkedKeys = Object.keys(yarnLock).filter((key) => key.includes(fileLinkDelimiter));

if (linkedKeys.length === 0) return yarnLock;

const updatedYarnLock = { ...yarnLock };
for (const key of linkedKeys) {
const [keyName, keyPath, ...rest] = key.split(fileLinkDelimiter);
if (!isAbsolute(keyPath)) {
const updatedKeyName = [keyName, standardize(resolve(projectRoot, keyPath)), ...rest].join(
fileLinkDelimiter
);
updatedYarnLock[updatedKeyName] = updatedYarnLock[key];
}
}

return updatedYarnLock;
}

/**
* Get a list of the absolute dependencies of this project, as resolved
* in the yarn.lock file, does not include other projects in the workspace
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"@scoped/foo@file:../linked/foo":
version "1.0.0"

"bar@file:../../deeper/linked/bar, @scoped/bar@file:../../deeper/linked/bar":
version "1.0.0"

"baz@file:/absolute/baz":
version "1.0.0"

# impossible but used for validating the transformer
"qux@^1.0.0, qux@file:../linked/qux, @scoped/qux@^1.0.0, @scoped/qux@file:../linked/qux":
version "1.0.0"
resolved "https://user@file:[email protected]/package"
22 changes: 22 additions & 0 deletions src/dev/build/tasks/__snapshots__/copy_source_task.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions src/dev/build/tasks/copy_source_task.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { join, resolve } from 'path';
import { mkdirSync, readFileSync, rmSync } from 'fs';
import { copyYarnLock } from './copy_source_task';

const repoRoot = resolve(`${__dirname}/__fixtures__/opensearch-dashboards-dev`);
const disposableRoot = join(repoRoot, 'build');
const buildRoot = join(repoRoot, 'build/opensearch-dashboards-dev');

describe('copy source task', () => {
beforeEach(() => {
mkdirSync(buildRoot, { recursive: true });
});

afterEach(() => {
rmSync(disposableRoot, { force: true, recursive: true });
});

it('transform relative paths while copying yarn.lock', async () => {
await copyYarnLock(repoRoot, buildRoot);
expect(readFileSync(join(buildRoot, 'yarn.lock'), 'utf8')).toMatchSnapshot();
});
});
71 changes: 69 additions & 2 deletions src/dev/build/tasks/copy_source_task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,21 @@
* under the License.
*/

import { createInterface } from 'readline';
import { join, isAbsolute, relative } from 'path';
import { createReadStream, createWriteStream } from 'fs';
import { copyAll, Task } from '../lib';

export const CopySource: Task = {
description: 'Copying source into platform-generic build directory',

async run(config, log, build) {
await copyAll(config.resolveFromRepo(), build.resolvePath(), {
const repoRoot = config.resolveFromRepo();
const buildRoot = build.resolvePath();

await copyAll(repoRoot, buildRoot, {
dot: false,
select: [
'yarn.lock',
'src/**',
'!src/**/*.{test,test.mocks,mock}.{js,ts,tsx}',
'!src/**/mocks.ts', // special file who imports .mock files
Expand All @@ -60,5 +65,67 @@ export const CopySource: Task = {
'opensearch_dashboards.d.ts',
],
});

await copyYarnLock(repoRoot, buildRoot);
},
};

/*
* Dependencies linked using `file:` in the `yarn.lock` file are
* referenced with relative paths. Moving them to the `build`
* folder, these paths need to be transformed.
*/
export const copyYarnLock = async (repoRoot: string, buildRoot: string) => {
const writeStream = createWriteStream(join(buildRoot, 'yarn.lock'));
const writeLine = (line: string) =>
new Promise((resolve) => {
if (writeStream.write(line + '\n')) return resolve();
writeStream.once('drain', resolve);
});

const returnPromise = new Promise((resolve) => {
writeStream.once('close', resolve);
});

/*
* Dependency patterns are comma separated `name@range` values, followed
* by a colon, like `AAAA:` and `AAAA, BBBB, CCCC:`
* Some are wrapped in quotes, like `"AAAA":` and `"AAAA, BBBB, CCCC":`
*
* The linked file will have `@file:` in it and the inner strings will
* not have commas or colons.
*
* Zero-width lookbehind and lookaheads allow getting only the part
* required for processing the patterns. `\S` is to avoid matching
* spaces after commas.
*/
const linkedFileDepPattern = /(?<=^"?|,\s*)(\S+?[^,:]+)@file:([^,:]+)(?=,\s*|"?:\s*$)/g;
const fileLinkDelimiter = '@file:';

const reader = createInterface({
input: createReadStream(join(repoRoot, 'yarn.lock')),
crlfDelay: Infinity,
});

for await (const line of reader) {
/*
* For added safety, we make sure the line doesn't start with whitespace
* before we check that it has `@file:`.
*/
if (!line.startsWith(' ') && line.includes(fileLinkDelimiter)) {
await writeLine(
line.replace(linkedFileDepPattern, (match, m1, m2) => {
if (isAbsolute(m2)) return `${m1}${fileLinkDelimiter}${m2}`;
// Join the relative path and repoRoot and find how to ge there from buildRoot
return `${m1}${fileLinkDelimiter}${relative(buildRoot, join(repoRoot, m2))}`;
})
);
} else {
await writeLine(line);
}
}

writeStream.end();

return returnPromise;
};
1 change: 1 addition & 0 deletions src/dev/precommit_hook/casing_check_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const IGNORE_DIRECTORY_GLOBS = [
'src/legacy/ui/public/flot-charts',
'test/functional/fixtures/opensearch_archiver/visualize_source-filters',
'packages/osd-pm/src/utils/__fixtures__/*',
'src/dev/build/tasks/__fixtures__/*',
];

/**
Expand Down

0 comments on commit 931b915

Please sign in to comment.