Scripts: `plugin-zip` does not include root-level files #40528

admturner · 2022-04-21T22:26:39Z

Description

When I run the command wp-scripts plugin-zip the resulting zip file does not include root-level files, only directories (and their containing files). I expect the resulting zip file to include either the defaults (the entrypoint, Readme, etc.) as described in the documentation, or the files specified in the package.json "files": [] list. Instead the zip file contains only the build/ directory files (or other directories specified in package.json) and none of the root-level files.

Step-by-step reproduction instructions

Create a basic plugin with a src/ directory, an entrypoint, and some other root-level files such as README and CHANGELOG.
Install the @wordpress/scripts package.
Run wp-scripts build to generate the build/ directory.
Run wp-scripts plugin-zip.
Open the resulting zip file to see the error.

Screenshots, screen recording, code snippet

With a test plugin with the directory structure:

build/index.asset.php
build/index.js
src/index.js
CHANGELOG.md 
LICENSE.md
README.md
package-lock.json
package.json
scripts-test.php

Running wp-scripts plugin-zip generates a zip file with only:

build/index.asset.php
build/index.js

I would expect it to generate:

build/index.asset.php
build/index.js
CHANGELOG.md 
LICENSE.md
README.md
package.json
scripts-test.php

The terminal output suggests everything worked:

> wp-scripts plugin-zip

Creating archive for `scripts-test` plugin... 🎁

Using Plugin Handbook best practices to discover files:

  Adding `CHANGELOG.md`.
  Adding `LICENSE.md`.
  Adding `README.md`.
  Adding `scripts-test.php`.
  Adding `build/index.asset.php`.
  Adding `build/index.js`.

Done. `scripts-test.zip` is ready! 🎉

But those root-level files aren't actually included it the zip file.

However, if I edit the plugin-zip.js file in my node_modules and replace:

zip.addLocalFile( file, dirname( file ) );

with:

if ( '.' !== dirname( file ) ) {
    zip.addLocalFile( file, dirname( file ) );
} else {
    zip.addLocalFile( file );
}

Then it works as expected. Maybe the problem is that zip.addLocalFile is failing silently when it doesn't recognize '.' as valid.

Environment info

@wordpress/scripts v22.5.0
Node.js v14.19.1
npm v6.14.16
WSL 2 on Windows 10

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

The text was updated successfully, but these errors were encountered:

seebz · 2022-04-25T09:40:06Z

I confirm this issue.
Root files are stored in a . (dot) folder.

How to reproduce :

Generate the Gutenride plugin

$ npx @wordpress/create-block gutenpride
...

Move into the plugin folder
```
$ cd gutenpride
```

Create the zip file

$ npm run plugin-zip

> gutenpride@0.1.0 plugin-zip
> wp-scripts plugin-zip

Creating archive for `gutenpride` plugin... 🎁

Using Plugin Handbook best practices to discover files:

  Adding `gutenpride.php`.
  Adding `readme.txt`.
  Adding `build/block.json`.
  Adding `build/index.asset.php`.
  Adding `build/index.css`.
  Adding `build/index.js`.
  Adding `build/style-index.css`.

Done. `gutenpride.zip` is ready! 🎉

List files in archive

$ unzip -l gutenpride.zip 
Archive:  gutenpride.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
      885  2022-04-25 11:12   ./gutenpride.php
     1826  2022-04-25 11:12   ./readme.txt
      446  2022-04-25 11:13   build/block.json
      150  2022-04-25 11:13   build/index.asset.php
       57  2022-04-25 11:13   build/index.css
     1513  2022-04-25 11:13   build/index.js
       83  2022-04-25 11:13   build/style-index.css
---------                     -------
     4960                     7 files

We can see than root files are in a . folder

skorasaurus · 2022-04-26T19:12:06Z

ran into this a few minutes ago as well; can confirm the above that it lists all of the files as being added but they are not; great, detailed report @admturner

amustaque97 · 2022-05-13T19:53:20Z

Hi 👋, thank you for reporting this issue. I'm able to reproduce the error. I tried in my local and found a solution for the mentioned issue. In our package.json file we use add something like this:

{
    "files": [ "*" ]
}

