[SakarPaisen]

Bugfix

Description

scripts/config_mapping/lifecycle_config.py accepted configurations in which two distinct run-target names produce the same run-target identifier hash. Since PR #527 the Launch Manager daemon identifies run targets by a portable FNV-1a hash of their name (identifier_hash.cpp), so two distinct names with the same hash cannot be told apart at runtime; such a configuration must be rejected at validation time.

This PR mirrors the exact portable hashing algorithm (64-bit FNV-1a over the UTF-8 bytes of the name, matching Fnv1aHash() in identifier_hash.cpp and the values pinned in identifier_hash_UT.cpp) in the config validator and adds a collision check:

• every name that maps to a hashed run-target identifier is hashed: all configured run-target names plus the implicit "Off" and "fallback_run_target" run-target identifiers created in graph.cpp;
• if two distinct names hash to the same value, a diagnostic identifying both names and the shared hash is printed via report_error() and the script exits with status 1, per the issue's definition of done;
• names are never mutated, renamed, appended to or silently deduplicated;
• normal valid configs and unrelated validation errors keep their current behavior and exit codes (verified by tests).

Regression tests force a deterministic collision by monkeypatching the hash function (no brute-force search for naturally colliding names).

Related ticket

closes #545

Test plan

• New unit tests in scripts/config_mapping/unit_tests.py:
  • test_fnv1a_hash_matches_identifier_hash_cpp_values (hash parity with identifier_hash_UT.cpp)
  • test_check_run_target_hash_collisions_accepts_distinct_names
  • test_check_run_target_hash_collisions_detects_collision (forced collision, diagnostic content, names untouched)
  • test_check_run_target_hash_collisions_implicit_run_target_names
  • test_main_exits_1_on_run_target_hash_collision (exit status 1)
  • test_main_unrelated_validation_errors_keep_exit_code (unrelated errors still exit 3)
• pytest scripts/config_mapping/unit_tests.py — 80 passed (the 2 collection errors from the schema_file fixture are pre-existing on the base commit and unrelated to this change)
• bazel test //scripts/config_mapping/tests:lifecycle_config_tests — not run locally (toolchain); please run in CI

This change and PR description were prepared with AI assistance.

[danth]

I assume the description in the issue was meant as an example, and it is not required to use this exact number? CC @MaciejKaszynski

I think it would be fine to just use CUSTOM_VALIDATION_FAILURE and delete this constant.

[MaciejKaszynski]

yea just needs to exit failure and fail the build.

[danth]

I think it would be useful to add a similar comment in identifier_hash.cpp which points to this file, so that someone changing the C++ code knows to update the Python script.

[danth]

Suggested change

	# identifiers in score/launch_manager/src/daemon/src/common/identifier_hash.cpp
	# (introduced by PR #527). std::size_t is 64-bit on all supported targets, so the
	# identifiers in score/launch_manager/src/daemon/src/common/identifier_hash.cpp.
	# std::size_t is 64-bit on all supported targets, so the

I don't think PR numbers need to be written in the code.

[danth]

Suggested change

	# that a detected collision is always reported and exits with status 1
	# (issue #545). Unrelated validation errors keep their existing exit codes.
	# that a detected collision is always reported.

I don't think issue numbers need to be written in the code.

[danth]

@SakarPaisen Thanks for contributing. Please ensure that you have signed the ECA using the same email address as your account here - without this, we will not be able to merge your pull request. :))

[danth]

I'm wondering if we could just have the smallest possible implementation of FNV-1a (I think this is 32 bits?) and use it everywhere.

At the moment, the script would be silently incorrect if we added support for a 32-bit architecture.

I suppose this does not need to be part of this PR since it involves the existing C++ code.

[NicolasFussberger]

FYI baselibs has this hash implemented for 32 and 64bit: https://github.com/eclipse-score/baselibs/blob/c6c20e53264a5b7ee171ea20f2ebb7bfb4db42f2/score/language/futurecpp/include/score/hash.hpp#L88-L111

Probably the c++ code at least should be updated to use this instead of custom implementation.
Though I guess this does not help us in python. Or we would need to do this check in c++ instead. We have the string and hash in the registry so when registering another registry entry we could easily see if the hash is existing but string is different.

[MaciejKaszynski]

I think it would be fine to just make the cpp use the std::uint32 and the python also always use the 32 bit version. The algorithm is a standard so the outcome will be the same.