Skip to content

๐Ÿ›‘ Prevent overwriting identical workspace data #607

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.

Sign up for GitHub

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
pepicrft merged 6 commits into from
Apr 27, 2021
Merged

๐Ÿ›‘ Prevent overwriting identical workspace data #607

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5420e9
๐Ÿ›‘ Prevent overwriting identical workspace data
ferologics Apr 21, 2021
0774ce3
๐Ÿ›  Address review comment
ferologics Apr 21, 2021
d3f08ba
๐Ÿคฆ Okay XCWorkspace != XCWorkspaceData
ferologics Apr 21, 2021
46a8341
๐Ÿ›  Use data with glo path
ferologics Apr 21, 2021
d789221
๐Ÿ˜Ž Shift the responsibility of data equality to XCWorkspace
ferologics Apr 26, 2021
69c7894
Update Sources/XcodeProj/Workspace/XCWorkspace.swift
ferologics Apr 26, 2021
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
1 change: 1 addition & 0 deletions Sources/XcodeProj/Project/XcodeProj.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ extension XcodeProj: Writable {
/// - Parameter override: if workspace should be overridden. Default is true.
/// If false will throw error if workspace already exists at the given path.
public func writeWorkspace(path: Path, override: Bool = true) throws {
guard let p = path.glob("*.xcworkspacedata").first, workspace.data != (try? XCWorkspaceData(path: p)) else { return }
Copy link
Copy Markdown
Collaborator

@kwridan kwridan Apr 22, 2021

Choose a reason for hiding this comment

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

There's a few caveats we should be mindful here:

  • The xcworkspacedata serialisation is a responsibility of the XCWorkspace class, a check of this nature can potentially go there, as that component knows the exact path and there wouldn't be a need to search / glob for it :)
  • XCWorkspace deletes and recreates the underlying contents.xcworkspacedata file when override is set, which I believe may be the source of the issue - this is also the same when viewing a Workspace rather than a standalone Project
if override, dataPath.exists {
   try dataPath.delete()
}
  • We need to mindful of not reloading and deserialising content from disk to verify equality as that could potentially be expensive for large workspaces (needs benchmarking), we may be able to compare the raw data without deserialisation - From my quick test locally on a test fixture with 100 projects it didn't have as big of an impact as I feared it would <1%

try workspace.write(path: XcodeProj.workspacePath(path), override: override)
}

Expand Down