I believe the issue with the package npm-packlist, we are using. They should add in their documentation saying to include all files of the current directory we can use * as well. I would like to share a complete list of ignore files supported by npm-packlist package.
https://github.com/npm/npm-packlist/blob/7e1fc7106c0b5b1c76f78154c94a28a9f0f099d4/lib/index.js#L54-L80

I'm sharing a screencast for reference.

Screen.Recording.2022-05-14.at.1.13.23.AM.mov

We can close this issue. Let me know what do you think?

admturner · 2022-05-23T22:35:47Z

Thanks, @amustaque97! I tried adding "files": [ "*" ] to my package.json file and I do see the same results you do, but it isn't changed from the original behavior. The problem isn't that it's ignoring files like .gitignore; the problem is that it's placing wanted root files like the plugin entrypoint in a dot-directory. As @seebz noted, the root files are being generated, but they're stored in a . (dot) folder instead of at the root level. I can see the ./ files using unzip -l but when I extract the files all of those ./ files aren't there.

Unless I'm missing something and this is the expected behavior, then it looks like the issue might be with the implementation of addLocalFile in the scripts/plugin-zip.js file of the @wordpress/scripts package. If I edit that file and replace (line 52):

zip.addLocalFile( file, dirname( file ) );

with:

if ( '.' !== dirname( file ) ) {
    zip.addLocalFile( file, dirname( file ) );
} else {
    zip.addLocalFile( file );
}

then when I run npm run plugin-zip I get;

 ~/w/w/w/w/p/gutenpride  unzip -l gutenpride.zip                                       303ms  Mon May 23 15:30:33 2022
Archive:  gutenpride.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
      447  2022-05-23 15:08   build/block.json
      139  2022-05-23 15:08   build/index.asset.php
       57  2022-05-23 15:08   build/index.css
     1513  2022-05-23 15:08   build/index.js
       83  2022-05-23 15:08   build/style-index.css
      885  2022-05-23 15:07   gutenpride.php
     1826  2022-05-23 15:07   readme.txt
---------                     -------
     4950                     7 files

By omitting the second argument from addLocalFile for root-level files it prevents creating a ./ directory for those files.

gziolo · 2022-05-24T07:44:39Z

if ( '.' !== dirname( file ) ) {
    zip.addLocalFile( file, dirname( file ) );
} else {
    zip.addLocalFile( file );
}

It looks like a simple fix to apply. Is anyone willing to open a PR?

amustaque97 · 2022-05-25T20:45:22Z

Hi @admturner , I'm really sorry but I'm not able to reproduce the same issue. Neither I'm able to create a directory whose name is . since it's a system reserved name nor programatically. Also, we should not forget that . and .. has special meaning in *NIX environments.

Since your machine is windows I actually tried to reproduce the same issue in Github pipeline with windows environment. No luck so far. Here is the link of the PR I created #41288

Here is the link of the Github pipeline output: https://github.com/WordPress/gutenberg/runs/6580805114?check_suite_focus=true#step:4:200

Here are a few screenshot of the error I get:
If I use GUI to create a folder

Programatically:

At last, I would request you to share zip file via some link or screencast so that I can also see and understand what's really going on. cc @seebz @skorasaurus
looking forward to hearing from you.
thank you.

admturner · 2022-05-27T15:17:24Z

I'm happy to provide a screencast; I'll do that as soon as I get a chance. To clarify my environment as well, while I'm on Windows I work in WSL2 using Ubuntu.

seebz · 2022-05-29T09:27:40Z

@amustaque97 commented:

At last, I would request you to share zip file via some link or screencast so that I can also see and understand what's really going on.

Here is the archive I just build on Xubuntu : gutenpride.zip

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.4 LTS
Release:	20.04
Codename:	focal

$ node --version
v16.15.0

$ npm --version
8.5.5

gziolo · 2022-05-29T11:21:06Z

It looks like the issue is isolated to some specific platform setup. The offending line would be:

gutenberg/packages/scripts/scripts/plugin-zip.js

Line 52 in 0c901d5

zip.addLocalFile( file, dirname( file ) );

When the issue happens then dirname( file ) resolves to .. I spent some time looking at the implementation in adm-zip and I isolated this case to the following lines:

