frioux / DBIx-Class-DeploymentHandler

https://metacpan.org/pod/DBIx::Class::DeploymentHandler
21 stars 26 forks source link

Install searchs for 1.000 instead of 1 for version 1 of the database. #78

Closed sergiotarxz closed 1 year ago

sergiotarxz commented 1 year ago

This is where the bug happens:

perl5/lib/perl5/DBIx/Class/DeploymentHandler/Dad.pm

30:  ref($version) ? $version->numify : $version;

Instead this should be done in Perl v5.36.0 to keep compatibility.

30:  ref($version) ? int($version->numify) : $version;

I am using perl v5.36.0.

sergiotarxz commented 1 year ago

If you are searching for a new maintainer please tell me.

I would really like to be able to use this library and I would be pleased to be able to maintain it if you are no longer interested or have no time.

mmcclimon commented 1 year ago

Thanks for the report. I don't think I understand what the bug is. Could you provide a little information on what you were trying to do, what you expected to happen, and what happened instead? Thanks.

sergiotarxz commented 1 year ago

Context

I have this 3 scripts.

prepare.pl

#!/usr/bin/env perl

use strict;
use warnings;
use aliased 'DBIx::Class::DeploymentHandler' => 'DH';
use Getopt::Long;
use FindBin;
use lib "$FindBin::Bin/../lib";
use LasTres::Schema;

my $force_overwrite = 1;

unless ( GetOptions( 'force_overwrite!' => \$force_overwrite ) ) {
    die "Invalid options";
}

my $schema = LasTres::Schema->Schema;

my $dh = DH->new(
    {
        schema              => $schema,
        script_directory    => "$FindBin::Bin/../dbicdh",
        databases           => 'PostgreSQL',
        sql_translator_args => { add_drop_table => 0 },
        force_overwrite     => $force_overwrite,
    }
);

$dh->prepare_deploy;
eval {
    $dh->prepare_upgrade;
};
if ($@) {
    $dh->prepare_install;
}

install.pl

#!/usr/bin/env perl

use strict;
use warnings;
use aliased 'DBIx::Class::DeploymentHandler' => 'DH';
use Getopt::Long;
use FindBin;
use lib "$FindBin::Bin/../lib";
use LasTres::Schema;

my $force_overwrite = 0;

unless ( GetOptions( 'force_overwrite!' => \$force_overwrite ) ) {
    die "Invalid options";
}

my $schema = LasTres::Schema->Schema;

my $dh = DH->new(
    {
        schema              => $schema,
        script_directory    => "$FindBin::Bin/../dbicdh",
        databases           => 'PostgreSQL',
        sql_translator_args => { add_drop_table => 0 },
        force_overwrite     => $force_overwrite,
    }
);

$dh->install;

upgrade.pl

#!/usr/bin/env perl

use strict;
use warnings;
use aliased 'DBIx::Class::DeploymentHandler' => 'DH';
use Getopt::Long;
use FindBin;
use lib "$FindBin::Bin/../lib";
use LasTres::Schema;

my $force_overwrite = 1;

unless ( GetOptions( 'force_overwrite!' => \$force_overwrite ) ) {
    die "Invalid options";
}

my $schema = LasTres::Schema->Schema;

my $dh = DH->new(
    {
        schema              => $schema,
        script_directory    => "$FindBin::Bin/../dbicdh",
        databases           => 'PostgreSQL',
        sql_translator_args => { add_drop_table => 0 },
        force_overwrite     => $force_overwrite,
    }
);

$dh->upgrade;

And this is my schema.

package LasTres::Schema 1;

use v5.36.0;

use strict;
use warnings;

use feature 'signatures';

use LasTres;

use parent 'DBIx::Class::Schema';

__PACKAGE__->load_namespaces();

sub Schema($class) {
    my $app = LasTres->new;
    my $config = $app->{config};
    my $database_config = $config->{database};
    my $dbname = $database_config->{dbname};
    my $host = $database_config->{host};
    my $port = $database_config->{port};
    my $user = $database_config->{user};
    my $password = $database_config->{password};
    my $dsn = 'dbi:Pg:';
    if (!defined $dbname) {
        die "The key database/dbname must be configured.";
    }
    $dsn .= "dbname=$dbname";
    if (defined $host) {
        $dsn .= ";host=$host";
    }
    if (defined $port) {
        $dsn .= ";port=$port";
    }
    # Undef is perfectly fine for username and password.
    return $class->connect($dsn, $user, $password);
}
1;

Steps

First I created my database.

postgres=# create database lastres;
CREATE DATABASE
postgres=# grant all privileges on database lastres to sergio;
GRANT
postgres=# alter database lastres owner to sergio;
ALTER DATABASE

After that I run prepare.pl.

sergio@bahdder ~/las_tres $ perl script/prepare.pl 
Overwriting existing DDL-YML file - /home/sergio/las_tres/script/../dbicdh/_source/deploy/1/001-auto.yml at inline delegation in DBIx::Class::DeploymentHandler for deploy_method->prepare_deploy (attribute declared in /home/sergio/perl5/lib/perl5/DBIx/Class/DeploymentHandler/WithApplicatorDumple.pm at line 51) line 18.
Overwriting existing DDL file - /home/sergio/las_tres/script/../dbicdh/PostgreSQL/deploy/1/001-auto.sql at inline delegation in DBIx::Class::DeploymentHandler for deploy_method->prepare_deploy (attribute declared in /home/sergio/perl5/lib/perl5/DBIx/Class/DeploymentHandler/WithApplicatorDumple.pm at line 51) line 18.

Result

But them it fails installing with install.pl

sergio@bahdder ~/las_tres $ perl script/install.pl 
Can't opendir($fh, '/home/sergio/las_tres/script/../dbicdh/PostgreSQL/deploy/1.000'): No such file or directory at /home/sergio/perl5/lib/perl5/DBIx/Class/DeploymentHandler/DeployMethod/SQL/Translator.pm line 129
DBIx::Class::Storage::TxnScopeGuard::DESTROY(): A DBIx::Class::Storage::TxnScopeGuard went out of scope without explicit commit or error. Rolling back. at script/install.pl line 0

These are the created files in dbicdh:

sergio@bahdder ~/las_tres $ tree dbicdh/
dbicdh/
├── PostgreSQL
│   └── deploy
│       └── 1
│           ├── 001-auto.sql
│           └── 001-auto-__VERSION.sql
└── _source
    └── deploy
        └── 1
            ├── 001-auto-__VERSION.yml
            └── 001-auto.yml

As you can see the install added .000 to my version, my patch avoids that behaviour.

mmcclimon commented 1 year ago

Thanks, that's very helpful. This is not a bug in DBIC::DH, but in your code. This will fix it for you:

-package LasTres::Schema 1;
+package LasTres::Schema;
+our $VERSION = 1;

You're seeing this because of the version line, and the way Perl interprets them differently in different situations, and the way that's interacting with the DeploymentHandler code.

[/tmp/dh] $ perl -E 'package Foo 1 {}; say $Foo::VERSION; say ref $Foo::VERSION'
1
version

[/tmp/dh] $ perl -E 'package Foo { our $VERSION = 1 }; say $Foo::VERSION; say ref $Foo::VERSION'
1

[/tmp/dh] $

The first version gives you a v-string, which is a special kind of version object; the second is just a number. DH coerces from non-objects (like the second) via numification, but v-strings are a little bit weird, and don't behave in exactly the way you were expecting.

sergiotarxz commented 1 year ago

I am sorry for the misunderstanding.