fix(build): remove duplicate test targets from tests/CMakeLists.txt - #93
fix(build): remove duplicate test targets from tests/CMakeLists.txt#93prakhar7017 wants to merge 1 commit into
Conversation
tests/CMakeLists.txt declared test_crypto_aes and test_crypto_sha512
twice each. A repeated add_executable target name is a hard CMake error,
not a warning:
CMake Error at tests/CMakeLists.txt:77 (add_executable):
add_executable cannot create target "test_crypto_aes" because
another target with the same name already exists.
CMake Error at tests/CMakeLists.txt:79 (add_test):
add_test given test NAME "test_crypto_aes" which already exists.
Configuration aborted, so the command the README and CLAUDE.md give for
building the test suite produced no build system at all and no test
could be built or run.
Kept the second copy of each pair, whose comments describe what the
suite actually covers, and gave test_crypto_ecc and test_crypto_rsa the
same one-line description for consistency. No target, source file or
link dependency changes.
42e6cea to
9e2edf6
Compare
srpatcha
left a comment
There was a problem hiding this comment.
The defect is real and I reproduced it on master:
$ git show origin/master:tests/CMakeLists.txt | grep -oE 'add_executable\([a-z_0-9]+' | sort | uniq -c | awk '$1>1'
2 test_crypto_aes
2 test_crypto_sha512
Two declarations each, and a duplicate add_executable name is a hard CMake
error, so -DEOS_BUILD_TESTS=ON cannot generate a single test target. The
CHANGELOG entry describes it accurately.
One thing you should know before this merges: #82 fixes the same two
duplicates, as part of repairing four breakages that landed together. It takes
tests/CMakeLists.txt from 30 add_executable lines to 28; so does this. The
two will conflict.
That does not make this PR wasted, because you made the better call on which
copy survives. #82 keeps the grouped comment:
# --- test_crypto_aes / _ecc / _rsa / _sha512: were sitting in tests/ unused ---which describes how the tests got there rather than what they cover. This keeps
the per-test descriptions:
# --- test_crypto_aes: AES-128/192/256 ECB + CBC, NIST FIPS-197 / SP 800-38A ---
# --- test_crypto_ecc: ECC point arithmetic and ECDSA ---
# --- test_crypto_rsa: RSA modular exponentiation and verify ---
# --- test_crypto_sha512: SHA-512 against NIST vectors ---Naming the standard each vector set comes from is worth more to the next reader
than a note about the file's history. I am folding your version into #82 with
attribution, so the outcome is yours even though the conflict resolves in that
PR's favour.
Approving on the merits. If #82 lands first this becomes empty and can close; if
this lands first, #82 rebases cleanly around it.
Worth flagging that this is not the only duplicate-target problem in the
platform. eos and eBoot both define test_crypto and test_multicore, and
since ebuild composes them with add_subdirectory, the integration build could
not configure either — same class of error, one repository apart. That one is
embeddedos-org/ebuild#85, fixed in embeddedos-org/eBoot#71.
Deduplicating test_crypto_aes and test_crypto_sha512 left the grouped
comment that had been sitting above the block:
# --- test_crypto_aes / _ecc / _rsa / _sha512: were sitting in tests/ unused ---
That records how the files got here, which stops being interesting the
moment they are wired up. #93 fixes the same two duplicates and keeps the
per-test descriptions instead:
# --- test_crypto_aes: AES-128/192/256 ECB + CBC, NIST FIPS-197 / SP 800-38A ---
# --- test_crypto_ecc: ECC point arithmetic and ECDSA ---
# --- test_crypto_rsa: RSA modular exponentiation and verify ---
# --- test_crypto_sha512: SHA-512 against NIST vectors ---
Naming the standard each vector set comes from is worth more to the next
reader than a note about the file's history. Adopting that here so the
better wording survives the conflict between the two PRs.
Comment-only; no target, source or link line changes.
28/28 tests passed, 0 build errors
Co-authored-by: prakhar7017 <prakhar7017@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem
tests/CMakeLists.txtdeclaredtest_crypto_aesandtest_crypto_sha512twiceeach — once in a group block at lines 59–74, then again individually at lines
76–84. A repeated
add_executabletarget name is a hard CMake error, not awarning:
CMake Error at tests/CMakeLists.txt:77 (add_executable):
add_executable cannot create target "test_crypto_aes" because another
target with the same name already exists. The existing target is an
executable created in source directory "D:/eos/tests".
CMake Error at tests/CMakeLists.txt:79 (add_test):
add_test given test NAME "test_crypto_aes" which already exists in this
directory.
CMake Error at tests/CMakeLists.txt:82 (add_executable):
add_executable cannot create target "test_crypto_sha512" because another
target with the same name already exists.
-- Configuring incomplete, errors occurred!
Configuration aborted, so the exact command
CLAUDE.mdandREADME.mdgive forbuilding the suite —