frioux / Win32-ServiceManager

https://metacpan.org/pod/Win32::ServiceManager
5 stars 3 forks source link

create_service argument 'args' issue #3

Open frioux opened 4 years ago

frioux commented 4 years ago

From Matteo Bottazzi:

I'm trying to set the 'args' argument in the create_service method in order to pass two paths to my executable ('command' argument). It seems to me that the '_sc_install' sub places the 'args' outside the binpath double quotes and, for this reason, it fails to create the service.

If I execute the direct sc command from cmd, it works fine this way:

binPath="\"C:\dev\WindowsService.exe\" \"C:/path_number_one\"
\"C:/path_number_two\""

So, if I modify your _scinstall sub from this `qw(sc create), $[1], qq(binpath= "$[2]") . ($[3] ? " $[3]" : ''), to this qw(sc create), $[1], "binpath= \"$[2]".($[3] ? " $_[3]" : '')."\""`, it works correctly.

This is the service creation code:

    $sc->create_service (
      name        => "$ServiceName",
      display     => "EPV zParser for $Profile Profile",
      description => 'Extract data from z/OS SMF (and many other log files)
and load it in a database',
      use_perl    => 0,
      use_nssm    => 0,
      command     => $WindowsService_FullPath,
      args        => '\"'.$WindowsServiceFolder.'\" \"'.$ProfileFolder.'\"',
      depends     => [],
      start       => 'auto',
      user        => '',
      password    => '',
    )

Am I doing something wrong or your code needs a patch?

frioux commented 4 years ago

Comment from @wesQ3:

The package docs have a note on the "args" param: "XXX: do these even make sense?" so I think it's safe to say these have never been used. I think we should drop the option since it def doesn't work as-is. Reviewing the (weird) syntax for sc commands with arguments1, I think it's best to just let the user configure the full binpath string in the "command" argument. In his case, that would be

    command => q(\"C:\dev\WindowsService.exe\" \"C:/path_number_one\"
\"C:/path_number_two\")

A link to that SC explanation and maybe an example in the docs would go a long way I think. A GH issue would be better to discuss anything further for posterity etc.

What do you think?

TeoBotta90 commented 4 years ago

I totally agree with you @wesQ3

frioux commented 4 years ago

Cool. If someone submits a pull request that makes setting args an error and documents the right way to do it I'll release it!

-- Sent from a rotary phone rented from Ma Bell

On Sun, Nov 17, 2019, 10:35 PM TeoBotta90 notifications@github.com wrote:

I totally agree with you @wesQ3 https://github.com/wesQ3

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/Win32-ServiceManager/issues/3?email_source=notifications&email_token=AAAB7Y2YYCQMELNA5T7TAX3QUIZRTA5CNFSM4JOFVMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEJLE2A#issuecomment-554873448, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y4ROBLWPZSQRPZPD63QUIZRTANCNFSM4JOFVMPQ .

TeoBotta90 commented 4 years ago

I've created a Pull Request. First one for me on GitHub, if there's something wrong in the process or in the code just le me know and I'll fix it!

wesQ3 commented 4 years ago

Hm, looking this over I think we could have a conflict with the check_command option. The exist check will fail if the user provides args in the command field. This wouldn't be a big deal except that check_command_default is on so anyone using args will have to set check_command => 0.

Maybe this just needs further documentation and descriptive errors to let the user know check_command doesn't work with args.

TeoBotta90 commented 4 years ago

You're right. Also, inside the pod there is an example of create_service that doesn't work until you set check_command => 0: ... command => 'C:\code\GR\script\server.pl -p 3001', ... This raises 'command not found'. Doing what you suggest is the easiest thing in my opinion, it would be ok by my side: only an improvement in the documentation.

The other possibility I see is the following:

What do you think? Again, it is simpler what you suggested ;)

wesQ3 commented 4 years ago

Yes I think we'll go with the simpler fix unless Frew has an objection :)

frioux commented 4 years ago

👍

-- Sent from a rotary phone rented from Ma Bell

On Tue, Nov 19, 2019, 8:41 AM Wes Malone notifications@github.com wrote:

Yes I think we'll go with the simpler fix unless Frew has an objection :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/frioux/Win32-ServiceManager/issues/3?email_source=notifications&email_token=AAAB7Y4DOSUHSQ4UECMKAHTQUQJJZA5CNFSM4JOFVMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEO3POQ#issuecomment-555595706, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAB7Y7GU4T5LZQMQ55PPT3QUQJJZANCNFSM4JOFVMPQ .