-
-
Notifications
You must be signed in to change notification settings - Fork 540
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
Fix calculation of Settings hash related to Cache commands #1869
Conversation
SettingsContentHasher used to receive SettingsDictionary and generate a string based on the keys and values. However, it wasn’t sorted according to the keys, so every run could yield a different hash for the settings. I also changed the implementation of `hash(_ settingsDictionary: SettingsDictionary) ` to match the naming, and return the hash of the settings (and not simply a string containing its contents).
In SynthesizedResourceInterfaceProjectMapper.swift
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the fix.
Do you think it would be possible to update the unit test to make it fail in case settings were not sorted?
@@ -16,7 +16,7 @@ public final class DependenciesContentHasher: DependenciesContentHashing { | |||
self.contentHasher = contentHasher | |||
} | |||
|
|||
// MARK: - HeadersContentHashing | |||
// MARK: - DependenciesContentHashing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy/paste traces.... :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagined 😃 There was another one as well.
ed2029b
to
e4535f5
Compare
@all-contributors add @natanrolnik for bug |
@pepibumur I've put up a pull request to add @natanrolnik! 🎉 |
@natanrolnik I think you'll have to update the associated to include the right hash. |
* Fix hashing calculation of SettingsDictionary SettingsContentHasher used to receive SettingsDictionary and generate a string based on the keys and values. However, it wasn’t sorted according to the keys, so every run could yield a different hash for the settings. I also changed the implementation of `hash(_ settingsDictionary: SettingsDictionary) ` to match the naming, and return the hash of the settings (and not simply a string containing its contents). * Fix a typo in a warning message In SynthesizedResourceInterfaceProjectMapper.swift * Update Changelog * Update tests * Fix another test
Short description 📝
In our projects, every time
tuist cache print-hashes
ran, it would result in different hashes.Solution 📦
When debugging Tuist, I noticed that the hash generated by
SettingsContentHasher
was different between runs.After investigating it a little bit, I found that
SettingsContentHasher
used to receive SettingsDictionary and generate a string based on the keys and values. However, it wasn’t sorted according to the keys, so every run could yield a different hash for the settings.I also changed the implementation of
hash(_ settingsDictionary: SettingsDictionary)
to match its naming, and return the hash of the settings instead of simply a string containing its contents.