Don't rewrite pubspec.lock and workspace_ref if not modified (#4589)

Author: sigurdm

lib/src/entrypoint.dart

@@ -405,7 +405,7 @@ See $workspacesDocUrl for more information.''',
   Future<void> writePackageConfigFiles() async {
     ensureDir(p.dirname(packageConfigPath));
 
-    _writeIfDifferent(
+    writeTextFileIfDifferent(
       packageConfigPath,
       await _packageConfigFile(
         cache,
@@ -416,7 +416,7 @@ See $workspacesDocUrl for more information.''',
                 ?.effectiveConstraint,
       ),
     );
-    _writeIfDifferent(packageGraphPath, await _packageGraphFile(cache));
+    writeTextFileIfDifferent(packageGraphPath, await _packageGraphFile(cache));
 
     if (workspaceRoot.workspaceChildren.isNotEmpty) {
       for (final package in workspaceRoot.transitiveWorkspace) {
@@ -430,7 +430,7 @@ See $workspacesDocUrl for more information.''',
         final workspaceRef = const JsonEncoder.withIndent(
           '  ',
         ).convert({'workspaceRoot': relativeRootPath});
-        writeTextFile(workspaceRefPath, '$workspaceRef\n');
+        writeTextFileIfDifferent(workspaceRefPath, '$workspaceRef\n');
       }
     }
   }
@@ -526,17 +526,6 @@ See $workspacesDocUrl for more information.''',
     return '$jsonText\n';
   }
 
-  void _writeIfDifferent(String path, String newContent) {
-    // Compare to the present package_config.json
-    // For purposes of equality we don't care about the `generated` timestamp.
-    final originalText = tryReadTextFile(path);
-    if (originalText != newContent) {
-      writeTextFile(path, newContent);
-    } else {
-      log.fine('`$path` is unchanged. Not rewriting.');
-    }
-  }
-
   /// Gets all dependencies of the [workspaceRoot] package.
   ///
   /// Performs version resolution according to [SolveType].

lib/src/io.dart

@@ -257,6 +257,17 @@ void writeTextFile(
   File(file).writeAsStringSync(contents, encoding: encoding);
 }
 
+void writeTextFileIfDifferent(String path, String newContent) {
+  // Compare to the present package_config.json
+  // For purposes of equality we don't care about the `generated` timestamp.
+  final originalText = tryReadTextFile(path);
+  if (originalText != newContent) {
+    writeTextFile(path, newContent);
+  } else {
+    log.fine('`$path` is unchanged. Not rewriting.');
+  }
+}
+
 /// Reads the contents of the binary file [file].
 void writeBinaryFile(String file, Uint8List data) {
   log.io('Writing ${data.length} bytes to file $file.');

lib/src/lock_file.dart

@@ -406,7 +406,7 @@ ${yamlToString(data)}
         detectWindowsLineEndings(readTextFile(lockFilePath));
 
     final serialized = serialize(p.dirname(lockFilePath), cache);
-    writeTextFile(
+    writeTextFileIfDifferent(
       lockFilePath,
       windowsLineEndings ? serialized.replaceAll('\n', '\r\n') : serialized,
     );

test/package_config_file_test.dart

@@ -313,42 +313,67 @@ void main() {
     });
   });
 
-  test(
-    'package_config and package_graph are not rewritten if unchanged',
-    () async {
-      final server = await servePackages();
-      server.serve('foo', '1.0.0');
-
-      await d.appDir(dependencies: {'foo': 'any'}).create();
+  test('pubspec.lock, package_config, package_graph and workspace_ref '
+      'are not rewritten if unchanged', () async {
+    final server = await servePackages();
+    server.serve('foo', '1.0.0');
+
+    await d.dir(appPath, [
+      d.appPubspec(
+        dependencies: {'foo': 'any'},
+        extras: {
+          'workspace': ['foo'],
+          'environment': {'sdk': '^3.5.0'},
+        },
+      ),
+      d.dir('foo', [d.libPubspec('foo', '1.0.0', resolutionWorkspace: true)]),
+    ]).create();
 
-      await pubGet();
-      final packageConfigFile = File(
-        p.join(sandbox, appPath, '.dart_tool', 'package_config.json'),
-      );
-      final packageConfig = jsonDecode(packageConfigFile.readAsStringSync());
-      final packageConfigTimestamp = packageConfigFile.lastModifiedSync();
-      final packageGraphFile = File(
-        p.join(sandbox, appPath, '.dart_tool', 'package_graph.json'),
-      );
-      final packageGraph = jsonDecode(packageGraphFile.readAsStringSync());
-      final packageGraphTimestamp = packageGraphFile.lastModifiedSync();
-      final s = p.separator;
-      await pubGet(
-        silent: allOf(
-          contains(
-            '`.dart_tool${s}package_config.json` is unchanged. Not rewriting.',
-          ),
-          contains(
-            '`.dart_tool${s}package_graph.json` is unchanged. Not rewriting.',
-          ),
+    await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'});
+    final packageConfigFile = File(
+      p.join(sandbox, appPath, '.dart_tool', 'package_config.json'),
+    );
+    final packageConfig = jsonDecode(packageConfigFile.readAsStringSync());
+    final packageConfigTimestamp = packageConfigFile.lastModifiedSync();
+    final lockFile = File(p.join(sandbox, appPath, 'pubspec.lock'));
+    final lockfileTimestamp = lockFile.lastModifiedSync();
+    final packageGraphFile = File(
+      p.join(sandbox, appPath, '.dart_tool', 'package_graph.json'),
+    );
+    final packageGraph = jsonDecode(packageGraphFile.readAsStringSync());
+    final packageGraphTimestamp = packageGraphFile.lastModifiedSync();
+    final workspaceRefFile = File(
+      p.join(
+        sandbox,
+        appPath,
+        'foo',
+        '.dart_tool',
+        'pub',
+        'workspace_ref.json',
+      ),
+    );
+    final workspaceRefTimestamp = workspaceRefFile.lastModifiedSync();
+    final s = p.separator;
+    await pubGet(
+      silent: allOf(
+        contains(
+          '`.dart_tool${s}package_config.json` is unchanged. Not rewriting.',
         ),
-      );
+        contains(
+          '`.dart_tool${s}package_graph.json` is unchanged. Not rewriting.',
+        ),
+      ),
+      environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
+    );
+    // The resolution of timestamps is not that good.
+    await Future<Null>.delayed(const Duration(seconds: 1));
+    expect(packageConfig, jsonDecode(packageConfigFile.readAsStringSync()));
+    expect(packageConfigFile.lastModifiedSync(), packageConfigTimestamp);
 
-      expect(packageConfig, jsonDecode(packageConfigFile.readAsStringSync()));
-      expect(packageConfigFile.lastModifiedSync(), packageConfigTimestamp);
+    expect(packageGraph, jsonDecode(packageGraphFile.readAsStringSync()));
+    expect(packageGraphFile.lastModifiedSync(), packageGraphTimestamp);
 
-      expect(packageGraph, jsonDecode(packageGraphFile.readAsStringSync()));
-      expect(packageGraphFile.lastModifiedSync(), packageGraphTimestamp);
-    },
-  );
+    expect(lockFile.lastModifiedSync(), lockfileTimestamp);
+    expect(workspaceRefFile.lastModifiedSync(), workspaceRefTimestamp);
+  });
 }