Use tar-fs instead of tar-stream in yarn pack (and fix packed emojis)
#3170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This lets tar-fs do the [header construction] for us. [header construction]: https://github.com/mafintosh/tar-fs/blob/b79d82a79c5e21f6187462d7daaba1fc03cdd1de/index.js#L101-L127 I tested this by comparing the output of this command before and after the change: ./bin/yarn.js pack >/dev/null && tar tvf yarn-v0.24.0-0.tgz | sort && wc -c < yarn-v0.24.0-0.tgz && rm *tgz Here's the diff between the outputs: ```diff diff --git a/before.txt b/after.txt index 5e7f370e..5565a808 100644 --- a/before.txt +++ b/after.txt @@ -7,13 +7,13 @@ -rw-r--r-- 0 0 0 657 Mar 4 07:19 package/Dockerfile.dev -rw-r--r-- 0 0 0 1346 Mar 4 07:19 package/LICENSE -rw-r--r-- 0 0 0 1789 Apr 17 15:10 package/jenkins_jobs.groovy --rw-r--r-- 0 0 0 3061 Mar 4 07:19 package/README.md --rw-r--r-- 0 0 0 3438 Apr 17 16:18 package/package.json +-rw-r--r-- 0 0 0 3057 Mar 4 07:19 package/README.md +-rw-r--r-- 0 0 0 3430 Apr 17 16:18 package/package.json -rwxr-xr-x 0 0 0 42 Mar 4 07:19 package/bin/yarnpkg -rwxr-xr-x 0 0 0 172 Mar 4 07:19 package/bin/node-gyp-bin/node-gyp -rwxr-xr-x 0 0 0 906 Mar 4 07:19 package/bin/yarn -rwxr-xr-x 0 0 0 929 Apr 10 15:59 package/bin/yarn.js drwxr-xr-x 0 0 0 0 Apr 10 15:59 package/bin drwxr-xr-x 0 0 0 0 Apr 17 17:04 package drwxr-xr-x 0 0 0 0 Mar 4 07:19 package/bin/node-gyp-bin - 6206 + 6177 ``` I extracted the tarballs into `./package-master` and `./package-feature`, then diffed them to find that this change has the side effect of fixing emojis in the tarball. You can see examples of the broken emoji here: * https://unpkg.com/[email protected]/package.json * https://unpkg.com/[email protected]/README.md ```diff diff --git a/package-master/README.md b/package-feature/README.md index aabfc24f..6aff13d8 100644 --- a/package-master/README.md +++ b/package-feature/README.md @@ -30,7 +30,7 @@ * **Network Performance.** Yarn efficiently queues up requests and avoids request waterfalls in order to maximize network utilization. * **Network Resilience.** A single request failing won't cause an install to fail. Requests are retried upon failure. * **Flat Mode.** Yarn resolves mismatched versions of dependencies to a single version to avoid creating duplicates. -* **More emojis.** ð��� +* **More emojis.** 🐈 ## Installing Yarn diff --git a/package-master/package.json b/package-feature/package.json index c89ad7a6..8e7e3bc7 100644 --- a/package-master/package.json +++ b/package-feature/package.json @@ -4,7 +4,7 @@ "version": "0.24.0-0", "license": "BSD-2-Clause", "preferGlobal": true, - "description": "ð��¦ð��� Fast, reliable, and secure dependency management.", + "description": "📦🐈 Fast, reliable, and secure dependency management.", "dependencies": { "babel-runtime": "^6.0.0", "bytes": "^2.4.0", ```
josephfrazier
added a commit
to josephfrazier/yarn
that referenced
this pull request
Apr 18, 2017
This makes it so that you don't have to put '/**' after a directory in the `files` field of package.json to ensure that the contents of the directory will be published. Fixes yarnpkg#2498 Fixes yarnpkg#2942 Fixes yarnpkg#2851 Includes and closes yarnpkg#3170
|
Is it fine if I close this PR in favor of #3175 (also one of your PRs)? The later seems to include both changesets. |
|
Ah, missed your note in the other PR :) Gonna close this one, then |
arcanis
pushed a commit
that referenced
this pull request
Apr 28, 2017
* Use tar-fs instead of tar-stream in `yarn pack` (and fix packed emojis) This lets tar-fs do the [header construction] for us. [header construction]: https://github.com/mafintosh/tar-fs/blob/b79d82a79c5e21f6187462d7daaba1fc03cdd1de/index.js#L101-L127 I tested this by comparing the output of this command before and after the change: ./bin/yarn.js pack >/dev/null && tar tvf yarn-v0.24.0-0.tgz | sort && wc -c < yarn-v0.24.0-0.tgz && rm *tgz Here's the diff between the outputs: ```diff diff --git a/before.txt b/after.txt index 5e7f370e..5565a808 100644 --- a/before.txt +++ b/after.txt @@ -7,13 +7,13 @@ -rw-r--r-- 0 0 0 657 Mar 4 07:19 package/Dockerfile.dev -rw-r--r-- 0 0 0 1346 Mar 4 07:19 package/LICENSE -rw-r--r-- 0 0 0 1789 Apr 17 15:10 package/jenkins_jobs.groovy --rw-r--r-- 0 0 0 3061 Mar 4 07:19 package/README.md --rw-r--r-- 0 0 0 3438 Apr 17 16:18 package/package.json +-rw-r--r-- 0 0 0 3057 Mar 4 07:19 package/README.md +-rw-r--r-- 0 0 0 3430 Apr 17 16:18 package/package.json -rwxr-xr-x 0 0 0 42 Mar 4 07:19 package/bin/yarnpkg -rwxr-xr-x 0 0 0 172 Mar 4 07:19 package/bin/node-gyp-bin/node-gyp -rwxr-xr-x 0 0 0 906 Mar 4 07:19 package/bin/yarn -rwxr-xr-x 0 0 0 929 Apr 10 15:59 package/bin/yarn.js drwxr-xr-x 0 0 0 0 Apr 10 15:59 package/bin drwxr-xr-x 0 0 0 0 Apr 17 17:04 package drwxr-xr-x 0 0 0 0 Mar 4 07:19 package/bin/node-gyp-bin - 6206 + 6177 ``` I extracted the tarballs into `./package-master` and `./package-feature`, then diffed them to find that this change has the side effect of fixing emojis in the tarball. You can see examples of the broken emoji here: * https://unpkg.com/[email protected]/package.json * https://unpkg.com/[email protected]/README.md ```diff diff --git a/package-master/README.md b/package-feature/README.md index aabfc24f..6aff13d8 100644 --- a/package-master/README.md +++ b/package-feature/README.md @@ -30,7 +30,7 @@ * **Network Performance.** Yarn efficiently queues up requests and avoids request waterfalls in order to maximize network utilization. * **Network Resilience.** A single request failing won't cause an install to fail. Requests are retried upon failure. * **Flat Mode.** Yarn resolves mismatched versions of dependencies to a single version to avoid creating duplicates. -* **More emojis.** ð��� +* **More emojis.** 🐈 ## Installing Yarn diff --git a/package-master/package.json b/package-feature/package.json index c89ad7a6..8e7e3bc7 100644 --- a/package-master/package.json +++ b/package-feature/package.json @@ -4,7 +4,7 @@ "version": "0.24.0-0", "license": "BSD-2-Clause", "preferGlobal": true, - "description": "ð��¦ð��� Fast, reliable, and secure dependency management.", + "description": "📦🐈 Fast, reliable, and secure dependency management.", "dependencies": { "babel-runtime": "^6.0.0", "bytes": "^2.4.0", ``` * When testing `yarn pack`, use fs.walk instead of fs.readdir This ensures that files inside directories are listed too. * Add failing test for packing directories recursively #2498 * `pack`: include contents of directories in `files` field This makes it so that you don't have to put '/**' after a directory in the `files` field of package.json to ensure that the contents of the directory will be published. Fixes #2498 Fixes #2942 Fixes #2851 Includes and closes #3170 * `pack` test: Use path.join() to create nested path * `path` test: Make output easier to understand Now, we can see just what the expected/actual difference is, rather than just getting a -1 vs 0 from an indexOf test. * `pack`: transform each [ "file-name" ] into [ "file-name", "file-name/**" ], whether it's a file or a folder See #3175 (comment) * Account for backslashes in paths when filtering files See #3175 (comment) * Use `path.sep` instead of slashes See #3175 (comment) * Revert "Use `path.sep` instead of slashes" This reverts commit c2df043. It caused an additional test to fail: https://ci.appveyor.com/project/kittens/yarn/build/2195/job/q5u26f85qlroy533#L3011 * Revert "Account for backslashes in paths when filtering files" This reverts commit 20646f5. I don't think it actually helps, see #3175 (comment) * Keep pattern in IgnoreFilter, use with minimatch() in matchesFilter This should help with Windows support. See #3175 (comment) * Update ignoreLinesToRegex tests
lovelypuppy0607
added a commit
to lovelypuppy0607/yarn
that referenced
this pull request
May 11, 2023
* Use tar-fs instead of tar-stream in `yarn pack` (and fix packed emojis) This lets tar-fs do the [header construction] for us. [header construction]: https://github.com/mafintosh/tar-fs/blob/b79d82a79c5e21f6187462d7daaba1fc03cdd1de/index.js#L101-L127 I tested this by comparing the output of this command before and after the change: ./bin/yarn.js pack >/dev/null && tar tvf yarn-v0.24.0-0.tgz | sort && wc -c < yarn-v0.24.0-0.tgz && rm *tgz Here's the diff between the outputs: ```diff diff --git a/before.txt b/after.txt index 5e7f370e..5565a808 100644 --- a/before.txt +++ b/after.txt @@ -7,13 +7,13 @@ -rw-r--r-- 0 0 0 657 Mar 4 07:19 package/Dockerfile.dev -rw-r--r-- 0 0 0 1346 Mar 4 07:19 package/LICENSE -rw-r--r-- 0 0 0 1789 Apr 17 15:10 package/jenkins_jobs.groovy --rw-r--r-- 0 0 0 3061 Mar 4 07:19 package/README.md --rw-r--r-- 0 0 0 3438 Apr 17 16:18 package/package.json +-rw-r--r-- 0 0 0 3057 Mar 4 07:19 package/README.md +-rw-r--r-- 0 0 0 3430 Apr 17 16:18 package/package.json -rwxr-xr-x 0 0 0 42 Mar 4 07:19 package/bin/yarnpkg -rwxr-xr-x 0 0 0 172 Mar 4 07:19 package/bin/node-gyp-bin/node-gyp -rwxr-xr-x 0 0 0 906 Mar 4 07:19 package/bin/yarn -rwxr-xr-x 0 0 0 929 Apr 10 15:59 package/bin/yarn.js drwxr-xr-x 0 0 0 0 Apr 10 15:59 package/bin drwxr-xr-x 0 0 0 0 Apr 17 17:04 package drwxr-xr-x 0 0 0 0 Mar 4 07:19 package/bin/node-gyp-bin - 6206 + 6177 ``` I extracted the tarballs into `./package-master` and `./package-feature`, then diffed them to find that this change has the side effect of fixing emojis in the tarball. You can see examples of the broken emoji here: * https://unpkg.com/[email protected]/package.json * https://unpkg.com/[email protected]/README.md ```diff diff --git a/package-master/README.md b/package-feature/README.md index aabfc24f..6aff13d8 100644 --- a/package-master/README.md +++ b/package-feature/README.md @@ -30,7 +30,7 @@ * **Network Performance.** Yarn efficiently queues up requests and avoids request waterfalls in order to maximize network utilization. * **Network Resilience.** A single request failing won't cause an install to fail. Requests are retried upon failure. * **Flat Mode.** Yarn resolves mismatched versions of dependencies to a single version to avoid creating duplicates. -* **More emojis.** ð��� +* **More emojis.** 🐈 ## Installing Yarn diff --git a/package-master/package.json b/package-feature/package.json index c89ad7a6..8e7e3bc7 100644 --- a/package-master/package.json +++ b/package-feature/package.json @@ -4,7 +4,7 @@ "version": "0.24.0-0", "license": "BSD-2-Clause", "preferGlobal": true, - "description": "ð��¦ð��� Fast, reliable, and secure dependency management.", + "description": "📦🐈 Fast, reliable, and secure dependency management.", "dependencies": { "babel-runtime": "^6.0.0", "bytes": "^2.4.0", ``` * When testing `yarn pack`, use fs.walk instead of fs.readdir This ensures that files inside directories are listed too. * Add failing test for packing directories recursively yarnpkg/yarn#2498 * `pack`: include contents of directories in `files` field This makes it so that you don't have to put '/**' after a directory in the `files` field of package.json to ensure that the contents of the directory will be published. Fixes yarnpkg/yarn#2498 Fixes yarnpkg/yarn#2942 Fixes yarnpkg/yarn#2851 Includes and closes yarnpkg/yarn#3170 * `pack` test: Use path.join() to create nested path * `path` test: Make output easier to understand Now, we can see just what the expected/actual difference is, rather than just getting a -1 vs 0 from an indexOf test. * `pack`: transform each [ "file-name" ] into [ "file-name", "file-name/**" ], whether it's a file or a folder See yarnpkg/yarn#3175 (comment) * Account for backslashes in paths when filtering files See yarnpkg/yarn#3175 (comment) * Use `path.sep` instead of slashes See yarnpkg/yarn#3175 (comment) * Revert "Use `path.sep` instead of slashes" This reverts commit c2df043343092dcc408f8792ad16eb86cad6ba3a. It caused an additional test to fail: https://ci.appveyor.com/project/kittens/yarn/build/2195/job/q5u26f85qlroy533#L3011 * Revert "Account for backslashes in paths when filtering files" This reverts commit 20646f5d4ac5207dbf929af4e96acebeca29d07f. I don't think it actually helps, see yarnpkg/yarn#3175 (comment) * Keep pattern in IgnoreFilter, use with minimatch() in matchesFilter This should help with Windows support. See yarnpkg/yarn#3175 (comment) * Update ignoreLinesToRegex tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This lets tar-fs do the header construction for us.
Test plan
I tested this by comparing the output of this command before and after
the change:
Here's the diff between the outputs:
I extracted the tarballs into
./package-masterand./package-feature,then diffed them to find that this change has the side effect of
fixing emojis in the tarball. You can see examples of the broken emoji
here: