-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/
-----------------------------------------------------------

Review request for Sqoop.
Description
-------

Allow Sqoop to build on Windows OS
Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949

Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.

The patch does the following:
- provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
- Update build.xml to invoke the appropriate script based on the host OS.
- Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
This addresses bug SQOOP-949.
    https://issues.apache.org/jira/browse/SQOOP-949
Diffs
-----

  build.xml c097fe8
  src/scripts/create-tool-scripts.cmd PRE-CREATION
  src/scripts/tool-script.cmd.template PRE-CREATION
  src/scripts/write-version-info.cmd PRE-CREATION

Diff: https://reviews.apache.org/r/9961/diff/
Testing
-------

Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
Thanks,

Ahmed El Baz

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17990
-----------------------------------------------------------
Looks good to me.   Thanks for creating these batch scripts.   I will test this on my windows VM.   One quick question?   WHy don't you redirect the error output from the tool list generation to NUL (the equivalent of /dev/null) instead of a file.  Don't we have to clean it up at the end otherwise?

- Venkat Ranganathan
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 15, 2013, 7:15 p.m., Venkat Ranganathan wrote:
> > Looks good to me.   Thanks for creating these batch scripts.   I will test this on my windows VM.   One quick question?   WHy don't you redirect the error output from the tool list generation to NUL (the equivalent of /dev/null) instead of a file.  Don't we have to clean it up at the end otherwise?

Thank you Venkat for taking a look.
The main purpose was debugging the scripts in case of errors. The file is overridden with every build.
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17990
-----------------------------------------------------------
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 15, 2013, 7:15 p.m., Venkat Ranganathan wrote:
> > Looks good to me.   Thanks for creating these batch scripts.   I will test this on my windows VM.   One quick question?   WHy don't you redirect the error output from the tool list generation to NUL (the equivalent of /dev/null) instead of a file.  Don't we have to clean it up at the end otherwise?
>
> Ahmed El Baz wrote:
>     Thank you Venkat for taking a look.
>     The main purpose was debugging the scripts in case of errors. The file is overridden with every build.

Thanks Ahmed.   It would be good to not have debug output statements or files around - but windows being new to many may be we can have it.
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17990
-----------------------------------------------------------
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 15, 2013, 7:15 p.m., Venkat Ranganathan wrote:
> > Looks good to me.   Thanks for creating these batch scripts.   I will test this on my windows VM.   One quick question?   WHy don't you redirect the error output from the tool list generation to NUL (the equivalent of /dev/null) instead of a file.  Don't we have to clean it up at the end otherwise?
>
> Ahmed El Baz wrote:
>     Thank you Venkat for taking a look.
>     The main purpose was debugging the scripts in case of errors. The file is overridden with every build.
>
> Venkat Ranganathan wrote:
>     Thanks Ahmed.   It would be good to not have debug output statements or files around - but windows being new to many may be we can have it.

Thank you Venkat. In fact I agree with you especially that errors are not much expected in this scenario, so redirection can be enabled on demand in case of errors. Also it will be consistent with the behavior in Linux.
I have updated the patch based on this feedback.

Thank you
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17990
-----------------------------------------------------------
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 15, 2013, 7:15 p.m., Venkat Ranganathan wrote:
> > Looks good to me.   Thanks for creating these batch scripts.   I will test this on my windows VM.   One quick question?   WHy don't you redirect the error output from the tool list generation to NUL (the equivalent of /dev/null) instead of a file.  Don't we have to clean it up at the end otherwise?
>
> Ahmed El Baz wrote:
>     Thank you Venkat for taking a look.
>     The main purpose was debugging the scripts in case of errors. The file is overridden with every build.
>
> Venkat Ranganathan wrote:
>     Thanks Ahmed.   It would be good to not have debug output statements or files around - but windows being new to many may be we can have it.
>
> Ahmed El Baz wrote:
>     Thank you Venkat. In fact I agree with you especially that errors are not much expected in this scenario, so redirection can be enabled on demand in case of errors. Also it will be consistent with the behavior in Linux.
>     I have updated the patch based on this feedback.
>    
>     Thank you

 +1 looks good.

Can you please also update the JIRA and add the Review board link as a web link than as a comment?

Thanks

