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

Fixes for Windows compatibility (backpatch to 3.7.x) #387

Merged
merged 3 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 17 additions & 26 deletions lib/src/executable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,42 @@ import 'dart:io';
import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:args/command_runner.dart';
import 'package:dart_dev/dart_dev.dart';
import 'package:dart_dev/src/dart_dev_tool.dart';
import 'package:dart_dev/src/utils/format_tool_builder.dart';
import 'package:dart_dev/src/utils/parse_flag_from_args.dart';
import 'package:io/ansi.dart';
import 'package:io/io.dart' show ExitCode;
import 'package:logging/logging.dart';
import 'package:path/path.dart' as p;

import '../utils.dart';
import 'dart_dev_runner.dart';
import 'tools/over_react_format_tool.dart';
import 'utils/assert_dir_is_dart_package.dart';
import 'utils/cached_pubspec.dart';
import 'utils/dart_dev_paths.dart';
import 'utils/dart_tool_cache.dart';
import 'utils/ensure_process_exit.dart';
import 'utils/format_tool_builder.dart';
import 'utils/logging.dart';
import 'utils/parse_flag_from_args.dart';

typedef _ConfigGetter = Map<String, DevTool> Function();

final _runScriptPath = p.join(cacheDirPath, 'run.dart');
final paths = DartDevPaths();

final _runScript = File(_runScriptPath);

const _configPath = 'tool/dart_dev/config.dart';

const _oldDevDartPath = 'tool/dev.dart';

final _relativeDevDartPath = p.relative(
p.absolute(_configPath),
from: p.absolute(p.dirname(_runScriptPath)),
);
final _runScript = File(paths.runScript);

