Skip to content
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

2.18.0 adding code hash sum #48

Merged
merged 15 commits into from
Apr 9, 2024
10 changes: 9 additions & 1 deletion changelog
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [2.17.2] - 2024-03-20

### Changed
- vhdlhelper.py with created function 'get_files_hash_value' (hash value of all .py and .vhd files in "tmVhdlProducer" recursively) for use in VHDL templates ("menu.info.files_hash_value").
- algodist.py with writing "files_hash_value" to log file.
- VHDL templates.

## [2.17.1] - 2024-02-28

### Fixed
Expand Down Expand Up @@ -310,7 +317,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- adjusted `--dryrun` and `--ratio` options.

[unreleased]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.17.1...HEAD
[unreleased]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.17.2...HEAD
[2.17.2]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.17.1...2.17.2
[2.17.1]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.17.0...2.17.1
[2.17.0]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.16.0...2.17.0
[2.16.0]: https://github.com/cms-l1-globaltrigger/tm-vhdlproducer/compare/2.15.1...2.16.0
Expand Down
2 changes: 1 addition & 1 deletion tmVhdlProducer/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "2.17.1"
__version__ = "2.17.2"
13 changes: 13 additions & 0 deletions tmVhdlProducer/algodist.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import argparse
import json
import logging
import subprocess
import uuid
import sys, os
from collections import namedtuple
Expand All @@ -32,6 +33,8 @@
import tmGrammar

from .constants import BRAMS_TOTAL, SLICELUTS_TOTAL, PROCESSORS_TOTAL, NR_CALOS, NR_MUONS
from . import vhdlhelper
from . import __version__

from .handles import Payload
from .handles import ObjectHandle
Expand Down Expand Up @@ -1694,6 +1697,16 @@ def dump_distribution(collection: ModuleCollection, filename: str):

def distribute(eventSetup, modules: int, config: str, ratio: float, reverse_sorting: bool, constraints: Dict[str, str] = None) -> ModuleCollection:
"""Distribution wrapper function, provided for convenience."""

