[ovs-dev] [PATCH 1/3] Windows installer

[Alin Serdean]

1. I will respin the patches to fix the author chain.
2. The license is used in a dialog. The user must accept the license before it can be installed.
3. The pid file gets generated in the binaries directory I can change to a temporary one if needed.

Alin.

-----Mesaj original-----
De la: Gurucharan Shetty [mailto:shettyg at nicira.com] 
Trimis: Thursday, April 23, 2015 4:33 AM
Către: Alin Serdean
Cc: dev at openvswitch.org
Subiect: Re: [ovs-dev] [PATCH 1/3] Windows installer

On Mon, Apr 20, 2015 at 11:58 AM, Alin Serdean <aserdean at cloudbasesolutions.com> wrote:
> This commit adds the windows installer to the OVS tree.
>
> Requirements are the following:
> Visual Studio Community 2013
> WiX Toolset 3.9
> Microsoft_VC120_CRT_x86.msm
>
> More detailed information on the requirements and build instructions 
> can be found under:
> https://github.com/cloudbase/ovs-windows-installer/blob/master/README.
> rst
>
> Signed-off-by: Alessandro Pilotti <apilotti at cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Thank you so much for doing this! You rock!
The Signed-off-by chain is a little odd. Who is the Author? If it is Alessandro, you should change this patch and make him the author of the commit. If it was you, you should add your Signed-off-by. If it was both, you should add a Co-authored-by from Alessandro and Signed-off-by from you. Can you please re-post with the correct submission chain?

I haven't looked at the the entire commit closely yet (or tried to build this), but I have a couple of high level comments.

---snip----
> diff --git a/windows/LICENSE b/windows/LICENSE new file mode 100644 
> index 0000000..68c771a
> --- /dev/null
Does the below LICENSE get installed somewhere after package installation? Or is it the LICENSE for the installer code?

> +++ b/windows/LICENSE
> @@ -0,0 +1,176 @@
> +
> +                                 Apache License
> +                           Version 2.0, January 2004
> +                        http://www.apache.org/licenses/
> +
> +   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
> +



---snip---
> +    <Component Id="OvsdbServerService" Directory="BINARIESDIR" Guid="{280201D5-35E7-45D6-83B9-293F1A4F7F0E}">
> +      <File Id="ovsdbserver.exe" Source="Services\ovsdb-server.exe" Checksum="yes" />
> +      <ServiceInstall
> +                Arguments='--log-file="[LOGSDIR]ovsdb-server.log" --pidfile --service --service-monitor --unixctl="[APPDATADIR]ovsdb-server.ctl" --remote=punix:"[APPDATADIR]db.sock" "[CONFDIR]conf.db"'

I think the above will write the pidfile in the configured directory which would be wrong. Probably it needs the entire path? Have you seen where it gets generated? Same for ovs-vswitchd.