JamieMason / shrinkpack

Fast, resilient, reproducible builds with npm install.
https://www.npmjs.com/package/shrinkpack
MIT License
793 stars 38 forks source link

Only consider files with '.tar' and '.tgz' extensions in bundle logic #82

Closed rhiadj closed 6 years ago

rhiadj commented 7 years ago

As part of a project I'm working on we'd like to use shrinkpack - however we'd like to use a git submodule in order to store the node packages in a separate repository.

This has several advantages - the main two being:

With this in mind we set up the repo as follows:

codebase
    |
    +--- .git 
    +--- README.md
    +--- package.json
    +--- node_shrinkwrap  <<  this is a git submodule - points to another repo hence has it's own .git
            |
            +--- .git
            +--- README.md

However we can't do this currently as whenever we run shrinkpack it removes the .git and README.md (and any other file) inside node_shrinkwrap as part of the deletion logic:

$ shrinkpack

> codebase@0.1.0 shrinkpack /Workspace/codebase
> shrinkpack --keep-optional

- .git
- README.md
+ ansi-regex-0.1.0.tar
+ strip-ansi-0.2.2.tar
... etc

In order to work around this I've tweaked the bundle logic slightly so that it only considers files ending in .tar and .tgz during various operations.

Interested to get your thoughts on this?

Notes: I tried to run the tests (on a vanilla clone) but got cryptic errors so wasn't able to add any additional tests for this new logic - or indeed check if any were failing (sorry about this I did try!).

Thanks

JamieMason commented 7 years ago

Thanks @rhiadj, very happy to include this πŸ‘ I'll pick it up when I next time I update shrinkpack.

rhiadj commented 7 years ago

Brilliant! Thanks @JamieMason - much appreciated πŸ‘

JamieMason commented 7 years ago

Sorry for the wait, this will make it in eventually :+1:

JamieMason commented 7 years ago

When finished, the new version that supports npm 5 should account for this: https://github.com/JamieMason/shrinkpack/blob/dev/src/lib/get-unused-tars.js#L6. Anything that isn't a tar or tgz will be ignored.

rhiadj commented 7 years ago

Great stuff - thanks for the update @JamieMason

Don't worry too much re the wait - I've been using a cloned/patched version from my repo for now but will def move back to your mainline release once this is in.

As always - much appreciated and keep up the fantastic work! This tool has saved me countless hours already!

JamieMason commented 6 years ago

Because #83 is blocked I thought I'd look at releasing this and some other small changes while we wait.

Unfortunately though the build is having a strange issue running npm install -g npm@4.4.3 in Travis and I'm now on holiday for a week. I will pick this back up when I return, thanks.

rhiadj commented 6 years ago

@JamieMason no worries at all - as it happens I've jumped ship to yarn now (which has similar functionality built in) so this isn't as important as it was. Obviously feel free to continue this as is/was. I'm very grateful for all your help - best of luck for the future!