Future<void> run(List<String> args) async {
attachLoggerToStdio(args);
final configExists = File(_configPath).existsSync();
final oldDevDartExists = File(_oldDevDartPath).existsSync();

final configExists = File(paths.config).existsSync();
final oldDevDartExists = File(paths.legacyConfig).existsSync();

if (!configExists) {
log.fine('No custom `tool/dart_dev/config.dart` file found; '
log.fine('No custom `${paths.config}` file found; '
'using default config.');
}
if (oldDevDartExists) {
log.warning(yellow.wrap(
'dart_dev v3 now expects configuration to be at `$_configPath`,\n'
'but `$_oldDevDartPath` still exists. View the guide to see how to upgrade:\n'
'dart_dev v3 now expects configuration to be at `${paths.config}`,\n'
'but `${paths.legacyConfig}` still exists. View the guide to see how to upgrade:\n'
'https://github.com/Workiva/dart_dev/blob/master/doc/v3-upgrade-guide.md'));
}

Expand All @@ -59,7 +51,7 @@ Future<void> run(List<String> args) async {

generateRunScript();
final process = await Process.start(
Platform.executable, [_runScriptPath, ...args],
Platform.executable, [paths.runScript, ...args],
mode: ProcessStartMode.inheritStdio);
ensureProcessExit(process);
exitCode = await process.exitCode;
Expand All @@ -69,7 +61,7 @@ Future<void> handleFastFormat(List<String> args) async {
assertDirIsDartPackage();

DevTool formatTool;
final configFile = File(_configPath);
final configFile = File(paths.config);
if (configFile.existsSync()) {
final toolBuilder = FormatToolBuilder();
parseString(content: configFile.readAsStringSync())
Expand Down Expand Up @@ -115,13 +107,13 @@ bool get shouldWriteRunScript =>
_runScript.readAsStringSync() != buildDartDevRunScriptContents();

String buildDartDevRunScriptContents() {
final hasCustomToolDevDart = File(_configPath).existsSync();
final hasCustomToolDevDart = File(paths.config).existsSync();
return '''
import 'dart:io';
import 'package:dart_dev/src/core_config.dart';
import 'package:dart_dev/src/executable.dart' as executable;
${hasCustomToolDevDart ? "import '$_relativeDevDartPath' as custom_dev;" : ""}
${hasCustomToolDevDart ? "import '${paths.configFromRunScriptForDart}' as custom_dev;" : ""}
void main(List<String> args) async {
await executable.runWithConfig(args,
Expand All @@ -146,8 +138,7 @@ Future<void> runWithConfig(
config = configGetter();
} catch (error) {
stderr
..writeln(
'Invalid "tool/dart_dev/config.dart" in ${p.absolute(p.current)}')
..writeln('Invalid "${paths.config}" in ${p.absolute(p.current)}')
..writeln()
..writeln('It should provide a `Map<String, DevTool> config;` getter,'
' but it either does not exist or threw unexpectedly:')
Expand Down
3 changes: 2 additions & 1 deletion lib/src/tools/analyze_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:logging/logging.dart';
import '../dart_dev_tool.dart';
import '../utils/arg_results_utils.dart';
import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
import '../utils/executables.dart' as exe;
import '../utils/logging.dart';
import '../utils/process_declaration.dart';
import '../utils/run_process_and_ensure_exit.dart';
Expand Down Expand Up @@ -161,7 +162,7 @@ ProcessDeclaration buildProcess(
verbose: context.verbose);
final entrypoints = buildEntrypoints(include: include, root: path);
logCommand(args, entrypoints, verbose: context.verbose);
return ProcessDeclaration('dartanalyzer', [...args, ...entrypoints],
return ProcessDeclaration(exe.dartanalyzer, [...args, ...entrypoints],
mode: ProcessStartMode.inheritStdio);
}

Expand Down
5 changes: 3 additions & 2 deletions lib/src/tools/format_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import '../../utils.dart';
import '../dart_dev_tool.dart';
import '../utils/arg_results_utils.dart';
import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
import '../utils/executables.dart' as exe;
import '../utils/logging.dart';
import '../utils/organize_directives/organize_directives_in_paths.dart';
import '../utils/package_is_immediate_dependency.dart';
Expand Down Expand Up @@ -537,10 +538,10 @@ FormatExecution buildExecution(
ProcessDeclaration buildFormatProcess([Formatter formatter]) {
switch (formatter) {
case Formatter.dartStyle:
return ProcessDeclaration('pub', ['run', 'dart_style:format']);
return ProcessDeclaration(exe.pub, ['run', 'dart_style:format']);
case Formatter.dartfmt:
default:
return ProcessDeclaration('dartfmt', []);
return ProcessDeclaration(exe.dartfmt, []);
}
}

Expand Down
5 changes: 3 additions & 2 deletions lib/src/tools/over_react_format_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:dart_dev/dart_dev.dart';
import 'package:dart_dev/utils.dart';

import '../tools/format_tool.dart';
import '../utils/executables.dart' as exe;

class OverReactFormatTool extends DevTool {
/// Wrap lines longer than this.
Expand All @@ -31,9 +32,9 @@ class OverReactFormatTool extends DevTool {
'over_react_format',
if (lineLength != null) '--line-length=$lineLength'
];
final process = ProcessDeclaration('pub', [...args, ...paths],
final process = ProcessDeclaration(exe.pub, [...args, ...paths],
mode: ProcessStartMode.inheritStdio);
logCommand('pub', paths, args, verbose: context?.verbose);
logCommand(exe.pub, paths, args, verbose: context?.verbose);
return runProcessAndEnsureExit(process);
}
}
3 changes: 2 additions & 1 deletion lib/src/tools/test_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:logging/logging.dart';

import '../dart_dev_tool.dart';
import '../utils/arg_results_utils.dart';
import '../utils/executables.dart' as exe;
import '../utils/logging.dart';
import '../utils/package_is_immediate_dependency.dart';
import '../utils/process_declaration.dart';
Expand Down Expand Up @@ -315,7 +316,7 @@ TestExecution buildExecution(
verbose: context.verbose);
logSubprocessHeader(_log, 'pub ${args.join(' ')}'.trim());
return TestExecution.process(
ProcessDeclaration('pub', args, mode: ProcessStartMode.inheritStdio));
ProcessDeclaration(exe.pub, args, mode: ProcessStartMode.inheritStdio));
}

// NOTE: This currently depends on https://github.com/dart-lang/build/pull/2445
Expand Down
3 changes: 2 additions & 1 deletion lib/src/tools/tuneup_check_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:logging/logging.dart';
import '../dart_dev_tool.dart';
import '../utils/arg_results_utils.dart';
import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
import '../utils/executables.dart' as exe;
import '../utils/logging.dart';
import '../utils/package_is_immediate_dependency.dart';
import '../utils/process_declaration.dart';
Expand Down Expand Up @@ -152,5 +153,5 @@ TuneupExecution buildExecution(
verbose: context.verbose);
logSubprocessHeader(_log, 'pub ${args.join(' ')}');
return TuneupExecution.process(
ProcessDeclaration('pub', args, mode: ProcessStartMode.inheritStdio));
ProcessDeclaration(exe.pub, args, mode: ProcessStartMode.inheritStdio));
}
3 changes: 2 additions & 1 deletion lib/src/tools/webdev_serve_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:pub_semver/pub_semver.dart';
import '../dart_dev_tool.dart';
import '../utils/arg_results_utils.dart';
import '../utils/assert_no_positional_args_nor_args_after_separator.dart';
import '../utils/executables.dart' as exe;
import '../utils/global_package_is_active_and_compatible.dart';
import '../utils/logging.dart';
import '../utils/process_declaration.dart';
Expand Down Expand Up @@ -226,5 +227,5 @@ WebdevServeExecution buildExecution(
verbose: context.verbose);
logSubprocessHeader(_log, 'pub ${args.join(' ')}'.trim());
return WebdevServeExecution.process(
ProcessDeclaration('pub', args, mode: ProcessStartMode.inheritStdio));
ProcessDeclaration(exe.pub, args, mode: ProcessStartMode.inheritStdio));
}
31 changes: 31 additions & 0 deletions lib/src/utils/dart_dev_paths.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import 'package:path/path.dart' as p;

/// A collection of paths to files and directories constructed to be compatible
/// with a given [p.Context].
class DartDevPaths {
final p.Context _context;

DartDevPaths({p.Context context}) : _context = context ?? p.context;

String cache([String subPath]) => _context.normalize(
_context.joinAll([..._cacheParts, if (subPath != null) subPath]));

String get _cacheForDart => p.url.joinAll(_cacheParts);

final List<String> _cacheParts = ['.dart_tool', 'dart_dev'];

String get config => _context.joinAll(_configParts);

String get configForDart => p.url.joinAll(_configParts);

final List<String> _configParts = ['tool', 'dart_dev', 'config.dart'];

String get configFromRunScriptForDart => p.url.relative(
p.url.absolute(configForDart),
from: p.url.absolute(_cacheForDart),
);

String get legacyConfig => _context.join('tool', 'dev.dart');

String get runScript => cache('run.dart');
}
10 changes: 2 additions & 8 deletions lib/src/utils/dart_tool_cache.dart
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
import 'dart:io';

import 'package:path/path.dart' as p;

const cacheDirPath = '.dart_tool/dart_dev';
import 'package:dart_dev/src/utils/dart_dev_paths.dart';

void createCacheDir({String subPath}) {
var path = cacheDirPath;
if (subPath != null) {
path = p.join(path, subPath);
}
final dir = Directory(path);
final dir = Directory(DartDevPaths().cache(subPath));
if (!dir.existsSync()) {
dir.createSync(recursive: true);
}
Expand Down
9 changes: 9 additions & 0 deletions lib/src/utils/executables.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import 'dart:io';

final dart = 'dart';

final dartanalyzer = Platform.isWindows ? 'dartanalyzer.bat' : 'dartanalyzer';

final dartfmt = Platform.isWindows ? 'dartfmt.bat' : 'dartfmt';

final pub = Platform.isWindows ? 'pub.bat' : 'pub';
6 changes: 4 additions & 2 deletions lib/src/utils/global_package_is_active_and_compatible.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import 'dart:io';

import 'package:pub_semver/pub_semver.dart';

import 'executables.dart' as exe;

/// Returns `true` if [packageName] is globally activated at a version
/// allowed by [constraint]. Returns `false` otherwise.
///
Expand All @@ -16,9 +18,9 @@ import 'package:pub_semver/pub_semver.dart';
bool globalPackageIsActiveAndCompatible(
String packageName, VersionConstraint constraint,
{Map<String, String> environment}) {
final executable = 'pub';
final executable = exe.pub;
final args = ['global', 'list'];
final result = Process.runSync('pub', ['global', 'list'],
final result = Process.runSync(exe.pub, ['global', 'list'],
environment: environment, stderrEncoding: utf8, stdoutEncoding: utf8);
if (result.exitCode != 0) {
throw ProcessException(
Expand Down
73 changes: 73 additions & 0 deletions test/utils/dart_dev_paths_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import 'package:dart_dev/src/utils/dart_dev_paths.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';

void main() {
group('DartDevPaths', () {
group('posix', () {
DartDevPaths paths;

setUp(() {
paths = DartDevPaths(context: p.posix);
});

test('cache', () {
expect(paths.cache(), '.dart_tool/dart_dev');
});

test('cache with subpath', () {
expect(paths.cache('sub/path'), '.dart_tool/dart_dev/sub/path');
});

test('config', () {
expect(paths.config, 'tool/dart_dev/config.dart');
});

test('configFromRunScriptForDart', () {
expect(paths.configFromRunScriptForDart,
'../../tool/dart_dev/config.dart');
});

test('legacyConfig', () {
expect(paths.legacyConfig, 'tool/dev.dart');
});

test('runScript', () {
expect(paths.runScript, '.dart_tool/dart_dev/run.dart');
});
});

group('windows', () {
DartDevPaths paths;

setUp(() {
paths = DartDevPaths(context: p.windows);
});

test('cache', () {
expect(paths.cache(), r'.dart_tool\dart_dev');
});

test('cache with subpath', () {
expect(paths.cache('sub/path'), r'.dart_tool\dart_dev\sub\path');
});

test('config', () {
expect(paths.config, r'tool\dart_dev\config.dart');
});

test('configFromRunScriptForDart', () {
expect(paths.configFromRunScriptForDart,
r'../../tool/dart_dev/config.dart');
Copy link
Member

Choose a reason for hiding this comment

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

Are the path separators for a relative path really not backslashes in windows? If so, how inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Turns out this is a path that will be used as an import in a .dart file. Dart always uses forward slashes regardless of the platform. So, nothing to do here.

});

test('legacyConfig', () {
expect(paths.legacyConfig, r'tool\dev.dart');
});

test('runScript', () {
expect(paths.runScript, r'.dart_tool\dart_dev\run.dart');
});
});
});
}