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 8 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
4 changes: 2 additions & 2 deletions .projenrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ const project = new TypeScriptLibraryProject({
'@types/fs-extra': Semver.caret('8.1.0'),
},
dependencies: {
'jsii': Semver.caret('1.5.0'),
'jsii-pacmak': Semver.caret('1.5.0'),
'jsii': Semver.caret('1.9.0'),
'jsii-pacmak': Semver.caret('1.9.0'),
'fs-extra': Semver.caret('9.0.0'),
'ncp': Semver.caret('2.0.0'),
'yargs': Semver.caret('11.1.1'),
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
},
"peerDependencies": {},
"dependencies": {
"jsii": "^1.5.0",
"jsii-pacmak": "^1.5.0",
"jsii": "^1.9.0",
"jsii-pacmak": "^1.9.0",
"fs-extra": "^9.0.0",
"ncp": "^2.0.0",
"yargs": "^11.1.1"
Expand Down
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 packageName = options.packageName?.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: packageName,
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
6 changes: 6 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,12 @@ export interface Options {
*/
jsii?: JsiiOutputOptions;

/**
* Package name
* @default - hash of the basepath to the module
*/
packageName?: string
Copy link
Copy Markdown
Contributor

@eladb eladb Jul 29, 2020

Choose a reason for hiding this comment

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

I am not sure that's the correct name for this because users don't really interact with this value after code generation. Maybe moduleKey or something like that.

Docstring help needs to indicate that it should be project-unique.


/**
* 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
111 changes: 82 additions & 29 deletions test/__snapshots__/cli.test.ts.snap
View file
Open in desktop

Large diffs are not rendered by default.

69 changes: 46 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,7 +45,7 @@ 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
Expand Down Expand Up @@ -73,7 +73,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 +97,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 Down Expand Up @@ -194,7 +194,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,9 +223,11 @@ 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
",
"src/main/resources/hello/world/javapackage@0.0.0.jsii.tgz": "��ko�H�����K���&@J�Hw�&�B ��ʏ�Yb���5�T��?1��.m�:F����˳��qU�Q5Q�8f��1 IR�R֭T��5��I�\\\\�I@'�R�����x��T��|\\"c������� ���G�\`s��f���*G�騨;���Q�B ��
_m]֔fU��mKs]�!M}Ь&c ��t���t��]�7�Ӛ�e�zXkZc�y�8<M��i� ��H����[��H���=_i%rR�%���4�O�_e��y�7�n�9�xܒx0����\`�CH�Rv��e���!���?� ���O�o϶F�ai����h���+>�L\\\\�!�I}�\\\\;g]�fS�fw���֙A�f���i�K[?���jgs�%�u������Y*�75�VJ�y��6�����}�Em�E���1��[��6���|-���\\\\^��{r����?��ZH{JB�ታE����&?���e �b�*]0T� k��T�Nv�[;d [���'�7/��� ",
}
`;

Expand All @@ -237,26 +239,26 @@ Object {
"author",
],
},
"description": "generated",
"fingerprint": "cNtb65kyZQOiBulmzr+bs6dCGdv4uhea1tFTkbRGJTw=",
"description": "1bc04b5291c26a46d918139138b992d2de976d6851d0893b0476b85bfbdfc6e6",
"fingerprint": "6oK+Fme17BaQrSRFIR9SO8fcGINzbxpouGPKqn9MAhs=",
"homepage": "http://generated",
"jsiiVersion": "1.6.0 (build 248e75b)",
"jsiiVersion": "1.9.0 (build 5c646d5)",
"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 +302,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,25 +317,29 @@ 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'})
class Operands():
@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:
\\"\\"\\"
:param lhs: -
:param rhs: -
\\"\\"\\"
self._values = {
'lhs': lhs,
'rhs': rhs,
\\"lhs\\": lhs,
\\"rhs\\": rhs,
}

@builtins.property
def lhs(self) -> jsii.Number:
return self._values.get('lhs')
return self._values.get(\\"lhs\\")

@builtins.property
def rhs(self) -> jsii.Number:
return self._values.get('rhs')
return self._values.get(\\"rhs\\")

def __eq__(self, rhs) -> bool:
return isinstance(rhs, self.__class__) and rhs._values == self._values
Expand All @@ -342,7 +348,9 @@ class Operands():
return not (rhs == self)

def __repr__(self) -> str:
return 'Operands(%s)' % ', '.join(k + '=' + repr(v) for k, v in self._values.items())
return \\"Operands(%s)\\" % \\", \\".join(
k + \\"=\\" + repr(v) for k, v in self._values.items()
)


__all__ = [
Expand All @@ -362,14 +370,29 @@ 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__\\",
]

publication.publish()
",
"my_python_module/submodule/_jsii/pythonpackage@0.0.0.jsii.tgz": "��ko�H�����K���6��D�������TE�z����5�T����<]Ҫ���ݙ��Ύ��ih��p.;f��y�$�T(��
Copy link
Copy Markdown
Contributor

@eladb eladb Jul 29, 2020

Choose a reason for hiding this comment

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

This needs to be filtered out

Copy link
Copy Markdown
Contributor Author

@campionfellin campionfellin Jul 29, 2020

Choose a reason for hiding this comment

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

Yeah... not really sure why my macbook keeps trying to add them... will try to fix

y�(��K�,�J�I��(yx��>�3�QP����o�3� j>7]*�� �fc��#�\`�ql����ȵţ���f@�w4]�_��f��� �z9^w+�tm�3@��{'�\\\\*0����)[��$��}bB�
%��.����X}!&�S�J1*1k�{.#ܥ�� <�b/�X�Sk���� �ZHj�����H�0a�V�ǫq��g��!K�1j}�Aoo��RW@ۮ�[����w�Z����H
�^�qC�l[������u+��Ո����S_2Ji�C8�,2�Q.RĄ8F�Y iEo�H �IuZ���J�Db��
r�)vx~4���(V���x���kc��X�maM�!
��ź&�V�$i�j���W�4
(wcC�m�7��޶�p����^yn�h�~�yh�uCo��O���=Jl�id���:�⃲V�d�JO��+�c��\\"�и�4�S?~R;1O8�{
��{e'���Zyԅ5N�#�B���[�G8b�>�tk�+��z�m����&��ܻ?���Z���)� }n�$�p�}3]���M��+�3��;��D>0��Z�T�W�����Y�:�f�Dy�{?�w�j�<�ޜ��E�WU�,�Pn;t���d<^��ż�]�+��7�k��� � ��$��̕>ƈg
�S~��<�rv$�ww�-ё�]�j���%,ޝf��qy'� S��t���7!^��1o�
y��O�@��,�e�}4�4t������4���!0ק�4σ�qӽ< _�'0�ρ(����uNu��R�Me8hε~�W����ڴ��̂1��r&�����|/�Uz׺R�B�J���ed�-��~дt��k�4
�����'KL�e5*�n�p�՜wnj���Y����tn���M��W�5\`�qyц��J�����ܴ��}��� p�u� �x-���sX;ߠ��&��w%�6��S�:�w&�f'P�괧Ơ9��R�B_�cOW���V?��z�������=N�B�мUnF_���a�5��~���z����^7�W��}� ��^�t�\`دM@�ו@k�Jj�(������$�)��dl�Fe����V��(�]�{L�ؔ���n��zm2��S�(�s7u�;����N��ڽ@m$rR�%������PPd���SP y��;�����㆜� ��t��/�B���z4�&�-���/���/ ����3�������,m=�\\"m�8���17�
;FR�>���׳y�As���IKаS����� 4V������;v���̓Z2Iɼ*�
�a���?��6��aLϷ1�ŏ��C�_x�c��yH���g@\\\\S i�AH>��aM�f��P�����&��c�*M00T} �~'��w���C^�^��'�/�~9M",
"my_python_module/submodule/py.typed": "
",
}
Expand Down
4 changes: 3 additions & 1 deletion test/srcmak.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ test('python + different entrypoint + submodule', async () => {
await mkdtemp(async target => {
await srcmak(source, {
entrypoint: 'different/entry.ts',
packageName: 'python.package',
python: {
outdir: target,
moduleName: 'my_python_module.submodule',
moduleName: 'my-python_module.submodule',
},
});

Expand Down Expand Up @@ -96,6 +97,7 @@ test('java + different entrypoint', async () => {
await mkdtemp(async target => {
await srcmak(source, {
entrypoint: 'different/entry.ts',
packageName: 'java.package',
java: {
outdir: target,
package: 'hello.world',
Expand Down
Loading
⚡