Skip to content

fix: Package name collisions #22

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.

Sign up for GitHub

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

Merged
eladb merged 20 commits into from
Aug 3, 2020
Merged

fix: Package name collisions #22

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
584bc50
fix: Package name collisions
campionfellin Jul 28, 2020
824bb84
chore: Update jsii version to 1.6
campionfellin Jul 28, 2020
0ed3c1d
fix: fallback to hash for default packageName and fix hyphens in pyth…
campionfellin Jul 29, 2020
8f34bab
fix: hypens to underscores for python package names
campionfellin Jul 29, 2020
58e1784
chore: sync up dependencies with projen
campionfellin Jul 29, 2020
0e460c8
Merge branch 'master' into issue273
Jul 29, 2020
d8b1e8b
Merge branch 'master' into issue273
campionfellin Jul 29, 2020
4508c2e
chore: clean up snapshots
campionfellin Jul 29, 2020
b25f102
Revert "chore: sync up dependencies with projen"
campionfellin Jul 29, 2020
8726b66
Revert "chore: Update jsii version to 1.6"
campionfellin Jul 29, 2020
036f654
Merge branch 'master' of https://github.com/aws/jsii-srcmak into issu…
campionfellin Jul 29, 2020
35ebfdf
fix: remove weird characters from snapshot
campionfellin Jul 29, 2020
8381fda
fix: back to yarn.lock from mainline
campionfellin Jul 30, 2020
c62fc33
fix: Respond to comments
campionfellin Jul 30, 2020
b0448a1
fix: Update test utils to ignore all jsii.tgz binary files
campionfellin Jul 30, 2020
d9727c0
fix: update snapshots
campionfellin Jul 30, 2020
8839018
fix: snapshots
campionfellin Jul 30, 2020
f6ab3b4
fix: snapshots
campionfellin Jul 30, 2020
2abca41
fix: snapshots
campionfellin Jul 30, 2020
6bfc372
fix: validate input options for python module name and java package
campionfellin Aug 3, 2020
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
8 changes: 5 additions & 3 deletions src/compile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as fs from 'fs-extra';
import * as path from 'path';
import { exec } from './util';
import { Options } from './options';
import * as crypto from 'crypto';

const compilerModule = require.resolve('jsii/bin/jsii');

