[PackageLoading] Make sure JSON-encoded rule keys are stable #1802
Conversation
|
@swift-ci please test |
|
@swift-ci smoke test |
|
@swift-ci test with toolchain |
Oops, yeah, this needs an availability check on macOS. (Sorting is only critical for Linux, where JSONEncoder uses native Swift dictionaries.) |
aciidgh
force-pushed
the
stable-rule-keys
branch
from
7c8c5f9 to
562e8fe
Compare
|
Yep. And sorry for stomping on your commit 😅 |
JSONEncoder does not guarantee stable element ordering unless the keys are sorted.
aciidgh
force-pushed
the
stable-rule-keys
branch
from
562e8fe to
3606348
Compare
|
@swift-ci smoke test |
|
@swift-ci test with toolchain |
| @@ -197,6 +197,14 @@ private func fromBytes<T: Decodable>(_ bytes: [UInt8]) throws -> T { | |||
| } | |||
|
|
|||
| private func toBytes<T: Encodable>(_ value: T) throws -> [UInt8] { | |||
| let encoded = try JSONEncoder().encode(value) | |||
| let encoder = JSONEncoder() | |||
| #if os(macOS) | |||
There was a problem hiding this comment.
Are you sure this #if os(macOS) is necessary?
The way #available works is that the '*' indicates that the code path inside the if body will be taken on any platforms not listed. So this by itself:
if #available(OSX 10.13, *) {
encoder.outputFormatting = [.sortedKeys]
}
should be entirely equivalent to:
#if os(macOS)
if #available(OSX 10.13, *) {
encoder.outputFormatting = [.sortedKeys]
}
#else
encoder.outputFormatting = [.sortedKeys]
#endif
There was a problem hiding this comment.
You're right. IDK why I thought that was required 🤷♂️. Fixed here #1803
JSONEncoder does not guarantee stable element ordering unless we specifically ask for the keys to be sorted.
This gets especially important as we plan to enable per-instance hash seeding for Dictionary, which breaks llbuild's rule keys on Linux without this change.