Postgres Pro
Downloads
Home   >   mailing lists

Re: Fix broken Install.bat when target directory contains a space - Mailing list pgsql-hackers

From Asif Naeem
Subject Re: Fix broken Install.bat when target directory contains a space
Date
Msg-id CAEB4t-Pb66bT5L1wA=pE7biwDLVGe3iF3PXddag4_arChWjoQg@mail.gmail.com
Whole thread Raw
In response to Re: Fix broken Install.bat when target directory contains a space  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Fix broken Install.bat when target directory contains a space
List pgsql-hackers
Tree view
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.paquier@gmail.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

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: PATCH: Add 'pid' column to pg_replication_slots
Next
From: Heikki Linnakangas
Date:
Subject: Re: Streaming replication and WAL archive interactions