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.

Sign up for GitHub

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

Jump to bottom

dart format hangs when reading from stdin #46600

Closed
johnniwinther opened this issue Jul 13, 2021 · 10 comments
Closed

dart format hangs when reading from stdin #46600

johnniwinther opened this issue Jul 13, 2021 · 10 comments
Assignees
@munificent
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-format Issues related to the 'dart format' tool P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@johnniwinther
Copy link
Member

After cb6d0e0 landed the presubmit performed by git cl upload takes a very long time - or doesn't complete, maybe - on my Windows machine. I've let the presubmit run for at least 15 minutes before giving up and instead bypassing the presubmit checks. The only output at that point being

Running Python 2 presubmit upload checks ...

I have verified that this behavior starts at the mentioned commit.
@johnniwinther johnniwinther added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Jul 13, 2021
@whesse whesse added area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jul 13, 2021
@whesse
Copy link
Contributor

whesse commented Jul 13, 2021

This seems to be a big problem if "dart format" takes much longer than the existing "dartfmt" in our published SDKs. The checked-in SDK for all platforms has just been updated to 2.14.0-293.0.dev.

I have a Windows machine and can check this, as does Johnni. Will not be able to check myself until later tonight or tomorrow, so someone else may want to do it.

@bkonyi
Copy link
Contributor

bkonyi commented Jul 13, 2021

I can reproduce this myself on Windows. Seems to happen whenever a Dart file is changed and dart format doesn't seem to be notified that we're done writing to stdin, so we just hang there forever.

We should be safe to just pass dart format the filename instead of piping the file contents over stdin since that fixes the issue, but we'll probably want to look into this more closely to make sure it's not a general problem.

FYI @munificent

dart-bot pushed a commit that referenced this issue Jul 13, 2021
@bkonyi
Pass filename instead of piping contents through stdin to dart format in
44ffad5 
PRESUBMIT.py

FIXES=#46600

Change-Id: I69358185b5a8dfc863f17cd4beb0209592d3be09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206681
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
@a-siva
Copy link
Contributor

a-siva commented Jul 15, 2021

Since a workaround has been submitted is this still a P1 ?

maybe we should edit the title of the bug to something like 'dart format' does not notify when writing to stdin

@whesse whesse added library-io os-windows and removed area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jul 15, 2021
@whesse
Copy link
Contributor

whesse commented Jul 15, 2021

We have seen problems like this before on Windows, with stdin behaving differently, especially with pipes and EOF.

Those issues have been closed, so I think we should keep this open, reproduce the failure, and find out the problem with end of stdin on windows.

@bkonyi bkonyi removed their assignment Jul 15, 2021
@mraleph mraleph changed the title Presubmit check takes forever dart format hangs when reading from stdin Jul 16, 2021
@mraleph mraleph mentioned this issue Jul 16, 2021
Closed
@mraleph mraleph added area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. and removed area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. library-io os-windows labels Jul 16, 2021
@natebosch
Copy link
Member

This is not a windows specific issue, dart format hang on linux as well when reading from stdin.

Since a workaround has been submitted is this still a P1

Formatting from stdin is a critical use case and this should be considered a release blocker for stable.

@bkonyi bkonyi added P1 A high priority bug; for example, a single project is unusable or has many test failures dart-cli-format Issues related to the 'dart format' tool labels Jul 16, 2021
@bkonyi
Copy link
Contributor

bkonyi commented Jul 16, 2021

@munificent can you take a look?

@munificent
Copy link
Member

Yes, I'll look into this today.

@munificent
Copy link
Member

Fix out in dart_style: dart-lang/dart_style#1046

I can't repro in dart_style itself, but I have a test for dartdev that repros the issue. I'll add that when I roll dart_style 2.0.3 into the SDK.

@munificent
Copy link
Member

This is fixed on master now. Rolling into the SDK: https://dart-review.googlesource.com/c/sdk/+/207502

When that looks good, I'll publish 2.0.3 and close this.

@munificent
Copy link
Member

This has rolled into the SDK, into Google, and been published as 2.0.3 now.

@mraleph mraleph mentioned this issue Aug 19, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@munificent munificent

Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-format Issues related to the 'dart format' tool P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@munificent @mraleph @whesse @natebosch @a-siva @johnniwinther @bkonyi