Absorb simp-build-helpers into this project - #274
Open
silug wants to merge 2 commits into
Open
Conversation
`simp/rake/build/auto.rb` has always hard-required `Simp::Build::ReleaseMapper` from the `simp-build-helpers` gem, and `Simp::Rake::Build::Helpers` loads `auto.rb` unconditionally -- so the entire `Simp::Rake::Build::*` task tree fails to load without it. That gem was never declared as a runtime dependency here, only as a `Gemfile` entry, so `gem install simp-rake-helpers` produced a `LoadError` for every build task. simp-core worked around it by declaring the gem in its own Gemfile. The gem was also broken. Its last release on RubyGems is 0.1.1 (2016-09-28), which calls `File.exists?` in `sanitize_iso_list`. That method no longer exists in current Rubies, so `rake build:auto` raised `NoMethodError` on its first ISO path check. The fix was committed to that project's `master` in 2023 but never released, and no git tags were ever pushed. Since the gem was a single 142-line class with no other consumers, absorb it rather than releasing a 10-year-dormant project. `Simp::Build::ReleaseMapper` keeps its namespace, so `auto.rb` needs no change, and its unit tests come along with it. Verified equivalent against the pre-move implementation: every error message is byte-identical and all `get_flavor` return values match, except the intentional typo fix noted below. - Add lib/simp/build/release_mapper.rb, reformatted to pass this project's enforced RuboCop config - Add spec/lib/simp/build/release_mapper_spec.rb plus fixtures under spec/lib/simp/files/release_mapper/ - Drop the simp-build-helpers Gemfile entry and remove it from the acceptance-test build project scaffold - `Simp::Build::SIMPBuildException` now inherits `StandardError` rather than `Exception`, matching the sibling `SIMPBuildException` classes in auto.rb and build.rb. `build:auto` wraps each distro/version/arch build in `rescue StandardError`, so ISO mapping failures are now collected in the "Failed ISOs" summary (exit 1) instead of aborting the whole run on the first failure. - Add the missing closing quote in the "Recognized SIMP ISOs for '<release>'" error message header Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the checksum branch was taken (`@do_checksums`, or a flavor with
non-unique ISO sizes) and verification *failed*, control fell out of the inner
`if` into the unconditional `result = flavor; break` -- so instead of moving on
to the next flavor and ultimately returning nil, `get_flavor` returned the
flavor with `result_isos` still `[]`.
Downstream this was silent rather than loud: `simp/rake/build/auto.rb:353`
drives `unpack` by iterating `target_data['isos']`, so an empty list meant
unpack was simply never invoked and `build:auto` carried on against an empty or
stale staging directory with a corrupt ISO accepted as valid. That made
`SIMP_BUILD_checksum=true` worse than useless.
Now takes `next` when the checksums do not account for every ISO the flavor
expects, so the search continues and `autoscan_unpack_list` raises
`No flavors for target release` if nothing verifies.
Inherited from simp-build-helpers rather than introduced by the port, but this
is where the code enters this repo and the ported spec only covered the
checksum-success path, so nothing caught it. Adds three regression tests, all
of which fail against the previous implementation:
- `#get_flavor` returns nil when checksum verification fails
- `#get_flavor` does not report a flavor with an empty ISO list
- `#autoscan_unpack_list` raises when checksum verification fails
Also drops a tautological guard: after
`matched_isos = iso_checksums.select { |_iso, sum| checksums.include?(sum) }`,
the subsequent `matched_isos.values.all? { |sum| checksums.include?(sum) }` was
always true, so only the `uniq.size` comparison did any work.
Thanks to @michael-riddle for catching this in review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
michael-riddle
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
simp/rake/build/auto.rb:15hard-requiresSimp::Build::ReleaseMapperfrom thesimp-build-helpersgem, andSimp::Rake::Build::Helpersloadsauto.rbunconditionally — so the entireSimp::Rake::Build::*task tree fails to load without that gem.Three things were wrong:
Undeclared runtime dependency.
simp-build-helperswas never in this project's gemspec, only in itsGemfile. Sogem install simp-rake-helpers+ any use of the build tasks =LoadError. simp-core papered over it by declaring the gem in its own Gemfile, andsimp-core/Rakefilecatches theLoadErrorand merely warns — so the failure mode was every build task silently disappearing.The published gem is broken on modern Ruby. The latest release on RubyGems is 0.1.1 (2016-09-28), which calls
File.exists?insanitize_iso_list. That method is gone from current Rubies, sorake build:autoraisedNoMethodErroron its first ISO path check. Verified against simp-core's realCentOS/8Stream/x86_64/release_mappings.yaml:The one-line fix was committed to that project's
masterin 2023 as 0.1.2 but never released, and the repo has no git tags.Nobody noticed, which suggests
build:autois dormant: simp-core hasrelease_mappings.yamlonly for CentOS 7/8/8Stream and RedHat 7/8 (nothing for EL9/10), the ISO build docs were last touched 2023-03, and no simp-core CI workflow invokesbuild:auto.Approach
The gem was a single 142-line class with no other consumers, so absorbing it beats tagging a release for a project dormant since 2016.
Simp::Build::ReleaseMapperkeeps its namespace, soauto.rbneeds no change.Changes
lib/simp/build/release_mapper.rb, reformatted to pass this project's enforced RuboCop configspec/lib/simp/build/release_mapper_spec.rbplus fixtures underspec/lib/simp/files/release_mapper/(matching the existingspec/lib/simp/files/<name>convention)simp-build-helpersGemfileentry and remove it from the acceptance-test build project scaffoldBehavior change worth a look
Simp::Build::SIMPBuildExceptionnow inheritsStandardErrorrather thanException, matching the siblingSIMPBuildExceptionclasses already inauto.rb:12andbuild.rb:13, and satisfyingLint/InheritException.build:autowraps each distro/version/arch build inrescue StandardError(auto.rb:420), andget_target_data(auto.rb:252) sits inside that block. So ISO mapping failures are now collected into the "Failed ISOs" summary and exit 1, instead of aborting the whole run on the first failure. That's what the per-ISO loop appears to intend, but it is a change — happy to keepExceptionwith an inline cop disable if you'd rather preserve it exactly.Also fixed a missing closing quote in the
Recognized SIMP ISOs for '<release>'error message header.Verification
bundle exec rake spec— 170 examples, 0 failures, 5 pending (all pre-existing)bundle exec rubocop— 69 files, no offensesget_flavorreturn values identical, except the intentional quote fixauto.rbresolvesSimp::Build::ReleaseMapperwith the gem absent, and thatSIMPBuildExceptionis rescuable asStandardErrorrelease_mappings.yamlMerge order
Please merge and release 6.1.0 before simp/simp-core#942, which raises its floor to
simp-rake-helpers ~> 6.1.