Expand All @@ -23,6 +24,8 @@ export async function compile(workdir: string, options: Options) {
// path to entrypoint without extension
const basepath = path.join(path.dirname(entrypoint), path.basename(entrypoint, '.ts'));

const moduleKey = options.moduleKey?.replace(/\./g, '').replace(/\//g, '') ?? crypto.createHash('sha256').update(basepath, 'utf8').digest('hex');

// jsii modules to include
const moduleDirs = options.deps ?? [];

Expand All @@ -46,9 +49,8 @@ export async function compile(workdir: string, options: Options) {
}
}


const pkg = {
name: 'generated',
name: moduleKey,
version: '0.0.0',
author: 'generated@generated.com',
main: `${basepath}.js`,
Expand All @@ -66,7 +68,7 @@ export async function compile(workdir: string, options: Options) {
if (options.python) {
targets.python = {
distName: 'generated',
module: options.python.moduleName,
module: options.python.moduleName.replace(/-/g, '_'),
Copy link
Copy Markdown
Contributor

@eladb eladb Jul 30, 2020

Choose a reason for hiding this comment

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

I am assuming java would have a similar issue. I wonder if the correct behavior here is to validate per the language constraints and throw an error so the user will be required to pass in a valid name for the module.

Copy link
Copy Markdown
Contributor Author

@campionfellin campionfellin Jul 30, 2020

Choose a reason for hiding this comment

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

Yeah! We can do some validation instead of just correcting it. I'd be worried in some cases though where the user isn't the one explicitly choosing what this value is. In some use-cases, like CRDs in cdk8s from the internet, they might not control the group.

Would we then add this correction behavior to all consumers?

Copy link
Copy Markdown
Contributor Author

@campionfellin campionfellin Jul 30, 2020

Choose a reason for hiding this comment

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

Maybe we can do a warn that lets the user know we're correcting it?

Copy link
Copy Markdown
Contributor

@eladb eladb Jul 30, 2020

Choose a reason for hiding this comment

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

I think the consumers of this library (eg cdk8s) should do the name mangling if needed. The constraints should be well documented

Copy link
Copy Markdown
Contributor Author

@campionfellin campionfellin Jul 30, 2020

Choose a reason for hiding this comment

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

Ok! I will add validation here then

};
}

Expand Down
9 changes: 9 additions & 0 deletions src/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ export interface Options {
*/
jsii?: JsiiOutputOptions;

/**
* Key for the module to prevent JSII collisions.
*
* Use your own if it's project-unique, otherwise use default.
*
* @default - hash of the basepath to the module
*/
moduleKey?: string

/**
* Produce python code.
* @default - python is not generated
Expand Down
2 changes: 1 addition & 1 deletion src/srcmak.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function srcmak(srcdir: string, options: Options = { }) {

// extract code based on selected languages
if (options.python) {
const reldir = options.python.moduleName.replace(/\./g, '/'); // jsii replaces "." with "/"
const reldir = options.python.moduleName.replace(/\./g, '/').replace(/-/g, '_'); // jsii replaces "." with "/"
const source = path.resolve(path.join(workdir, 'dist/python/src', reldir));
const target = path.join(options.python.outdir, reldir);
await fs.move(source, target, { overwrite: true });
Expand Down
53 changes: 24 additions & 29 deletions test/__snapshots__/cli.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,15 @@ public final class $Module extends JsiiModule {
private final Map<String, Class<?>> cache = new HashMap<>();

public $Module() {
super(\\"generated\\", \\"0.0.0\\", $Module.class, \\"generated@0.0.0.jsii.tgz\\");
super(\\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060\\", \\"0.0.0\\", $Module.class, \\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060@0.0.0.jsii.tgz\\");
}

@Override
protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException {
if (!MODULE_TYPES.containsKey(fqn)) {
throw new ClassNotFoundException(\\"Unknown JSII type: \\" + fqn);
}
String className = MODULE_TYPES.get(fqn);
if (!this.cache.containsKey(className)) {
this.cache.put(className, this.findClass(className));
}
return this.cache.get(className);
return this.cache.computeIfAbsent(MODULE_TYPES.get(fqn), this::findClass);
}

private Class<?> findClass(final String binaryName) {
Expand All @@ -76,7 +72,7 @@ public final class $Module extends JsiiModule {
* A sophisticaed multi-language calculator.
*/

@software.amazon.jsii.Jsii(module = mypackage.$Module.class, fqn = \\"generated.Calculator\\")
@software.amazon.jsii.Jsii(module = mypackage.$Module.class, fqn = \\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Calculator\\")
public class Calculator extends software.amazon.jsii.JsiiObject {

protected Calculator(final software.amazon.jsii.JsiiObjectRef objRef) {
Expand Down Expand Up @@ -126,7 +122,7 @@ public class Calculator extends software.amazon.jsii.JsiiObject {
* Math operands.
*/

@software.amazon.jsii.Jsii(module = mypackage.$Module.class, fqn = \\"generated.Operands\\")
@software.amazon.jsii.Jsii(module = mypackage.$Module.class, fqn = \\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands\\")
@software.amazon.jsii.Jsii.Proxy(Operands.Jsii$Proxy.class)
public interface Operands extends software.amazon.jsii.JsiiSerializable {

Expand All @@ -149,7 +145,7 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
/**
* A builder for {@link Operands}
*/
public static final class Builder implements software.amazon.jsii.Builder<Operands> {
public static final class Builder {
private java.lang.Number lhs;
private java.lang.Number rhs;

Expand Down Expand Up @@ -178,7 +174,6 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
* @return a new instance of {@link Operands}
* @throws NullPointerException if any required attribute was not provided
*/
@Override
public Operands build() {
return new Jsii$Proxy(lhs, rhs);
}
Expand Down Expand Up @@ -229,7 +224,7 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
data.set(\\"rhs\\", om.valueToTree(this.getRhs()));

final com.fasterxml.jackson.databind.node.ObjectNode struct = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
struct.set(\\"fqn\\", om.valueToTree(\\"generated.Operands\\"));
struct.set(\\"fqn\\", om.valueToTree(\\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands\\"));
struct.set(\\"data\\", data);

final com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
Expand Down Expand Up @@ -258,8 +253,8 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
}
}
",
"src/main/resources/mypackage/$Module.txt": "generated.Calculator=mypackage.Calculator
generated.Operands=mypackage.Operands
"src/main/resources/mypackage/$Module.txt": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Calculator=mypackage.Calculator
4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands=mypackage.Operands
",
}
`;
Expand All @@ -272,29 +267,29 @@ Object {
"author",
],
},
"description": "generated",
"fingerprint": "+fSgo9PQwFAKmX2JcaRILwTXE/p1H1bq+QKuOREAfuE=",
"description": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060",
"fingerprint": "64TWv4C2KsEvkRVfUjD1gQTbNC2rsTlISDNv4F9JUw0=",
"homepage": "http://generated",
"jsiiVersion": "1.6.0 (build 248e75b)",
"license": "Apache-2.0",
"name": "generated",
"name": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060",
"repository": Object {
"type": "git",
"url": "http://generated",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "generated",
"npm": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060",
},
},
"types": Object {
"generated.Calculator": Object {
"assembly": "generated",
"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Calculator": Object {
"assembly": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060",
"docs": Object {
"summary": "A sophisticaed multi-language calculator.",
},
"fqn": "generated.Calculator",
"fqn": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Calculator",
"initializer": Object {},
"kind": "class",
"locationInModule": Object {
Expand All @@ -318,7 +313,7 @@ Object {
},
"name": "ops",
"type": Object {
"fqn": "generated.Operands",
"fqn": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands",
},
},
],
Expand All @@ -344,7 +339,7 @@ Object {
},
"name": "ops",
"type": Object {
"fqn": "generated.Operands",
"fqn": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands",
},
},
],
Expand All @@ -370,7 +365,7 @@ Object {
},
"name": "ops",
"type": Object {
"fqn": "generated.Operands",
"fqn": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands",
},
},
],
Expand All @@ -383,13 +378,13 @@ Object {
],
"name": "Calculator",
},
"generated.Operands": Object {
"assembly": "generated",
"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands": Object {
"assembly": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060",
"datatype": true,
"docs": Object {
"summary": "Math operands.",
},
"fqn": "generated.Operands",
"fqn": "4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands",
"kind": "interface",
"locationInModule": Object {
"filename": "lib/main.ts",
Expand Down Expand Up @@ -449,7 +444,7 @@ import publication
from ._jsii import *


class Calculator(metaclass=jsii.JSIIMeta, jsii_type=\\"generated.Calculator\\"):
class Calculator(metaclass=jsii.JSIIMeta, jsii_type=\\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Calculator\\"):
\\"\\"\\"A sophisticaed multi-language calculator.\\"\\"\\"
def __init__(self) -> None:
jsii.create(Calculator, self, [])
Expand Down Expand Up @@ -488,7 +483,7 @@ class Calculator(metaclass=jsii.JSIIMeta, jsii_type=\\"generated.Calculator\\"):
return jsii.invoke(self, \\"sub\\", [ops])


@jsii.data_type(jsii_type=\\"generated.Operands\\", jsii_struct_bases=[], name_mapping={'lhs': 'lhs', 'rhs': 'rhs'})
@jsii.data_type(jsii_type=\\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060.Operands\\", jsii_struct_bases=[], name_mapping={'lhs': 'lhs', 'rhs': 'rhs'})
class Operands():
def __init__(self, *, lhs: jsii.Number, rhs: jsii.Number) -> None:
\\"\\"\\"Math operands.
Expand Down Expand Up @@ -538,7 +533,7 @@ import jsii
import jsii.compat
import publication

__jsii_assembly__ = jsii.JSIIAssembly.load(\\"generated\\", \\"0.0.0\\", __name__[0:-6], \\"generated@0.0.0.jsii.tgz\\")
__jsii_assembly__ = jsii.JSIIAssembly.load(\\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060\\", \\"0.0.0\\", __name__[0:-6], \\"4c6b576fbe7e27053874813d9754cb2e46811d806c1e0aa9aae8add4c3763060@0.0.0.jsii.tgz\\")

__all__ = [
\\"__jsii_assembly__\\",
Expand Down
41 changes: 18 additions & 23 deletions test/__snapshots__/srcmak.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,15 @@ public final class $Module extends JsiiModule {
private final Map<String, Class<?>> cache = new HashMap<>();

public $Module() {
super(\\"generated\\", \\"0.0.0\\", $Module.class, \\"generated@0.0.0.jsii.tgz\\");
super(\\"javapackage\\", \\"0.0.0\\", $Module.class, \\"javapackage@0.0.0.jsii.tgz\\");
}

@Override
protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException {
if (!MODULE_TYPES.containsKey(fqn)) {
throw new ClassNotFoundException(\\"Unknown JSII type: \\" + fqn);
}
String className = MODULE_TYPES.get(fqn);
if (!this.cache.containsKey(className)) {
this.cache.put(className, this.findClass(className));
}
return this.cache.get(className);
return this.cache.computeIfAbsent(MODULE_TYPES.get(fqn), this::findClass);
}

private Class<?> findClass(final String binaryName) {
Expand All @@ -73,7 +69,7 @@ public final class $Module extends JsiiModule {
"src/main/java/hello/world/Hello.java": "package hello.world;


@software.amazon.jsii.Jsii(module = hello.world.$Module.class, fqn = \\"generated.Hello\\")
@software.amazon.jsii.Jsii(module = hello.world.$Module.class, fqn = \\"javapackage.Hello\\")
public class Hello extends software.amazon.jsii.JsiiObject {

protected Hello(final software.amazon.jsii.JsiiObjectRef objRef) {
Expand All @@ -97,7 +93,7 @@ public class Hello extends software.amazon.jsii.JsiiObject {
"src/main/java/hello/world/Operands.java": "package hello.world;


@software.amazon.jsii.Jsii(module = hello.world.$Module.class, fqn = \\"generated.Operands\\")
@software.amazon.jsii.Jsii(module = hello.world.$Module.class, fqn = \\"javapackage.Operands\\")
@software.amazon.jsii.Jsii.Proxy(Operands.Jsii$Proxy.class)
public interface Operands extends software.amazon.jsii.JsiiSerializable {

Expand All @@ -114,7 +110,7 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
/**
* A builder for {@link Operands}
*/
public static final class Builder implements software.amazon.jsii.Builder<Operands> {
public static final class Builder {
private java.lang.Number lhs;
private java.lang.Number rhs;

Expand Down Expand Up @@ -143,7 +139,6 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
* @return a new instance of {@link Operands}
* @throws NullPointerException if any required attribute was not provided
*/
@Override
public Operands build() {
return new Jsii$Proxy(lhs, rhs);
}
Expand Down Expand Up @@ -194,7 +189,7 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
data.set(\\"rhs\\", om.valueToTree(this.getRhs()));

final com.fasterxml.jackson.databind.node.ObjectNode struct = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
struct.set(\\"fqn\\", om.valueToTree(\\"generated.Operands\\"));
struct.set(\\"fqn\\", om.valueToTree(\\"javapackage.Operands\\"));
struct.set(\\"data\\", data);

final com.fasterxml.jackson.databind.node.ObjectNode obj = com.fasterxml.jackson.databind.node.JsonNodeFactory.instance.objectNode();
Expand Down Expand Up @@ -223,8 +218,8 @@ public interface Operands extends software.amazon.jsii.JsiiSerializable {
}
}
",
"src/main/resources/hello/world/$Module.txt": "generated.Hello=hello.world.Hello
generated.Operands=hello.world.Operands
"src/main/resources/hello/world/$Module.txt": "javapackage.Hello=hello.world.Hello
javapackage.Operands=hello.world.Operands
",
}
`;
Expand All @@ -237,26 +232,26 @@ Object {
"author",
],
},
"description": "generated",
"fingerprint": "cNtb65kyZQOiBulmzr+bs6dCGdv4uhea1tFTkbRGJTw=",
"description": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6",
"fingerprint": "i19np+HVFMpwiHENNJeGk6AC3MVvpkkG7VtDtf9ouXs=",
"homepage": "http://generated",
"jsiiVersion": "1.6.0 (build 248e75b)",
"license": "Apache-2.0",
"name": "generated",
"name": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6",
"repository": Object {
"type": "git",
"url": "http://generated",
},
"schema": "jsii/0.10.0",
"targets": Object {
"js": Object {
"npm": "generated",
"npm": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6",
},
},
"types": Object {
"generated.Foo": Object {
"assembly": "generated",
"fqn": "generated.Foo",
"1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6.Foo": Object {
"assembly": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6",
"fqn": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6.Foo",
"initializer": Object {},
"kind": "class",
"locationInModule": Object {
Expand Down Expand Up @@ -300,7 +295,7 @@ import publication
from ._jsii import *


class Hello(metaclass=jsii.JSIIMeta, jsii_type=\\"generated.Hello\\"):
class Hello(metaclass=jsii.JSIIMeta, jsii_type=\\"pythonpackage.Hello\\"):
def __init__(self) -> None:
jsii.create(Hello, self, [])

Expand All @@ -315,7 +310,7 @@ class Hello(metaclass=jsii.JSIIMeta, jsii_type=\\"generated.Hello\\"):
return jsii.invoke(self, \\"add\\", [ops])


@jsii.data_type(jsii_type=\\"generated.Operands\\", jsii_struct_bases=[], name_mapping={'lhs': 'lhs', 'rhs': 'rhs'})
@jsii.data_type(jsii_type=\\"pythonpackage.Operands\\", jsii_struct_bases=[], name_mapping={'lhs': 'lhs', 'rhs': 'rhs'})
class Operands():
def __init__(self, *, lhs: jsii.Number, rhs: jsii.Number) -> None:
\\"\\"\\"
Expand Down Expand Up @@ -362,7 +357,7 @@ import jsii
import jsii.compat
import publication

__jsii_assembly__ = jsii.JSIIAssembly.load(\\"generated\\", \\"0.0.0\\", __name__[0:-6], \\"generated@0.0.0.jsii.tgz\\")
__jsii_assembly__ = jsii.JSIIAssembly.load(\\"pythonpackage\\", \\"0.0.0\\", __name__[0:-6], \\"pythonpackage@0.0.0.jsii.tgz\\")

__all__ = [
\\"__jsii_assembly__\\",
Expand Down
Loading
⚡