Skip to content

traverse local depedencies from package.swift #382

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
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@
},
{
"command": "swift.openExternal",
"when": "view == packageDependencies && viewItem == remote"
"when": "view == packageDependencies && viewItem != local"
}
]
},
Expand Down
12 changes: 11 additions & 1 deletion src/SwiftPackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,19 @@ export interface WorkspaceState {
version: number;
}

/** revision + (branch || version)
* ref: https://github.com/apple/swift-package-manager/blob/e25a590dc455baa430f2ec97eacc30257c172be2/Sources/Workspace/CheckoutState.swift#L19:L23
*/
export interface CheckoutState {
revision: string;
branch: string | null;
version: string | null;
}

export interface WorkspaceStateDependency {
packageRef: { identity: string; kind: string; location: string; name: string };
state: { name: string; path?: string };
state: { name: string; path?: string; checkoutState?: CheckoutState };
subpath: string;
}

export interface PackagePlugin {
Expand Down
2 changes: 1 addition & 1 deletion src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ async function executeTaskWithUI(
*/
function openInExternalEditor(packageNode: PackageNode) {
try {
const uri = vscode.Uri.parse(packageNode.path, true);
const uri = vscode.Uri.parse(packageNode.location, true);
vscode.env.openExternal(uri);
} catch {
// ignore error
Expand Down
230 changes: 200 additions & 30 deletions src/ui/PackageDependencyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,18 @@ import * as vscode from "vscode";
import * as fs from "fs/promises";
import * as path from "path";
import configuration from "../configuration";
import { getRepositoryName, buildDirectoryFromWorkspacePath } from "../utilities/utilities";
import { buildDirectoryFromWorkspacePath } from "../utilities/utilities";
import { WorkspaceContext } from "../WorkspaceContext";
import { FolderEvent } from "../WorkspaceContext";
import { FolderContext } from "../FolderContext";
import contextKeys from "../contextKeys";
import { WorkspaceState } from "../SwiftPackage";
import {
Dependency,
PackageContents,
SwiftPackage,
WorkspaceState,
WorkspaceStateDependency,
} from "../SwiftPackage";

/**
* References:
Expand All @@ -41,6 +47,7 @@ export class PackageNode {
constructor(
public name: string,
public path: string,
public location: string,
public version: string,
public type: "local" | "remote" | "editing"
) {}
Expand Down Expand Up @@ -148,39 +155,129 @@ export class PackageDependenciesProvider implements vscode.TreeDataProvider<Tree
}
if (!element) {
const workspaceState = await folderContext.swiftPackage.loadWorkspaceState();
// Build PackageNodes for all dependencies. Because Package.resolved might not
// be up to date with edited dependency list, we need to remove the edited
// dependencies from the list before adding in the edit version
const children = [
...this.getLocalDependencies(workspaceState),
...this.getRemoteDependencies(folderContext),
];
const editedChildren = await this.getEditedDependencies(workspaceState);
const uneditedChildren: PackageNode[] = [];
for (const child of children) {
const editedVersion = editedChildren.find(item => item.name === child.name);
if (!editedVersion) {
uneditedChildren.push(child);
}
return await this.getDependencyGraph(workspaceState, folderContext);
}

return this.getNodesInDirectory(element.path);
}

private async getDependencyGraph(
workspaceState: WorkspaceState | undefined,
folderContext: FolderContext
): Promise<PackageNode[]> {
if (!workspaceState) {
return [];
}
const inUseDependencies = await this.getInUseDependencies(workspaceState, folderContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment saying why we have to do this

return (
workspaceState?.object.dependencies
.filter(dependency => inUseDependencies.has(dependency.packageRef.identity))
.map(dependency => {
const type = this.dependencyType(dependency);
const version = this.dependencyDisplayVersion(dependency);
const packagePath = this.dependencyPackagePath(
dependency,
folderContext.folder.fsPath
);
const location = dependency.packageRef.location;
return new PackageNode(
dependency.packageRef.identity,
packagePath,
location,
version,
type
);
}) ?? []
);
}

/**
* * Returns a set of all dependencies that are in use in the workspace.
* Why tranverse is necessary here?
* * If we have an implicit local dependency of a dependency, you may not be able to see it in either `Package.swift` or `Package.resolved` unless tranversing from root Package.swift.
* Why not using `swift package show-dependencies`?
* * it costs more time and it triggers the file change of `workspace-state.json` which is not necessary
* Why not using `workspace-state.json` directly?
* * `workspace-state.json` contains all necessary dependencies but it also contains dependencies that are not in use.
* Here is the implementation details:
* 1. local/remote/edited dependency has remote/edited dependencies, Package.resolved covers them
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adam-fowler

After I tried more cases, I just found item 1 is wrong. originally I saw edited remote package is in Package.resolved <-- but it's not . perhaps I prepared a wrong test case
the test case is

  1. A depends on B, B depends on C
  2. swift package edit C, C will dissapear from Package.resolved

This means we can only check if C is in use by traversing from root or we just show edited dependencies out anyway.

then we have to go back to the original discuss point.
It seems traversing is the only way to get correct dependencies graph

Currently I added getEditedDependencySet which read edited packages from workspace-state.json, but it's still wrong. because once A is removed from Package.swift. C is still in workspace-state.json as edited. so it will still be in the list (deleting state.json could fix the issue)

lemme summarize a bit again

  1. tranverse from root, this is the best result [ no missing items, no extra items, but concern about performance]
  2. load dependencies from workspace-state.json [no missing items, but sometimes extra items, no performance issue (deleting state.json could fix the issue)]
  3. current PR, traversing local dependencies + reading editing from state.json + remote packages from pin [extra editing packages sometimes (deleting state.json could fix the issue)]

my preference is 1 >= 2 >= 3

What do you think

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. This is a real performance issue
  2. Yeah workspace-state.json would be the best solution, but it isn't kept up to date. I don't know what the implications of deleting the workspace-state.json every time we resolve is. I have added this issue to the SwiftPM repo Removing a dependency from the Package.swift does not remove it from workspace-state.json swift-package-manager#5727
  3. I'm happy to go with this temporarily. It improves on our current situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also created a PR swiftlang/swift-package-manager#5700
if it can be accepted, workspace-state.json file could be watched in future

swiftlang/swift-package-manager#5727 I'm not sure if they will treat it as a bug, because they may still treat the unused dependencies as "ManagedDepdency", hopefully, they can fix this issue

* 2. remote/edited dependency has a local dependency, the local dependency must have been declared in root Package.swift
* 3. local dependency has a local dependency, traverse it and find the local dependencies only recursively
* 4. pins include all remote and edited packages for 1, 2
*/
private async getInUseDependencies(
workspaceState: WorkspaceState,
folderContext: FolderContext
): Promise<Set<string>> {
const localDependencies = await this.getLocalDependencySet(workspaceState, folderContext);
const remoteDependencies = this.getRemoteDependencySet(folderContext);
const editedDependencies = this.getEditedDependencySet(workspaceState);
return new Set<string>([
...localDependencies,
...remoteDependencies,
...editedDependencies,
]);
}

private getRemoteDependencySet(folderContext: FolderContext | undefined): Set<string> {
return new Set<string>(folderContext?.swiftPackage.resolved?.pins.map(pin => pin.identity));
}

private getEditedDependencySet(workspaceState: WorkspaceState): Set<string> {
return new Set<string>(
workspaceState.object.dependencies
.filter(dependency => this.dependencyType(dependency) === "editing")
.map(dependency => dependency.packageRef.identity)
);
}

/**
* @param workspaceState the workspace state read from `Workspace-state.json`
* @param folderContext the folder context of the current folder
* @returns all local in-use dependencies
*/
private async getLocalDependencySet(
workspaceState: WorkspaceState,
folderContext: FolderContext
): Promise<Set<string>> {
const rootDependencies = folderContext.swiftPackage.dependencies ?? [];
const workspaceStateDependencies = workspaceState.object.dependencies ?? [];
const workspacePath = folderContext.folder.fsPath;

const showingDependencies: Set<string> = new Set<string>();
const stack: Dependency[] = rootDependencies;

while (stack.length > 0) {
const top = stack.pop();
if (!top) {
continue;
}

if (showingDependencies.has(top.identity)) {
continue;
}

if (top.type !== "local" && top.type !== "fileSystem") {
continue;
}
return [...uneditedChildren, ...editedChildren].sort((first, second) =>
first.name.localeCompare(second.name)

showingDependencies.add(top.identity);
const workspaceStateDependency = workspaceStateDependencies.find(
workspaceStateDependency =>
workspaceStateDependency.packageRef.identity === top.identity
);
}
if (!workspaceStateDependency) {
continue;
}

const buildDirectory = buildDirectoryFromWorkspacePath(folderContext.folder.fsPath, true);
const packagePath = this.dependencyPackagePath(workspaceStateDependency, workspacePath);
const childDependencyContents = (await SwiftPackage.loadPackage(
vscode.Uri.file(packagePath)
)) as PackageContents;

if (element instanceof PackageNode) {
// Read the contents of a package.
const packagePath =
element.type === "remote"
? path.join(buildDirectory, "checkouts", getRepositoryName(element.path))
: element.path;
return this.getNodesInDirectory(packagePath);
} else {
// Read the contents of a directory within a package.
return this.getNodesInDirectory(element.path);
stack.push(...childDependencyContents.dependencies);
}
return showingDependencies;
}

/**
Expand All @@ -204,6 +301,7 @@ export class PackageDependenciesProvider implements vscode.TreeDataProvider<Tree
new PackageNode(
dependency.packageRef.identity,
dependency.packageRef.location,
dependency.packageRef.location,
"local",
"local"
)
Expand All @@ -221,6 +319,7 @@ export class PackageDependenciesProvider implements vscode.TreeDataProvider<Tree
new PackageNode(
pin.identity,
pin.location,
pin.location,
pin.state.version ?? pin.state.branch ?? pin.state.revision.substring(0, 7),
"remote"
)
Expand All @@ -244,6 +343,7 @@ export class PackageDependenciesProvider implements vscode.TreeDataProvider<Tree
new PackageNode(
item.packageRef.identity,
item.state.path!,
item.state.path!,
"local",
"editing"
)
Expand Down Expand Up @@ -277,4 +377,74 @@ export class PackageDependenciesProvider implements vscode.TreeDataProvider<Tree
}
});
}

/// - Dependency display helpers

/**
* Get type of WorkspaceStateDependency for displaying in the tree: real version | edited | local
* @param dependency
* @return "local" | "remote" | "editing"
*/
private dependencyType(dependency: WorkspaceStateDependency): "local" | "remote" | "editing" {
if (dependency.state.name === "edited") {
return "editing";
} else if (
dependency.packageRef.kind === "local" ||
dependency.packageRef.kind === "fileSystem"
) {
// need to check for both "local" and "fileSystem" as swift 5.5 and earlier
// use "local" while 5.6 and later use "fileSystem"
return "local";
} else {
return "remote";
}
}

/**
* Get version of WorkspaceStateDependency for displaying in the tree
* @param dependency
* @return real version | editing | local
*/
private dependencyDisplayVersion(dependency: WorkspaceStateDependency): string {
const type = this.dependencyType(dependency);
if (type === "editing") {
return "editing";
} else if (type === "local") {
return "local";
} else {
return (
dependency.state.checkoutState?.version ??
dependency.state.checkoutState?.branch ??
dependency.state.checkoutState?.revision.substring(0, 7) ??
"unknown"
);
}
}

/**
* * Get package source path of dependency
* `editing`: dependency.state.path ?? workspacePath + Packages/ + dependency.subpath
* `local`: dependency.packageRef.location
* `remote`: buildDirectory + checkouts + dependency.packageRef.location
* @param dependency
* @param workspaceFolder
* @return the package path based on the type
*/
private dependencyPackagePath(
dependency: WorkspaceStateDependency,
workspaceFolder: string
): string {
const type = this.dependencyType(dependency);
if (type === "editing") {
return (
dependency.state.path ?? path.join(workspaceFolder, "Packages", dependency.subpath)
);
} else if (type === "local") {
return dependency.state.path ?? dependency.packageRef.location;
} else {
// remote
const buildDirectory = buildDirectoryFromWorkspacePath(workspaceFolder, true);
return path.join(buildDirectory, "checkouts", dependency.subpath);
}
}
}