Repository navigation
Bugfix 2788 - #2796
Bugfix 2788#2796
Conversation
There was a problem hiding this comment.
You should be able to unnest these:
return createTempDir
.flatMap(.merge) { temp in
copyItem(binaryURL, temp)
}
.flatMap(.merge) { workspace in
strip(workspace, keepingArchitectures)
}
.flatMap(.merge) { workspace in
replace(binaryURL, workspace)
}| return architecturesInPackage(binaryURL) | ||
| .filter { !keepingArchitectures.contains($0) } | ||
| .flatMap(.concat) { stripArchitecture(binaryURL, $0) } | ||
| let fileManager = FileManager().reactive |
There was a problem hiding this comment.
Should this use FileManager.default?
There was a problem hiding this comment.
I don't remember the specific details, but there is (was?) a concurrency issue with the default FileManager.
Ah, here it is:
https://useyourloaf.com/blog/nsfilemanager-defaultmanager-is-not-thread-safe/
and once again, I know things that are no longer true. :-) I'll update it.
|
|
||
| public func replaceItem(at originalItemURL: URL, withItemAt newItemURL: URL) -> SignalProducer<(), CarthageError> { | ||
| do { | ||
| guard (try FileManager().replaceItemAt(originalItemURL, withItemAt: newItemURL, backupItemName: nil, options: .usingNewMetadataOnly)) != nil else { |
There was a problem hiding this comment.
I missed this before, but this should be using self.base, not FileManager()
There was a problem hiding this comment.
Ah. You're right. Stand by…
| return fileManager.replaceItem(at: original, withItemAt: modified) | ||
| } | ||
|
|
||
| return createTempDir |
There was a problem hiding this comment.
Would you mind adding a comment here that explains why we're going through the trouble of copying/stripping/replacing instead of just stripping?
When stripping frameworks, copy the source into a temporary directory, process it there, and then move it back.
This is a rework of my previous PR, which didn't actually do what I thought it did.