Skip to content

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

@bdfreese

Description

@bdfreese

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:

  • the __exec() helper function returns bytes, but sometimes we compare this to GetVersion, which returns a regular string (e.g., in _needs_update())
  • when we write .pod-version at the end of _update_repo_impl(), we open the file with w option instead of wb, while we updated to use the b option in other file ops when upgrading to python 3

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions