diff --git a/.github/workflows/examples.yaml b/.github/workflows/examples.yaml index 7e416f7..2583658 100644 --- a/.github/workflows/examples.yaml +++ b/.github/workflows/examples.yaml @@ -102,7 +102,7 @@ jobs: - uses: nttld/setup-ndk@v1 id: setup-ndk with: - ndk-version: r27c + ndk-version: r29 add-to-path: true - name: flutter example run: | diff --git a/.github/workflows/test_publish.yml b/.github/workflows/test_publish.yml index c6d856e..d18b84e 100644 --- a/.github/workflows/test_publish.yml +++ b/.github/workflows/test_publish.yml @@ -28,7 +28,7 @@ jobs: - uses: nttld/setup-ndk@v1 id: setup-ndk with: - ndk-version: r27c + ndk-version: r29 add-to-path: true - uses: subosito/flutter-action@v2 with: @@ -47,7 +47,7 @@ jobs: - uses: nttld/setup-ndk@v1 id: setup-ndk with: - ndk-version: r27c + ndk-version: r29 add-to-path: true - uses: subosito/flutter-action@v2 with: @@ -66,7 +66,7 @@ jobs: - uses: nttld/setup-ndk@v1 id: setup-ndk with: - ndk-version: r27c + ndk-version: r29 add-to-path: true - uses: subosito/flutter-action@v2 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index c518bcc..44d564d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # native_toolchain_cmake +## 0.2.8 + +- fix: properly resolve user-defines for android_home on Windows, [#37](https://github.com/rainyl/native_toolchain_cmake/issues/37) + ## 0.2.7 - new: add `CMakeBuilder.runStandalone` to allow running builder without `BuildInput input` and `BuildOutputBuilder output` diff --git a/lib/src/builder/builder.dart b/lib/src/builder/builder.dart index 17888e2..f00718f 100644 --- a/lib/src/builder/builder.dart +++ b/lib/src/builder/builder.dart @@ -11,6 +11,7 @@ import 'dart:io'; import 'package:code_assets/code_assets.dart'; import 'package:hooks/hooks.dart'; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import '../native_toolchain/msvc.dart'; import '../utils/env_from_bat.dart'; @@ -246,6 +247,50 @@ class CMakeBuilder implements Builder { ); } + /// Unwraps the `workspace_pubspec.defines` envelope that `package:hooks` + /// wraps `user_defines` in. + /// + /// When a build hook is invoked through `flutter build` / `hooks_runner`, + /// `input.json['user_defines']` has the shape: + /// + /// ```json + /// { + /// "workspace_pubspec": { + /// "base_path": ".../pubspec.yaml", + /// "defines": { "env_file": ..., "android": {...}, ... } + /// } + /// } + /// ``` + /// + /// Direct callers of [runStandalone] usually pass the `defines` map directly + /// (without the envelope). This helper accepts both shapes and returns the + /// inner defines map. Non-Map values are logged and ignored. + @visibleForTesting + static Map unwrapUserDefinesForTesting( + Map? userDefines, { + Logger? logger, + }) => _unwrapUserDefines(userDefines, logger: logger); + + static Map _unwrapUserDefines(Map? userDefines, {Logger? logger}) { + if (userDefines == null) return const {}; + // Workspace envelope (hooks_runner / flutter build path). + final workspace = userDefines['workspace_pubspec']; + if (workspace is Map) { + final defines = workspace['defines']; + if (defines is Map) return defines; + if (defines is Map) return defines.cast(); + if (defines != null) { + logger?.warning( + 'user_defines.workspace_pubspec.defines expected Map, ' + 'got ${defines.runtimeType}; ignored.', + ); + } + return const {}; + } + // Direct flat defines (manual runStandalone / tests). + return userDefines; + } + /// Runs the CMake generate and build process with explicit arguments instead /// of [BuildInput] or [BuildOutputBuilder]. /// @@ -296,60 +341,58 @@ class CMakeBuilder implements Builder { // ninja_version: null # "1.10.2" // windows: // cmake_version: null # "3.31.6" - final androidConfig = userDefines["android"] as Map?; - final iosConfig = userDefines["ios"] as Map?; - final linuxConfig = userDefines["linux"] as Map?; - final macOSConfig = userDefines["macos"] as Map?; - final windowsConfig = userDefines["windows"] as Map?; - - var cmakeVersion = userDefines["cmake_version"] as String?; - cmakeVersion = switch (targetOS) { - OS.android => androidConfig?["cmake_version"] as String? ?? cmakeVersion, - OS.iOS => iosConfig?["cmake_version"] as String? ?? cmakeVersion, - OS.linux => linuxConfig?["cmake_version"] as String? ?? cmakeVersion, - OS.macOS => macOSConfig?["cmake_version"] as String? ?? cmakeVersion, - OS.windows => windowsConfig?["cmake_version"] as String? ?? cmakeVersion, - _ => cmakeVersion, - }; + logger?.fine('userDefines: $userDefines'); - var ninjaVersion = userDefines["ninja_version"] as String?; - ninjaVersion = switch (targetOS) { - OS.android => androidConfig?["ninja_version"] as String? ?? ninjaVersion, - OS.iOS => iosConfig?["ninja_version"] as String? ?? ninjaVersion, - OS.linux => linuxConfig?["ninja_version"] as String? ?? ninjaVersion, - OS.macOS => macOSConfig?["ninja_version"] as String? ?? ninjaVersion, - OS.windows => windowsConfig?["ninja_version"] as String? ?? ninjaVersion, - _ => ninjaVersion, - }; + // Peel `workspace_pubspec.defines` if present (hooks_runner / flutter + // build path). Direct callers passing the flat defines map are unaffected. + final userDefinesFlat = _unwrapUserDefines(userDefines, logger: logger); - var userConfig = UserConfig( + // Per-OS overrides win over the top-level entries; wrong types are logged + // and ignored instead of throwing. See [UserConfig.parseFromUserDefines]. + var userConfig = UserConfig.parseFromUserDefines( targetOS: targetOS, - cmakeVersion: cmakeVersion, - ninjaVersion: ninjaVersion, - ndkVersion: androidConfig?["ndk_version"] as String?, - androidHome: androidConfig?["android_home"] as String?, - preferAndroidNinja: userDefines["prefer_android_ninja"] as bool?, - preferAndroidCmake: userDefines["prefer_android_cmake"] as bool?, + userDefines: userDefinesFlat, + logger: logger, ); - // optional host specific build config - final envFile = userDefines["env_file"] as String?; - - if (envFile != null) { + // Android-specific fallback chain for android_home: + // 1. user_defines.android.android_home (handled in parseFromUserDefines) + // 2. env_file ANDROID_HOME (handled below) + // 3. ANDROID_HOME system env var (handled below) + // Each step only fills in when the previous one is null. + final envFile = userDefinesFlat[UserConfigKeys.envFile]; + if (envFile is String && envFile.isNotEmpty) { final userEnvConfig = await getUserEnvConfig(input: input, packageRoot: packageRoot, envFile: envFile); final androidHome = userEnvConfig['ANDROID_HOME']; - if (androidHome != null) { + if (androidHome != null && userConfig.androidHome == null) { final androidHomeEntity = Directory(androidHome); if (androidHomeEntity.existsSync()) { userConfig = userConfig.copyWith(androidHome: androidHomeEntity.absolute.path); } else { logger?.warning( - "ANDROID_HOME=$androidHome is set in envFile=$envFile but does not exist, ignoring", + 'ANDROID_HOME=$androidHome is set in envFile=$envFile but does not exist, ignoring', ); } } + } else if (envFile != null) { + logger?.warning('user_defines.env_file expected String, got ${envFile.runtimeType}; ignored.'); } + // Last-resort fallback to the ANDROID_HOME system environment variable. + if (userConfig.androidHome == null) { + final envAndroidHome = Platform.environment['ANDROID_HOME']; + if (envAndroidHome != null && envAndroidHome.isNotEmpty) { + final dir = Directory(envAndroidHome); + if (dir.existsSync()) { + userConfig = userConfig.copyWith(androidHome: dir.absolute.path); + } else { + logger?.warning('ANDROID_HOME=$envAndroidHome from environment does not exist, ignoring.'); + } + } + } + + logger?.fine('Resolved userConfig: $userConfig'); + final task = RunCMakeBuilder( targetOS: targetOS, targetArchitecture: targetArchitecture, diff --git a/lib/src/builder/user_config.dart b/lib/src/builder/user_config.dart index 1373ab1..834db25 100644 --- a/lib/src/builder/user_config.dart +++ b/lib/src/builder/user_config.dart @@ -4,11 +4,44 @@ import 'dart:io'; import 'package:code_assets/code_assets.dart'; +import 'package:logging/logging.dart'; + +/// Keys allowed under the top-level `user_defines` map and (where applicable) +/// the per-OS sub-maps (`android`, `ios`, `linux`, `macos`, `windows`). +class UserConfigKeys { + const UserConfigKeys._(); + + static const cmakeVersion = 'cmake_version'; + static const ninjaVersion = 'ninja_version'; + static const ndkVersion = 'ndk_version'; + static const androidHome = 'android_home'; + static const envFile = 'env_file'; + static const preferAndroidCmake = 'prefer_android_cmake'; + static const preferAndroidNinja = 'prefer_android_ninja'; + + /// Per-OS sub-map keys indexed by [OS]. + static const osConfigKey = { + OS.android: 'android', + OS.iOS: 'ios', + OS.linux: 'linux', + OS.macOS: 'macos', + OS.windows: 'windows', + }; +} class UserConfig { final OS targetOS; - /// for [OS.android], i.e., ANDROID_HOME, will try to load from environment variable if not specified. + /// for [OS.android], i.e., ANDROID_HOME. + /// + /// Always stored with forward slashes only and without a trailing slash so + /// it can be interpolated directly into glob patterns. `package:glob` + /// treats `\` as an escape character rather than a path separator, so raw + /// Windows paths obtained from `Platform.environment`, env files or + /// `Directory.absolute.path` need to be normalised at construction time. + /// Use [copyWith] / [parseFromUserDefines] to derive new instances so the + /// invariant is preserved; do not assign this field manually from a + /// non-normalised string. final String? androidHome; /// for [OS.android], if not specified, use the latest one @@ -44,7 +77,111 @@ class UserConfig { }) : preferAndroidCmake = preferAndroidCmake ?? targetOS == OS.android, preferAndroidNinja = preferAndroidNinja ?? targetOS == OS.android, androidHome = - androidHome ?? (envVarAndroidHomeAsDefault ? Platform.environment['ANDROID_HOME'] : null); + _normalizeAndroidHome(androidHome) ?? + (envVarAndroidHomeAsDefault ? _normalizeAndroidHome(Platform.environment['ANDROID_HOME']) : null); + + /// Normalises an [androidHome] path for safe interpolation into glob + /// patterns: backslashes become forward slashes and trailing slashes are + /// removed. `null` and empty strings are returned as `null`. + static String? _normalizeAndroidHome(String? path) { + if (path == null) return null; + final normalized = path.replaceAll(r'\', '/').replaceFirst(RegExp(r'/+$'), ''); + return normalized.isEmpty ? null : normalized; + } + + /// Parses the `user_defines` map (as surfaced by `hooks`) into a [UserConfig]. + /// + /// Resolution order for each scalar value: + /// 1. the per-OS sub-map (`android`, `ios`, ...) entry, if present; + /// 2. the top-level entry, if present; + /// 3. `null` (left to the caller / [UserConfig] defaults to fill in). + /// + /// Keys with the wrong type are logged via [logger] at `warning` level and + /// ignored rather than throwing a [TypeError]. This mirrors the lenient + /// behaviour expected of build hooks where users edit YAML by hand. + /// + /// Android-only keys ([ndkVersion], [androidHome]) are read solely from the + /// `android` sub-map regardless of [targetOS]. + /// + /// System environment variable `ANDROID_HOME` is **not** consulted here; the + /// caller decides whether to fall back to it. Pass + /// `envVarAndroidHomeAsDefault: true` (the default) to [UserConfig] if that + /// fallback is desired. + // ignore: prefer_constructors_over_static_methods + static UserConfig parseFromUserDefines({ + required OS targetOS, + required Map userDefines, + Logger? logger, + }) { + final osConfigKey = UserConfigKeys.osConfigKey[targetOS]; + final osConfig = osConfigKey == null ? null : _optMap(userDefines, osConfigKey, logger: logger); + + final cmakeVersion = + _optString(osConfig, UserConfigKeys.cmakeVersion, logger: logger) ?? + _optString(userDefines, UserConfigKeys.cmakeVersion, logger: logger); + final ninjaVersion = + _optString(osConfig, UserConfigKeys.ninjaVersion, logger: logger) ?? + _optString(userDefines, UserConfigKeys.ninjaVersion, logger: logger); + + // Android-only keys, read only from the android sub-map. + final androidConfig = _optMap(userDefines, UserConfigKeys.osConfigKey[OS.android]!, logger: logger); + final ndkVersion = _optString(androidConfig, UserConfigKeys.ndkVersion, logger: logger); + final androidHome = _optString(androidConfig, UserConfigKeys.androidHome, logger: logger); + + final preferCmake = + _optBool(osConfig, UserConfigKeys.preferAndroidCmake, logger: logger) ?? + _optBool(userDefines, UserConfigKeys.preferAndroidCmake, logger: logger); + final preferNinja = + _optBool(osConfig, UserConfigKeys.preferAndroidNinja, logger: logger) ?? + _optBool(userDefines, UserConfigKeys.preferAndroidNinja, logger: logger); + + return UserConfig( + targetOS: targetOS, + cmakeVersion: cmakeVersion, + ninjaVersion: ninjaVersion, + ndkVersion: ndkVersion, + androidHome: androidHome, + preferAndroidCmake: preferCmake, + preferAndroidNinja: preferNinja, + envVarAndroidHomeAsDefault: false, + ); + } + + /// Returns [map]?[key] when it is a [String], otherwise `null`. + /// + /// Logs a warning when the entry exists but has the wrong type. Tolerates + /// numeric values (common from YAML where `3.22.1` parses as a double) by + /// accepting them. + static String? _optString(Map? map, String key, {Logger? logger}) { + if (map == null) return null; + final v = map[key]; + if (v == null) return null; + if (v is String) return v; + if (v is num) return v.toString(); + logger?.warning('user_defines.$key expected String or num, got ${v.runtimeType}; ignored.'); + return null; + } + + /// Returns [map]?[key] when it is a [bool], otherwise `null`. + static bool? _optBool(Map? map, String key, {Logger? logger}) { + if (map == null) return null; + final v = map[key]; + if (v == null) return null; + if (v is bool) return v; + logger?.warning('user_defines.$key expected bool, got ${v.runtimeType}; ignored.'); + return null; + } + + /// Returns [map]?[key] when it is a [Map] with String keys, otherwise `null`. + static Map? _optMap(Map? map, String key, {Logger? logger}) { + if (map == null) return null; + final v = map[key]; + if (v == null) return null; + if (v is Map) return v; + if (v is Map) return v.cast(); + logger?.warning('user_defines.$key expected Map, got ${v.runtimeType}; ignored.'); + return null; + } UserConfig copyWith({ OS? targetOS, @@ -62,5 +199,18 @@ class UserConfig { ninjaVersion: ninjaVersion ?? this.ninjaVersion, preferAndroidNinja: preferAndroidNinja ?? this.preferAndroidNinja, ndkVersion: ndkVersion ?? this.ndkVersion, + envVarAndroidHomeAsDefault: false, ); + + @override + String toString() => + 'UserConfig(' + 'targetOS: $targetOS, ' + 'cmakeVersion: $cmakeVersion, ' + 'ninjaVersion: $ninjaVersion, ' + 'ndkVersion: $ndkVersion, ' + 'androidHome: $androidHome, ' + 'preferAndroidCmake: $preferAndroidCmake, ' + 'preferAndroidNinja: $preferAndroidNinja' + ')'; } diff --git a/pubspec.yaml b/pubspec.yaml index 287da52..66e807b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,7 +1,7 @@ name: native_toolchain_cmake description: >- A library to invoke and build CMake projects for Dart Native Assets. -version: 0.2.7 +version: 0.2.8 repository: https://github.com/rainyl/native_toolchain_cmake topics: diff --git a/test/builder/cmake_builder_cross_android_test.dart b/test/builder/cmake_builder_cross_android_test.dart index 81f8e5e..d338757 100644 --- a/test/builder/cmake_builder_cross_android_test.dart +++ b/test/builder/cmake_builder_cross_android_test.dart @@ -22,19 +22,15 @@ void main() { Architecture.x64: 'elf64-x86-64', }; - /// From https://docs.flutter.dev/reference/supported-platforms. - const flutterAndroidNdkVersionLowestBestEffort = 19; - /// From https://docs.flutter.dev/reference/supported-platforms. const flutterAndroidNdkVersionLowestSupported = 21; /// From https://docs.flutter.dev/reference/supported-platforms. - const flutterAndroidNdkVersionHighestSupported = 34; + const flutterAndroidNdkVersionHighestSupported = 35; for (final linkMode in [DynamicLoadingBundled()]) { for (final target in targets) { for (final apiLevel in [ - flutterAndroidNdkVersionLowestBestEffort, flutterAndroidNdkVersionLowestSupported, flutterAndroidNdkVersionHighestSupported, ]) { diff --git a/test/builder/user_config_test.dart b/test/builder/user_config_test.dart new file mode 100644 index 0000000..2ebeeb5 --- /dev/null +++ b/test/builder/user_config_test.dart @@ -0,0 +1,294 @@ +import 'package:code_assets/code_assets.dart'; +import 'package:logging/logging.dart'; +import 'package:native_toolchain_cmake/src/builder/builder.dart'; +import 'package:native_toolchain_cmake/src/builder/user_config.dart'; +import 'package:test/test.dart'; + +void main() { + late Logger logger; + late List warnings; + + setUp(() { + warnings = []; + logger = Logger('UserConfigTest') + ..onRecord.listen((r) { + if (r.level >= Level.WARNING) warnings.add(r); + }); + }); + + group('parseFromUserDefines', () { + test('top-level cmake_version used when no per-OS override', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.linux, + userDefines: {UserConfigKeys.cmakeVersion: '3.22.1'}, + logger: logger, + ); + expect(cfg.cmakeVersion, '3.22.1'); + expect(cfg.ninjaVersion, isNull); + expect(cfg.androidHome, isNull); + expect(cfg.ndkVersion, isNull); + expect(cfg.preferAndroidCmake, isFalse); + expect(cfg.preferAndroidNinja, isFalse); + expect(warnings, isEmpty); + }); + + test('per-OS sub-map overrides top-level for cmake/ninja version', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.macOS, + userDefines: { + UserConfigKeys.cmakeVersion: '3.22.1', + UserConfigKeys.ninjaVersion: '1.10.2', + 'macos': {UserConfigKeys.cmakeVersion: '3.31.6', UserConfigKeys.ninjaVersion: '1.12.0'}, + }, + logger: logger, + ); + expect(cfg.targetOS, OS.macOS); + expect(cfg.cmakeVersion, '3.31.6'); + expect(cfg.ninjaVersion, '1.12.0'); + expect(warnings, isEmpty); + }); + + test('android sub-map populates ndkVersion and androidHome', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + 'android': { + UserConfigKeys.ndkVersion: '28.2.13676358', + UserConfigKeys.androidHome: r'C:\Android\Sdk', + }, + }, + logger: logger, + ); + expect(cfg.targetOS, OS.android); + expect(cfg.ndkVersion, '28.2.13676358'); + // Path normalised to forward slashes. + expect(cfg.androidHome, 'C:/Android/Sdk'); + // Defaults for android. + expect(cfg.preferAndroidCmake, isTrue); + expect(cfg.preferAndroidNinja, isTrue); + expect(warnings, isEmpty); + }); + + test('prefer_android_cmake from android sub-map overrides top-level', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + UserConfigKeys.preferAndroidCmake: true, + UserConfigKeys.preferAndroidNinja: true, + 'android': {UserConfigKeys.preferAndroidCmake: false, UserConfigKeys.preferAndroidNinja: false}, + }, + logger: logger, + ); + expect(cfg.preferAndroidCmake, isFalse); + expect(cfg.preferAndroidNinja, isFalse); + expect(warnings, isEmpty); + }); + + test('non-android targetOS still reads android.* keys (backward compat)', () { + // Original implementation unconditionally read ndkVersion/androidHome + // from the `android` sub-map regardless of targetOS; downstream only + // consults these fields on OS.android, so storing them on other OSes is + // harmless and preserves backward compatibility. + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.windows, + userDefines: { + 'android': { + UserConfigKeys.ndkVersion: '28.2.13676358', + UserConfigKeys.androidHome: r'C:\Android\Sdk', + }, + UserConfigKeys.cmakeVersion: '3.31.6', + }, + logger: logger, + ); + expect(cfg.targetOS, OS.windows); + expect(cfg.cmakeVersion, '3.31.6'); + expect(cfg.ndkVersion, '28.2.13676358'); + expect(cfg.androidHome, 'C:/Android/Sdk'); + }); + + test('numeric cmake_version tolerated as String (YAML can parse as double)', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.linux, + userDefines: {UserConfigKeys.cmakeVersion: 3.22}, + logger: logger, + ); + expect(cfg.cmakeVersion, '3.22'); + expect(warnings, isEmpty); + }); + + test('wrong-typed cmake_version logs warning and is ignored', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.linux, + userDefines: { + UserConfigKeys.cmakeVersion: ['3.22.1'], + }, + logger: logger, + ); + expect(cfg.cmakeVersion, isNull); + expect(warnings, isNotEmpty); + expect(warnings.first.message, contains('cmake_version')); + }); + + test('wrong-typed prefer_android_cmake logs warning and is ignored', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: {UserConfigKeys.preferAndroidCmake: 'no'}, + logger: logger, + ); + // Falls back to the android default (true). + expect(cfg.preferAndroidCmake, isTrue); + expect(warnings, isNotEmpty); + expect(warnings.first.message, contains('prefer_android_cmake')); + }); + + test('non-Map android sub-map logs warning and is ignored', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: {'android': r'C:\Android\Sdk'}, + logger: logger, + ); + expect(cfg.androidHome, isNull); + expect(cfg.ndkVersion, isNull); + expect(warnings, isNotEmpty); + }); + + test('does not consult ANDROID_HOME env var', () { + // Regression guard: parseFromUserDefines must leave androidHome null + // when no android.android_home is provided, regardless of Platform env. + // (We cannot reliably mutate Platform.environment here, but the absence + // of the key in userDefines is sufficient because the static parser + // disables envVarAndroidHomeAsDefault.) + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: {}, + logger: logger, + ); + const env = String.fromEnvironment('ANDROID_HOME'); + // env is '' under normal test runs; assert our explicit contract. + expect( + cfg.androidHome, + env.isEmpty ? null : env, + reason: 'parseFromUserDefines must not pull ANDROID_HOME from env', + ); + }); + + test('toString contains the relevant fields', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + 'android': {UserConfigKeys.ndkVersion: '28.2.13676358'}, + UserConfigKeys.cmakeVersion: '3.22.1', + }, + logger: logger, + ); + final s = cfg.toString(); + expect(s, contains('targetOS: android')); + expect(s, contains('cmakeVersion: 3.22.1')); + expect(s, contains('ndkVersion: 28.2.13676358')); + }); + + test('copyWith preserves fields and keeps preferAndroid defaults', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + 'android': {UserConfigKeys.androidHome: r'C:\Android\Sdk'}, + }, + logger: logger, + ); + final updated = cfg.copyWith(androidHome: r'D:\Other\Sdk'); + // copyWith goes through the ctor, so the new value is normalised too. + expect(updated.androidHome, 'D:/Other/Sdk'); + // other fields preserved + expect(updated.targetOS, OS.android); + expect(updated.preferAndroidCmake, cfg.preferAndroidCmake); + expect(updated.preferAndroidNinja, cfg.preferAndroidNinja); + }); + + test('androidHome is normalised to forward slashes and no trailing slash', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + 'android': {UserConfigKeys.androidHome: r'C:\Android\Sdk\\'}, + }, + logger: logger, + ); + expect(cfg.androidHome, 'C:/Android/Sdk'); + }); + + test('empty androidHome collapses to null', () { + final cfg = UserConfig.parseFromUserDefines( + targetOS: OS.android, + userDefines: { + 'android': {UserConfigKeys.androidHome: ''}, + }, + logger: logger, + ); + // parseFromUserDefines passes envVar=false, so ANDROID_HOME env won't + // leak in; the empty string collapses to null during normalisation. + expect(cfg.androidHome, isNull); + }); + + test('UserConfig ctor normalises Platform.environment ANDROID_HOME', () { + // Indirect: ctor with envVarAndroidHomeAsDefault=true and a literal + // backslash path should also be normalised; cannot mutate Platform + // environment, so we verify the explicit-pass path instead. + final cfg = UserConfig( + targetOS: OS.android, + androidHome: r'C:\Path\With\Backslashes\', + envVarAndroidHomeAsDefault: false, + ); + expect(cfg.androidHome, 'C:/Path/With/Backslashes'); + }); + }); + + group('CMakeBuilder.unwrapUserDefinesForTesting', () { + test('peels workspace_pubspec.defines envelope', () { + const env = { + 'workspace_pubspec': { + 'base_path': 'D:/flutter/native_toolchain_cmake/example/flutter/pubspec.yaml', + 'defines': { + 'env_file': null, + 'cmake_version': null, + 'ninja_version': null, + 'prefer_android_cmake': null, + 'prefer_android_ninja': null, + 'android': { + 'android_home': r'D:\test_ndk', + 'ndk_version': null, + 'cmake_version': null, + 'ninja_version': null, + }, + }, + }, + }; + final flat = CMakeBuilder.unwrapUserDefinesForTesting(env); + expect(flat['android'], isA()); + expect((flat['android'] as Map)['android_home'], r'D:\test_ndk'); + }); + + test('passes through flat defines (no envelope)', () { + const flat = { + 'env_file': '.env', + 'cmake_version': '3.22.1', + 'android': {'android_home': r'C:\Android\Sdk'}, + }; + final result = CMakeBuilder.unwrapUserDefinesForTesting(flat); + expect(result, same(flat)); + }); + + test('null input returns empty', () { + expect(CMakeBuilder.unwrapUserDefinesForTesting(null), isEmpty); + }); + + test('envelope with wrong-typed defines logs warning', () { + final records = []; + final l = Logger('UnwrapTest')..onRecord.listen(records.add); + const env = { + 'workspace_pubspec': {'base_path': 'foo', 'defines': 'not a map'}, + }; + final result = CMakeBuilder.unwrapUserDefinesForTesting(env, logger: l); + expect(result, isEmpty); + expect(records.where((r) => r.level >= Level.WARNING), isNotEmpty); + }); + }); +} diff --git a/test/native_toolchain/ndk_test.dart b/test/native_toolchain/ndk_test.dart index 1e7f144..4e4632a 100644 --- a/test/native_toolchain/ndk_test.dart +++ b/test/native_toolchain/ndk_test.dart @@ -2,6 +2,10 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:io'; + +import 'package:code_assets/code_assets.dart'; +import 'package:native_toolchain_cmake/src/builder/user_config.dart'; import 'package:native_toolchain_cmake/src/native_toolchain/android_ndk.dart'; import 'package:native_toolchain_cmake/src/tool/tool_requirement.dart'; import 'package:test/test.dart'; @@ -20,4 +24,61 @@ void main() { final satisfied = requirement.satisfy(resolved); expect(satisfied?.length, 4); }); + + // Regression test for https://github.com/rainyl/native_toolchain_cmake/issues/37 + // + // `package:glob` treats `\` as an escape character rather than a path + // separator, so a Windows `androidHome` such as `C:\Android\Sdk` would be + // fed into `Glob('$androidHome/ndk/*/')` and silently match nothing. The + // fix normalises backslashes to forward slashes at [UserConfig] construction + // time. This test reproduces the original failure mode by feeding a real + // on-disk SDK tree via a backslash-laden path (on Windows) or a + // forward-slash path (other OSes), then asserts the NDK is discovered. + test('issue-37-windows-backslash', () async { + // Build a fake SDK layout: /ndk// + final tempUri = await tempDirForTest(); + final sdkDir = await Directory.fromUri(tempUri.resolve("android_sdk")).create(recursive: true); + + // Use an obviously-fake, very-high version so this fixture never collides + // with any real NDK installed under $HOME/AppData/Local/Android/Sdk (which + // the resolver also searches on Windows regardless of androidHome). + const ndkVersion = '99.99.99999'; + final ndkDir = Directory.fromUri(sdkDir.uri.resolve('ndk/').resolve('$ndkVersion/')); + await ndkDir.create(recursive: true); + // tryResolveClang lists `toolchains/llvm/prebuilt/`; create it empty so + // the resolver returns no clang/ar/lld instances without throwing. + await Directory.fromUri(ndkDir.uri.resolve('toolchains/llvm/prebuilt/')).create(recursive: true); + + // Use the OS-native path which on Windows contains backslashes; on + // other platforms it is already forward-slash. Either way the test must + // pass, demonstrating that UserConfig normalisation keeps the glob + // pattern valid. + final rawAndroidHome = sdkDir.absolute.path; + expect( + UserConfig(targetOS: OS.android, androidHome: rawAndroidHome).androidHome, + rawAndroidHome.replaceAll(r'\', '/').replaceAll(RegExp(r'/+$'), ''), + reason: 'UserConfig must normalise backslashes to forward slashes', + ); + + final userConfig = UserConfig( + targetOS: OS.android, + androidHome: rawAndroidHome, + ndkVersion: ndkVersion, + envVarAndroidHomeAsDefault: false, + ); + + // If the androidHome backslashes are NOT normalised before being fed to + // `Glob('/ndk/*/')` package:glob treats `\` as an escape and + // the pattern matches nothing under our fixture, so the resolver cannot + // find an NDK with the requested version and throws. + final resolved = await androidNdk.defaultResolver!.resolve(logger: logger, userConfig: userConfig); + + final ndkInstances = resolved.where((t) => t.tool == androidNdk).toList(); + expect(ndkInstances, hasLength(1), reason: 'Only the fixture NDK should match ndkVersion filter'); + expect( + ndkInstances.single.uri.toFilePath().replaceAll(r'\', '/'), + contains('/ndk/$ndkVersion/'), + reason: 'Resolved NDK URI must point at the fixture we created', + ); + }); }