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  (Michael Paquier <michael.paquier@gmail.com>)
Список 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.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 по дате отправления:

Предыдущее
От: Andres Freund
Дата:
Сообщение: Re: PATCH: Add 'pid' column to pg_replication_slots
Следующее
От: Heikki Linnakangas
Дата:
Сообщение: Re: Streaming replication and WAL archive interactions