Skip to content

Resolve workspace root and workPackage when invoking pub from any sub-directory #4186

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 11 commits into from
Mar 22, 2024
5 changes: 2 additions & 3 deletions lib/src/command/add.dart
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,7 @@ Specify multiple sdk packages with descriptors.''');
}

String? overridesFileContents;
final overridesPath =
p.join(entrypoint.workspaceRoot.dir, Pubspec.pubspecOverridesFilename);
final overridesPath = entrypoint.workspaceRoot.pubspecOverridesPath;
try {
overridesFileContents = readTextFile(overridesPath);
} on IOException {
Expand All @@ -268,7 +267,7 @@ Specify multiple sdk packages with descriptors.''');
/// gets a report on the other packages that might change version due
/// to this new dependency.
await entrypoint
.withPubspec(
.withWorkPubspec(
Pubspec.parse(
newPubspecText,
cache.sources,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/command/remove.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ To remove a dependency override of a package prefix the package name with
final rootPubspec = entrypoint.workspaceRoot.pubspec;
final newPubspec = _removePackagesFromPubspec(rootPubspec, targets);

await entrypoint.withPubspec(newPubspec).acquireDependencies(
await entrypoint.withWorkPubspec(newPubspec).acquireDependencies(
SolveType.get,
precompile: !isDryRun && argResults.flag('precompile'),
dryRun: isDryRun,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/command/upgrade.dart
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ be direct 'dependencies' or 'dev_dependencies', following packages are not:
}

await entrypoint
.withPubspec(_updatedPubspec(newPubspecText, entrypoint))
.withWorkPubspec(_updatedPubspec(newPubspecText, entrypoint))
.acquireDependencies(
solveType,
dryRun: _dryRun,
Expand Down
172 changes: 136 additions & 36 deletions lib/src/entrypoint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import 'solver.dart';
import 'solver/report.dart';
import 'solver/solve_suggestions.dart';
import 'source/cached.dart';
import 'source/root.dart';
import 'source/unknown.dart';
import 'system_cache.dart';
import 'utils.dart';
Expand All @@ -58,25 +59,112 @@ class Entrypoint {
///
/// [workspaceRoot] will be the package in the nearest parent directory that
/// has `resolution: null`
// TODO(https://github.com/dart-lang/pub/issues/4127): make this actually
// true.
final String workingDir;

Package? _workspaceRoot;
/// Finds the [workspaceRoot] and [workPackage] based on [workingDir].
///
/// Works by iterating through the parent directories from [workingDir].
///
/// [workPackage] is the package of first dir we find with a `pubspec.yaml`
/// file.
///
/// [workspaceRoot] is the package of the first dir we find with a
/// `pubspec.yaml` that does not have `resolution: workspace`.
///
/// [workPackage] and [workspaceRoot] can be the same. And will always be the
/// same when no `workspace` is involved.
/// =
/// If [workingDir] doesn't exist, [fail].
///
/// If no `pubspec.yaml` is found without `resolution: workspace` we [fail].
static ({Package root, Package work}) _loadWorkspace(
String workingDir,
SystemCache cache,
) {
if (!dirExists(workingDir)) {
fail('The directory `$workingDir` does not exist.');
}
// Keep track of all the pubspecs met when walking up the file system.
// The first of these is the workingPackage.
final pubspecsMet = <String, Pubspec>{};
for (final dir in parentDirs(workingDir)) {
final Pubspec pubspec;

try {
pubspec = Pubspec.load(
dir,
cache.sources,
containingDescription: RootDescription(dir),
allowOverridesFile: true,
);
} on FileException {
continue;
}
pubspecsMet[p.canonicalize(dir)] = pubspec;
final Package root;
if (pubspec.resolution == Resolution.none) {
root = Package.load(
dir,
cache.sources,
loadPubspec: (
path, {
expectedName,
required withPubspecOverrides,
}) =>
pubspecsMet[p.canonicalize(path)] ??
Pubspec.load(
path,
cache.sources,
expectedName: expectedName,
allowOverridesFile: withPubspecOverrides,
containingDescription: RootDescription(path),
),
withPubspecOverrides: true,
);
for (final package in root.transitiveWorkspace) {
if (identical(pubspecsMet.entries.first.value, package.pubspec)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to find the Package object that has the first Pubspec that we loaded. Such that we can refer it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you relying on the iteration order for .entries -- that's seems a little surprising.

Maybe, make like a firstPubspecMet variable that is nullable?

Is this because the root loads the packages? Ah.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can rely on the insertion order of a map.

return (root: root, work: package);
}
}
assert(false);
}
}
if (pubspecsMet.isEmpty) {
throw FileException(
'Found no `pubspec.yaml` file in `${p.normalize(p.absolute(workingDir))}` or parent directories',
p.join(workingDir, 'pubspec.yaml'),
);
} else {
final firstEntry = pubspecsMet.entries.first;
throw FileException(
'''
Found a pubspec.yaml at ${firstEntry.key}. But it has resolution `${firstEntry.value.resolution.name}`.
But found no workspace root including it in parent directories.

See $workspacesDocUrl for more information.''',
p.join(workingDir, 'pubspec.yaml'),
);
}
}

/// Stores the result of [_loadWorkspace].
/// Only access via [workspaceRoot], [workPackage] and [canFindWorkspaceRoot].
({Package root, Package work})? _packages;

/// Only access via [workspaceRoot], [workPackage] and [canFindWorkspaceRoot].
({Package root, Package work}) get _getPackages =>
_packages ??= _loadWorkspace(workingDir, cache);

/// The root package this entrypoint is associated with.
///
/// For a global package, this is the activated package.
Package get workspaceRoot => _workspaceRoot ??= Package.load(
null,
workingDir,
cache.sources,
withPubspecOverrides: true,
);
Package get workspaceRoot => _getPackages.root;

/// True if we can find a `pubspec.yaml` to resolve in [workingDir] or any
/// parent directory.
bool get canFindWorkspaceRoot {
try {
workspaceRoot;
_getPackages;
return true;
} on FileException {
return false;
Expand All @@ -87,8 +175,6 @@ class Entrypoint {
///
/// It will be the package in the nearest parent directory to `workingDir`.
/// Example: if a workspace looks like this:
// TODO(https://github.com/dart-lang/pub/issues/4127): make this actually
// true.
///
/// foo/ pubspec.yaml # contains `workspace: [- 'bar'] bar/ pubspec.yaml #
/// contains `resolution: workspace`.
Expand All @@ -98,7 +184,7 @@ class Entrypoint {
///
/// Running `pub add` in `foo` will have foo as workPackage, and add
/// dependencies to `foo/pubspec.yaml`.
Package get workPackage => workspaceRoot;
Package get workPackage => _getPackages.work;

/// The system-wide cache which caches packages that need to be fetched over
/// the network.
Expand Down Expand Up @@ -193,9 +279,9 @@ class Entrypoint {
var packages = {
for (var packageEntry in packageConfig.nonInjectedPackages)
packageEntry.name: Package.load(
packageEntry.name,
packageEntry.resolvedRootDir(packageConfigPath),
cache.sources,
expectedName: packageEntry.name,
),
};
packages[workspaceRoot.name] = workspaceRoot;
Expand Down Expand Up @@ -229,58 +315,72 @@ class Entrypoint {
this._example,
this._packageGraph,
this.cache,
this._workspaceRoot,
this._packages,
this.isCachedGlobal,
);

/// An entrypoint representing a package at [rootDir].
/// An entrypoint for the workspace containing [workingDir]/
///
/// If [checkInCache] is `true` (the default) an error will be thrown if
/// [rootDir] is located inside [cache.rootDir].

Entrypoint(
this.workingDir,
this.cache, {
({Pubspec pubspec, List<Package> workspacePackages})? preloaded,
bool checkInCache = true,
}) : _workspaceRoot = preloaded == null
? null
: Package(
preloaded.pubspec,
workingDir,
preloaded.workspacePackages,
),
isCachedGlobal = false {
}) : isCachedGlobal = false {
if (checkInCache && p.isWithin(cache.rootDir, workingDir)) {
fail('Cannot operate on packages inside the cache.');
}
}

/// Creates an entrypoint at the same location, that will use [pubspec] for
/// resolution.
Entrypoint withPubspec(Pubspec pubspec) {
/// resolution of the [workPackage].
Entrypoint withWorkPubspec(Pubspec pubspec) {
final existingPubspecs = <String, Pubspec>{};
// First extract all pubspecs from the workspace.
for (final package in workspaceRoot.transitiveWorkspace) {
existingPubspecs[package.dir] = package.pubspec;
}
// Then override the one of the workPackage.
existingPubspecs[p.canonicalize(workPackage.dir)] = pubspec;
final newWorkspaceRoot = Package.load(
workspaceRoot.dir,
cache.sources,
loadPubspec: (
dir, {
expectedName,
required withPubspecOverrides,
}) =>
existingPubspecs[p.canonicalize(dir)] ??
Pubspec.load(
dir,
cache.sources,
containingDescription: RootDescription(dir),
),
);
final newWorkPackage = newWorkspaceRoot.transitiveWorkspace
.firstWhere((package) => package.dir == workPackage.dir);
return Entrypoint._(
workingDir,
_lockFile,
_example,
_packageGraph,
cache,
Package(
pubspec,
workingDir,
workspaceRoot.workspaceChildren,
),
(root: newWorkspaceRoot, work: newWorkPackage),
isCachedGlobal,
);
}

/// Creates an entrypoint given package and lockfile objects.
/// If a SolveResult is already created it can be passed as an optimization.
Entrypoint.global(
Package this._workspaceRoot,
Package package,
this._lockFile,
this.cache, {
SolveResult? solveResult,
}) : workingDir = _workspaceRoot.dir,
}) : _packages = (root: package, work: package),
workingDir = package.dir,
isCachedGlobal = true {
if (solveResult != null) {
_packageGraph =
Expand Down Expand Up @@ -410,7 +510,7 @@ class Entrypoint {
}) async {
workspaceRoot; // This will throw early if pubspec.yaml could not be found.
summaryOnly = summaryOnly || _summaryOnlyEnvironment;
final suffix = workspaceRoot.dir == '.' ? '' : ' in ${workspaceRoot.dir}';
final suffix = workspaceRoot.dir == '.' ? '' : ' in `${workspaceRoot.dir}`';

if (enforceLockfile && !fileExists(lockFilePath)) {
throw ApplicationException('''
Expand Down Expand Up @@ -978,7 +1078,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
}
var touchedLockFile = false;
late final lockFile = _loadLockFile(lockFilePath, cache);
late final root = Package.load(null, dir, cache.sources);
late final root = Package.load(dir, cache.sources);

if (!lockfileNewerThanPubspecs) {
if (isLockFileUpToDate(lockFile, root)) {
Expand Down
29 changes: 29 additions & 0 deletions lib/src/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1229,3 +1229,32 @@ String escapeShellArgument(String x) =>
RegExp(r'^[a-zA-Z0-9-_=@.^]+$').stringMatch(x) == null
? "'${x.replaceAll(r'\', r'\\').replaceAll("'", r"'\''")}'"
: x;

/// Returns all parent directories of [path], starting from [path] to the
/// filesystem root.
///
/// If [path] is relative the directories will also be.
///
/// If [from] is passed, directories are made relative to that.
///
/// Examples:
/// parentDirs('/a/b/c') => ('/a/b/c', '/a/b', '/a', '/')
/// parentDirs('./d/e', from: '/a/b/c') => ('./d/e', './d', '.', '..', '../..', '../../..')
Iterable<String> parentDirs(String path, {String? from}) sync* {
var relative = false;
var d = path;
while (true) {
if (relative) {
yield p.relative(d, from: from);
} else {
yield d;
}
if (!p.isWithin(from ?? p.current, d)) {
d = p.normalize(p.join(from ?? p.current, d));
relative = true;
}
final parent = p.dirname(d);
if (parent == d) break;
d = parent;
}
}
Loading