Cyclic symlinks are followed indefinitely until crashing

[fzyzcjy]

Hi thanks for the lib! When I use build_runner it has bugs as follows. It just follows .symlinks and .dart_tool folder infinitely, then filename is too long.

But IMHO, build_runner should not look at symlinks and dart_tool folders!

flutter pub get && flutter packages pub run build_runner watch --delete-conflicting-outputs 
Running "flutter pub get" in proj...                                3.0s
[INFO] Generating build script...
[INFO] Generating build script completed, took 377ms

[WARNING] Deleted previous snapshot due to missing asset graph.
[INFO] Creating build script snapshot......
[INFO] Creating build script snapshot... completed, took 9.4s

[INFO] There was output on stdout while compiling the build script snapshot, run with `--verbose` to see it (you will need to run a `clean` first to re-snapshot).

[INFO] Setting up file watchers...
[INFO] Setting up file watchers completed, took 12ms

[INFO] Waiting for all file watchers to be ready...
[SEVERE] Error from directory watcher for package:yplusplus

If you see this consistently then it is recommended that you enable the polling file watcher with --use-polling-watcher.


You have hit a bug in build_runner
Please file an issue with reproduction steps at https://github.com/dart-lang/build/issues


FileSystemException: Directory listing failed, path = '/Users/tom/QAPMain/Development/rzzsdxx_frontend/proj/ios/.symlinks/plugins/vision_utils/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/1/example/ios/.symlinks/plugins/vision_utils/.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/vision_utils/0/example/ios/.symlinks/plugins/vision_utils/example/ios/.symlinks/plugins/sentry_flutter/test/mo' (OS Error: File name too long, errno = 63)
Instance of '_StringStackTrace'

pub finished with exit code 1

i.e.

[jakemac53]

This looks like a bug in the watcher package. The build_runner tool watches the root package dir, and I don't think its really avoidable for us.

But I think that its probably accurate that the watcher package should not follow cyclic symlinks. It might be fairly expensive to detect though.

[DamonChen117]

too many bugs, regret to use flutter at the beginning

[JosephNK]

create a build.yaml file in the root folder and write it using exclude .

targets:
  $default:
    sources:
      exclude:
        # Example that excludes intellij's swap files
        - **/*___jb_tmp___

https://github.com/dart-lang/build/blob/master/docs/faq.md#how-do-i-avoid-running-builders-on-unnecessary-inputs

[bwilkerson]

The root of the problem is in the way the watcher is using Directory.list. If both of the arguments are true (which is how it's being used today) the Directory.list has an exponential algorithm (dart-lang/sdk#61407).

But I think that its probably accurate that the watcher package should not follow cyclic symlinks. It might be fairly expensive to detect though.

I think it would be fairly easy to not follow cyclic links. All we'd need to do is set followLinks to false and then implement any recursion into symbolic links that we want (and not where when we find a cyclic link).

I'm guessing that this would be a breaking change, so we might need an optional argument to the watcher to change its behavior. What I would like is to have a mode where the file watcher doesn't follow any symbolic links. I'm not actually sure why we would ever want to follow links, so I'm asking for insights.

@jakemac53 @davidmorgan

[srawlins]

I'm guessing that this would be a breaking change, so we might need an optional argument to the watcher to change its behavior.

Do you mean that changing the behavior to visit all directories recursively, but only once per directory, instead of potentially many times per directory, would be breaking? I can't think of a good reason someone would depend on that old behavior, haha.

I'm not actually sure why we would ever want to follow links, so I'm asking for insights.

If by "we" you mean "any user of the watcher package," there are plenty of reasons people might want to use symlinks. They're a feature of the OS, and it would be detrimental to not support them at all. It would be like saying the watcher package can only read EBCDIC-encoded files, and no ASCII or UTF-8 files. The watcher package isn't just used for watching Dart source files; it can be used for watching any files on disk.

[davidmorgan]

Hmmm, tricky. I don't like the awkward failure mode but don't see an obvious fix that's better than doing nothing.

Using symlinks in a good way seems useful--for example building a symlink tree to some real output folder. And that would work just fine today. So that seems like a useful feature.

I don't really understand why you would want to create cyclic symlinks, that seems like a bad idea for lots of reasons. Is there a reasonable use case for it that makes it worth doing extra work / paying a runtime penalty to support?

[bwilkerson]

Do you mean that changing the behavior to visit all directories recursively, but only once per directory, instead of potentially many times per directory, would be breaking? I can't think of a good reason someone would depend on that old behavior, haha.

I can't either, but if someone is depending on it then changing it would be a breaking change. Unfortunately sometimes people depend on broken behavior in a way that makes fixing it break them.

But, no, I wasn't talking about changing the way Directory.list works, I was only talking about changing the way the DirectoryWatcher works. We could stop following symlinks, but that might be breaking depending on how the OS watchers are being used. I have a meeting later today to understand better how it all works and whether this would, in fact, be breaking.

If by "we" you mean ...

I think I was thinking about the analysis server.

I don't really understand why you would want to create cyclic symlinks, that seems like a bad idea for lots of reasons. Is there a reasonable use case for it that makes it worth doing extra work / paying a runtime penalty to support?

"Reasonable" is debatable, but yes, there's an actual user who's running into this because flutter pub get is creating cyclic symlinks. The only support we need to add to the analysis server is to have a way for the DirectoryWatcher to stop following symlinks when watching files, and that will negate all of the runtime penalty that I'm aware of.

[davidmorgan]

"Reasonable" is debatable, but yes, there's an actual user who's running into this because flutter pub get is creating cyclic symlinks.

Still trying to understand--why does it create cyclic symlinks?

[bwilkerson]

Sorry, I should have provided more details.

When flutter pub get runs in a package that is using one or more plugins it creates a (hidden) directory that contains symlinks to each of those plugins. In this particular case, the user is developing a Flutter plugin, and they decided to create some examples of how to use their plugin. The examples are in subdirectories inside the package containing the plugin. When they run flutter pub get in the example directories it creates symlinks back to the root of the plugin package, creating the circularity.

[davidmorgan]

Hmm. Possibly that's a bug in the symlink creation?

As far as I can find the intention is to symlink to a package under the pub cache so that they are relative and not absolute, first paragraph here

flutter/flutter#41146

if the package is included as a relative path dependency then it's already possible to use a relative path, there is no need to symlink it.

This sounds a bit like the "good symlink use cases" vs "bad symlink use cases": the intended use case here is to stage packages from the pub cache under the current package, but it's also being used to stage packages that are not in the pub cache and so can have nested packages and create loops?