Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multi-engine template resolution #796

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

theory
Copy link
Collaborator

@theory theory commented Oct 21, 2023

When adding a change to multiple engines, Sqitch would randomly pick one engine's templates or the others and create them all for the same engine. In other words, adding a change to both "sqlite" and "pg" targets would result in all change files generated from templates from only one of those engines -- whichever it happened to find first.

Due to the use of a hash reference for templates in add.pm. Fix it by making a copy of the hash instead. This leads to duplication of effort when multiple targets use the same engine, but the cost is generally low relative to how often add is called -- and likely is measured in milliseconds.

Resolves #795.

@theory theory requested a review from autarch October 21, 2023 20:52
@theory theory self-assigned this Oct 21, 2023
@theory theory force-pushed the fix-multi-engine-template-resolution branch from 96ced2a to c2ec89f Compare October 21, 2023 21:49
When adding a change to multiple engines, Sqitch would randomly pick one
engine's templates or the others and create them all for the same
engine. In other words, adding a change to both "sqlite" and "pg"
targets would result in all change files generated from templates from
only one of those engines -- whichever it happened to find first.

Due to the use of a hash reference for templates in add.pm. Fix it by
making a copy of the hash instead. This leads to duplication of effort
when multiple targets use the same engine, but the cost is generally low
relative to how often `add` is called -- and likely is measured in
milliseconds.

Resolves #795.
@theory theory force-pushed the fix-multi-engine-template-resolution branch from c2ec89f to 0702d9a Compare October 21, 2023 21:50
@theory theory merged commit 0702d9a into develop Nov 19, 2023
230 of 239 checks passed
@theory theory deleted the fix-multi-engine-template-resolution branch November 19, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqitch add suddently uses same template for all engines while add.all = true
1 participant