update-hash: Support __hash_filename__ key for some cases

A lot recipes treats filename key as something special, so for them support
additional key __hash_filename__, in order to avoid such clash.

update-hash

@@ -44,7 +44,7 @@ import tempfile
 # ConfigParser syntax in order to be strictly validated, to prevent misuse
 # and allow easy extension (ex: to other hashes).
 
-FILENAME_KEY = 'filename'
+FILENAME_KEY_LIST = ['filename', '__hash_filename__']
 HASH_MAP = {
     'md5sum': hashlib.md5,
 }
@@ -97,7 +97,7 @@ def main():
                         name, value = line.split('=', 1)
                         name = name.strip()
                         value = value.strip()
-                        if name == FILENAME_KEY:
+                        if name in FILENAME_KEY_LIST:
                             hash_file_path = value
                             current_section.append(line)
                         else:

[Łukasz Nowak]

@vpelletier @jerome

The filename key in sections for various recipes (like hexagonit.recipe.download and slapos.recipe.build:download) has special meaning. That's why I added this functionality.

What do you think about this idea?

What do you think about the name __hash_filename__?

[Jérome Perrin]

@luke the intermediate -hash] sections like on nexedi/slapos!631 (merged) does not work ?

[Vincent Pelletier]

I'm having a hard time deciding which approach (dedicated key vs. separate section) I prefer.

If we continue the dedicated key way, I would suggest _update_hash_filename_: single-underscores on both ends to match existing (even-more-)magic _buildout_section_name_.

[Łukasz Nowak]

Thanks for checking it.

@luke the intermediate -hash] sections like on nexedi/slapos!631 (merged) does not work ?

They work, they just seem hackish for me.

If we continue the dedicated key way, I would suggest _update_hash_filename_: single-underscores on both ends to match existing (even-more-)magic _buildout_section_name_.

+1, I'll name it this way.

I'm having a hard time deciding which approach (dedicated key vs. separate section) I prefer.

For me having dedicated key have big advantage: no need for special section, much cleaner profiles, so less places to make an error. I am in favour of _update_hash_filename_ than -hash] section, which was looking like:

buildout.hash.cfg
[template-hash]
filename = test.txt
md5sum = oink

buildout.cfg
[template]
md5sum = ${template-hash:md5sum}

against:

[template]
_update_hash_filename_ = test.txt
md5sum = oink

[template]
md5sum = ${template-hash:md5sum}

The second way with complicated profiles is much easier to read, debug, code and manage.

[Vincent Pelletier]

Dedicated key approach results in shorter code, yes. But then, some recipe will use md5sum for something different, and it will be the same thing over again for that parameter as well:

[template]
_update_hash_filename_ = test.txt
_update_hash_md5sum_ = oink

[template]
url = (...)/${:_update_hash_filename_}
some-other-name-for-reference-md5sum = ${:_update_hash_md5sum_}

While the "separate section" approach solves everything right now without need for changing this tool, at the cost of a trivial extra section and 2 inter-section references (which are not trivial, granted, and risk getting in the way when copy/pasting).

EDIT: I'm still not prefering one over the other. I'm only saying that I do find advantages in both.

[Łukasz Nowak]

True.

So: how we do? Voting? More reviewers?

Defending _update_hash_filename_: I think that md5sum will most probably mean checksum of the content, but one never knows the creativity of recipe developers...

[Vincent Pelletier]

I think that md5sum will most probably mean checksum of the content

This seems much more reasonable than filename indeed.

So: how we do?

As far as I'm concerned, I think you can go ahead. Your change does not prevent using separate sections if one really wants to, it only makes it slightly less likely to be chosen if one hesitates (because it provided an alternative, and very likely examples using it will follow soon).

[Jérome Perrin]

I think I don't have preference. Both ways are OK to me, we can make this tool also _update_hash_filename_.

[Łukasz Nowak]

I go with _update_hash_filename_. I'll check the current state and as soon as tests are OK (!= passing, as we have issues on the test suites), I'll merge.

[Łukasz Nowak]

One last question: how we document update-hash usage and do we have template for buildout.hash.cfg? If not is it ok if I add README.update-hash.rst, on the same level as update-hash?

[Jérome Perrin]

I think we could make a section somewhere to describe all the tricks with hashes for profile authors, especially the git integration, first update-hash is best used as a git commit hook and there's also a "merge tool" ( https://lab.nexedi.com/nexedi/slapos/blob/2bd95ea0481e18604f15255cd8a8a709fa29957d/update-hash-mergetool ) to resolve automatically most of conflicts with hashes.

[Łukasz Nowak]

section somewhere

The somewhere is where? README.update-hash.rst is good candidate?

[Jérome Perrin]

I was thinking that maybe there's a place in https://slapos.nexedi.com/ that could be extended, I read a bit, maybe https://slapos.nexedi.com/slapos-Tutorial.Create.Software.Release ? but yes, having this page in the repository is more convenient.

https://slapos.nexedi.com/slapos-Tutorial.Create.Software.Release already have a link to README.software.rst so we could continue this pattern of having the web site documentation include a link to the latest technical reference that lives in the repository.

Instances generated from a Software Release take parameters (typically to customise instantiation and instance behaviour) and publish results (typically allowing access to requester). The structure of these values is constrained by how the Software Release was implemented, and must be documented so it can be used. Instance descriptors are intended to provide such documentation in a form allowing automated generation of a user interface to consult and provide parameters, and to consult published results (see also Instance descriptors).