Skip to content

Commit

Permalink
feat: add support for bin-scripts (python only) (#1941)
Browse files Browse the repository at this point in the history
### Problem
JSII is missing functionality for packing and using bin scripts from Python package.

### Solution
Were extended assembly and project-info properties for bin-scripts.
Added generation scripts for each script in bin section.
Added section `scripts` to `pyhon.py` script.

### Testing

Was build project with changed scripts and checked that file `.jsii` contains `bin` section.
Also was checked that without `bin` section in `package.json` empty `bin` section was not created in `.jsii` file.
Was created package and verified that scripts were created and paths to it were added to `setup.py`.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
kozlove-aws authored Dec 10, 2020
1 parent b7b7275 commit 61ef5ed
Show file tree
Hide file tree
Showing 21 changed files with 261 additions and 33 deletions.
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[];
}

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');
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

0 comments on commit 61ef5ed

Please sign in to comment.