Skip to content

[pigeon] switched to using isolates instead of subprocesses to run pigeon_lib #210

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

Merged
gaaclarke merged 4 commits into from
Sep 25, 2020
Merged

[pigeon] switched to using isolates instead of subprocesses to run pigeon_lib #210

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c120192
[pigeon] switched to using isolates instead of subprocesses to run pi…
gaaclarke Sep 23, 2020
a623c51
format code
gaaclarke Sep 25, 2020
4c0c483
started using system temporary files
gaaclarke Sep 25, 2020
e45a22f
format
gaaclarke Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/pigeon/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.1.8

* Started spawning pigeon_lib in an isolate instead of a subprocess. The
subprocess could have lead to errors if the dart version on $PATH didn't match
the one that comes with flutter.

## 0.1.7

* Fixed Dart compilation for later versions that support null safety, opting out
Expand Down
28 changes: 19 additions & 9 deletions packages/pigeon/bin/pigeon.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:convert';
import 'dart:async';
import 'dart:io';
import 'dart:isolate';
import 'package:path/path.dart' as path;
import 'package:pigeon/pigeon_lib.dart';

Future<void> main(List<String> args) async {
Expand All @@ -13,20 +15,28 @@ Future<void> main(List<String> args) async {
(opts.input != null) ? 'import \'${opts.input}\';\n' : '';
final String code = """$importLine
import 'dart:io';
import 'dart:isolate';
import 'package:pigeon/pigeon_lib.dart';

void main(List<String> args) async {
exit(await Pigeon.run(args));
void main(List<String> args, SendPort sendPort) async {
sendPort.send(await Pigeon.run(args));
}
""";
// TODO(aaclarke): Start using a system temp file.

@xster xster Sep 24, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use https://api.dart.dev/stable/2.9.3/dart-io/Directory/systemTemp.html?

@gaaclarke gaaclarke Sep 25, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that API isn't so helpful though because I should probably make a temp directory in there instead of placing the file directly in the system temp. It would be nice if we had something like mktemp -d. At the very least i need a GUID to make sure I don't collide. I'll get around to this later.

@xster xster Sep 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry wrong link. I meant to point to https://api.dart.dev/stable/2.9.3/dart-io/Directory/createTempSync.html

@gaaclarke gaaclarke Sep 25, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, dang you, I did it, haha. I had to do some trickery to get relative paths for the dart imports. Now it says stuff like:

import '../../../../../../Users/aaclarke/dev/packages/packages/pigeon/pigeons/message.dart';

I didn't check to see if dart actually supports absolute path imports.

@xster xster Sep 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this whole problem go away once we do flutter/flutter#65071?

@gaaclarke gaaclarke Sep 25, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. There always has to be a link between the generated code that the isolate executes and the pigeon file.

const String tempFilename = '_pigeon_temp_.dart';
final String tempFilename = path.join(Directory.current.path, '_pigeon_temp_.dart');
final File tempFile = await File(tempFilename).writeAsString(code);
final Process process =
await Process.start('dart', <String>[tempFilename] + args);
process.stdout.transform(utf8.decoder).listen((String data) => print(data));
process.stderr.transform(utf8.decoder).listen((String data) => print(data));
final int exitCode = await process.exitCode;
final ReceivePort receivePort = ReceivePort();
Isolate.spawnUri(Uri.parse(tempFilename), args, receivePort.sendPort);
final Completer<int> completer = Completer<int>();
receivePort.listen((dynamic message) {
try {
// ignore: avoid_as
completer.complete(message as int);
} catch (exception) {
completer.completeError(exception);
}
});
final int exitCode = await completer.future;
tempFile.deleteSync();
exit(exitCode);
}
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'dart:mirrors';
import 'ast.dart';

/// The current version of pigeon.
const String pigeonVersion = '0.1.7';
const String pigeonVersion = '0.1.8';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: pigeon
version: 0.1.7
version: 0.1.8
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
homepage: https://github.com/flutter/packages/tree/master/packages/pigeon
dependencies:
Expand Down