Analysis Server double-watches file system

[scheglov]

See https://dart-review.googlesource.com/c/sdk/+/339232/2/pkg/analysis_server/test/domain_analysis_test.dart#195

Apparently we add watchers for both the included directory, and nested package.

[newFile][path: /home/test/AndroidManifest.xml]
[_notifyWatchers]
  /home/test/AndroidManifest.xml isWithin /home
  /home/test/AndroidManifest.xml isWithin /home/test

This is a simple example that demonstrates that PhysicalResourceProvider behaves the same as our MemoryResourceProvider.

import 'package:analyzer/file_system/physical_file_system.dart';

main() {
  PhysicalResourceProvider.INSTANCE
      .getFolder('/Users/scheglov/tmp/20231203')
      .watch()
      .changes
      .listen((event) {
    print('[root][event: $event]');
  });

  PhysicalResourceProvider.INSTANCE
      .getFolder('/Users/scheglov/tmp/20231203/test')
      .watch()
      .changes
      .listen((event) {
    print('[test][event: $event]');
  });
}

When I create a.dart file, both watches are notified.

[root][event: add /Users/scheglov/tmp/20231203/test/a.dart]
[test][event: add /Users/scheglov/tmp/20231203/test/a.dart]

@DanTup do you know if this is an expected behavior, or accidental?

I think this does not affect significantly analysis of Dart files, we add changed files to the list, but if it was not yet (or ever be) read as FileState, we quickly exit from collectAffected().

[DanTup]

@DanTup do you know if this is an expected behavior, or accidental?

I don't see why we should need this - I presume it just happens because we enumerate all contexts and watch() them:

https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/lib/src/context_manager.dart#L559-L567

As far as I can tell, we don't make any attempt to keep track of the active watchers and reuse watchers on same/parent folders. We could probably combine all of analysisContext.contextRoot.included from collection.contexts and remove any item that would be covered by another watcher further up, although from your test code I wonder if you're thinking it should be handled inside PhysicalResourceProvider (because otherwise your test could would still show duplicates)?

[scheglov]

Ah, no, I was not implying that it should be handled on ResourceProvider level.
My example was just me trying that MemoryResourceProvider behaves as package:watcher does.
ResourceProvider should do what asked - if there is intersecting watches, it is up to the client to know why.
OTOH, in DAS we are asked to analyze the set of files, and watching is an implementation detail.
So, it should be handled somewhere in DAS.

Yes, it seems that there is common idea about during-the-build watching, and post-watching.
Maybe we can share the set, I looked, but have not implemented it yet.