Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher error for emacs temp file #2635

Closed
bradyt opened this issue Mar 2, 2020 · 7 comments · Fixed by #2637
Closed

Watcher error for emacs temp file #2635

bradyt opened this issue Mar 2, 2020 · 7 comments · Fixed by #2637

Comments

@bradyt
Copy link

bradyt commented Mar 2, 2020

Seems to be similar to this issue: #1134.

If I use pub run build_runner watch, and edit a file with Emacs,

[SEVERE] built_value_generator:built_value on lib/src/.#types.dart:

NoSuchMethodError: The getter 'element' was called on null.
Receiver: null
Tried calling: element
[SEVERE] Unhandled build failure!
FileSystemException: Cannot create file, path = '.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/cone_lib/0/lib/src/.' (OS Error: Is a directory, errno = 21)
[SEVERE] Failed after 7ms

Environment information:

$ dart --version
Dart VM version: 2.7.1 (Thu Jan 23 13:02:26 2020 +0100) on "macos_x64"
# pubspec.lock
...
  build_runner:
    dependency: "direct dev"
    description:
      name: build_runner
      url: "https://pub.dartlang.org"
    source: hosted
    version: "1.7.4"
...
  built_value:
    dependency: "direct main"
    description:
      name: built_value
      url: "https://pub.dartlang.org"
    source: hosted
    version: "7.0.9"
  built_value_generator:
    dependency: "direct dev"
    description:
      name: built_value_generator
      url: "https://pub.dartlang.org"
    source: hosted
    version: "7.0.9"
...
@bradyt bradyt changed the title Watcher crashed for emacs temp file Watcher error for emacs temp file Mar 2, 2020
@jakemac53
Copy link
Contributor

Looks like it is using a # as a prefix for the file names which confuses things unsurprisingly as that is a uri fragment. It seems to be getting stripped. This could be a problem with the watcher package or something in our code although we don't really work with uris, I would guess its something in watcher.

@jakemac53
Copy link
Contributor

Hmm we do actually allow getting uris for AssetIds, which it looked like is partly where we had issues before.

Given that I don't think its even possible to create a sensible uri from this I am inclined to close this as not planned.

cc @natebosch

@jakemac53
Copy link
Contributor

jakemac53 commented Mar 2, 2020

See here which I think is the actual culprit

p.fromUri(output.primaryInput.path));

Any idea why we are using p.fromUri for that @natebosch ?

@natebosch
Copy link
Member

Any idea why we are using p.fromUri for that @natebosch ?

It's because the argument is the path part, which is always URI separator, not the OS separator.

The real problem is that we aren't escaping properly when we go from OS to URI style path somewhere I think. That path should really have the # escaped as %23

natebosch added a commit that referenced this issue Mar 2, 2020
Fixes #2635

We normalize to URI path separators, and have considered them to be
valid URIs in some places. This isn't safe since do don't do any
encoding so special characters like `#` are not escaped. We can't do
actual URI encoding for AssetIds since `$lib$` is used unencoded in too
many places. We will consider AssetIds to be, as much as possible posix
style instead of URI style. Remove some cases of treating them as URIs.

- Use the current OS style for `errorCacheDirectory` since it is only
  ever used for OS file operations and does not get stored anywhere.
- Use `p.posix.split` over `p.fromUri` when normalizing separators in
  asset ID paths just before an OS file operation.
natebosch added a commit that referenced this issue Mar 2, 2020
Fixes #2635

We normalize to URI path separators, and have considered them to be
valid URIs in some places. This isn't safe since do don't do any
encoding so special characters like `#` are not escaped. We can't do
actual URI encoding for AssetIds since `$lib$` is used unencoded in too
many places. We will consider AssetIds to be, as much as possible posix
style instead of URI style. Remove some cases of treating them as URIs.

- Use the current OS style for `errorCacheDirectory` since it is only
  ever used for OS file operations and does not get stored anywhere.
- Use `p.posix.split` over `p.fromUri` when normalizing separators in
  asset ID paths just before an OS file operation.
@natebosch
Copy link
Member

@bradyt - can you try a git override for build_runner_core and see if it's working? Assuming it is I can publish.

We expect there may be build errors when you have files named .dart that aren't valid Dart files, but we hope that this resolves the stack trace.

@bradyt
Copy link
Author

bradyt commented Mar 2, 2020

Ah, maybe there wasn't really anything broken. I tried the following as you suggested.

dependency_overrides:
  build_runner_core:
    version: '4.4.1-dev'
    git:
      url: https://github.com/dart-lang/build.git
      path: build_runner_core

Then I no longer see this:

FileSystemException: Cannot create file, path = '.dart_tool/build/098d3ee73e6cc294616d3a2e2c3c81ad/error_cache/cone_lib/0/lib/src/.' (OS Error: Is a directory, errno = 21)

But I still see the following:

[SEVERE] built_value_generator:built_value on lib/src/.#types.dart:

NoSuchMethodError: The getter 'element' was called on null.
Receiver: null
Tried calling: element

But nothing actually "breaks".

(Almost completely unrelated, but for context; if for example, I had an incorrect name in a Built class, then I would see logs like, Please make the following changes to use BuiltValue:, the .g.dart file would disappear, and that would break my usage of tools like ls lib/**/*.dart | entr -s 'pub run test'.)

@natebosch
Copy link
Member

Treating these files as inputs to other build steps is harder to change - currently most builders assume that files ending in .dart are Dart files, and so adding non Dart files with that extension is likely going to cause problems.

I don't think we have a easy answer to that problem - there is a lot of stuff built on that assumption, and we don't want to hardcode support for a particular editor's behavior around "special" files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants