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.

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

Jump to bottom

ubergen: monocdk/aws-cdk-lib include files that are .npmignored in submodules #12292

Closed
eladb opened this issue Dec 31, 2020 · 1 comment · Fixed by #13642
Closed

ubergen: monocdk/aws-cdk-lib include files that are .npmignored in submodules #12292

eladb opened this issue Dec 31, 2020 · 1 comment · Fixed by #13642
Assignees
@rix0rrr
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort monocdk Related to the monocdk package p1

Comments

@eladb
Copy link
Contributor

eladb commented Dec 31, 2020

The .npmignore files in monocdk submodules are not respected since there could be only one npmignore in an npm package (the root one). This means that the monocdk module includes files that are not supposed to be there.

Reproduction Steps

For example, monocdk/lib/lambda-layer-awscli submodule includes a layer directory which is .npmignored in the normal module.

What did you expect to happen?

ubergen needs to process the .npmignore files in submodules and either exclude those files from the staging of monocdk or "hoist" them into the top-level npmignore files of monocdk (with the proper subdirectory prefix).

What actually happened?

The monocdk library bundles files that are not supposed to be included.

Environment

  • CDK CLI Version : 1.81.0
  • Framework Version: 1.81.0
  • Node.js Version: N/A
  • OS : N/A
  • Language (Version): N/A

This is 🐛 Bug Report
@eladb eladb added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 31, 2020
@eladb eladb changed the title monnocdk: submodule npmignore files are not respected ubergen: submodule npmignore files are not respected Dec 31, 2020
@eladb eladb changed the title ubergen: submodule npmignore files are not respected ubergen: submodule npmignore files are not respected in the mono package Dec 31, 2020
@eladb eladb changed the title ubergen: submodule npmignore files are not respected in the mono package ubergen: monocdk/aws-cdk-lib include files that are .npmignored in submodules Dec 31, 2020
@nija-at nija-at added aws-cdk-lib Related to the aws-cdk-lib package monocdk Related to the monocdk package effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 7, 2021
@nija-at nija-at assigned rix0rrr and unassigned nija-at Mar 16, 2021
rix0rrr added a commit that referenced this issue Mar 17, 2021
@rix0rrr
chore(ubergen): copy .npmignore into monopackage
a54fabe 
`.npmignore` did not use to be copied into the directories of the monopackage,
leading to files that *ought* to have been excluded to not be excluded.

Remove the file from the list of files that doesn't get copied to fix this.

This fix is not along the lines of the method suggested in the linked issue,
but it does address the original problem.

Fixes #12292.
@rix0rrr rix0rrr mentioned this issue Mar 17, 2021
Merged
@mergify mergify bot closed this as completed in #13642 Mar 18, 2021
mergify bot pushed a commit that referenced this issue Mar 18, 2021
@rix0rrr
chore(ubergen): copy .npmignore into monopackage (#13642)
95438b5 
`.npmignore` did not use to be copied into the directories of the monopackage,
leading to files that *ought* to have been excluded to not be excluded.

Remove the file from the list of files that doesn't get copied to fix this.

This fix is not along the lines of the method suggested in the linked issue,
but it does address the original problem.

Fixes #12292.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

eladb pushed a commit that referenced this issue Mar 24, 2021
@rix0rrr
chore(ubergen): copy .npmignore into monopackage (#13642)
1b3731d 
`.npmignore` did not use to be copied into the directories of the monopackage,
leading to files that *ought* to have been excluded to not be excluded.

Remove the file from the list of files that doesn't get copied to fix this.

This fix is not along the lines of the method suggested in the linked issue,
but it does address the original problem.

Fixes #12292.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
@rix0rrr @hollanddd
chore(ubergen): copy .npmignore into monopackage (aws#13642)
b783680 
`.npmignore` did not use to be copied into the directories of the monopackage,
leading to files that *ought* to have been excluded to not be excluded.

Remove the file from the list of files that doesn't get copied to fix this.

This fix is not along the lines of the method suggested in the linked issue,
but it does address the original problem.

Fixes aws#12292.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rix0rrr rix0rrr

Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort monocdk Related to the monocdk package p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(ubergen): copy .npmignore into monopackage aws/aws-cdk
3 participants
@rix0rrr @eladb @nija-at