From 96ced2ab79a84b4425926bdbe8b9eb5fc5544ef5 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Sat, 21 Oct 2023 16:47:57 -0400 Subject: [PATCH] Fix multi-engine template resolution 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. --- Changes | 3 +++ lib/App/Sqitch/Command/add.pm | 2 +- t/add.t | 11 +++++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Changes b/Changes index bfc80831..6bd4cc5d 100644 --- a/Changes +++ b/Changes @@ -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 diff --git a/lib/App/Sqitch/Command/add.pm b/lib/App/Sqitch/Command/add.pm index ccba27dc..2110b94c 100644 --- a/lib/App/Sqitch/Command/add.pm +++ b/lib/App/Sqitch/Command/add.pm @@ -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 ( diff --git a/t/add.t b/t/add.t index 533cd13e..a98d3ca0 100644 --- a/t/add.t +++ b/t/add.t @@ -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; @@ -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';