Skip to content

Commit

Permalink
Fix multi-engine template resolution
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theory committed Oct 21, 2023
1 parent 5724e0a commit 96ced2a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
3 changes: 3 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ Revision history for Perl extension App::Sqitch
- Removed the duplicate change name from the output of the list of
changes to be deployed or reverted with `-VV`. Thanks to Erik Wienhold
for the PR (#787)!
- Fixed invalid template resolution when adding a singe to multiple
engines at once. Thanks to Christian Riedel for the detailed bug report
(#795)!

1.4.0 2023-08-01T23:37:30Z
- Fixed Snowflake warehouse and role setup to properly quote identifiers
Expand Down
2 changes: 1 addition & 1 deletion lib/App/Sqitch/Command/add.pm
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ sub _config_templates {
sub all_templates {
my ($self, $name) = @_;
my $config = $self->sqitch->config;
my $tmpl = $self->templates;
my $tmpl = { %{ $self->templates } };

# Read all the template directories.
for my $dir (
Expand Down
11 changes: 9 additions & 2 deletions t/add.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use strict;
use warnings;
use utf8;
use Test::More tests => 242;
use Test::More tests => 243;
#use Test::More 'no_plan';
use App::Sqitch;
use App::Sqitch::Target;
Expand Down Expand Up @@ -301,7 +301,14 @@ is_deeply $add->all_templates($tname), {
deploy => file('etc/templates/deploy/pg.tmpl'),
revert => file('etc/templates/revert/pg.tmpl'),
verify => file('etc/templates/verify/pg.tmpl'),
}, 'Should find all templates in directory';
}, 'Should find all pg templates in directory';

# Make sure it works for a second name.
is_deeply $add->all_templates('sqlite'), {
deploy => file('etc/templates/deploy/sqlite.tmpl'),
revert => file('etc/templates/revert/sqlite.tmpl'),
verify => file('etc/templates/verify/sqlite.tmpl'),
}, 'Should find all sqlite templates in directory';

# Now let it find the templates in the user dir.
$usrdir = dir 'etc';
Expand Down

0 comments on commit 96ced2a

Please sign in to comment.