[ffigen] Fix path normalization bug (#1549)

pkgs/ffigen/CHANGELOG.md

@@ -1,6 +1,8 @@
-## 14.1.0-wip
+## 14.0.1
 
-- Fix bug with nullable types in `ObjCBlock`'s type arguments.
+- Fix bug with nullable types in `ObjCBlock`'s type arguments:
+  https://github.com/dart-lang/native/issues/1537
+- Fix a path normalization bug: https://github.com/dart-lang/native/issues/1543
 
 ## 14.0.0
 

pkgs/ffigen/lib/src/code_generator/utils.dart

@@ -134,7 +134,7 @@ String findDart() {
   final dartExe = 'dart${p.extension(path)}';
   while (path.isNotEmpty) {
     path = p.dirname(path);
-    final dartPath = p.join(path, dartExe);
+    final dartPath = p.normalize(p.join(path, dartExe));
     if (File(dartPath).existsSync()) return dartPath;
   }
   throw Exception(

pkgs/ffigen/lib/src/config_provider/spec_utils.dart

@@ -30,15 +30,15 @@ String _replaceSeparators(String path) {
   }
 }
 
-/// Replaces the path separators according to current platform. If a relative
-/// path is passed in, it is resolved relative to the config path, and the
-/// absolute path is returned.
-String _normalizePath(String path, String? configFilename) {
-  final skipNormalization =
+/// Replaces the path separators according to current platform, and normalizes .
+/// and .. in the path. If a relative path is passed in, it is resolved relative
+/// to the config path, and the absolute path is returned.
+String normalizePath(String path, String? configFilename) {
+  final resolveInConfigDir =
       (configFilename == null) || p.isAbsolute(path) || path.startsWith('**');
-  return _replaceSeparators(skipNormalization
+  return _replaceSeparators(p.normalize(resolveInConfigDir
       ? path
-      : p.absolute(p.join(p.dirname(configFilename), path)));
+      : p.absolute(p.join(p.dirname(configFilename), path))));
 }
 
 Map<String, LibraryImport> libraryImportsExtractor(
@@ -67,7 +67,7 @@ YamlMap loadSymbolFile(String symbolFilePath, String? configFileName,
     PackageConfig? packageConfig) {
   final path = symbolFilePath.startsWith('package:')
       ? packageConfig!.resolve(Uri.parse(symbolFilePath))!.toFilePath()
-      : _normalizePath(symbolFilePath, configFileName);
+      : normalizePath(symbolFilePath, configFileName);
 
   return loadYaml(File(path).readAsStringSync()) as YamlMap;
 }
@@ -274,7 +274,7 @@ YamlHeaders headersExtractor(
   for (final key in yamlConfig.keys) {
     if (key == strings.entryPoints) {
       for (final h in yamlConfig[key]!) {
-        final headerGlob = _normalizePath(h, configFilename);
+        final headerGlob = normalizePath(h, configFilename);
         // Add file directly to header if it's not a Glob but a File.
         if (File(headerGlob).existsSync()) {
           final osSpecificPath = headerGlob;
@@ -294,7 +294,7 @@ YamlHeaders headersExtractor(
     if (key == strings.includeDirectives) {
       for (final h in yamlConfig[key]!) {
         final headerGlob = h;
-        final fixedGlob = _normalizePath(headerGlob, configFilename);
+        final fixedGlob = normalizePath(headerGlob, configFilename);
         includeGlobs.add(quiver.Glob(fixedGlob));
       }
     }
@@ -433,13 +433,13 @@ String llvmPathExtractor(List<String> value) {
 OutputConfig outputExtractor(
     dynamic value, String? configFilename, PackageConfig? packageConfig) {
   if (value is String) {
-    return OutputConfig(_normalizePath(value, configFilename), null, null);
+    return OutputConfig(normalizePath(value, configFilename), null, null);
   }
   value = value as Map;
   return OutputConfig(
-    _normalizePath(value[strings.bindings] as String, configFilename),
+    normalizePath(value[strings.bindings] as String, configFilename),
     value.containsKey(strings.objCBindings)
-        ? _normalizePath(value[strings.objCBindings] as String, configFilename)
+        ? normalizePath(value[strings.objCBindings] as String, configFilename)
         : null,
     value.containsKey(strings.symbolFile)
         ? symbolFileOutputExtractor(
@@ -455,7 +455,7 @@ SymbolFile symbolFileOutputExtractor(
   if (output.scheme != 'package') {
     _logger.warning('Consider using a Package Uri for ${strings.symbolFile} -> '
         '${strings.output}: $output so that external packages can use it.');
-    output = Uri.file(_normalizePath(output.toFilePath(), configFilename));
+    output = Uri.file(normalizePath(output.toFilePath(), configFilename));
   } else {
     output = packageConfig!.resolve(output)!;
   }

pkgs/ffigen/lib/src/header_parser/sub_parsers/macro_parser.dart

@@ -168,7 +168,7 @@ late Set<String> _macroVarNames;
 
 /// Creates a temporary file for parsing macros in current directory.
 File createFileForMacros() {
-  final fileNameBase = p.join(strings.tmpDir, 'temp_for_macros');
+  final fileNameBase = p.normalize(p.join(strings.tmpDir, 'temp_for_macros'));
   final fileExt = 'hpp';
 
   // Find a filename which doesn't already exist.

pkgs/ffigen/pubspec.yaml

@@ -3,7 +3,7 @@
 # BSD-style license that can be found in the LICENSE file.
 
 name: ffigen
-version: 14.1.0-wip
+version: 14.0.1
 description: >
   Generator for FFI bindings, using LibClang to parse C, Objective-C, and Swift
   files.

pkgs/ffigen/test/unit_tests/normalize_path_test.dart

@@ -0,0 +1,58 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// 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:ffigen/src/config_provider/spec_utils.dart';
+import 'package:path/path.dart' as p;
+import 'package:test/test.dart';
+
+void main() {
+  final abs = Platform.isWindows ? r'C:\' : '/';
+  final config = p.joinAll([abs, 'foo', 'package', 'ffigen.yaml']);
+
+  group('normalizePath', () {
+    test('All separators replaced with platform separators', () {
+      // Unix style.
+      expect(normalizePath('relative/header.h', null),
+          p.joinAll(['relative', 'header.h']));
+
+      // Windows style.
+      expect(normalizePath(r'relative\header.h', null),
+          p.joinAll(['relative', 'header.h']));
+    });
+
+    test('Relative path normalized and resolved relative to config', () {
+      expect(normalizePath('relative/src/header.h', config),
+          p.joinAll([abs, 'foo', 'package', 'relative', 'src', 'header.h']));
+      expect(normalizePath('../src/header.h', config),
+          p.joinAll([abs, 'foo', 'src', 'header.h']));
+      expect(normalizePath('./././src/header.h', config),
+          p.joinAll([abs, 'foo', 'package', 'src', 'header.h']));
+      expect(normalizePath('.././src/.././header.h', config),
+          p.joinAll([abs, 'foo', 'header.h']));
+    });
+
+    test('Absolute path normalized but not resolved', () {
+      expect(normalizePath(p.joinAll([abs, 'absolute/src/header.h']), config),
+          p.joinAll([abs, 'absolute', 'src', 'header.h']));
+      expect(normalizePath(p.joinAll([abs, './src/.././header.h']), config),
+          p.joinAll([abs, 'header.h']));
+    });
+
+    test('Glob path normalized but not resolved', () {
+      expect(normalizePath('**/glob/*/header.h', config),
+          p.joinAll(['**', 'glob', '*', 'header.h']));
+      expect(normalizePath('**/glob/./*/../header.h', config),
+          p.joinAll(['**', 'glob', 'header.h']));
+    });
+
+    test('Null configFilename normalized but not resolved', () {
+      expect(normalizePath(p.joinAll(['relative/src/header.h']), null),
+          p.joinAll(['relative', 'src', 'header.h']));
+      expect(normalizePath(p.joinAll(['./src/.././header.h']), null),
+          p.joinAll(['header.h']));
+    });
+  });
+}