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

[Backport 2.x] Adds @osd/cross-platform #2714

Merged
merged 1 commit into from
Nov 1, 2022
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Vis Builder] Change classname prefix wiz to vb ([#2581](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2581/files))
- [Vis Builder] Change wizard to vis_builder in file names and paths ([#2587](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2587))
- [Windows] Facilitate building and running OSD and plugins on Windows platforms ([#2601](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2601))
- [Windows] Add `@osd/cross-platform` package to standardize path handling across platforms ([#2703](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2703))
- [Multi DataSource] Address UX comments on Data source list and create page ([#2625](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2625))
- [Vis Builder] Rename wizard to visBuilder in i18n id and formatted message id ([#2635](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2635))
- [Vis Builder] Rename wizard to visBuilder in class name, type name and function name ([#2639](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2639))
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
"@osd/apm-config-loader": "1.0.0",
"@osd/config": "1.0.0",
"@osd/config-schema": "1.0.0",
"@osd/cross-platform": "1.0.0",
"@osd/i18n": "1.0.0",
"@osd/interpreter": "1.0.0",
"@osd/logging": "1.0.0",
Expand Down
5 changes: 3 additions & 2 deletions packages/osd-config-schema/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
"osd:bootstrap": "yarn build"
},
"devDependencies": {
"typescript": "4.0.2",
"tsd": "^0.21.0"
"@osd/cross-platform": "1.0.0",
"tsd": "^0.21.0",
"typescript": "4.0.2"
},
"peerDependencies": {
"lodash": "^4.17.21",
Expand Down
6 changes: 3 additions & 3 deletions packages/osd-config-schema/src/errors/schema_error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import { relative, sep } from 'path';
import { SchemaError } from '.';

import { standardize, getRepoRoot } from '@osd/cross-platform';

/**
* Make all paths in stacktrace relative.
*/
Expand All @@ -46,9 +48,7 @@ export const cleanStack = (stack: string) =>
}

const path = parts[1];
// Cannot use `standardize` from `@osd/utils
let relativePath = relative(process.cwd(), path);
if (process.platform === 'win32') relativePath = relativePath.replace(/\\/g, '/');
const relativePath = standardize(relative(getRepoRoot(path) || '.', path));

return line.replace(path, relativePath);
})
Expand Down
28 changes: 28 additions & 0 deletions packages/osd-cross-platform/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# `@osd/cross-platform` — OpenSearch Dashboards cross-platform helpers

This package contains the helpers to work around the differences across platforms, such as the difference in the path segment separator and the possibility of referencing a path using the short 8.3 name (SFN), a long name, and a long UNC on Windows.

Some helpers are functions that `standardize` the reference to a path or help `getRepoRoot`, and some are constants referencing the `PROCESS_WORKING_DIR` or `REPO_ROOT`.

### Example

When the relative reference of `path` to the working directory is needed, using the code below would produce different results on Linux that it would on Windows and if the process was started in a Windows shell that used short paths, the results differ from a Windows shell that used long paths.
```js
import { relative } from 'path';

const relativePath = relative(process.cwd(), path);

// Output on Linux: relative-path/to/a/file
// Windows: relative-path\to\a\file
// Windows SFN: RELATI~1\to\a\file
```

To avoid those differences, helper functions and constants can be used:
```js
import { relative } from 'path';
import { standardize, PROCESS_WORKING_DIR } from '@osd/cross-platform';

const relativePath = standardize(relative(PROCESS_WORKING_DIR, path));

// Output: relative-path/to/a/file
```
15 changes: 15 additions & 0 deletions packages/osd-cross-platform/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "@osd/cross-platform",
"main": "./target/index.js",
"version": "1.0.0",
"license": "Apache-2.0",
"private": true,
"scripts": {
"build": "tsc",
"osd:bootstrap": "yarn build"
},
"devDependencies": {
"typescript": "4.0.2",
"tsd": "^0.21.0"
}
}
13 changes: 13 additions & 0 deletions packages/osd-cross-platform/src/__snapshots__/path.test.ts.snap

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

8 changes: 8 additions & 0 deletions packages/osd-cross-platform/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export * from './path';
export * from './process';
export * from './repo_root';
208 changes: 208 additions & 0 deletions packages/osd-cross-platform/src/path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import path from 'path';
import fs from 'fs';
import { access, rmdir, mkdir, writeFile, symlink } from 'fs/promises';

import {
resolveToFullNameSync,
resolveToFullPathSync,
resolveToShortNameSync,
resolveToShortPathSync,
shortNamesSupportedSync,
realPathSync,
realShortPathSync,
standardize,
} from './path';

const tmpTestFolder = './__test_artifacts__';
const longFolderName = '.long-folder-name';
const longFileName = '.long-file-name.txt';
const longSymlinkName = '.sym.link';
const shortFolderName = 'LONG-F~1';
const shortFileName = 'LONG-F~1.TXT';
const dummyWindowsPath = 'C:\\a\\b\\c';
const dummyWindowsPOSIXPath = 'C:/a/b/c';
const dummyPOSIXPath = '/a/b/c';

const onWindows = process.platform === 'win32' ? describe : xdescribe;
const onWindowsWithShortNames = shortNamesSupportedSync() ? describe : xdescribe;

// Save the real process.platform
const realPlatform = Object.getOwnPropertyDescriptor(process, 'platform')!;

describe('Cross Platform', () => {
describe('path', () => {
onWindows('on Windows', () => {
onWindowsWithShortNames('when 8.3 is supported', () => {
beforeAll(async () => {
// Cleanup
try {
// If leftover artifacts were found, get rid of them
await access(tmpTestFolder);
await rmdir(tmpTestFolder, { recursive: true });
} catch (ex) {
// Do nothing; if `rmdir` failed, let the `mkdir` below throw the error
}

await mkdir(tmpTestFolder);
await mkdir(path.resolve(tmpTestFolder, longFolderName));
await writeFile(path.resolve(tmpTestFolder, longFolderName, longFileName), '');
await symlink(
path.resolve(tmpTestFolder, longFolderName),
path.resolve(tmpTestFolder, longSymlinkName),
'junction'
);
});

afterAll(async () => {
try {
await rmdir(tmpTestFolder, { recursive: true });
} catch (ex) {
// Do nothing
}
});

it('can synchronously extract full name of a folder', () => {
const name = path.basename(
resolveToFullPathSync(path.resolve(tmpTestFolder, shortFolderName))
);
expect(name).toBe(longFolderName);
});

it('can synchronously extract full name of a file', () => {
const name = path.basename(
resolveToFullNameSync(path.resolve(tmpTestFolder, shortFolderName, shortFileName))
);
expect(name).toBe(longFileName);
});

it('can synchronously extract short name of a folder', () => {
const name = path.basename(
resolveToShortPathSync(path.resolve(tmpTestFolder, longFolderName))
);
expect(name).toBe(shortFolderName);
});

it('can synchronously extract short name of a file', () => {
const name = path.basename(
resolveToShortNameSync(path.resolve(tmpTestFolder, longFolderName, longFileName))
);
expect(name).toBe(shortFileName);
});

it('can synchronously extract full name of a symbolic link', () => {
const name = path.basename(realPathSync(path.resolve(tmpTestFolder, longSymlinkName)));
expect(name).toBe(longFolderName);
});

it('can synchronously extract short name of a symbolic link', () => {
const name = path.basename(
realShortPathSync(path.resolve(tmpTestFolder, longSymlinkName))
);
expect(name).toBe(shortFolderName);
});
});
});

describe('on platforms other than Windows', () => {
let mockPathNormalize: jest.SpyInstance<string, [p: string]>;
let mockPathResolve: jest.SpyInstance<string, string[]>;
let mockFSRealPathSync: jest.SpyInstance<string>;

beforeAll(() => {
Object.defineProperty(process, 'platform', {
...Object.getOwnPropertyDescriptor(process, 'property'),
value: 'linux',
});

mockPathNormalize = jest.spyOn(path, 'normalize').mockReturnValue(dummyPOSIXPath);
mockPathResolve = jest.spyOn(path, 'resolve').mockReturnValue(dummyPOSIXPath);
mockFSRealPathSync = jest
.spyOn(fs, 'realpathSync')
.mockReturnValue(dummyPOSIXPath) as jest.SpyInstance<string>;
});

afterAll(() => {
// Restore the real property value after each test
Object.defineProperty(process, 'platform', realPlatform);
mockPathNormalize.mockRestore();
mockPathResolve.mockRestore();
mockFSRealPathSync.mockRestore();
});

it('all short and full name methods return just the normalized paths', () => {
expect(shortNamesSupportedSync()).toBe(false);
expect(resolveToFullPathSync(dummyPOSIXPath)).toBe(dummyPOSIXPath);
expect(resolveToShortPathSync(dummyPOSIXPath)).toBe(dummyPOSIXPath);
});
});

describe('standardize', () => {
describe('on Windows', () => {
let mockPathNormalize: jest.SpyInstance<string, [p: string]>;

beforeAll(() => {
Object.defineProperty(process, 'platform', {
...Object.getOwnPropertyDescriptor(process, 'property'),
value: 'win32',
});

mockPathNormalize = jest.spyOn(path, 'normalize').mockReturnValue(dummyWindowsPath);
});

afterAll(() => {
// Restore the real property value after each test
Object.defineProperty(process, 'platform', realPlatform);
mockPathNormalize.mockRestore();
});

it('produces a path in native format', () => {
expect(standardize(dummyWindowsPath, false, false)).toMatchSnapshot();
});

it('produces a path in native format even for POSIX input', () => {
expect(standardize(dummyWindowsPOSIXPath, false, false)).toMatchSnapshot();
});

it('produces a path in native format with escaped backslashes', () => {
expect(standardize(dummyWindowsPath, false, true)).toMatchSnapshot();
});

it('produces a path in POSIX format', () => {
expect(standardize(dummyWindowsPath)).toMatchSnapshot();
});
});

describe('on POSIX-compatible platforms', () => {
let mockPathNormalize: jest.SpyInstance<string, [p: string]>;

beforeAll(() => {
Object.defineProperty(process, 'platform', {
...Object.getOwnPropertyDescriptor(process, 'property'),
value: 'linux',
});

mockPathNormalize = jest.spyOn(path, 'normalize').mockReturnValue(dummyPOSIXPath);
});

afterAll(() => {
// Restore the real property value after each test
Object.defineProperty(process, 'platform', realPlatform);
mockPathNormalize.mockRestore();
});

it('produces a path in POSIX format', () => {
expect(standardize(dummyPOSIXPath)).toMatchSnapshot();
});

it('ignores additional parameters', () => {
expect(standardize(dummyPOSIXPath, false, true)).toMatchSnapshot();
});
});
});
});
});
Loading