bazel-xcode / PodToBUILD

An easy way to integrate CocoaPods into Bazel
Apache License 2.0
323 stars 69 forks source link

Bug with .pod-version hashes/files in python3 #223

Open bdfreese opened 1 year ago

bdfreese commented 1 year ago

There seems to be an issue after the python3 upgrade where (re-)running @rules_pods//:update_pods updates the .pod-version hash/file, even if there were no actual changes to the WORKSPACE file or any pods's files or pod_repository declarations.

The main issue seems to be the use of python's builtin hash function (see HashFile, etc. in update_pods here).

The python3 documentation for the hash function notes that the hashes:

are “salted” with an unpredictable random value. Although they remain constant within an individual Python process, they are not predictable between repeated invocations of Python.

so future executions will not produce the same hash by design. So every separate run of update_pods will always have new values for the hash, even if we're hashing the same thing. We make .pod-version by hashing and concat-ing the update_pods.py file, repo tool file, and current pod config/context; and now in py3, these always produce new hashes across runs, even if the files are the same, and the script thinks they need updating.

The simplest fix seems like it would be to replace usage of hash with hashlib.md5 or some similar unsalted built in hash function (or making a custom one, since these don't really need to be cryptographically secure or anything).

(Worth noting that since the current .pod-version includes a hash of the current update_pods.py file contents, .pod-versions will update one last time after a fix is merged, but should be stable after that)

[This may qualify as a separate issue, but including here as it relates to the .pod-version calculation as well and would be good to fix all at once if needed] There also may be a couple str<->bytes conversion issues in update_pods.py:

Unclear whether python is smart enough to handle the conversions the correct way all the time; it'd probably be good to have some tests around these and make them consistent, for consistency's sake at least.

slice-dharamd commented 1 year ago

Facing this issue, can we have this fixed ?