Error resolving symlink after upgrade to Gulp v5

[hangy]

Before you open this issue, please complete the following tasks:

• [x] use the search bar at the top of the page to search this repository for similar issues or discussions that have already been opened.
• [ ] if you are looking for help from the gulp team or community, open a discussion.
• [ ] if you think there is a problem with the plugin you're using, open a discussion.
• [x] if you think there is a bug in our code, open this issue.

What were you expecting to happen?

We have a few symlinked icons in our repo. The symlinked files should have been processed by the pipe, and written to the dest.

What actually happened?

After migrating from Gulp v4.0.2 to v5.0.0, Gulp claims that the files do not exist.

Please give us a sample of your gulpfile

The full file is available on GitHub. The main task relevant to this issue could be boiled down to this:

import { dest, src } from "gulp";
import svgmin from "gulp-svgmin";

export function icons() {
  return src("*.svg", { cwd: "./icons" }).
    pipe(
      svgmin({
        // @ts-ignore
        configFile: "icons/svgo.config.js",
      })
    ).
    pipe(dest("./html/images/icons/dist"));
}

Terminal output / screenshots

#22 2.076 [19:15:24] Starting 'icons'... #22 2.102 [19:15:24] 'icons' errored after 26 ms #22 2.103 [19:15:24] Error: ENOENT: no such file or directory, lstat 'material/baseline-account_box-24px.svg' #22 2.104 [19:15:24] Finished 'default' after 49 ms

(GitHub Actions log)

Please provide the following information:

• OS & version [e.g. MacOS Catalina 10.15.4]: GitHub Actions (ubuntu-latest)
• node version (run node -v): v20.15.0
• npm version (run npm -v): 10.7.0
• gulp version (run gulp -v): CLI version: 3.0.0, Local version: 5.0.0

Additional information

Gulp v4 performs this task just fine. The icons are definitely available in the repo: https://github.com/openfoodfacts/openfoodfacts-server/tree/e33bea5931cab7f321db55349290137b3551ad09/icons

[macgyver]

I too was surprised after updating gulp to see that it now chokes on symlinks

[mcspr]

I believe the issue is w/ vinyl-fs handling of symlinks, introduced a while ago https://github.com/gulpjs/vinyl-fs/blame/02e9373d05fd783caeb9223acfacf2c032ca8710/lib/file-operations.js#L164

Patch introducing it is quite large, so I am not sure of the specific intention behind the change. Main issue is that it does ignore the fact that readlink returns raw value of the link and does not actually resolve the path of the file.

With a small test task

export function testme() {
  return pipeline(
    src("*.svg", { cwd: "./source/icons" }),
    dest("./build/icons")));
}

For example, if there is only one ./source/icons/source.svg file and the rest are symlinked to it

  fs.readlink("/abs/path/source/icons/foo.svg", ...)

But... since it returns just source.svg, fs.lstat("source.svg", ...) fails with ENOENT

Simply disabling this handler works around the issue. Destination would contain copy of the symlink

-    src("*.svg", { cwd: "./source/icons" }),
+    src("*.svg", { resolveSymlinks: false, cwd: "./source/icons" }),

Similarly, naively patching vinyl-fs onReadlink callback. Destination would contain copy of the source.svg for every symlink

diff --git a/node_modules/vinyl-fs/lib/file-operations.js b/node_modules/vinyl-fs/lib/file-operations.js
index 81f21732..9489fbd7 100644
--- a/node_modules/vinyl-fs/lib/file-operations.js
+++ b/node_modules/vinyl-fs/lib/file-operations.js
@@ -1,5 +1,6 @@
 'use strict';

+var nodepath = require('path');
 var fs = require('graceful-fs');
 var date = require('value-or-function').date;
 var Writable = require('streamx').Writable;
@@ -169,6 +169,10 @@ function findSymlinkHardpath(path, callback) {
       return callback(readlinkErr);
     }

+    if (!nodepath.isAbsolute(resolvedPath)) {
+      resolvedPath = nodepath.join(nodepath.dirname(path), resolvedPath);
+    }
+
     fs.lstat(resolvedPath, onLstat);

     function onLstat(lstatErr, stat) {

edit: Or, use realpath. gulpjs/vinyl-fs#333 commit replaced it with the readlink + lstat combo - https://github.com/gulpjs/vinyl-fs/pull/333/commits/ad46d76660f3cbd48f7be5e7db99552b51b9ddac

-  fs.readlink(path, onReadLink);
+  fs.realpath(path, onReadLink); 

Depending on what behaviour is deemed correct.

And, this should probably go to vinyl-fs repo issue tracker as well? I am just not sure what is correct or broken here, and how much change is required.