Add entry.isSymlink method [feat]

[ayushmanchhabra]

Closes: #39

This only identifies if an entry is a symlink. How the symlink is extracted is left up to the developer.

[overlookmotel]

Thanks for doing this. Please ping me when you've completed the work and it's ready for review.

[ayushmanchhabra]

@overlookmotel ready for review

[overlookmotel]

Again, thanks very much for your work on this.

Not had time to review the core implementation yet, but a few things in meantime:

Requested changes

async

isSymlink doesn't need to be an async function. It doesn't do anything async. Can you make it a normal function please?

Naming nit

Fine by me if you want getting an enty's mode to be a separate function. But please can you change the name from _modeFromEntry to just _mode? This is a method of Entry class, so repeating "FromEntry" is redundant (mode of entry from entry?)

Test fixtures

For the tests, could you just create the ZIP including a symlink any add it to the tests/fixtures folder, same as the rest? That'd match the rest of the tests, and remove the need for archiver dev dependency.

Also, I'd like to set up CI to run on Windows and Mac as well as Linux (not asking you to do this, I'll do it). But once that's done, if the ZIP file is just static in the repo, that will catch any oddities with a ZIP created on Windows unzips fine on Windows, but not on Linux etc.

Test fixtures pt 2

My biggest concern with this has been whether the flags/attributes indicating a symlink are a universal standard, or something that different ZIP impls handle differently.

To test this, would you be able to create ZIP files with a few common ZIP programs, e.g. WinZip, Mac OS Archive Utility, and add them all as test fixtures. If tests pass on all those, then that would give me more confidence to merge this.

Tests style

It's important that zip.close() gets called, even in case of an error.

Could you please use the try / finally pattern like in the docs for the test?

I think it's important the tests are written as the library should be used, as often people (and AIs like Github Copilot) use the tests as a "canonical example" of correct usage.

Questions

And 1 question: If a link is a symlink, how does user get the path of the file it links to?

[ayushmanchhabra]

async

Method is no longer async.

Naming nit

Renamed method to _mode.

Test fixtures

I've commited the fixture to the repo and removed Archiver.

Test fixtures pt 2

Need some time to come up with zips created by different utilities. Other than WinZip and MacOS Archive Utility do any other come to mind?

Questions

Symlinks are files with the path of the target file as their content. For example if these are the file paths:

/dir/file.txt
/symlink_to_file.txt

then symlink_to_file.txt will contain

/dir/file.txt

[overlookmotel]

Just to say, this is the only thing we're waiting for to get this merged:

Need some time to come up with zips created by different utilities. Other than WinZip and MacOS Archive Utility do any other come to mind?

But I didn't answer your question. Answer is: I guess 7-Zip and WinRAR would be good to add. From googling, they seem to be the most popular programs beyond the ones you already mentioned. Do you have time for that?

[ayushmanchhabra]

Yup, should be able to add fixtures for 7Zip, WinRaR and MacOS Archive today. Windows handles symlinks in a different way (.lnk file). Will see how to parse that using Node.

[ayushmanchhabra]

Test for MacOS Archive Utility is failing. Looking into this.

To create the zip, I went on a Mac, I created a folder, created a file inside the folder, created an alias for the file and zipped it. Then I sent it over to my pc (linux).

$: npm run jest ./test/symlink.test.js 

> [email protected] jest
> jest ./test/symlink.test.js

 FAIL  test/symlink.test.js
  Identify symlinks
    ✓ reads symlink from NodeJS generated zip file (5 ms)
    ✓ reads symlink from WinZip generated zip file (1 ms)
    ✓ reads symlink from 7-Zip generated zip file (1 ms)
    ✓ reads symlink from WinRAR generated zip file (1 ms)
    ✕ reads symlink from MacOS Archive Utility generated zip file (7 ms)

  ● Identify symlinks › reads symlink from MacOS Archive Utility generated zip file

    expect(received).toEqual(expected) // deep equality

    - Expected  - 2
    + Received  + 2

    @@ -15,12 +15,12 @@
          "fileName": "__MACOSX/MacOS Archive Utility/._.DS_Store",
          "isSymlink": false,
        },
        Object {
          "fileName": "MacOS Archive Utility/file.txt alias",
    -     "isSymlink": true,
    +     "isSymlink": false,
        },
        Object {
          "fileName": "__MACOSX/MacOS Archive Utility/._file.txt alias",
    -     "isSymlink": true,
    +     "isSymlink": false,
        },
      ]

      123 |             }
      124 |
    > 125 |             expect(zipMetadata).toEqual([
          |                                 ^
      126 |                     {fileName: 'MacOS Archive Utility/', isSymlink: false},
      127 |                     {fileName: 'MacOS Archive Utility/file.txt', isSymlink: false},
      128 |                     {fileName: 'MacOS Archive Utility/.DS_Store', isSymlink: false},

      at Object.toEqual (test/symlink.test.js:125:23)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 4 passed, 5 total
Snapshots:   0 total
Time:        0.338 s, estimated 1 s
Ran all test suites matching /.\/test\/symlink.test.js/i.

[overlookmotel]

It would be simpler I think if you removed the .DS_Store files before zipping. They are invisible in Finder, but if you close Finder, and then navigate to the folder in terminal you ca see it with ls -la and delete it with rm.

That may not be the source of the problem, but it will at least reduce irrelevant noise.

[ayushmanchhabra]

Turns out the __MACOSX dir is created during/after zipping. The MacOS alias/symlink format created by MacOS Archive Utility is different from the Linux version. Will have to look into this more deeply. Closing this for now, will reopen once I've figured out how to identify those symlinks.

[overlookmotel]

I'm sorry this is so difficult. Thanks very much for all your efforts.

This was exactly the kind of thing I was worried about - that there is not a consistent cross-platform method of representing symlinks in ZIP files, so it's difficult for us to support it.