Remove the cache machinery left behind when caching was dropped - #6
Open
jolierabideau wants to merge 1 commit into
Open
jolierabideau wants to merge 1 commit into
jolierabideau wants to merge 1 commit into
Conversation
d88e189 removed the caching step from action.yaml, but the code and documentation supporting it stayed: - cache_key() had no remaining caller. - configure_macports still wrote a cache-key entry to GITHUB_OUTPUT, and action.yaml does not declare such an output, so nothing could read it. That line also carried a typo inherited from upstream, `printf 'cache-key=%sn\n'`, which appended a stray "n" to every key. - The macos variable in main() existed only to build the cache key. - README.md advertised "the caching of an installation" and kept a "Cache scopes" section describing behaviour the action no longer has. None of this was reachable, so there is no behaviour change: the three outputs action.yaml declares — package, prefix and version — are all still written. Should caching ever be restored, note that upstream has since fixed the cache key to account for the architecture, in melusina-org/setup-macports 8326514, along with a probe_architecture helper worth taking at the same time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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
d88e189removed the caching step fromaction.yaml, but the code and documentation supporting it stayed behind:cache_key()has no remaining caller.configure_macportsstill wrote acache-keyentry to$GITHUB_OUTPUT.action.yamldoes not declare such an output, so nothing could read it. That line also carried a typo inherited from upstream (3ab7103, pre-fork):printf 'cache-key=%sn\n'appended a straynto every key. Still live upstream, where it does matter.macosvariable inmain()existed only to build the cache key — likewisepackage, which was never used.README.mdadvertised "the caching of an installation" in its opening paragraph and kept a## Cache scopessection describing behaviour this action no longer has.Change
Delete all of it. No behaviour change: none of it was reachable, and the three outputs
action.yamlactually declares —package,prefix,version— are all still written. I cross-checked declared outputs against emitted ones.parametersis also emitted without being declared, but that predates and is unrelated to caching, so I left it alone.Verification
sh -n configure_macportspassesmacos,macports,failwith)Why this is based on
sync-retryStacked on #4 (which is itself stacked on #5) so CI can actually go green. Based on
mainthis would fail for two reasons that have nothing to do with it — the staleport versionassertion (#5) and the silent ports-tree sync failure (#4). Review order: #5, then #4, then this. GitHub retargets each automatically as its parent merges.If you would rather merge this independently of the other two, say so and I will retarget it to
main; just expect its CI to be red until those land.Context
Found while auditing
melusina-org/setup-macportsfor changes worth carrying over. Three of upstream's four commits since the fork are caching work that does not apply here, and this is the code they would have touched. Worth noting for the future: upstream's8326514fixes the cache key to account for architecture (x86_64 vs arm64 were being conflated) and adds aprobe_architecturehelper — both worth taking if caching is ever restored.🤖 Generated with Claude Code
This change is