From: | Asif Naeem <anaeem(dot)it(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fix broken Install.bat when target directory contains a space |
Date: | 2015-04-21 07:33:34 |
Message-ID: | CAEB4t-Pb66bT5L1wA=pE7biwDLVGe3iF3PXddag4_arChWjoQg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
The v2 patch looks good to me, just a minor concern on usage message i.e.
C:\PG\postgresql\src\tools\msvc>install
> Invalid command line options.
> Usage: "install.bat <targetdir> [installtype]"
> installtype: client
It seems that there are two install options i.e. client, all (any other
string other than client is being considered or treated as all), the
following install command works i.e.
install C:\PG\postgresql\inst option_does_not_exist
As your patch effects this area of code, I thought to share these findings
with you, BTW, it is a minor thing that can be handled in another patch, If
you like please feel free to change status to ready for committer. Thanks.
On Fri, Apr 17, 2015 at 10:36 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> wrote:
> On Thu, Apr 16, 2015 at 5:40 PM, Asif Naeem wrote:
> > Along with fixing the space in installation path, it is also changing the
> > behavior of install script, why not just "if NOT [%1]==[] GOTO
> RUN_INSTALL",
> > checking for "/?" seems good for help message but it seem not well
> handled
> > in the script, it is still saying "Invalid command line options.", along
> > with this, option "/?" seems not handled by any other .bat build script.
> > Other than this, with the patch applied, is it an acceptable behavior
> that
> > (2) shows usage message as 'Usage: install.pl <targetdir>
> [installtype]' but
> > (3) shows usage message as 'Usage: "install.bat <path>"'. Thanks.
>
> Thanks for your review!
>
> OK, let's remove the use of /? then, consistency with the other
> scripts is a good argument for its removal.Attached is an updated
> patch that does the following regarding missing arguments:
> >install
> Invalid command line options.
> Usage: "install.bat <targetdir> [installtype]"
> installtype: client
> >install
> Installing version 9.5 for release in /?
> Copying build output files...Could not copy release\postgres\postgres.exe
> to /?\
> bin\postgres.exe
> at Install.pm line 40.
> Install::lcopy("release\\postgres\\postgres.exe",
> "/?\\bin\\postgres.exe
> ") called at Install.pm line 324
> Install::CopySolutionOutput("release", "/?") called at Install.pm
> line 9
> 3
> Install::Install("/?", undef) called at install.pl line 13
>
> This patch fixes of course the issue with spaces included in the
> target path. I updated as well the Usage in install.bat to be
> consistent with install.pl.
> Regards,
> --
> Michael
>
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2015-04-21 07:38:25 | Re: Streaming replication and WAL archive interactions |
Previous Message | Andres Freund | 2015-04-21 07:19:00 | Re: PATCH: Add 'pid' column to pg_replication_slots |