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

Error during registry setup steps when deploying from Windows #289

Closed
sbuttgereit opened this issue Apr 12, 2016 · 5 comments
Closed

Error during registry setup steps when deploying from Windows #289

sbuttgereit opened this issue Apr 12, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@sbuttgereit
Copy link

When performing an initial deployment from a Windows 10 system, the following error is encountered:

> sqitch deploy db:pg://scb@127.0.0.1:5432/fit_data
Adding registry tables to db:pg://scb@127.0.0.1:5432/fit_data
Use of uninitialized value $ret in scalar chomp at C:/strawberry/perl/site/lib/App/Sqitch.pm line 484.

For the record this is Windows 10 using Strawberry Perl (This is perl 5, version 20, subversion 1 (v5.20.1) built for MSWin32-x64-multi-thread) installed via Chocolatey; Sqitch is version .9994 installed via cpanminus.

I know nothing of Perl (this is really the first Perl program I've tried to comprehend), but I think I've at least traced this issue to where it's happening. The error is thrown when sqitch.pm is trying to figure out whether we're on Postgres XC or not... specifically (starting line 189):

# Is this XC?
    $opts = ' DISTRIBUTE BY REPLICATION' if $self->_probe('-c', q{
        SELECT count(*)
          FROM pg_catalog.pg_proc p
          JOIN pg_catalog.pg_namespace n ON p.pronamespace = n.oid
         WHERE nspname = 'pg_catalog'
           AND proname = 'pgxc_version';
    });

Ultimately this ends up in a "probe" subroutine which in turn calls "capture". Adding print statements, through these calls appear (at least to my uneducated eye) to be sensible... the command line parameters generally. I get to this routine in subroutine capture in sqitch.pm (line 362):

sub capture {
    my $self = shift;
    local $SIG{__DIE__} = sub {
        ( my $msg = shift ) =~ s/\s+at\s+.+/\n/ms;
        die $msg;
    };
    capturex @_;
}

Up to the capturex line, my print statements seem sensible. I assume that the return of capturex is the return of the capture subroutine (which gets passed back to subroutine probe where the error is actually thrown.) I know that capturex is from another package that Sqitch depends on, so this may not ultimately be a Sqitch problem... but reporting here as it is ultimately a blocker for anyone in a pure Windows environment.

I have tested my setup and project using a Linux environment and things are fine there (same version of Sqitch at least). I also have tested running Sqitch commands in Windows after the Linux environment has built out the registry and every else seems to work fine in Windows. Finally I have also tested in the new Windows Subsystem for Linux... not even close getting Sqitch to install into that environment so, but that's not a Sqitch problem as yet (and for some while I expect).

@theory theory added the bug label Apr 12, 2016
@theory theory added this to the v1.0.0 milestone Apr 12, 2016
@theory theory self-assigned this Apr 12, 2016
@theory
Copy link
Collaborator

theory commented Apr 12, 2016

Thank you. Does this patch fix the issue?

diff --git a/lib/App/Sqitch.pm b/lib/App/Sqitch.pm
index 4f59a54..dd6da36 100644
--- a/lib/App/Sqitch.pm
+++ b/lib/App/Sqitch.pm
@@ -481,7 +481,7 @@ sub spool {

 sub probe {
     my ($ret) = shift->capture(@_);
-    chomp $ret;
+    chomp $ret if $ret;
     return $ret;
 }

@sbuttgereit
Copy link
Author

Sorry for not responding sooner. Just tested and things seem to work as expected with that change. I was able to successfully deploy from Windows to a freshly created database with no registry. The registry was created as expected and I see the expected data in the registry.

@theory theory closed this as completed in 587e4ce Apr 18, 2016
@theory
Copy link
Collaborator

theory commented Apr 18, 2016

Awesome, thanks!

@decibel
Copy link
Contributor

decibel commented May 8, 2016

FWIW, EXISTS() would be more efficient than count(). Probably a non-issue in this case, but it's a better pattern to follow. It also makes it clearer that we only care if there's a single row, not about an exact count.

@theory
Copy link
Collaborator

theory commented May 9, 2016

Meh, should only ever return 1 or 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants