Skip to content

Commit

Permalink
Adjust messages, move check, fixed time comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
@sigurdm
sigurdm committed Sep 30, 2022
1 parent 206a5a7 commit 15013b7
Show file tree
Hide file tree
Showing 13 changed files with 126 additions and 132 deletions.
2 changes: 1 addition & 1 deletion lib/src/command/cache_add.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class CacheAddCommand extends PubCommand {
}

// Download it.
await cache.downloadPackage(id, allowOutdatedHashChecks: true);
await cache.downloadPackage(id);
}

if (argResults['all']) {
Expand Down
12 changes: 1 addition & 11 deletions lib/src/command/dependency_services.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,17 +484,7 @@ class DependencyServicesApplyCommand extends PubCommand {
// This happens when we resolved a package from a legacy server
// not providing archive_sha256. As a side-effect of downloading
// the package we compute and store the sha256.
await cache.downloadPackage(
package,
allowOutdatedHashChecks: false,
);
listedId = PackageId(
listedId.name,
listedId.version,
description.withSha256(
cache.hosted.sha256FromCache(listedId, cache),
),
);
listedId = await cache.downloadPackage(package);
}
updatedPackages.add(listedId);
} else {
Expand Down
18 changes: 4 additions & 14 deletions lib/src/entrypoint.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,6 @@ class Entrypoint {
/// shown --- in case of failure, a reproduction command is shown.
///
/// Updates [lockFile] and [packageRoot] accordingly.
///
/// If [enforceLockfile] is true no changes to the current lockfile are
/// allowed. Instead the existing lockfile is loaded, verified against
/// pubspec.yaml and all dependencies downloaded.
Future<void> acquireDependencies(
SolveType type, {
Iterable<String>? unlock,
Expand Down Expand Up @@ -360,17 +356,11 @@ class Entrypoint {

// We have to download files also with --dry-run to ensure we know the
// archive hashes for downloaded files.
final newLockFile = await result.downloadCachedPackages(cache,
allowOutdatedHashChecks: true);
final newLockFile = await result.downloadCachedPackages(cache);

final report = SolveReport(
type,
root,
lockFile,
newLockFile,
result.availableVersions,
cache,
);
type, root, lockFile, newLockFile, result.availableVersions, cache,
dryRun: dryRun);
if (!onlyReportSuccessOrFailure) {
await report.show();
}
Expand All @@ -383,7 +373,7 @@ class Entrypoint {
if (onlyReportSuccessOrFailure) {
log.message('Got dependencies$suffix.');
} else {
await report.summarize(dryRun: dryRun);
await report.summarize();
}

if (!dryRun) {
Expand Down
6 changes: 2 additions & 4 deletions lib/src/global_packages.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,7 @@ class GlobalPackages {
// We want the entrypoint to be rooted at 'dep' not the dummy-package.
result.packages.removeWhere((id) => id.name == 'pub global activate');

final lockFile = await result.downloadCachedPackages(
cache,
allowOutdatedHashChecks: true,
);
final lockFile = await result.downloadCachedPackages(cache);
final sameVersions =
originalLockFile != null && originalLockFile.samePackageIds(lockFile);

Expand All @@ -247,6 +244,7 @@ To recompile executables, first run `$topLevelProgram pub global deactivate $nam
lockFile,
result.availableVersions,
cache,
dryRun: false,
).show();
}

Expand Down
6 changes: 1 addition & 5 deletions lib/src/null_safety_analysis.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,7 @@ class NullSafetyAnalysis {

if (source is CachedSource) {
// TODO(sigurdm): Consider using withDependencyType here.
await source.downloadToSystemCache(
dependencyId,
_systemCache,
allowOutdatedHashChecks: true,
);
await source.downloadToSystemCache(dependencyId, _systemCache);
}

final libDir =
Expand Down
46 changes: 42 additions & 4 deletions lib/src/solver/report.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import '../lock_file.dart';
import '../log.dart' as log;
import '../package.dart';
import '../package_name.dart';
import '../source/hosted.dart';
import '../source/root.dart';
import '../system_cache.dart';
import '../utils.dart';
Expand All @@ -27,6 +28,7 @@ class SolveReport {
final LockFile _previousLockFile;
final LockFile _newLockFile;
final SystemCache _cache;
final bool _dryRun;

/// The available versions of all selected packages from their source.
///
Expand All @@ -44,14 +46,50 @@ class SolveReport {
this._previousLockFile,
this._newLockFile,
this._availableVersions,
this._cache,
);
this._cache, {
required bool dryRun,
}) : _dryRun = dryRun;

/// Displays a report of the results of the version resolution in
/// [_newLockFile] relative to the [_previousLockFile] file.
Future<void> show() async {
await _reportChanges();
await _reportOverrides();
_checkContentHashesMatchOldLockfile();
}

_checkContentHashesMatchOldLockfile() {
for (final newId in _newLockFile.packages.values) {
var newDescription = newId.description;
// Use the cached content-hashes after downloading to ensure that
// content-hashes from legacy servers gets used.
if (newDescription is ResolvedHostedDescription) {
final cachedHash = newDescription.sha256;
assert(cachedHash != null);
final oldId = _previousLockFile.packages[newId.name];
if (oldId != null &&
cachedHash != null &&
oldId.version == newId.version) {
final oldDecription = oldId.description;
if (oldDecription is ResolvedHostedDescription &&
oldDecription.description == newDescription.description) {
final oldHash = oldDecription.sha256;
if (oldHash != null && !fixedTimeBytesEquals(cachedHash, oldHash)) {
log.warning('''
The content of ${newId.name}-${newId.version} from ${newDescription.description.url} doesn't match the pubspec.lock.
This indicates one of:
* The content has changed on the server since you created the pubspec.lock.
* The pubspec.lock has been corrupted.
${_dryRun ? '' : '\nThe pubspec.lock has been updated.'}
See $contentHashesDocumentationUrl for more information.
''');
}
}
}
}
}
}

/// Displays a one-line message summarizing what changes were made (or would
Expand All @@ -64,7 +102,7 @@ class SolveReport {
/// If [type] is `SolveType.UPGRADE` it also shows the number of packages that
/// are not at the latest available version and the number of outdated
/// packages.
Future<void> summarize({bool dryRun = false}) async {
Future<void> summarize() async {
// Count how many dependencies actually changed.
var dependencies = _newLockFile.packages.keys.toSet();
dependencies.addAll(_previousLockFile.packages.keys);
Expand All @@ -90,7 +128,7 @@ class SolveReport {
}
}

if (dryRun) {
if (_dryRun) {
if (numChanged == 0) {
log.message('No dependencies would change$suffix.');
} else if (numChanged == 1) {
Expand Down
75 changes: 18 additions & 57 deletions lib/src/solver/result.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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:typed_data';

import 'package:collection/collection.dart';
import 'package:pub_semver/pub_semver.dart';

Expand All @@ -18,7 +16,6 @@ import '../pubspec.dart';
import '../source/cached.dart';
import '../source/hosted.dart';
import '../system_cache.dart';
import '../utils.dart';

/// The result of a successful version resolution.
class SolveResult {
Expand Down Expand Up @@ -64,20 +61,23 @@ class SolveResult {
/// If there is a mismatch between the previous content-hash from pubspec.lock
/// and the new one a warning will be printed but the new one will be
/// returned.
Future<LockFile> downloadCachedPackages(
SystemCache cache, {
required bool allowOutdatedHashChecks,
}) async {
await Future.wait(packages.map((id) async {
if (id.source is CachedSource) {
await withDependencyType(_root.dependencyType(id.name), () async {
await cache.downloadPackage(
id,
allowOutdatedHashChecks: allowOutdatedHashChecks,
);
});
}
}));
Future<LockFile> downloadCachedPackages(SystemCache cache) async {
final resolvedPackageIds = await Future.wait(
packages.map((id) async {
if (id.source is CachedSource) {
return await withDependencyType(_root.dependencyType(id.name),
() async {
return await cache.downloadPackage(
id,
);
});
}
return id;
}),
);

// Invariant: the content-hashes in PUB_CACHE matches those provided by the
// server.

// Don't factor in overridden dependencies' SDK constraints, because we'll
// accept those packages even if their constraints don't match.
Expand All @@ -94,46 +94,7 @@ class SolveResult {
});
}
return LockFile(
packages.map((id) {
var description = id.description;
// Use the cached content-hashes after downloading to ensure that
// content-hashes from legacy servers gets used.
if (description is ResolvedHostedDescription) {
Uint8List? cachedHash =
description.description.source.sha256FromCache(id, cache);
if (cachedHash == null) {
// This should not happen.
throw StateError(
'Archive for ${id.name}-${id.version} has no content hash.');
}
final originalEntry = _previousLockFile.packages[id.name];
if (originalEntry != null && originalEntry.version == id.version) {
final originalDescription = originalEntry.description;
if (originalDescription is ResolvedHostedDescription) {
final Uint8List? originalHash = originalDescription.sha256;
if (originalHash != null) {
if (!bytesEquals(cachedHash, originalHash)) {
log.warning('''
The content of ${id.name}-${id.version} on the server doesn't match what was locked in your pubspec.lock.
This might indicate that:
* The content has changed on the server since you created the pubspec.lock.
* The pubspec.lock has been corrupted.
Your pubspec.lock has been updated according to what is on the server.
See $contentHashesDocumentationUrl for more information.
''');
}
}
}
}

return PackageId(
id.name, id.version, description.withSha256(cachedHash));
}
return id;
}).toList(),
resolvedPackageIds,
sdkConstraints: sdkConstraints,
mainDependencies: MapKeySet(_root.dependencies),
devDependencies: MapKeySet(_root.devDependencies),
Expand Down
6 changes: 1 addition & 5 deletions lib/src/source/cached.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ abstract class CachedSource extends Source {
dirExists(getDirectoryInCache(id, cache));

/// Downloads the package identified by [id] to the system cache.
Future<void> downloadToSystemCache(
PackageId id,
SystemCache cache, {
required bool allowOutdatedHashChecks,
});
Future<PackageId> downloadToSystemCache(PackageId id, SystemCache cache);

/// Returns the [Package]s that have been downloaded to the system cache.
List<Package> getCachedPackages(SystemCache cache);
Expand Down
11 changes: 5 additions & 6 deletions lib/src/source/git.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,10 @@ class GitSource extends CachedSource {
/// itself; each of the commit-specific directories are clones of a directory
/// in `cache/`.
@override
Future<void> downloadToSystemCache(
Future<PackageId> downloadToSystemCache(
PackageId id,
SystemCache cache, {
required bool allowOutdatedHashChecks,
}) async {
SystemCache cache,
) async {
return await _pool.withResource(() async {
final ref = id.toRef();
final description = ref.description;
Expand All @@ -322,8 +321,7 @@ class GitSource extends CachedSource {

var revisionCachePath = _revisionCachePath(id, cache);
final path = description.path;
return await _revisionCacheClones.putIfAbsent(revisionCachePath,
() async {
await _revisionCacheClones.putIfAbsent(revisionCachePath, () async {
if (!entryExists(revisionCachePath)) {
await _clone(_repoCachePath(ref, cache), revisionCachePath);
await _checkOut(revisionCachePath, resolvedRef);
Expand All @@ -332,6 +330,7 @@ class GitSource extends CachedSource {
_updatePackageList(revisionCachePath, path);
}
});
return id;
});
}

Expand Down
Loading

0 comments on commit 15013b7

Please sign in to comment.