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.

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

pack: include contents of directories in files field (#3175) #24

Merged
merged 1 commit into from
Apr 29, 2017

Conversation

GulajavaMinistudio
Copy link
Owner

  • Use tar-fs instead of tar-stream in yarn pack (and fix packed emojis)

This lets tar-fs do the header construction for us.

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 --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:

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#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#2498
Fixes yarnpkg#2942
Fixes yarnpkg#2851

Includes and closes yarnpkg#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#3175 (comment)

  • Account for backslashes in paths when filtering files

See yarnpkg#3175 (comment)

  • Use path.sep instead of slashes

See yarnpkg#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
yarnpkg#3175 (comment)

  • Keep pattern in IgnoreFilter, use with minimatch() in matchesFilter

This should help with Windows support. See
yarnpkg#3175 (comment)

  • Update ignoreLinesToRegex tests

Summary

Test plan

* 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
@GulajavaMinistudio GulajavaMinistudio merged commit de56bcb into GulajavaMinistudio:master Apr 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants