Make the full_hash look like the dag_hash#8911
Conversation
|
Somewhat off topic, but you wouldn't have any idea how to solve #7694 would you? |
|
Sorry, I shouldn't volunteer information when my understanding is so limited. But I'm curious, do you want to replace those long patch hashes with a unique hash prefix (like git does with commits), or are you thinking of using a different hashing algorithm which results in a shorter hash (e.g. 32 characters instead of 64) as this topic does with the spec While reviewing another topic I'm working on (#8718), @tgamblin mentioned that spack allows you to enter unique prefixes of hashes and have them work. So I wonder if that's an option here? |
|
@scottwittenburg Let me update #7694 to be more specific so as not to derail your PR with a long off-topic conversation 😄 |
tgamblin
left a comment
tgamblin
left a comment
There was a problem hiding this comment.
LGTM except one minor change
| if sys.version_info[0] >= 3: | ||
| b32_hash = b32_hash.decode('utf-8') | ||
|
|
||
| self._full_hash = b32_hash |
There was a problem hiding this comment.
This should only cache if the spec is concrete, like the dag_hash does. I suspect that in practice, it always will be, but why not make it mirror dag_hash?
There was a problem hiding this comment.
Well, this function raises an exception immediately if the spec is not concrete, so your suspicion is correct 😄 and I think we could be good here.
There was a problem hiding this comment.
Oh let's not start pointing out all our "duh" moments or I'll start feeling bad.
Thanks for the review and merge!
3 participants
As requested in the review of #8718, this change makes the format/length of the
full_hashmatch that of thedag_hash.