Venkat
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17990
-----------------------------------------------------------
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17991
-----------------------------------------------------------
Hi Ahmed,
thank you very much for all your work to get Sqoop compiled on Windows. I'm wondering whether there is need to document any extra steps or limitations?
build.xml
<https://reviews.apache.org/r/9961/#comment38008>

    Is there a need to rename this?

src/scripts/create-tool-scripts.cmd
<https://reviews.apache.org/r/9961/#comment38006>

    Nit: Trailing white space characters

src/scripts/tool-script.cmd.template
<https://reviews.apache.org/r/9961/#comment38007>

    I've missed the place where the sqoop.cmd gets generated, would you mind pointing me?
Jarcec

- Jarek Cecho
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 15, 2013, 7:50 p.m., Jarek Cecho wrote:
> > build.xml, line 985
> > <https://reviews.apache.org/r/9961/diff/1/?file=271086#file271086line985>
> >
> >     Is there a need to rename this?

Thank you Jarek for your comments.
Yes in fact I have wrapped the doc.uptodate in a condition "skip-real-docs" which adds an OS constraint that host OS is not Windows. As I mentioned in the Jira, the real-docs target is Linux specific, hence the need to introduce the wrapper condition.
I verified Linux builds are not affected
> On March 15, 2013, 7:50 p.m., Jarek Cecho wrote:
> > src/scripts/create-tool-scripts.cmd, line 46
> > <https://reviews.apache.org/r/9961/diff/1/?file=271087#file271087line46>
> >
> >     Nit: Trailing white space characters

Fixed. Thank you :)
> On March 15, 2013, 7:50 p.m., Jarek Cecho wrote:
> > src/scripts/tool-script.cmd.template, line 26
> > <https://reviews.apache.org/r/9961/diff/1/?file=271088#file271088line26>
> >
> >     I've missed the place where the sqoop.cmd gets generated, would you mind pointing me?

This change is the first part of getting Sqoop to work on Windows, which handles build problems, and provides build scripts.
I will be posting another Jira for runtime scripts like sqoop.cmd and configure-sqoop.cmd, and also a fix for unit tests on Windows.
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review17991
-----------------------------------------------------------
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review18248
-----------------------------------------------------------
Hi Ahmed

I applied this to my local repo and can build.   Thanks for the patch.   Couple of quick points.   May be the MSYY GIt issue but the patch did not apply cleanly (the new files were created on the project rootdir instead of under src/scripts directory.   May be a git format-patch would work better on Windows?   Not sure if others also hit similar issues

I think this patch is good to go, butI thought  we wanted to collect all of tge patches in lieu of the temporary branch approach.

- Venkat Ranganathan
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9961/#review18746
-----------------------------------------------------------

Ship it!
I think this patch is good.   I did not click ship-it before

- Venkat Ranganathan
On March 15, 2013, 7 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9961/
> -----------------------------------------------------------
>
> (Updated March 15, 2013, 7 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Allow Sqoop to build on Windows OS
> Associated Jira: https://issues.apache.org/jira/browse/SQOOP-949
>
> Sqoop fails to build on Windows due to dependencies on Linux scripts like write-version-info.sh and create-tool-scripts.sh which fail to build on Windows without Cygwin.
>
> The patch does the following:
> - provide Windows scripts src/scripts/create-tool-scripts.cmd and src/scripts/write-version-info.cmd which perform the same logic as the .sh scripts on Linux.
> - Update build.xml to invoke the appropriate script based on the host OS.
> - Skip real-docs target on Windows OS builds since as it invokes Linux specific "make" executable
>
>
> This addresses bug SQOOP-949.
>     https://issues.apache.org/jira/browse/SQOOP-949
>
>
> Diffs
> -----
>
>   build.xml c097fe8
>   src/scripts/create-tool-scripts.cmd PRE-CREATION
>   src/scripts/tool-script.cmd.template PRE-CREATION
>   src/scripts/write-version-info.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9961/diff/
>
>
> Testing
> -------
>
> Verified builds are successful on both Linux and Windows, and Unit tests are fully passing on Linux. Fixes to unit tests on Windows will go in a separate patch
>
>
> Thanks,
>
> Ahmed El Baz
>
>