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: add support for bin-scripts (python only) #1941

Merged
merged 18 commits into from
Dec 10, 2020
Merged
Show file tree
Hide file tree
Changes from 17 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
13 changes: 13 additions & 0 deletions packages/@jsii/kernel/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@ export interface LoadResponse {
readonly types: number;
}

export interface InvokeScriptRequest {
readonly assembly: string;
readonly script: string;
readonly args?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
readonly args?: string[];
readonly args?: readonly string[];

}

export interface InvokeScriptResponse {
readonly status: number | null;
readonly stdout: Buffer;
readonly stderr: Buffer;
readonly signal: string | null;
}

export interface CreateRequest {
/**
* The FQN of the class of which an instance is requested (or "Object")
Expand Down
76 changes: 62 additions & 14 deletions packages/@jsii/kernel/lib/kernel.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as spec from '@jsii/spec';
import * as cp from 'child_process';
import * as fs from 'fs-extra';
import * as os from 'os';
import * as path from 'path';
Expand Down Expand Up @@ -65,24 +66,11 @@ export class Kernel {
);
}

if (!this.installDir) {
this.installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jsii-kernel-'));
fs.mkdirpSync(path.join(this.installDir, 'node_modules'));
this._debug('creating jsii-kernel modules workdir:', this.installDir);

process.on('exit', () => {
if (this.installDir) {
this._debug('removing install dir', this.installDir);
fs.removeSync(this.installDir); // can't use async version during exit
}
});
}

const pkgname = req.name;
const pkgver = req.version;

// check if we already have such a module
const packageDir = path.join(this.installDir, 'node_modules', pkgname);
const packageDir = this._getPackageDir(pkgname);
if (fs.pathExistsSync(packageDir)) {
// module exists, verify version
const epkg = fs.readJsonSync(path.join(packageDir, 'package.json'));
Expand Down Expand Up @@ -147,6 +135,50 @@ export class Kernel {
};
}

public invokeBinScript(
req: api.InvokeScriptRequest,
): api.InvokeScriptResponse {
const packageDir = this._getPackageDir(req.assembly);
if (fs.pathExistsSync(packageDir)) {
// module exists, verify version
const epkg = fs.readJsonSync(path.join(packageDir, 'package.json'));

if (!epkg.bin) {
throw new Error('There is no bin scripts defined for this package.');
}

const scriptPath = epkg.bin[req.script];

if (!epkg.bin) {
throw new Error(`Script with name ${req.script} was not defined.`);
}

const result = cp.spawnSync(
path.join(packageDir, scriptPath),
req.args ?? [],
{
encoding: 'utf-8',
env: {
...process.env,
// Make sure the current NODE_OPTIONS are honored if we shell out to node
NODE_OPTIONS: process.execArgv.join(' '),
// Make sure "this" node is ahead of $PATH just in case
PATH: `${path.dirname(process.execPath)}:${process.env.PATH}`,
},
shell: true,
},
);

return {
stdout: result.stdout,
stderr: result.stderr,
status: result.status,
signal: result.signal,
};
}
throw new Error(`Package with name ${req.assembly} was not loaded.`);
}

public create(req: api.CreateRequest): api.CreateResponse {
return this._create(req);
}
Expand Down Expand Up @@ -493,6 +525,22 @@ export class Kernel {
}
}

private _getPackageDir(pkgname: string): string {
if (!this.installDir) {
this.installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jsii-kernel-'));
fs.mkdirpSync(path.join(this.installDir, 'node_modules'));
this._debug('creating jsii-kernel modules workdir:', this.installDir);

process.on('exit', () => {
if (this.installDir) {
this._debug('removing install dir', this.installDir);
fs.removeSync(this.installDir); // can't use async version during exit
}
});
}
return path.join(this.installDir, 'node_modules', pkgname);
}

// prefixed with _ to allow calling this method internally without
// getting it recorded for testing.
private _create(req: api.CreateRequest): api.CreateResponse {
Expand Down
14 changes: 13 additions & 1 deletion packages/@jsii/kernel/test/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2134,6 +2134,18 @@ defineTest('Override transitive property', (sandbox) => {
expect(propValue).toBe('N3W');
});

defineTest('invokeBinScript() return output', (sandbox) => {
const result = sandbox.invokeBinScript({
assembly: 'jsii-calc',
script: 'calc',
});

expect(result.stdout).toEqual('Hello World!\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably should also asset the value of stderr, status and signal.

expect(result.stderr).toEqual('');
expect(result.status).toEqual(0);
expect(result.signal).toBeNull();
});

// =================================================================================================

const testNames: { [name: string]: boolean } = {};
Expand Down Expand Up @@ -2204,7 +2216,7 @@ async function preparePackage(module: string, useCache = true) {
});
const stdout = new Array<Buffer>();
child.stdout.on('data', (chunk) => stdout.push(Buffer.from(chunk)));
child.once('exit', (code, signal) => {
child.once('close', (code, signal) => {
if (code === 0) {
return ok();
}
Expand Down
15 changes: 6 additions & 9 deletions packages/@jsii/kernel/test/recording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ export function recordInteraction(kernel: Kernel, inputOutputLogPath: string) {
const logfile = fs.createWriteStream(inputOutputLogPath);
(kernel as any).logfile = logfile;

Object.getOwnPropertyNames(Kernel.prototype)
.filter((p) => !p.startsWith('_'))
.forEach((api) => {
const old = Object.getOwnPropertyDescriptor(Kernel.prototype, api)!;

Object.entries(Object.getOwnPropertyDescriptors(Kernel.prototype))
.filter(([p, v]) => !p.startsWith('_') && typeof v.value === 'function')
.forEach(([api, old]) => {
Object.defineProperty(kernel, api, {
...old,
value(...args: any[]) {
logInput({ api, ...args[0] });
try {
Expand Down Expand Up @@ -68,12 +67,10 @@ export function recordInteraction(kernel: Kernel, inputOutputLogPath: string) {
});

function logInput(obj: any) {
const inputLine = `${JSON.stringify(obj)}\n`;
logfile.write(`> ${inputLine}`);
logfile.write(`> ${JSON.stringify(obj)}\n`);
}

function logOutput(obj: any) {
const outputLine = `${JSON.stringify(obj)}\n`;
logfile.write(`< ${outputLine}`);
logfile.write(`< ${JSON.stringify(obj)}\n`);
}
}
15 changes: 15 additions & 0 deletions packages/@jsii/python-runtime/src/jsii/_kernel/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
GetResponse,
InvokeRequest,
InvokeResponse,
InvokeScriptRequest,
KernelResponse,
LoadRequest,
ObjRef,
Expand Down Expand Up @@ -242,6 +243,20 @@ def __init__(self, provider_class: Type[BaseProvider] = ProcessProvider) -> None
def load(self, name: str, version: str, tarball: str) -> None:
self.provider.load(LoadRequest(name=name, version=version, tarball=tarball))

def invokeBinScript(
self, pkgname: str, script: str, args: Optional[List[Any]] = None
) -> None:
if args is None:
args = []

self.provider.invokeBinScript(
InvokeScriptRequest(
pkgname=pkgname,
script=script,
args=_make_reference_for_native(self, args),
)
)

# TODO: Is there a way to say that obj has to be an instance of klass?
def create(self, klass: Type, obj: Any, args: Optional[List[Any]] = None) -> ObjRef:
if args is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
GetResponse,
InvokeRequest,
InvokeResponse,
InvokeScriptRequest,
InvokeScriptResponse,
DeleteRequest,
DeleteResponse,
SetRequest,
Expand Down Expand Up @@ -45,6 +47,10 @@ class BaseProvider(metaclass=abc.ABCMeta):
def load(self, request: LoadRequest) -> LoadResponse:
...

@abc.abstractmethod
def invokeBinScript(self, request: InvokeScriptRequest) -> InvokeScriptResponse:
...

@abc.abstractmethod
def create(self, request: CreateRequest) -> CreateResponse:
...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
GetResponse,
InvokeRequest,
InvokeResponse,
InvokeScriptRequest,
InvokeScriptResponse,
SetRequest,
SetResponse,
StaticGetRequest,
Expand Down Expand Up @@ -158,6 +160,10 @@ def __init__(self):
LoadRequest,
_with_api_key("load", self._serializer.unstructure_attrs_asdict),
)
self._serializer.register_unstructure_hook(
InvokeScriptRequest,
_with_api_key("invokeBinScript", self._serializer.unstructure_attrs_asdict),
)
self._serializer.register_unstructure_hook(
CreateRequest,
_with_api_key("create", self._serializer.unstructure_attrs_asdict),
Expand Down Expand Up @@ -343,6 +349,9 @@ def _process(self) -> _NodeProcess:
def load(self, request: LoadRequest) -> LoadResponse:
return self._process.send(request, LoadResponse)

def invokeBinScript(self, request: InvokeScriptRequest) -> InvokeScriptResponse:
return self._process.send(request, InvokeScriptResponse)

def create(self, request: CreateRequest) -> CreateResponse:
return self._process.send(request, CreateResponse)

Expand Down
19 changes: 19 additions & 0 deletions packages/@jsii/python-runtime/src/jsii/_kernel/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ class LoadResponse:
types: int


@attr.s(auto_attribs=True, frozen=True, slots=True)
class InvokeScriptRequest:

pkgname: str
script: str
args: List[Any] = attr.Factory(list)


@attr.s(auto_attribs=True, frozen=True, slots=True)
class InvokeScriptResponse:

status: int
stdout: str
stderr: str
output: List[str]


@attr.s(auto_attribs=True, frozen=True, slots=True)
class CreateRequest:

Expand Down Expand Up @@ -226,6 +243,7 @@ class StatsResponse:
GetRequest,
StaticGetRequest,
InvokeRequest,
InvokeScriptRequest,
StaticInvokeRequest,
StatsRequest,
]
Expand All @@ -237,6 +255,7 @@ class StatsResponse:
DeleteResponse,
GetResponse,
InvokeResponse,
InvokeScriptResponse,
SetResponse,
StatsResponse,
Callback,
Expand Down
4 changes: 4 additions & 0 deletions packages/@jsii/python-runtime/src/jsii/_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def load(cls, *args, _kernel=kernel, **kwargs):
# Give our record of the assembly back to the caller.
return assembly

def invokeBinScript(cls, pkgname, script, *args, _kernel=kernel):
response = _kernel.invokeBinScript(pkgname, script, *args)
print(response.stdout)


class JSIIMeta(_ClassPropertyMeta, type):
def __new__(cls, name, bases, attrs, *, jsii_type=None):
Expand Down
1 change: 0 additions & 1 deletion packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
from scope.jsii_calc_lib import IFriendly, EnumFromScopedModule, Number
from scope.jsii_calc_lib.custom_submodule_name import IReflectable, ReflectableEntry


# Note: The names of these test functions have been chosen to map as closely to the
# Java Compliance tests as possible.
# Note: While we could write more expressive and better tests using the functionality
Expand Down
7 changes: 7 additions & 0 deletions packages/@jsii/spec/lib/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ export interface Assembly extends AssemblyConfiguration, Documentable {
* @default none
*/
readme?: { markdown: string };

/**
* List of bin-scripts
*
* @default none
*/
bin?: { readonly [script: string]: string };
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/jsii-calc/bin/run
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/usr/bin/env node
require('./run.js');
2 changes: 2 additions & 0 deletions packages/jsii-calc/bin/run.cmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@echo off
node "%~dp0\run" %*
5 changes: 5 additions & 0 deletions packages/jsii-calc/bin/run.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env node

/* eslint-disable no-console */

console.info('Hello World!');
3 changes: 3 additions & 0 deletions packages/jsii-calc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
"bugs": {
"url": "https://github.com/aws/jsii/issues"
},
"bin": {
"calc": "bin/run"
},
"repository": {
"type": "git",
"url": "https://github.com/aws/jsii.git",
Expand Down
5 changes: 4 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
],
"url": "https://aws.amazon.com"
},
"bin": {
"calc": "bin/run"
},
"bundled": {
"@fixtures/jsii-calc-bundled": "^0.19.0"
},
Expand Down Expand Up @@ -14342,5 +14345,5 @@
}
},
"version": "0.0.0",
"fingerprint": "55ZmA4GbUYPUmTXM2oFDEND8/Yk2Vzw1FThRWEOigAM="
"fingerprint": "XfCnzPEGbEJR6hhBX8zWyFzWJP2wH9ztW2ZcW6Wdb+4="
}
Loading