🛑 Prevent overwriting identical workspace data #607
Conversation
Codecov Report
@@ Coverage Diff @@
## main #607 +/- ##
==========================================
- Coverage 84.28% 84.20% -0.09%
==========================================
Files 154 154
Lines 8688 8700 +12
==========================================
+ Hits 7323 7326 +3
- Misses 1365 1374 +9
Continue to review full report at Codecov.
|
|
@ferologics to resolve the error, could you try running |
ferologics
force-pushed
the
project-workspace-overwrite
branch
from
f69787e to
0774ce3
Compare
There was a problem hiding this comment.
Thanks @ferologics! This has been one annoying quirk of regenerating projects that would be great to solve.
@yonaskolb would this have any negative side effects for XcodeGen?
From my testing, I noticed if you have a workspace with several projects open, Xcode seems to tolerate regenerating the individual projects, however if you regenerate the entire workspace (including the .xcworkspace) that's when we start hitting the issue of needing to close and re-open Xcode.
Prototyping locally something like this may do the trick:
- extracted a
rawContents()method inXCWorkspaceDatawhich can be used by its write method
// XCWorkspaceData.swift
func rawContents() -> String {
let document = AEXMLDocument()
let workspace = document.addChild(name: "Workspace", value: nil, attributes: ["version": "1.0"])
_ = children
.map { $0.xmlElement() }
.map(workspace.addChild)
return document.xmlXcodeFormat
}
// MARK: - <Writable>
public func write(path: Path, override: Bool = true) throws {
let rawXml = rawContents()
if override, path.exists {
try path.delete()
}
try path.write(rawXml)
}- Updated
XCWorkspace.writeto check the raw contents before deleting
// XCWorkspace.swift
public func write(path: Path, override: Bool = true) throws {
let dataPath = path + "contents.xcworkspacedata"
if override, dataPath.exists {
if let existingContent: String = try? dataPath.read(),
existingContent == data.rawContents() {
// Raw data matches what's on disk
// there's no need to overwrite the contents
// this mitigates Xcode forcing users to
// close and re-open projects/workspaces
// on regneration.
return
}
try dataPath.delete()
}
try dataPath.mkpath()
try data.write(path: dataPath)
}Benchmarking reading the raw contents vs deserialisation they were roughly the same - not sure now which is better 🤷♂️
| @@ -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 } | |||
There was a problem hiding this comment.
There's a few caveats we should be mindful here:
- The
xcworkspacedataserialisation is a responsibility of theXCWorkspaceclass, 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 :) XCWorkspacedeletes and recreates the underlyingcontents.xcworkspacedatafile whenoverrideis 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%
|
Thanks for giving this a thorough look @kwridan 🙇 I agree with your observations about both the responsibility and the root cause. I adjusted the code according to your suggestion. |
|
After spending more time debugging this with a project I still got the occasional "The file “project.xcworkspace” has been modified by another application." when regenerating. A closer look at the Clean slate: <?xml version="1.0" encoding="UTF-8"?>
<Workspace
version = "1.0">
</Workspace>Existing: <?xml version="1.0" encoding="UTF-8"?>
<Workspace
version = "1.0">
<FileRef
location = "self:">
</FileRef>
</Workspace>The problematic |
There was a problem hiding this comment.
The problematic is added at some point in the lifetime of the project. I am not exactly sure what it's doing there, or what it's purpose is
This comes from XCWorkspaceDataElementLocationType that has a case self(String) // Single project workspace in xcodeproj directory. I have not dived deeper into why this one is added as I don't really follow what's the issue with it - the generation should be deterministic, so what exact modification / command creates this change in the data? But I believe we are fine with the fact that this data changes as long as it happens only when the generated .xcworkspace is different as well.
Co-authored-by: Marek Fořt <[email protected]>
|
I believe what you're witnessing is an internal Xcode behaviour: My current understanding of it based on past observations is as follows:
<FileRef
location = "self:">
</FileRef>
<?xml version="1.0" encoding="UTF-8"?>
<Workspace
version = "1.0">
<FileRef
location = "group:App/App.xcodeproj">
</FileRef>
<FileRef
location = "group:Modules/MyFramework.xcodeproj">
</FileRef>
</Workspace>As such we'd need to figure out which |
Partially resolves tuist/tuist#2809
for projects usingApp.xcodeproj-- withoutApp.xcworkspaceat project root. I haven't investigated the issue beyond the scope of.xcodeprojgeneration.Edit: The implementation has been updated to prevent overwriting identical workspace data (regardless of whether it's a project or a workspace)
Short description 📝
Prevent overwriting identical workspace data
Solution 📦
Check if previous workspace data is equal to newly generated workspace data.
Implementation 👩💻👨💻
XCWorkspacethat prevents overwriting of identical dataHow can I test this change @fortmarek? I ran into complications when trying with
1.40.0tag viaswift build -c release --product tuistand using that with Xcode12.4version: