Skip to content

Fix .packages entries of relative path deps when using --directory #2916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions lib/src/entrypoint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,18 @@ class Entrypoint {

/// Writes .packages and .dart_tool/package_config.json
Future<void> writePackagesFiles() async {
writeTextFile(packagesFile, lockFile.packagesFile(cache, root.name));
writeTextFile(
packagesFile,
lockFile.packagesFile(cache,
entrypoint: root.name, relativeFrom: root.dir));
ensureDir(p.dirname(packageConfigFile));
writeTextFile(
packageConfigFile,
await lockFile.packageConfigFile(
cache,
entrypoint: root.name,
entrypointSdkConstraint: root.pubspec.sdkConstraints[sdk.identifier],
));
await lockFile.packageConfigFile(cache,
entrypoint: root.name,
entrypointSdkConstraint:
root.pubspec.sdkConstraints[sdk.identifier],
relativeFrom: root.dir));
}

/// Gets all dependencies of the [root] package.
Expand Down Expand Up @@ -549,7 +552,8 @@ class Entrypoint {
// If we can't load the pubspec, the user needs to re-run "pub get".
}

dataError('${p.join(source.getDirectory(id), 'pubspec.yaml')} has '
dataError(
'${p.join(source.getDirectory(id, relativeFrom: '.'), 'pubspec.yaml')} has '
'changed since the pubspec.lock file was generated, please run "pub '
'get" again.');
}
Expand All @@ -576,7 +580,7 @@ class Entrypoint {
if (source is! CachedSource) return true;

// Get the directory.
var dir = source.getDirectory(package);
var dir = source.getDirectory(package, relativeFrom: '.');
// See if the directory is there and looks like a package.
return dirExists(dir) && fileExists(p.join(dir, 'pubspec.yaml'));
});
Expand Down Expand Up @@ -616,7 +620,9 @@ class Entrypoint {
}

final source = cache.source(lockFileId.source);
final lockFilePackagePath = root.path(source.getDirectory(lockFileId));
final lockFilePackagePath = root.path(
source.getDirectory(lockFileId, relativeFrom: root.dir),
);

// Make sure that the packagePath agrees with the lock file about the
// path to the package.
Expand Down Expand Up @@ -765,7 +771,8 @@ class Entrypoint {
cache.load(id).pubspec.sdkConstraints[sdk.identifier],
);
if (pkg.languageVersion != languageVersion) {
dataError('${p.join(source.getDirectory(id), 'pubspec.yaml')} has '
dataError(
'${p.join(source.getDirectory(id, relativeFrom: '.'), 'pubspec.yaml')} has '
'changed since the pubspec.lock file was generated, please run '
'"pub get" again.');
}
Expand Down
32 changes: 21 additions & 11 deletions lib/src/global_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,14 @@ To recompile executables, first run `global deactivate ${dep.name}`.
// Load the package graph from [result] so we don't need to re-parse all
// the pubspecs.
final entrypoint = Entrypoint.global(
Package(result.pubspecs[dep.name],
cache.source(dep.source).getDirectory(id)),
lockFile,
cache,
solveResult: result);
Package(
result.pubspecs[dep.name],
(cache.source(dep.source) as CachedSource).getDirectoryInCache(id),
),
lockFile,
cache,
solveResult: result,
);
if (!sameVersions) {
// Only precompile binaries if we have a new resolution.
await entrypoint.precompileExecutables();
Expand All @@ -261,10 +264,12 @@ To recompile executables, first run `global deactivate ${dep.name}`.
// TODO(sigurdm): Use [Entrypoint.writePackagesFiles] instead.
final packagesFilePath = _getPackagesFilePath(package);
final packageConfigFilePath = _getPackageConfigFilePath(package);
writeTextFile(packagesFilePath, lockFile.packagesFile(cache));
ensureDir(p.dirname(packageConfigFilePath));
final dir = p.dirname(packagesFilePath);
writeTextFile(
packageConfigFilePath, await lockFile.packageConfigFile(cache));
packagesFilePath, lockFile.packagesFile(cache, relativeFrom: dir));
ensureDir(p.dirname(packageConfigFilePath));
writeTextFile(packageConfigFilePath,
await lockFile.packageConfigFile(cache, relativeFrom: dir));
}

