Fixes #174: Lost confgure_requires prereq Crypt::OpenSSL::Guess

[timlegge]

No description provided.

[timlegge]

@toddr-bot please review

[toddr-bot]

Both fixes look correct and straightforward. Two observations:

Commit 1 — CONFIGURE_REQUIRES (#174):
The CONFIGURE_REQUIRES EUMM key is the right fix — META_MERGE.configure_requires only populates distribution metadata but doesn't trigger installation before Makefile.PL runs. EUMM 6.52+ supports CONFIGURE_REQUIRES, and the file already requires 6.64, so no compatibility concern.

Minor style nit: the new block uses double quotes ("CONFIGURE_REQUIRES", "Crypt::OpenSSL::Guess") while the rest of the file uses single quotes. Functionally irrelevant, but worth noting for consistency.

Commit 2 — padding.t skip for "no digest set" (#175):
Good catch. On OpenSSL 3.0.x builds where ripemd160 is unavailable (legacy provider not loaded), the error surfaces as "no digest set" rather than "invalid digest". The regex addition is correct.

However, t/rsa.t:40 has the same invalid digest skip pattern without the no digest set variant:

skip "OpenSSL error: invalid digest - $hash", 5 if $@ =~ /invalid digest/i;

In practice, rsa.t is protected by a broader version-based skip (lines 162-164) that skips ripemd160 entirely on OpenSSL 3.0.2–3.0.6. But it's still an inconsistency — if a future OpenSSL version reintroduces ripemd160 partially, rsa.t could hit the same failure. Worth applying the same fix for consistency, or leaving a note that the version-based skip covers it.

LGTM overall — both fixes are correct and minimal.

[timlegge]

@toddr-bot review again

[toddr-bot]

PR Review — Fixes #174: Lost confgure_requires prereq Crypt::OpenSSL::Guess

Both fixes are correct and address real issues. The CONFIGURE_REQUIRES addition is the right approach — META_MERGE.configure_requires only populates distribution metadata but doesn't ensure the module is installed before Makefile.PL runs, so the EUMM key is necessary (and compatible, since the file already requires EUMM 6.64). The test skip pattern updates handle OpenSSL 3.x "no digest set" errors correctly. The only substantive concern is the inconsistency between rsa.t (/no digest/) and padding.t (/no digest set/) — aligning them would reduce maintenance burden and avoid masking future failures. Neither issue is blocking; merge-ready.

---

🟡 Important

1. Inconsistent skip regex between rsa.t and padding.t (`t/rsa.t`, L40)

The regex here matches /no digest/i (broad), while t/padding.t:51 matches the more specific /no digest set/i. If the actual OpenSSL error is "no digest set", both files should use the same pattern. The broader no digest in rsa.t could inadvertently skip tests on future OpenSSL versions that emit a different "no digest …" message for a real failure. Recommend aligning both files to the same pattern — either both /no digest set/i (precise) or both /no digest/i (broad), with the precise form preferred to avoid masking genuine errors.

skip "OpenSSL error: invalid digest - $hash", 5 if $@ =~ /invalid digest|no digest/i;

🟢 Suggestions

1. Quoting style inconsistency (`Makefile.PL`, L28-30)

The new CONFIGURE_REQUIRES block uses double quotes for the key and module name ("Crypt::OpenSSL::Guess"), while the rest of the file uses single quotes ('Crypt::OpenSSL::Bignum', 'PREREQ_PM', etc.). Functionally irrelevant, but for consistency with the rest of this file, single quotes would be preferable:

'CONFIGURE_REQUIRES' => {
    'Crypt::OpenSSL::Guess' => 0.11,
},

"CONFIGURE_REQUIRES" => {
    "Crypt::OpenSSL::Guess" => 0.11,
},

---

Checklist

• No hardcoded secrets
• Test skip patterns are consistent — warning quietened critic to -3, added test #1

---

Summary

Both fixes are correct and address real issues. The CONFIGURE_REQUIRES addition is the right approach — META_MERGE.configure_requires only populates distribution metadata but doesn't ensure the module is installed before Makefile.PL runs, so the EUMM key is necessary (and compatible, since the file already requires EUMM 6.64). The test skip pattern updates handle OpenSSL 3.x "no digest set" errors correctly. The only substantive concern is the inconsistency between rsa.t (/no digest/) and padding.t (/no digest set/) — aligning them would reduce maintenance burden and avoid masking future failures. Neither issue is blocking; merge-ready.

---

Automated review by Kōan853f4df
3761a41