# hash value of the content of all .py and .vhd files in 'file_path'.
file_path = os.path.dirname(__file__)
files_hash_value = vhdlhelper.get_files_hash_value(file_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculating the software hash is not part of the resource distribution, consider moving this into module vhdlproducer or __main__.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved get_files_hash_value to constants.py to avoid circular references

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constants module is not ideal, I moved the code to the __main__ module (entry point for hash calculation).


logging.info("====================")
logging.info("VHDL producer")
logging.info("version: %s", vhdlhelper.VersionHelper(__version__))
logging.info("hash: %s", files_hash_value)
logging.info("====================")
logging.info("distributing menu...")

constraints = constraints or {}
Expand Down
9 changes: 5 additions & 4 deletions tmVhdlProducer/templates/vhdl/algo_index.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
-- Scale set:
-- {{ menu.info.scale_set }}

-- VHDL producer version
-- v{{ menu.info.sw_version }}
-- VHDL producer
-- version: {{ menu.info.sw_version }}
-- hash value: {{ menu.info.files_hash_value }}

-- tmEventSetup version
-- v{{ menu.info.version }}
-- tmEventSetup
-- version: {{ menu.info.version }}

-- HB 2016-09-16: constants for algo_mapping_rop.
type global_index_array is array (0 to NR_ALGOS-1) of integer;
Expand Down
9 changes: 5 additions & 4 deletions tmVhdlProducer/templates/vhdl/gtl_module_instances.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
-- Scale set:
-- {{ menu.info.scale_set }}

-- VHDL producer version
-- v{{ menu.info.sw_version }}
-- VHDL producer
-- version: {{ menu.info.sw_version }}
-- hash value: {{ menu.info.files_hash_value }}

-- tmEventSetup version
-- v{{ menu.info.version }}
-- tmEventSetup
-- version: {{ menu.info.version }}

-- ========================================================
-- Instantiations of conditions
Expand Down
9 changes: 5 additions & 4 deletions tmVhdlProducer/templates/vhdl/gtl_module_signals.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
-- Scale set:
-- {{ menu.info.scale_set }}

-- VHDL producer version
-- v{{ menu.info.sw_version }}
-- VHDL producer
-- version: {{ menu.info.sw_version }}
-- hash value: {{ menu.info.files_hash_value }}

-- tmEventSetup version
-- v{{ menu.info.version }}
-- tmEventSetup
-- version: {{ menu.info.version }}

-- Signal definition of pt, eta and phi for correlation conditions.
{%- include "signals/signal_correlation_conditions_parameter.vhd" %}
Expand Down
9 changes: 5 additions & 4 deletions tmVhdlProducer/templates/vhdl/ugt_constants.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
-- Scale set:
-- {{ menu.info.scale_set }}

-- VHDL producer version
-- v{{ menu.info.sw_version }}
-- VHDL producer
-- version: {{ menu.info.sw_version }}
-- hash value: {{ menu.info.files_hash_value }}

-- tmEventSetup version
-- v{{ menu.info.version }}
-- tmEventSetup
-- version: {{ menu.info.version }}

-- Algorithms
constant NR_ALGOS : positive := {{ module.algorithms | length }}; -- number of algorithmns (min. 32 for FDL registers width !!!) - written by TME
Expand Down
33 changes: 32 additions & 1 deletion tmVhdlProducer/vhdlhelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
* MassCutHelper
"""

import subprocess
import math
import string
import re, os
Expand All @@ -53,6 +54,9 @@
from . import algodist
from . import __version__

import hashlib
from glob import glob

# -----------------------------------------------------------------------------
# Precompiled regular expressions
# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -251,6 +255,30 @@ def bx_encode_4_array(value: int) -> str:
"""
return format([2, 1, 0, -1, -2].index(value), 'd')

def get_files_hash_value(dir_path):
"""Calculate hash value of the content of all .py and .vhd files in 'dir_path'.
"""
py_files = dir_path+"/**/*.py"
vhd_files = dir_path+"/**/*.vhd"

x = hashlib.sha256()
Copy link
Member

@arnobaer arnobaer Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h = hashlib.sha256() or sha256_hash = hashlib.sha256() would increase code readability (note hash is a built in function, so try to avoid hash = hashlib.sha256()).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand! Why is "h" better than "x"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

h for hash. While x is a perfectly valid variable name, it is quite generic and provides no hints about the variable's intended use. In contrast, h or sha256_hash offers immediate context. See also the Zen of Python's "Explicit is better than implicit" principle (PEP 20), https://peps.python.org/pep-0020/


filename = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filenames = [](plural for lists)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


for filename_py in glob(py_files, recursive=True):
filename.append(filename_py)
for filename_vhd in glob(vhd_files, recursive=True):
filename.append(filename_vhd)
for i in range(0, len(filename)):
with open(filename[i], 'rb') as f:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for a range iterator.

for filename in filenames:
    with open(filename, "rb") as f:
        ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

while True:
# Reading is buffered, so we can read smaller chunks.
chunk = f.read(x.block_size)
if not chunk:
break
x.update(chunk)

return x.hexdigest()

# -----------------------------------------------------------------------------
# Factories
Expand Down Expand Up @@ -355,7 +383,10 @@ def __init__(self, collection):
self.scale_set = eventSetup.getScaleSetName()
self.version = VersionHelper(tmEventSetup.__version__)
self.sw_version = VersionHelper(__version__)

# hash value of all .py and .vhd files in directory "tmVhdlProducer" recursively
file_path = os.path.dirname(__file__)
self.files_hash_value = get_files_hash_value(file_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to avoid calculating the hash multiple times, this gives potential for discrepancies and introduces an overhead. Try to calculate the hash once in __main__ and pass it like the __version__ variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


class ModuleHelper(VhdlHelper):
"""Module template helper.

Expand Down
Loading
Loading