Fix path-activated packages not picking up source changes (#4748) For path-activated global packages, binstubs now always run `dart pub global run` instead of trying to use snapshots. Since path packages are mutable, previously cached snapshots would become stale when source code changed, causing the bug where changes weren't picked up without manually deleting the snapshot directory.
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index 592592f..b0793e0 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -791,9 +791,12 @@ binStubScript, overwrite: true, isRefreshingBinstub: true, - snapshot: executable.pathOfGlobalSnapshot( - entrypoint.workspaceRoot.dir, - ), + snapshot: + entrypoint.isCachedGlobal + ? executable.pathOfGlobalSnapshot( + entrypoint.workspaceRoot.dir, + ) + : null, ); } } @@ -849,9 +852,12 @@ script, overwrite: overwriteBinStubs, isRefreshingBinstub: false, - snapshot: entrypoint.pathOfSnapshot( - exec.Executable.adaptProgramName(package.name, script), - ), + snapshot: + entrypoint.isCachedGlobal + ? entrypoint.pathOfSnapshot( + exec.Executable.adaptProgramName(package.name, script), + ) + : null, ); if (previousPackage != null) { collided[executable] = previousPackage; @@ -927,9 +933,10 @@ /// If [overwrite] is `true`, this will replace an existing binstub with that /// name for another package. /// - /// [snapshot] is a path to a snapshot file. If that snapshot exists the - /// binstub will invoke that directly. Otherwise, it will run - /// `pub global run`. + /// If [snapshot] is provided, the binstub will check for its existence and + /// invoke it directly if present. Otherwise, it will run `pub global run`. + /// If [snapshot] is `null`, the binstub will always run `pub global run` + /// (used for path-activated packages where snapshots would become stale). /// /// If a collision occurs, returns the name of the package that owns the /// existing binstub. Otherwise returns `null`. @@ -938,7 +945,7 @@ String executable, String script, { required bool overwrite, - required String snapshot, + required String? snapshot, required bool isRefreshingBinstub, }) { var binStubPath = p.join(_binStubDir, executable); @@ -959,50 +966,63 @@ final pubInvocation = runningFromTest ? Platform.script.toFilePath() : 'pub'; + final runPubGlobal = '${package.name}:$script'; + final String binstub; - // We need an absolute path since relative ones won't be relative to the - // right directory when the user runs this. - snapshot = p.absolute(snapshot); - // Batch files behave in funky ways if they are modified while updating. - // To ensure that the byte-offsets of everything stays the same even if the - // snapshot filename changes we insert some padding in lines containing the - // snapshot. - // 260 is the maximal short path length on Windows. Hopefully that is - // enough. - final padding = ' ' * (260 - snapshot.length); if (Platform.isWindows) { - binstub = ''' + final header = ''' @echo off rem This file was created by pub v${sdk.version}. rem Package: ${package.name} rem Version: ${package.version} rem Executable: $executable rem Script: $script -if exist "$snapshot" $padding( +'''; + if (snapshot != null) { + // We need an absolute path since relative ones won't be relative to the + // right directory when the user runs this. + snapshot = p.absolute(snapshot); + // Batch files behave in funky ways if they are modified while updating. + // To ensure that the byte-offsets of everything stays the same even if + // the snapshot filename changes we insert some padding in lines + // containing the snapshot. + // 260 is the maximal short path length on Windows. + final padding = ' ' * (260 - snapshot.length); + binstub = ''' +${header}if exist "$snapshot" $padding( call dart "$snapshot" $padding%* rem The VM exits with code 253 if the snapshot version is out-of-date. rem If it is, we need to delete it and run "pub global" manually. if not errorlevel 253 ( goto error ) - call dart $pubInvocation global run ${package.name}:$script %* + call dart $pubInvocation global run $runPubGlobal %* ) else ( - call dart $pubInvocation global run ${package.name}:$script %* + call dart $pubInvocation global run $runPubGlobal %* ) goto eof :error exit /b %errorlevel% :eof '''; + } else { + binstub = ''' +${header}call dart $pubInvocation global run $runPubGlobal %* +'''; + } } else { - binstub = ''' + final header = ''' #!/usr/bin/env sh # This file was created by pub v${sdk.version}. # Package: ${package.name} # Version: ${package.version} # Executable: $executable # Script: $script -if [ -f $snapshot ]; then +'''; + if (snapshot != null) { + snapshot = p.absolute(snapshot); + binstub = ''' +${header}if [ -f $snapshot ]; then dart "$snapshot" "\$@" # The VM exits with code 253 if the snapshot version is out-of-date. # If it is, we need to delete it and run "pub global" manually. @@ -1010,11 +1030,16 @@ if [ \$exit_code != 253 ]; then exit \$exit_code fi - dart $pubInvocation -v global run ${package.name}:$script "\$@" + dart $pubInvocation -v global run $runPubGlobal "\$@" else - dart $pubInvocation global run ${package.name}:$script "\$@" + dart $pubInvocation global run $runPubGlobal "\$@" fi '''; + } else { + binstub = ''' +${header}dart $pubInvocation global run $runPubGlobal "\$@" +'''; + } } // Write the binstub to a temporary location, make it executable and move
diff --git a/test/global/activate/path_package_test.dart b/test/global/activate/path_package_test.dart index bdc1f65..ff19582 100644 --- a/test/global/activate/path_package_test.dart +++ b/test/global/activate/path_package_test.dart
@@ -5,9 +5,11 @@ import 'package:path/path.dart' as p; import 'package:pub/src/io.dart'; import 'package:test/test.dart'; +import 'package:test_process/test_process.dart'; import '../../descriptor.dart' as d; import '../../test_pub.dart'; +import '../binstubs/utils.dart'; void main() { test('activates a package at a local path', () async { @@ -63,7 +65,7 @@ }, ); - test("Doesn't precompile binaries when activating from path", () async { + test("Doesn't precompile the path package's own binaries", () async { final server = await servePackages(); server.serve( 'bar', @@ -82,8 +84,46 @@ args: ['global', 'activate', '--source', 'path', '../foo'], output: allOf([ contains('Activated foo 1.0.0 at path'), - isNot(contains('Built')), + isNot(contains('Built foo:foo')), ]), ); }); + + // Regression test for #4409 + test( + 'path-activated binstub picks up source changes without reactivation', + () async { + await d.dir('foo', [ + d.pubspec({ + 'name': 'foo', + 'executables': {'foo': 'foo'}, + }), + d.dir('bin', [d.file('foo.dart', "main() => print('first');")]), + ]).create(); + + await runPub(args: ['global', 'activate', '--source', 'path', '../foo']); + + final binstub = p.join(d.sandbox, cachePath, 'bin', binStubName('foo')); + + var process = await TestProcess.start( + binstub, + [], + environment: getEnvironment(), + ); + expect(process.stdout, emitsThrough('first')); + await process.shouldExit(); + + await d.dir('foo', [ + d.dir('bin', [d.file('foo.dart', "main() => print('second');")]), + ]).create(); + + process = await TestProcess.start( + binstub, + [], + environment: getEnvironment(), + ); + expect(process.stdout, emitsThrough('second')); + await process.shouldExit(); + }, + ); }