/// Finishes activating package [package] by saving [lockFile] in the cache.
Expand Down Expand Up @@ -355,7 +360,7 @@ To recompile executables, first run `global deactivate ${dep.name}`.
if (source is CachedSource) {
// For cached sources, the package itself is in the cache and the
// lockfile is the one we just loaded.
entrypoint = Entrypoint.global(cache.load(id), lockFile, cache);
entrypoint = Entrypoint.global(cache.loadCached(id), lockFile, cache);
} else {
// For uncached sources (i.e. path), the ID just points to the real
// directory for the package.
Expand Down Expand Up @@ -515,8 +520,13 @@ To recompile executables, first run `global deactivate ${dep.name}`.
await _writePackageConfigFiles(id.name, entrypoint.lockFile);
await entrypoint.precompileExecutables();
var packageExecutables = executables.remove(id.name) ?? [];
_updateBinStubs(entrypoint, cache.load(id), packageExecutables,
overwriteBinStubs: true, suggestIfNotOnPath: false);
_updateBinStubs(
entrypoint,
cache.load(id),
packageExecutables,
overwriteBinStubs: true,
suggestIfNotOnPath: false,
);
successes.add(id.name);
} catch (error, stackTrace) {
var message = 'Failed to reactivate '
Expand Down
13 changes: 10 additions & 3 deletions lib/src/lock_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:collection';
import 'dart:convert';

import 'package:collection/collection.dart' hide mapMap;
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;
import 'package:pub_semver/pub_semver.dart';
import 'package:source_span/source_span.dart';
Expand Down Expand Up @@ -218,7 +219,11 @@ class LockFile {
///
/// If [entrypoint] is passed, a relative entry is added for its "lib/"
/// directory.
String packagesFile(SystemCache cache, [String entrypoint]) {
String packagesFile(
SystemCache cache, {
String entrypoint,
@required String relativeFrom,
}) {
var header = '''
This file is deprecated. Tools should instead consume
`.dart_tools/package_config.json`.
Expand All @@ -231,7 +236,8 @@ Generated by pub on ${DateTime.now()}.''';
Map<String, Uri>.fromIterable(ordered(packages.keys), value: (name) {
var id = packages[name];
var source = cache.source(id.source);
return p.toUri(p.join(source.getDirectory(id), 'lib'));
return p.toUri(
p.join(source.getDirectory(id, relativeFrom: relativeFrom), 'lib'));
});

if (entrypoint != null) map[entrypoint] = Uri.parse('lib/');
Expand All @@ -254,12 +260,13 @@ Generated by pub on ${DateTime.now()}.''';
SystemCache cache, {
String entrypoint,
VersionConstraint entrypointSdkConstraint,
@required String relativeFrom,
}) async {
final entries = <PackageConfigEntry>[];
for (final name in ordered(packages.keys)) {
final id = packages[name];
final source = cache.source(id.source);
final rootPath = source.getDirectory(id);
final rootPath = source.getDirectory(id, relativeFrom: relativeFrom);
Uri rootUri;
if (p.isRelative(rootPath)) {
// Relative paths are relative to the root project, we want them
Expand Down
8 changes: 6 additions & 2 deletions lib/src/null_safety_analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ class NullSafetyAnalysis {
return nullSafetyComplianceOfPackages(
result.packages.where((id) => id.name != fakeRootName),
Package(rootPubspec,
packageId.source.bind(_systemCache).getDirectory(packageId)));
packageId.source.bind(_systemCache).getDirectory(packageId)),
containingPath);
}

/// Decides if all dependendencies (transitively) have a language version
Expand All @@ -133,7 +134,10 @@ class NullSafetyAnalysis {
///
/// Assumes the root package is opted in.
Future<NullSafetyAnalysisResult> nullSafetyComplianceOfPackages(
Iterable<PackageId> packages, Package rootPackage) async {
Iterable<PackageId> packages,
Package rootPackage,
String containingPath,
) async {
NullSafetyAnalysisResult firstBadPackage;
for (final dependencyId in packages) {
final packageInternalAnalysis =
Expand Down
3 changes: 2 additions & 1 deletion lib/src/solver/package_lister.dart
Original file line number Diff line number Diff line change
Expand Up @@ -448,5 +448,6 @@ class _RootSource extends BoundSource {
@override
Future<Pubspec> doDescribe(PackageId id) => throw _unsupported;
@override
String getDirectory(PackageId id) => throw _unsupported;
String getDirectory(PackageId id, {String relativeFrom}) =>
throw _unsupported;
}
5 changes: 4 additions & 1 deletion lib/src/source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,10 @@ abstract class BoundSource {
/// Returns the directory where this package can (or could) be found locally.
///
/// If the source is cached, this will be a path in the system cache.
String getDirectory(PackageId id);
///
/// If id is a relative path id, the directory will be relative from
/// [relativeFrom]. Returns an absolute path if [relativeFrom] is not passed.
String getDirectory(PackageId id, {String relativeFrom});

/// Returns metadata about a given package. Information about remotely hosted
/// packages can be cached for up to [maxAge].
Expand Down
10 changes: 8 additions & 2 deletions lib/src/source/cached.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ abstract class CachedSource extends BoundSource {
/// Otherwise, defers to the subclass.
@override
Future<Pubspec> doDescribe(PackageId id) async {
var packageDir = getDirectory(id);
var packageDir = getDirectoryInCache(id);
if (fileExists(path.join(packageDir, 'pubspec.yaml'))) {
return Pubspec.load(packageDir, systemCache.sources,
expectedName: id.name);
Expand All @@ -40,6 +40,12 @@ abstract class CachedSource extends BoundSource {
return await describeUncached(id);
}

@override
String getDirectory(PackageId id, {String relativeFrom}) =>
getDirectoryInCache(id);

String getDirectoryInCache(PackageId id);

/// Loads the (possibly remote) pubspec for the package version identified by
/// [id].
///
Expand All @@ -49,7 +55,7 @@ abstract class CachedSource extends BoundSource {

/// Determines if the package identified by [id] is already downloaded to the
/// system cache.
bool isInSystemCache(PackageId id) => dirExists(getDirectory(id));
bool isInSystemCache(PackageId id) => dirExists(getDirectoryInCache(id));

/// Downloads the package identified by [id] to the system cache.
Future<Package> downloadToSystemCache(PackageId id);
Expand Down
4 changes: 2 additions & 2 deletions lib/src/source/git.dart
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class BoundGitSource extends CachedSource {

/// Returns the path to the revision-specific cache of [id].
@override
String getDirectory(PackageId id) =>
String getDirectoryInCache(PackageId id) =>
p.join(_revisionCachePath(id), id.description['path']);

@override
Expand Down Expand Up @@ -396,7 +396,7 @@ class BoundGitSource extends CachedSource {
result.add(RepairResult(id, success: false));

// Delete the revision cache path, not the subdirectory that contains the package.
tryDeleteEntry(getDirectory(id));
tryDeleteEntry(getDirectoryInCache(id));
}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/src/source/hosted.dart
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,12 @@ class BoundHostedSource extends CachedSource {
@override
Future<Package> downloadToSystemCache(PackageId id) async {
if (!isInSystemCache(id)) {
var packageDir = getDirectory(id);
var packageDir = getDirectoryInCache(id);
ensureDir(p.dirname(packageDir));
await _download(id, packageDir);
}

return Package.load(id.name, getDirectory(id), systemCache.sources);
return Package.load(id.name, getDirectoryInCache(id), systemCache.sources);
}

/// The system cache directory for the hosted source contains subdirectories
Expand All @@ -431,7 +431,7 @@ class BoundHostedSource extends CachedSource {
/// Each of these subdirectories then contains a subdirectory for each
/// package downloaded from that site.
@override
String getDirectory(PackageId id) {
String getDirectoryInCache(PackageId id) {
var parsed = source._parseDescription(id.description);
var dir = _urlToDirectory(parsed.last);
return p.join(systemCacheRoot, dir, '${parsed.first}-${id.version}');
Expand Down
6 changes: 5 additions & 1 deletion lib/src/source/path.dart
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ class BoundPathSource extends BoundSource {
}

@override
String getDirectory(PackageId id) => id.description['path'];
String getDirectory(PackageId id, {String relativeFrom}) {
return id.description['relative']
? p.relative(id.description['path'], from: relativeFrom)
: id.description['path'];
}

/// Ensures that [description] is a valid path description and returns a
/// normalized path to the package.
Expand Down
2 changes: 1 addition & 1 deletion lib/src/source/sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class BoundSdkSource extends BoundSource {
}

@override
String getDirectory(PackageId id) {
String getDirectory(PackageId id, {String relativeFrom}) {
try {
return _verifiedPackagePath(id);
} on PackageNotFoundException catch (error) {
Expand Down
5 changes: 3 additions & 2 deletions lib/src/source/unknown.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class _BoundUnknownSource extends BoundSource {

/// Returns the directory where this package can be found locally.
@override
String getDirectory(PackageId id) => throw UnsupportedError(
"Cannot find a package from an unknown source '${source.name}'.");
String getDirectory(PackageId id, {String relativeFrom}) =>
throw UnsupportedError(
"Cannot find a package from an unknown source '${source.name}'.");
}
9 changes: 9 additions & 0 deletions lib/src/system_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ class SystemCache {
return Package.load(id.name, source(id.source).getDirectory(id), sources);
}

Package loadCached(PackageId id) {
final bound = source(id.source);
if (bound is CachedSource) {
return Package.load(id.name, bound.getDirectoryInCache(id), sources);
} else {
throw ArgumentError('Call only on Cached ids.');
}
}

/// Determines if the system cache contains the package identified by [id].
bool contains(PackageId id) {
var source = this.source(id.source);
Expand Down
28 changes: 28 additions & 0 deletions test/get/path/relative_path_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,34 @@ void main() {
{'foo': '../relative/foo', 'bar': '../relative/bar'}).validate();
});

test('path is relative to containing pubspec when using --directory',
() async {
await d.dir('relative', [
d.dir('foo', [
d.libDir('foo'),
d.libPubspec('foo', '0.0.1', deps: {
'bar': {'path': '../bar'}
})
]),
d.dir('bar', [d.libDir('bar'), d.libPubspec('bar', '0.0.1')])
]).create();

await d.dir(appPath, [
d.appPubspec({
'foo': {'path': '../relative/foo'}
})
]).create();

await pubGet(
args: ['--directory', appPath],
workingDirectory: d.sandbox,
output: contains('Changed 2 dependencies in myapp!'));

await d.appPackagesFile(
{'foo': '../relative/foo', 'bar': '../relative/bar'},
).validate();
});

test('relative path preserved in the lockfile', () async {
await d
.dir('foo', [d.libDir('foo'), d.libPubspec('foo', '0.0.1')]).create();
Expand Down
6 changes: 3 additions & 3 deletions test/must_pub_get_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void main() {

await pubGet();

await createLockFile(appPath, sandbox: ['foo']);
await createLockFile(appPath, dependenciesInSandBox: ['foo']);

// Ensure that the pubspec looks newer than the lockfile.
await _touch('pubspec.yaml');
Expand Down Expand Up @@ -154,7 +154,7 @@ void main() {

await pubGet();

await createLockFile(appPath, sandbox: ['foo']);
await createLockFile(appPath, dependenciesInSandBox: ['foo']);

// Ensure that the pubspec looks newer than the lockfile.
await _touch('pubspec.yaml');
Expand Down Expand Up @@ -285,7 +285,7 @@ foo:http://example.com/

await pubGet();

await createPackagesFile(appPath, sandbox: ['foo']);
await createPackagesFile(appPath, dependenciesInSandBox: ['foo']);

// Ensure that the pubspec looks newer than the lockfile.
await _touch('pubspec.lock');
Expand Down
Loading