Re: Fix broken Install.bat when target directory contains a space
| От | Asif Naeem |
|---|---|
| Тема | Re: Fix broken Install.bat when target directory contains a space |
| Дата | |
| Msg-id | CAEB4t-Pb66bT5L1wA=pE7biwDLVGe3iF3PXddag4_arChWjoQg@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Fix broken Install.bat when target directory contains a space (Michael Paquier <michael.paquier@gmail.com>) |
| Ответы |
Re: Fix broken Install.bat when target directory contains a space
|
| Список | pgsql-hackers |
The v2 patch looks good to me, just a minor concern on usage message i.e.
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.
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.
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 по дате отправления: