google / process.dart

BSD 3-Clause "New" or "Revised" License
52 stars 26 forks source link

Encoding parameters should be nullable #64

Closed lambda-fairy closed 2 years ago

lambda-fairy commented 3 years ago

In the dart:io versions of the Process.run and runSync methods, the stderrEncoding and stdoutEncoding parameters are nullable. This library should be updated to match.

Due to backward compatibility constraints, this has to be done in two phases:

  1. Make the parameters nullable, but with a covariant keyword. This can be done in a patch release.
  2. Once all dependents are migrated, remove the covariant. This requires a major release.
topilski commented 3 years ago

Target dart2js failed: Exception: ../.pub-cache/hosted/pub.dartlang.org/process-4.2.2/lib/src/interface/local_process_manager.dart:85:25: Error: The argument type 'Encoding?' can't be assigned to the parameter type 'Encoding' because 'Encoding?' is nullable and 'Encoding' isn't.

tvolkert commented 3 years ago

@topilski what Dart version are you running? process 4.2.2 requires >=2.12.0-0

vanshg395 commented 3 years ago

I am facing same problem

My Dart Version: sdk: ">=2.12.0 <3.0.0"

$ flutter build web

💪 Building with sound null safety 💪

Target dart2js failed: Exception: ../../../Tools/flutter/.pub-cache/hosted/pub.dartlang.org/process-4.2.2/lib/src/interface/local_process_manager.dart:85:25:
Error: The argument type 'Encoding?' can't be assigned to the parameter type 'Encoding' because 'Encoding?' is nullable and 'Encoding' isn't.
 - 'Encoding' is from 'dart:convert'.
        stdoutEncoding: stdoutEncoding,
                        ^
../../../Tools/flutter/.pub-cache/hosted/pub.dartlang.org/process-4.2.2/lib/src/interface/local_process_manager.dart:86:25:
Error: The argument type 'Encoding?' can't be assigned to the parameter type 'Encoding' because 'Encoding?' is nullable and 'Encoding' isn't.
 - 'Encoding' is from 'dart:convert'.
        stderrEncoding: stderrEncoding,
                        ^
../../../Tools/flutter/.pub-cache/hosted/pub.dartlang.org/process-4.2.2/lib/src/interface/local_process_manager.dart:116:25:
Error: The argument type 'Encoding?' can't be assigned to the parameter type 'Encoding' because 'Encoding?' is nullable and 'Encoding' isn't.
 - 'Encoding' is from 'dart:convert'.
        stdoutEncoding: stdoutEncoding,
                        ^
../../../Tools/flutter/.pub-cache/hosted/pub.dartlang.org/process-4.2.2/lib/src/interface/local_process_manager.dart:117:25:
Error: The argument type 'Encoding?' can't be assigned to the parameter type 'Encoding' because 'Encoding?' is nullable and 'Encoding' isn't.
 - 'Encoding' is from 'dart:convert'.
        stderrEncoding: stderrEncoding,
                        ^
Error: Compilation failed.

Compiling lib/main.dart for the Web...                              9.5s
Exception: Failed to compile application for the Web.
tvolkert commented 3 years ago

@vanshg395 @topilski what do you see for "Dart version" when you run flutter doctor -v?

tvolkert commented 3 years ago

Aha, I see the issue.

In https://github.com/dart-lang/sdk/commit/d44457f79d087b52a0468609d44013e5fd9f09f0 (reviewed in https://dart-review.googlesource.com/c/sdk/+/151121), the encoding parameters to Process.run and Process.runSync were changed to be nullable. This change landed in Dart SDK 2.10. However, the change didn't update the web versions of those libraries (https://github.com/dart-lang/sdk/blob/cf5587e8612a3e19b9f622444572ab69fb9321ac/sdk/lib/_internal/js_runtime/lib/io_patch.dart#L361-L381).

So when compiling for web, the dart:io API is currently broken.

@leafpeterson

tvolkert commented 3 years ago

I'm gonna revert https://github.com/google/process.dart/pull/65 and publish that revert as process 4.2.3

tvolkert commented 3 years ago

Ok, the change has been reverted, and 4.2.3 has been published.

@vanshg395 @topilski you can update your packages, and the issue should be fixed.

FYI @lambda-fairy

tvolkert commented 3 years ago

I filed https://github.com/dart-lang/sdk/issues/46726 to track the fix needed in the Dart SDK

SchabanBo commented 3 years ago

@topilski It works again. Thanks 👍

topilski commented 3 years ago

Hello @tvolkert thank you for fast fix, I will check!

tvolkert commented 3 years ago

The Dart SDK was fixed in https://github.com/dart-lang/sdk/commit/5de7dfb90bf1d006c64992938a9e2a2a0908d4d4. Once that's released to stable (and a stable version of Flutter contains the release as well), the fix to this issue can proceed.

lambda-fairy commented 3 years ago

That fix is in 2.14.0. I'm gonna try land this again 🙂