https://github.com/cthackers/adm-zip/blob/bebbabf600a6ae5d64ee31cbc89a1274bd1d8c1f/adm-zip.js#L244

zipPath = zipPath ? fixPath(zipPath) : "";

https://github.com/cthackers/adm-zip/blob/bebbabf600a6ae5d64ee31cbc89a1274bd1d8c1f/adm-zip.js#L81-L85

function fixPath(zipPath) {
    const { join, normalize, sep } = pth.posix;
    // convert windows file separators and normalize
    return join(".", normalize(sep + zipPath.split("\\").join(sep) + sep));
}

Now, based on the reports I can see that people have the following files listed:

  Adding `CHANGELOG.md`.
  Adding `LICENSE.md`.
  Adding `README.md`.
  Adding `scripts-test.php`.

As far as I can tell from debugging dirname( 'CHANGELOG.md' ) resolves to '.'. So the question is why fixPath in adm-zip would produce different results. It's also very likely that we could fix it on our end by never passing '.' (replacing it with '' to addLocalFile which should do the trick.

Either one of the proposed fixes from this issue should resolve it or:

const zipDirectory = dirname( file );
zip.addLocalFile( file, zipDirectory !== '.' ? zipDirectory : '' );

I can confirm it works the same way on macOs as before.

amustaque97 · 2022-05-30T13:27:15Z

Kudos to you for doing in depth investigation. Only reason why I wanted to double check is to understand testing instructions and changes verification. Let’s fix it on our end and make sure nothing breaks. I will submit a PR in sometime.

Thanks once again @gziolo 😃😃

gziolo · 2022-06-01T11:35:58Z

I have just landed #41439 from @amustaque97. We should have the regular npm publishing happening today (or maybe later this week due to WordCamp EU). I hope this change resolve the issue for everyone 😄

gziolo added Needs Testing Needs further testing to be confirmed. [Package] Scripts /packages/scripts labels Apr 22, 2022

skorasaurus added [Type] Bug An existing feature does not function as intended and removed Needs Testing Needs further testing to be confirmed. labels Apr 26, 2022

gziolo added Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts labels Apr 27, 2022

gziolo added this to To do in Core JS Apr 27, 2022

amustaque97 self-assigned this May 30, 2022

amustaque97 mentioned this issue May 30, 2022

Scripts: plugin-zip to include root path files #41439

Merged

github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 30, 2022

gziolo closed this as completed in #41439 Jun 1, 2022

Core JS automation moved this from To do to Done Jun 1, 2022

priethor removed the [Status] In Progress Tracking issues with work in progress label May 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scripts: `plugin-zip` does not include root-level files #40528

Scripts: `plugin-zip` does not include root-level files #40528

admturner commented Apr 21, 2022

seebz commented Apr 25, 2022

skorasaurus commented Apr 26, 2022

amustaque97 commented May 13, 2022

admturner commented May 23, 2022 •

edited

gziolo commented May 24, 2022

amustaque97 commented May 25, 2022 •

edited

admturner commented May 27, 2022

seebz commented May 29, 2022

gziolo commented May 29, 2022 •

edited

amustaque97 commented May 30, 2022

gziolo commented Jun 1, 2022

Scripts: plugin-zip does not include root-level files #40528

Scripts: plugin-zip does not include root-level files #40528

Comments

admturner commented Apr 21, 2022

Description

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

Environment info

Please confirm that you have searched existing issues in the repo.

Please confirm that you have tested with all plugins deactivated except Gutenberg.

seebz commented Apr 25, 2022

skorasaurus commented Apr 26, 2022

amustaque97 commented May 13, 2022

admturner commented May 23, 2022 • edited

gziolo commented May 24, 2022

amustaque97 commented May 25, 2022 • edited

admturner commented May 27, 2022

seebz commented May 29, 2022

gziolo commented May 29, 2022 • edited

amustaque97 commented May 30, 2022

gziolo commented Jun 1, 2022

Scripts: `plugin-zip` does not include root-level files #40528

Scripts: `plugin-zip` does not include root-level files #40528

admturner commented May 23, 2022 •

edited

amustaque97 commented May 25, 2022 •

edited

gziolo commented May 29, 2022 •

edited