From 89e17cdddb3f3905bffe483c6e68f2ed5df1c50c Mon Sep 17 00:00:00 2001 From: saimskywalker Date: Thu, 27 Aug 2026 14:54:08 +0800 Subject: [PATCH] fix: the README's commands, and three ways the generated scripts break MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by cloning the repo fresh and following the README literally. The two commands at the top of the README, `dart run binary_sdk_bridge`, cannot work for anyone: the package is not on pub.dev, so it can be neither a path/hosted dependency nor `pub global activate`d by name, and there were no install instructions at all. CONTRIBUTING already used the form that does work. Adds an Install section and switches the examples to the executable that `pub global activate --source git` installs. In the generated `tool/fetch_ios_sdk.sh`, the advice line about `flutter build` used backticks inside a double-quoted `echo` — command substitution. The script ran `flutter build` and printed its output in place of the sentence. Binary names were substituted with a bare `FRAMEWORK`/`AAR` token, which is a substring of the scripts' own `FRAMEWORKS_DIR`, `XCFRAMEWORK` and `VENDOR_AAR_URL`. A vendor name with a dash or a dot — `Acme-SDK`, which the validator deliberately accepts, with a test saying so — rewrote those into names bash cannot assign: Acme-SDKS_DIR="$PKG_DIR/ios/acme_ads/Frameworks" URL="${VENDOR_Acme-SDK_URL:-}" The iOS script died on its first statement; the Android one skipped its "no URL" guard and handed curl a garbage URL. Placeholders are now `@...@`-delimited, and the env-file keys are the literal `VENDOR_AAR_URL` / `VENDOR_AAR_SHA256` the script's own help text documents. The `swift package describe` line the script prints ended in `\\`, so pasting it ended the command instead of continuing it. Two smaller things the native flavour got wrong: its fetch script told you to look in Flutter's `Runner.app/Frameworks/`, which a plain iOS app does not have, and the CLI's "Next:" output told you to add a pub path dependency on a package that has no pubspec. The next steps now come from `BridgeGenerator.nextSteps`, so they are a pure value the tests can read. Also: the README claimed exactly one file carries a TODO (there is one per platform) and its options table omitted `--description`. --- CHANGELOG.md | 21 +++++ README.md | 26 +++++- bin/binary_sdk_bridge.dart | 11 +-- lib/src/generator.dart | 30 ++++++ lib/src/templates/script_templates.dart | 62 +++++++++---- test/generator_test.dart | 116 ++++++++++++++++++++++++ 6 files changed, 236 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bba553a..72fa0e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Changelog +## Unreleased + +Fixes found by running the README end to end as a new user would. + +- The generated `tool/fetch_ios_sdk.sh` ran `flutter build` while trying to + explain it: the backticks in the advice line were command substitution + inside a double-quoted `echo`. +- A vendor binary name containing a dash or a dot — `Acme-SDK`, which the + validator deliberately accepts — was substituted into the generated scripts' + own shell variable names, producing `Acme-SDKS_DIR=...` and + `${VENDOR_Acme-SDK_URL}`. The iOS script died on its first statement and the + Android one built a garbage URL. The env-file keys are now literally + `VENDOR_AAR_URL` / `VENDOR_AAR_SHA256`, as the script's own help text says. +- The `swift package describe` command the script prints ended in `\\`, so + pasting it broke the line instead of continuing it. +- `--flavor native` no longer tells you to add a pub path dependency to a + package that has no `pubspec.yaml`, and its fetch script no longer points at + Flutter's `Runner.app`. +- The README's install step was missing entirely, and its example commands + (`dart run binary_sdk_bridge`) could not work from a fresh checkout. + ## 0.1.0 First release. diff --git a/README.md b/README.md index bf4ca8f..80404fb 100644 --- a/README.md +++ b/README.md @@ -4,17 +4,36 @@ Wrap a **closed-source binary SDK** — Swift Package Manager on iOS, a Gradle module on Android — with the vendor binary kept out of version control and *optional at build time*. +## Install + +Not on pub.dev yet, so install it from git: + +```bash +dart pub global activate --source git \ + https://github.com/saimskywalker/binary-sdk-bridge.git +``` + +That puts `binary-sdk-bridge` in `~/.pub-cache/bin`; add that directory to your +`PATH`, or call it as `dart pub global run binary_sdk_bridge`. From a clone, +`dart run bin/binary_sdk_bridge.dart` works with no install at all. + +Requires the Dart SDK (3.5 or newer). Generating needs nothing else — the +vendor binary is fetched later, by the generated `tool/fetch_*.sh`, and is +never required to build. + +## Usage + Two flavours, same core: ```bash # Flutter plugin: native wrappers + Dart API + plugin classes -dart run binary_sdk_bridge \ +binary-sdk-bridge \ --name acme_ads --org com.example \ --ios-framework AcmeSDK --android-aar AcmeSDK \ --out packages # Native only: an SPM package and a Gradle module, no Flutter anywhere -dart run binary_sdk_bridge --flavor native \ +binary-sdk-bridge --flavor native \ --name acme_sdk --org com.example \ --ios-framework AcmeSDK --android-aar AcmeSDK \ --out vendor @@ -116,7 +135,7 @@ acme_ads/ └── tool/fetch_{ios,android}_sdk.sh ``` -Exactly one file has a `TODO`: the bridge. That is the only place the vendor's +Exactly one file per platform has a `TODO`: the bridge. That is the only place the vendor's own API appears, which is the point — everything around it is already decided. ## Notes for the generated Android module @@ -174,6 +193,7 @@ right. | `--ios-framework` | — | `.xcframework` base name; omit to skip iOS | | `--android-aar` | — | `.aar` base name; omit to skip Android | | `--out` | `.` | directory to create the package in | +| `--description` | *generic* | `description:` for the generated pubspec (Flutter flavour only) | | `--ios-target` | `15.0` | iOS deployment target | | `--min-sdk` | `24` | Android `minSdk` | | `--compile-sdk` | `36` | Android `compileSdk` | diff --git a/bin/binary_sdk_bridge.dart b/bin/binary_sdk_bridge.dart index f1819f9..633a1a5 100644 --- a/bin/binary_sdk_bridge.dart +++ b/bin/binary_sdk_bridge.dart @@ -124,16 +124,13 @@ void main(List arguments) { stdout ..writeln('Created ${root.path}') ..writeln() - ..writeln('Next:') - ..writeln(' 1. Add it to your app: ' - '${spec.pluginName}: {path: ${root.path}}') - ..writeln(' 2. Drop the vendor binary in with tool/fetch_*.sh') - ..writeln(' 3. Fill in the TODO in the bridge — that is the only place ' - 'the vendor API appears'); + ..writeln('Next:'); + generator.nextSteps(root.path).forEach(stdout.writeln); } String _usage(ArgParser parser) => ''' -binary-sdk-bridge — wrap a closed-source binary SDK as a Flutter plugin. +binary-sdk-bridge — wrap a closed-source binary SDK as a Flutter plugin +or as native-only SPM + Gradle modules. Usage: binary-sdk-bridge --name --org [options] diff --git a/lib/src/generator.dart b/lib/src/generator.dart index f955483..2e9e607 100644 --- a/lib/src/generator.dart +++ b/lib/src/generator.dart @@ -105,6 +105,36 @@ class BridgeGenerator { return files; } + /// What to do with the package that was just written, as printed lines. + /// + /// Flavour-specific because the native flavour emits no `pubspec.yaml`: the + /// pub path-dependency line is not something a native consumer can act on, + /// and following it puts an unresolvable dependency in their app. + List nextSteps(String packagePath) { + final lines = []; + var number = 1; + void step(String text) => lines.add(' ${number++}. $text'); + void detail(String text) => lines.add(' $text'); + + if (spec.isFlutter) { + step('Add it to your app: ${spec.pluginName}: {path: $packagePath}'); + } else { + step('Add it to your app — this flavour has no pubspec:'); + if (spec.hasIos) { + detail('iOS — add $packagePath/ios/${spec.pluginName} as a local ' + 'Swift package'); + } + if (spec.hasAndroid) { + detail('Android — include $packagePath/android as a Gradle module'); + } + } + step('Drop the vendor binary in with tool/fetch_*.sh'); + step('Fill in the TODO in the bridge — that is the only place the vendor ' + 'API appears'); + + return lines; + } + /// Writes [plan] under `[outputDir]/`. /// /// Refuses to touch an existing directory unless [force] is set: silently diff --git a/lib/src/templates/script_templates.dart b/lib/src/templates/script_templates.dart index 9f25b71..3173f31 100644 --- a/lib/src/templates/script_templates.dart +++ b/lib/src/templates/script_templates.dart @@ -5,10 +5,17 @@ import '../spec.dart'; /// It refuses an unpinned download rather than trusting on first use: the /// artifact links into a shipping app, and a vendor URL whose contents can /// change silently is not something to accept sight-unseen. +/// +/// Placeholders are wrapped in `@...@` on purpose. A bare `FRAMEWORK` token +/// is a SUBSTRING of the shell variables `FRAMEWORKS_DIR` and `XCFRAMEWORK`, +/// so substituting it rewrote those identifiers too — and a vendor name with a +/// dash or a dot (`Acme-SDK`, which [BridgeSpec] accepts, because vendors ship +/// names like that) turned them into names bash cannot assign, killing the +/// script on its first line. String fetchIosSh(BridgeSpec spec) => r''' #!/usr/bin/env bash # -# Fetch the vendor xcframework into ios/PLUGIN/Frameworks/. +# Fetch the vendor xcframework into ios/@PLUGIN@/Frameworks/. # # The binary is NOT committed. This script is the only supported way to put it # in place, so every machine and CI runner ends up with a byte-identical, @@ -21,26 +28,26 @@ String fetchIosSh(BridgeSpec spec) => r''' set -euo pipefail PKG_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -FRAMEWORKS_DIR="$PKG_DIR/ios/PLUGIN/Frameworks" +FRAMEWORKS_DIR="$PKG_DIR/ios/@PLUGIN@/Frameworks" ENV_FILE="$PKG_DIR/tool/sdk_source.env" -DEST="$FRAMEWORKS_DIR/FRAMEWORK.xcframework" +DEST="$FRAMEWORKS_DIR/@FRAMEWORK@.xcframework" mkdir -p "$FRAMEWORKS_DIR" install_from_dir() { rm -rf "$DEST" cp -R "$1" "$DEST" - echo "==> installed $(basename "$1") as FRAMEWORK.xcframework" + echo "==> installed $(basename "$1") as @FRAMEWORK@.xcframework" } # SPM caches manifest evaluation by CONTENT, so a freshly-arrived binary is # invisible to the Package.swift probe until the cache is dropped. clear_spm_cache() { -CACHE_COMMENT +@CACHE_COMMENT@ local mobile_dir="$PKG_DIR/../.." local cache for cache in \ -CACHE_PATHS +@CACHE_PATHS@ do if [[ -e "$cache" ]]; then rm -rf "$cache" @@ -51,12 +58,12 @@ CACHE_PATHS echo "In Xcode, also run File > Packages > Reset Package Caches if it is open." echo echo "VERIFY the binary actually linked -- a green build is NOT evidence." -VERIFY_NOTE +@VERIFY_NOTE@ echo - echo " swift package --package-path PKG_PATH \\\\" + echo " swift package --package-path @PKG_PATH@ \\" echo " describe --type json | grep '\"type\" : \"binary\"'" echo - echo " ls build/ios/iphonesimulator/Runner.app/Frameworks/" +@BUNDLE_CHECK@ } # --- local directory form -------------------------------------------------- @@ -125,17 +132,21 @@ install_from_dir "$XCFRAMEWORK" clear_spm_cache ''' .replaceAll( - 'CACHE_COMMENT', spec.isFlutter ? _flutterComment : _nativeComment) + '@CACHE_COMMENT@', spec.isFlutter ? _flutterComment : _nativeComment) .replaceAll( - 'PKG_PATH', + '@PKG_PATH@', spec.isFlutter ? 'ios/Flutter/ephemeral/Packages/.packages/${spec.pluginName}' : 'ios/${spec.pluginName}', ) - .replaceAll('CACHE_PATHS', spec.isFlutter ? _flutterCaches : _nativeCaches) - .replaceAll('VERIFY_NOTE', spec.isFlutter ? _flutterVerify : _nativeVerify) - .replaceAll('PLUGIN', spec.pluginName) - .replaceAll('FRAMEWORK', spec.iosFrameworkName ?? 'Vendor'); + .replaceAll( + '@CACHE_PATHS@', spec.isFlutter ? _flutterCaches : _nativeCaches) + .replaceAll( + '@VERIFY_NOTE@', spec.isFlutter ? _flutterVerify : _nativeVerify) + .replaceAll('@BUNDLE_CHECK@', + spec.isFlutter ? _flutterBundleCheck : _nativeBundleCheck) + .replaceAll('@PLUGIN@', spec.pluginName) + .replaceAll('@FRAMEWORK@', spec.iosFrameworkName ?? 'Vendor'); /// Flutter resolves through its own ephemeral graph AND Xcode's cloned /// SourcePackages, on top of SwiftPM's global manifest cache. @@ -149,14 +160,25 @@ const _nativeCaches = r''' "$HOME/Library/Caches/org.swift.swiftpm/manifests" \ "$PKG_DIR/.build"'''; +/// The backticks are escaped because this is a DOUBLE-quoted `echo`: unescaped, +/// bash reads them as command substitution and the script literally runs +/// `flutter build` while printing this advice. const _flutterVerify = - r''' echo "`flutter build` passes -quiet to xcodebuild, which suppresses the" + r''' echo "\`flutter build\` passes -quiet to xcodebuild, which suppresses the" echo "#warning the bridge emits. Check one of these instead:"'''; const _nativeVerify = r''' echo "A build that succeeds without the binary looks identical to one" echo "that linked it. Check one of these instead:"'''; +/// `Runner.app` is Flutter's app bundle, so the native flavour cannot point at +/// it — there is no Runner target in a plain iOS app. +const _flutterBundleCheck = + r''' echo " ls build/ios/iphonesimulator/Runner.app/Frameworks/"'''; + +const _nativeBundleCheck = + r''' echo " ls YourApp.app/Frameworks/ # inside the built app bundle"'''; + String fetchAndroidSh(BridgeSpec spec) => r''' #!/usr/bin/env bash # @@ -174,7 +196,7 @@ set -euo pipefail PKG_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" LIBS_DIR="$PKG_DIR/android/libs" ENV_FILE="$PKG_DIR/tool/sdk_source.env" -DEST="$LIBS_DIR/AAR.aar" +DEST="$LIBS_DIR/@AAR@.aar" mkdir -p "$LIBS_DIR" @@ -185,7 +207,7 @@ if [[ $# -ge 1 ]]; then exit 1 fi cp "$1" "$DEST" - echo "==> installed $(basename "$1") as AAR.aar" + echo "==> installed $(basename "$1") as @AAR@.aar" echo " sha256: $(shasum -a 256 "$DEST" | cut -d' ' -f1)" echo echo "Record that hash — for a hand-delivered .aar it is the only evidence" @@ -234,9 +256,9 @@ if [[ "$ACTUAL_SHA" != "$EXPECTED_SHA" ]]; then fi cp "$TMP_DIR/sdk.aar" "$DEST" -echo "==> checksum ok, installed AAR.aar" +echo "==> checksum ok, installed @AAR@.aar" ''' - .replaceAll('AAR', spec.androidAarName ?? 'Vendor'); + .replaceAll('@AAR@', spec.androidAarName ?? 'Vendor'); String gitignore(BridgeSpec spec) { final lines = [ diff --git a/test/generator_test.dart b/test/generator_test.dart index c6fba03..59e8a90 100644 --- a/test/generator_test.dart +++ b/test/generator_test.dart @@ -260,6 +260,122 @@ void main() { }); }); + group('the generated shell scripts are shell, not almost-shell', () { + String script(BridgeSpec spec, String needle) => BridgeGenerator(spec) + .plan() + .firstWhere((f) => f.relativePath.contains(needle)) + .contents; + + BridgeSpec named(String binary) => BridgeSpec( + pluginName: 'acme_ads', + organization: 'com.example', + iosFrameworkName: binary, + androidAarName: binary, + ); + + /// Every `NAME=` assignment and `${NAME...}` reference in a shell script + /// has to be a legal shell identifier, or bash never gets as far as the + /// logic. + void assertShellIdentifiersAreLegal(String source, String label) { + final legal = RegExp(r'^[A-Za-z_][A-Za-z0-9_]*$'); + + for (final line in source.split('\n')) { + final assignment = RegExp(r'^([^\s=]+)=').firstMatch(line); + if (assignment != null) { + expect(legal.hasMatch(assignment.group(1)!), isTrue, + reason: '$label: "${assignment.group(1)}" is not a shell name'); + } + for (final ref in RegExp(r'\$\{([^}:]+)').allMatches(line)) { + final name = ref.group(1)!; + if (name.contains('[')) continue; // ${BASH_SOURCE[0]} + expect(legal.hasMatch(name), isTrue, + reason: '$label: "\$$name" is not a shell name'); + } + } + } + + // The binary name is substituted into these scripts, and a bare + // `FRAMEWORK`/`AAR` token is a SUBSTRING of `FRAMEWORKS_DIR`, + // `XCFRAMEWORK` and `VENDOR_AAR_URL`. Substituting those too turned a + // perfectly ordinary vendor name into `Acme-SDKS_DIR=...`, which bash + // cannot assign — the script died on its first statement. `Acme-SDK` and + // `a.b` are names `BridgeSpec` deliberately accepts. + for (final binary in ['AcmeSDK', 'Acme-SDK', 'a.b', 'Acme_SDK']) { + test('a vendor name like "$binary" keeps the scripts assignable', () { + assertShellIdentifiersAreLegal( + script(named(binary), 'fetch_ios_sdk.sh'), 'ios/$binary'); + assertShellIdentifiersAreLegal( + script(named(binary), 'fetch_android_sdk.sh'), 'android/$binary'); + }); + } + + test('the env-file keys the script documents are the ones it reads', () { + // The heredoc tells the user to write VENDOR_AAR_URL; reading anything + // else means the documented file silently does nothing. + final android = script(named('Acme-SDK'), 'fetch_android_sdk.sh'); + expect(android, contains(r'URL="${VENDOR_AAR_URL:-}"')); + expect(android, contains(r'EXPECTED_SHA="${VENDOR_AAR_SHA256:-}"')); + expect(android, contains('VENDOR_AAR_URL=https://.../Vendor.aar')); + }); + + test('no unescaped backtick reaches a generated script', () { + // A backtick inside a double-quoted `echo` is COMMAND SUBSTITUTION: the + // advice line about `flutter build` ran `flutter build` instead of + // printing it. + for (final flavor in BridgeFlavor.values) { + for (final file in BridgeGenerator(_spec(flavor: flavor)).plan()) { + if (!file.relativePath.endsWith('.sh')) continue; + final source = file.contents; + for (var i = 0; i < source.length; i++) { + if (source[i] != '`') continue; + expect(i > 0 && source[i - 1] == r'\', isTrue, + reason: '${file.relativePath} (${flavor.name}) has an ' + 'unescaped backtick at offset $i'); + } + } + } + }); + + test('the verify command it prints is one a user can paste', () { + // `\\\\` in the file survives the double-quoted echo as `\\`, which + // ends the line without continuing it. + final ios = script(_spec(), 'fetch_ios_sdk.sh'); + expect(ios, contains(r'--package-path')); + expect(ios, isNot(contains(r'\\\\'))); + expect(ios, contains(r'\\"')); + }); + + test('the native flavour never points at a Flutter app bundle', () { + // There is no Runner target in a plain iOS app. + final ios = BridgeGenerator(_spec(flavor: BridgeFlavor.native)) + .plan() + .firstWhere((f) => f.relativePath.contains('fetch_ios_sdk.sh')) + .contents; + expect(ios, isNot(contains('Runner.app'))); + }); + }); + + group('the next steps the CLI prints', () { + test('the Flutter flavour gets the pub path dependency', () { + final steps = BridgeGenerator(_spec()).nextSteps('packages/acme_ads'); + expect(steps.join('\n'), contains('acme_ads: {path: packages/acme_ads}')); + }); + + test('the native flavour is not told to add a pub dependency', () { + // It emits no pubspec.yaml, so `acme_ads: {path: ...}` is an + // instruction that cannot be followed — `flutter pub get` fails on it. + final steps = BridgeGenerator(_spec(flavor: BridgeFlavor.native)) + .nextSteps('vendor/acme_ads'); + final text = steps.join('\n'); + + expect(text, isNot(contains('{path:'))); + expect(text, contains('vendor/acme_ads/ios/acme_ads')); + expect(text, contains('Swift package')); + expect(text, contains('vendor/acme_ads/android')); + expect(text, contains('Gradle module')); + }); + }); + group('the native flavour carries no Flutter', () { List nativeFiles() => BridgeGenerator(_spec(flavor: BridgeFlavor.native)).plan();