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

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

A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
I did this in a utility script "BuildJar" which can be used for other components as well.
This change is specific to Windows scripts, Linux scripts are not affected.
This addresses bug SQOOP-954.
    https://issues.apache.org/jira/browse/SQOOP-954
Diffs
-----

  bin/BuildJar.ps1 PRE-CREATION
  bin/configure-sqoop.cmd PRE-CREATION
  bin/sqoop.cmd PRE-CREATION
  conf/sqoop-env-template.cmd PRE-CREATION

Diff: https://reviews.apache.org/r/10055/diff/
Testing
-------
Thanks,

Ahmed El Baz

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18234
-----------------------------------------------------------
Hi Ahmed,
would you mind removing trailing white space characters?

- Jarek Cecho
On March 20, 2013, 11:56 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 20, 2013, 11:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

I went through the patch.  Thanks for creating it.   First as Jarcec mentioned can you please removed the trailing spaces?

It seems that you are generating the jar file for every run.   I understand the need to regenerate if more dependency files are added but do we need to generate for every run?

Also, you have a try finally block to generate and check for Jar location.   Can the JDK path be validated earlier (or in try /catch/finally so that it is properly caught.

- Venkat Ranganathan
On March 20, 2013, 11:56 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 20, 2013, 11:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 22, 2013, 6:11 a.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > I went through the patch.  Thanks for creating it.   First as Jarcec mentioned can you please removed the trailing spaces?
> >
> > It seems that you are generating the jar file for every run.   I understand the need to regenerate if more dependency files are added but do we need to generate for every run?
> >
> > Also, you have a try finally block to generate and check for Jar location.   Can the JDK path be validated earlier (or in try /catch/finally so that it is properly caught.
> >
> >

I have updated the patch with the following changes:

1) Removed trailing spaces
2) Made an early check for the JDK path and exiting with error if not found
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18247
-----------------------------------------------------------
On March 20, 2013, 11:56 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 20, 2013, 11:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 22, 2013, 2:51 a.m., Jarek Cecho wrote:
> > Hi Ahmed,
> > would you mind removing trailing white space characters?

Done. Thank you
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18234
-----------------------------------------------------------
On March 20, 2013, 11:56 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 20, 2013, 11:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 22, 2013, 6:11 a.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > I went through the patch.  Thanks for creating it.   First as Jarcec mentioned can you please removed the trailing spaces?
> >
> > It seems that you are generating the jar file for every run.   I understand the need to regenerate if more dependency files are added but do we need to generate for every run?
> >
> > Also, you have a try finally block to generate and check for Jar location.   Can the JDK path be validated earlier (or in try /catch/finally so that it is properly caught.
> >
> >
>
> Ahmed El Baz wrote:
>     I have updated the patch with the following changes:
>    
>     1) Removed trailing spaces
>     2) Made an early check for the JDK path and exiting with error if not found

Hi Ahmed,
would you mind reuploading updated patch on the review board as well?

Jarcec
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18247
-----------------------------------------------------------
On March 20, 2013, 11:56 p.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 20, 2013, 11:56 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

(Updated March 29, 2013, 3:01 a.m.)
Review request for Sqoop.
Changes
-------

Updating the review with the updated patch based on Jarek's comment on associated Jira
Description
-------

A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
I did this in a utility script "BuildJar" which can be used for other components as well.
This change is specific to Windows scripts, Linux scripts are not affected.
This addresses bug SQOOP-954.
    https://issues.apache.org/jira/browse/SQOOP-954
Diffs (updated)
-----

  bin/BuildJar.ps1 PRE-CREATION
  bin/configure-sqoop.cmd PRE-CREATION
  bin/sqoop.cmd PRE-CREATION
  conf/sqoop-env-template.cmd PRE-CREATION

Diff: https://reviews.apache.org/r/10055/diff/
Testing
-------
Thanks,

Ahmed El Baz

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

Thanks for the new patch.  It looks good.  I still have one issue and suggestion.  The powershell script to generate the jar file is very good!  You are generating a jar file everytime and the jar file is generated under SQOOP_HOME.   There may be installations for the SQOOP_HOME may not be writable by user.   Also, I think the main motivation is to overcome the environment strings limitation.   Since JDK 1.6, Java has the ability to provide an option to provide a shortcut for all jars in a file (This probably should be done for the Unix classpaths also).   Please see http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html  

I am thinking whether this should be a simpler change to just add all jars in SQOOP_LIB.  We have to say %SQOOP_HOME%\lib\*.   Of course, this introduces dependency on 1.6+ versions of JDK, but given that 1.5 is EOLed this should be OK

Thanks

- Venkat Ranganathan
On March 29, 2013, 3:01 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 29, 2013, 3:01 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On March 29, 2013, 7:03 p.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > Thanks for the new patch.  It looks good.  I still have one issue and suggestion.  The powershell script to generate the jar file is very good!  You are generating a jar file everytime and the jar file is generated under SQOOP_HOME.   There may be installations for the SQOOP_HOME may not be writable by user.   Also, I think the main motivation is to overcome the environment strings limitation.   Since JDK 1.6, Java has the ability to provide an option to provide a shortcut for all jars in a file (This probably should be done for the Unix classpaths also).   Please see http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html  
> >
> > I am thinking whether this should be a simpler change to just add all jars in SQOOP_LIB.  We have to say %SQOOP_HOME%\lib\*.   Of course, this introduces dependency on 1.6+ versions of JDK, but given that 1.5 is EOLed this should be OK
> >
> > Thanks

Thank you a lot Venkat for the valuable comments,

I have considered the wildcard option, however, there are some limitations why it was not preferable to go this route, and using the referencing jar would give more flexibility:
1) The need to specify particular jars to include, or exclude some jars and not include all jars by default in a dorectory by using wildcard. For example, in configure-sqoop a list of dependency jars for HBase are returned by invoking "hbase classpath" which returns a list of jars. In this case using a wrapper Jar releases us from worrying about the length of jars returned, and it is not possible to use the * in this case, unless we do some logic to get common dirs.
2) As you can see also in configure-jar, Sqoop has dependency on other components rather than just SQOOP_HOME\lib, like HBase, SQOOP_CONF, ZOOCFGDIR.
3) Using the wrapper jar would scale regardless of how many directories we include. I understand it is hard the number of folders increases to the limit where we see the long command error, but even in this case the wrapper jar would work just fine.

I would like to unederstand more about scenarios where we anticipate SQOOP_HOME would not be writable on Windows systems.

Thank you again,
Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18523
-----------------------------------------------------------
On March 29, 2013, 3:01 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 29, 2013, 3:01 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/BuildJar.ps1 PRE-CREATION
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/

> On March 29, 2013, 7:03 p.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > Thanks for the new patch.  It looks good.  I still have one issue and suggestion.  The powershell script to generate the jar file is very good!  You are generating a jar file everytime and the jar file is generated under SQOOP_HOME.   There may be installations for the SQOOP_HOME may not be writable by user.   Also, I think the main motivation is to overcome the environment strings limitation.   Since JDK 1.6, Java has the ability to provide an option to provide a shortcut for all jars in a file (This probably should be done for the Unix classpaths also).   Please see http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html  
> >
> > I am thinking whether this should be a simpler change to just add all jars in SQOOP_LIB.  We have to say %SQOOP_HOME%\lib\*.   Of course, this introduces dependency on 1.6+ versions of JDK, but given that 1.5 is EOLed this should be OK
> >
> > Thanks
>
> Ahmed El Baz wrote:
>     Thank you a lot Venkat for the valuable comments,
>    
>     I have considered the wildcard option, however, there are some limitations why it was not preferable to go this route, and using the referencing jar would give more flexibility:
>     1) The need to specify particular jars to include, or exclude some jars and not include all jars by default in a dorectory by using wildcard. For example, in configure-sqoop a list of dependency jars for HBase are returned by invoking "hbase classpath" which returns a list of jars. In this case using a wrapper Jar releases us from worrying about the length of jars returned, and it is not possible to use the * in this case, unless we do some logic to get common dirs.
>     2) As you can see also in configure-jar, Sqoop has dependency on other components rather than just SQOOP_HOME\lib, like HBase, SQOOP_CONF, ZOOCFGDIR.
>     3) Using the wrapper jar would scale regardless of how many directories we include. I understand it is hard the number of folders increases to the limit where we see the long command error, but even in this case the wrapper jar would work just fine.
>    
>     I would like to unederstand more about scenarios where we anticipate SQOOP_HOME would not be writable on Windows systems.
>    
>     Thank you again,
>     Ahmed

Thanks Ahmed for the explanation.

I thought we are primarily limited by the 8K limit in the command line so if we can potentially limit the large jar file dirs in this format, then it would be fit within the limit.
Good point of hbase -classpath option.  May be we can have improvement on Hbase to return the hbase classpath with jar dirs properly added

For example, when people install Hadoop on Windows and decide that Hadoop stack will be installed under a terminal server and this is shared across multiple users - or it  may be installed in a common location and mapped based on logon scripts.   And the directory can become inaccessible for people running sqoop jobs.   This is a scheme used by some  Hadoop distributions today.

Thanks
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18523
-----------------------------------------------------------
On March 29, 2013, 3:01 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 29, 2013, 3:01 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.

> On March 29, 2013, 7:03 p.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > Thanks for the new patch.  It looks good.  I still have one issue and suggestion.  The powershell script to generate the jar file is very good!  You are generating a jar file everytime and the jar file is generated under SQOOP_HOME.   There may be installations for the SQOOP_HOME may not be writable by user.   Also, I think the main motivation is to overcome the environment strings limitation.   Since JDK 1.6, Java has the ability to provide an option to provide a shortcut for all jars in a file (This probably should be done for the Unix classpaths also).   Please see http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html  
> >
> > I am thinking whether this should be a simpler change to just add all jars in SQOOP_LIB.  We have to say %SQOOP_HOME%\lib\*.   Of course, this introduces dependency on 1.6+ versions of JDK, but given that 1.5 is EOLed this should be OK
> >
> > Thanks
>
> Ahmed El Baz wrote:
>     Thank you a lot Venkat for the valuable comments,
>    
>     I have considered the wildcard option, however, there are some limitations why it was not preferable to go this route, and using the referencing jar would give more flexibility:
>     1) The need to specify particular jars to include, or exclude some jars and not include all jars by default in a dorectory by using wildcard. For example, in configure-sqoop a list of dependency jars for HBase are returned by invoking "hbase classpath" which returns a list of jars. In this case using a wrapper Jar releases us from worrying about the length of jars returned, and it is not possible to use the * in this case, unless we do some logic to get common dirs.
>     2) As you can see also in configure-jar, Sqoop has dependency on other components rather than just SQOOP_HOME\lib, like HBase, SQOOP_CONF, ZOOCFGDIR.
>     3) Using the wrapper jar would scale regardless of how many directories we include. I understand it is hard the number of folders increases to the limit where we see the long command error, but even in this case the wrapper jar would work just fine.
>    
>     I would like to unederstand more about scenarios where we anticipate SQOOP_HOME would not be writable on Windows systems.
>    
>     Thank you again,
>     Ahmed
>
> Venkat Ranganathan wrote:
>     Thanks Ahmed for the explanation.
>    
>     I thought we are primarily limited by the 8K limit in the command line so if we can potentially limit the large jar file dirs in this format, then it would be fit within the limit.
>     Good point of hbase -classpath option.  May be we can have improvement on Hbase to return the hbase classpath with jar dirs properly added
>    
>     For example, when people install Hadoop on Windows and decide that Hadoop stack will be installed under a terminal server and this is shared across multiple users - or it  may be installed in a common location and mapped based on logon scripts.   And the directory can become inaccessible for people running sqoop jobs.   This is a scheme used by some  Hadoop distributions today.
>    
>     Thanks
>

I had this comment written befoe, but got caught up in the saved reviews instead of publishing.  Sorry about that.   Can you check my comments and can we simplify this
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18523
-----------------------------------------------------------
On March 29, 2013, 3:01 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated March 29, 2013, 3:01 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.

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

(Updated April 22, 2013, 3:26 a.m.)
Review request for Sqoop.
Changes
-------

As per the discussions below, updating configure-sqoop script to use the parent directories of classpath jars rather than using a single jar which references all other jars in its manifest.
For HBase we use HBASE_HOME and HBASE_HOM\lib, rather than invoking hbase -classpath as in the Linux version.
Removed the BuildJar.ps1 script since we no longer need to construct a jar with all classpath jars in its manifest.
Description
-------

A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
I did this in a utility script "BuildJar" which can be used for other components as well.
This change is specific to Windows scripts, Linux scripts are not affected.
This addresses bug SQOOP-954.
    https://issues.apache.org/jira/browse/SQOOP-954
Diffs (updated)
-----

  bin/configure-sqoop.cmd PRE-CREATION
  bin/sqoop.cmd PRE-CREATION
  conf/sqoop-env-template.cmd PRE-CREATION

Diff: https://reviews.apache.org/r/10055/diff/
Testing
-------
Thanks,

Ahmed El Baz

> On March 29, 2013, 7:03 p.m., Venkat Ranganathan wrote:
> > Hi Ahmed
> >
> > Thanks for the new patch.  It looks good.  I still have one issue and suggestion.  The powershell script to generate the jar file is very good!  You are generating a jar file everytime and the jar file is generated under SQOOP_HOME.   There may be installations for the SQOOP_HOME may not be writable by user.   Also, I think the main motivation is to overcome the environment strings limitation.   Since JDK 1.6, Java has the ability to provide an option to provide a shortcut for all jars in a file (This probably should be done for the Unix classpaths also).   Please see http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html  
> >
> > I am thinking whether this should be a simpler change to just add all jars in SQOOP_LIB.  We have to say %SQOOP_HOME%\lib\*.   Of course, this introduces dependency on 1.6+ versions of JDK, but given that 1.5 is EOLed this should be OK
> >
> > Thanks
>
> Ahmed El Baz wrote:
>     Thank you a lot Venkat for the valuable comments,
>    
>     I have considered the wildcard option, however, there are some limitations why it was not preferable to go this route, and using the referencing jar would give more flexibility:
>     1) The need to specify particular jars to include, or exclude some jars and not include all jars by default in a dorectory by using wildcard. For example, in configure-sqoop a list of dependency jars for HBase are returned by invoking "hbase classpath" which returns a list of jars. In this case using a wrapper Jar releases us from worrying about the length of jars returned, and it is not possible to use the * in this case, unless we do some logic to get common dirs.
>     2) As you can see also in configure-jar, Sqoop has dependency on other components rather than just SQOOP_HOME\lib, like HBase, SQOOP_CONF, ZOOCFGDIR.
>     3) Using the wrapper jar would scale regardless of how many directories we include. I understand it is hard the number of folders increases to the limit where we see the long command error, but even in this case the wrapper jar would work just fine.
>    
>     I would like to unederstand more about scenarios where we anticipate SQOOP_HOME would not be writable on Windows systems.
>    
>     Thank you again,
>     Ahmed
>
> Venkat Ranganathan wrote:
>     Thanks Ahmed for the explanation.
>    
>     I thought we are primarily limited by the 8K limit in the command line so if we can potentially limit the large jar file dirs in this format, then it would be fit within the limit.
>     Good point of hbase -classpath option.  May be we can have improvement on Hbase to return the hbase classpath with jar dirs properly added
>    
>     For example, when people install Hadoop on Windows and decide that Hadoop stack will be installed under a terminal server and this is shared across multiple users - or it  may be installed in a common location and mapped based on logon scripts.   And the directory can become inaccessible for people running sqoop jobs.   This is a scheme used by some  Hadoop distributions today.
>    
>     Thanks
>
>
> Venkat Ranganathan wrote:
>     I had this comment written befoe, but got caught up in the saved reviews instead of publishing.  Sorry about that.   Can you check my comments and can we simplify this

Thank you Venkatesh,

I have update the patch to use the jar dirs for classpath locations, rather than the powershell script to generate a single jar encapsulating the classpath in its manifest. As discussed, we will need to have a corresponding change for the HBASE case where hbase.cmd -classpath is invoked to return a list of jar files. For now we use HBASE_HOME and HBASE_HOME\lib in the case of Windows.

Thanks,
Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review18523
-----------------------------

On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19517
-----------------------------------------------------------
Thanks Ahmed.  I found some small issues.   Can you please address them?

THanks
bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40266>

    Shouldn't this be HADOOP_COMMON_HOME without % around it - Otherwise it would be expanded to null string, right?

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40267>

    See previous comment

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40268>

    Same as above

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40269>

    Wouldn't HADOOP_CLASSPATH be additive - that is subsequent invocations will be adding to the HADOOP_CLASSPATH with the same values again and again (it is something like always calling . bin/sqoop in Unix systems.   Should there be a setlocal either in this
- Venkat Ranganathan
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On April 22, 2013, 7:06 a.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 71
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line71>
> >
> >     Shouldn't this be HADOOP_COMMON_HOME without % around it - Otherwise it would be expanded to null string, right?

Hi Venkat,

The echo is done in a if exist block which only checks if the given path exists or refers to a a file/dir which does not exist. The case you are referring to should be caught earlier in the "if not defined HADOOP_COMMON_HOME" block.

Thanks
> On April 22, 2013, 7:06 a.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 77
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line77>
> >
> >     See previous comment

same response as above
> On April 22, 2013, 7:06 a.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 82
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line82>
> >
> >     Same as above

Same response as above

Thank you
> On April 22, 2013, 7:06 a.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 129
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line129>
> >
> >     Wouldn't HADOOP_CLASSPATH be additive - that is subsequent invocations will be adding to the HADOOP_CLASSPATH with the same values again and again (it is something like always calling . bin/sqoop in Unix systems.   Should there be a setlocal either in this

Thanks Venkat,

There is already a setlocal in the parent script (sqoop.cmd) which calls into this script (configure-sqoop.cmd).
Thanks,

Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19517
-----------------------------------------------------------
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19534
-----------------------------------------------------------
Thanks Ahmed for a quick turnaround.  Please see the comments below.
bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40320>

    I am not sure I see a if exists block.   This line I checked says %HADOOP_COMMON_HOME% does not exist.   May be you are checking against a different version of the file.  But it looks like you understand the issue.   So, please fix it in the right location

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40321>

    I see a setlocal enabledelayedexpansion.   Does it also localize environment variables.   I thought it did not.
- Venkat Ranganathan
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On April 22, 2013, 5:37 p.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 71
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line71>
> >
> >     I am not sure I see a if exists block.   This line I checked says %HADOOP_COMMON_HOME% does not exist.   May be you are checking against a different version of the file.  But it looks like you understand the issue.   So, please fix it in the right location

Hi Venkat,

Sorry if I did not make it clear enough. I meant to say that the case undefined variables while prinitng should not happen since there are two gates (please see below).
I have copied the snippets below from the uploaded diff, so it should be uploaded fine.

1) The first one is to check if variables are not set, and set them accordingly.

if not defined HADOOP_COMMON_HOME (
 ...
)

if not defined HADOOP_MAPRED_HOME (
 ...
)

if not defined HBASE_HOME (
  ...
)
2) The second step is to verify the set paths actually exist
if not exist %HADOOP_COMMON_HOME% (
  ...
)

if not exist %HADOOP_MAPRED_HOME% (
  ...
)

if not exist "%HBASE_HOME%" (
  ...
)

Thanks,
Ahmed
> On April 22, 2013, 5:37 p.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 129
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line129>
> >
> >     I see a setlocal enabledelayedexpansion.   Does it also localize environment variables.   I thought it did not.

Yes, I have verified that it does not leak the environment variables with subsequent runs.
Thanks,
Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19534
-----------------------------------------------------------
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19555
-----------------------------------------------------------
HI ahmed

Please see below for more comments

- Venkat Ranganathan
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40375>

    Ahmed
    
    If you do this test on Windows in a batch file
    
    if not exist %dir% (
       statements
    )
    then the statements will be executed even if %dir% is undefined.   Can you verify that

bin/configure-sqoop.cmd
<https://reviews.apache.org/r/10055/#comment40376>

    ok.  thanks
- Venkat Ranganathan
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On April 22, 2013, 11:04 p.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 71
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line71>
> >
> >     Ahmed
> >    
> >     If you do this test on Windows in a batch file
> >    
> >     if not exist %dir% (
> >        statements
> >     )
> >     then the statements will be executed even if %dir% is undefined.   Can you verify that

Thank you Venkat,

Yes I know this but if you look at the script you will see that in the case where if not defined HADOOP_COMMON_HOME and HADOOP_MAPRED_HOME are not defined, we print an error message and exit the script, so the print statement "echo Error: %HADOOP_MAPRED_HOME% does not exist!" is not reachable in this case.

I do not think it makes a big difference if I display the variable name, so I will make this change and upload a patch.

Thank you Venkat.
Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19557
-----------------------------------------------------------
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On April 22, 2013, 11:04 p.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 71
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line71>
> >
> >     Ahmed
> >    
> >     If you do this test on Windows in a batch file
> >    
> >     if not exist %dir% (
> >        statements
> >     )
> >     then the statements will be executed even if %dir% is undefined.   Can you verify that
>
> Ahmed El Baz wrote:
>     Thank you Venkat,
>    
>     Yes I know this but if you look at the script you will see that in the case where if not defined HADOOP_COMMON_HOME and HADOOP_MAPRED_HOME are not defined, we print an error message and exit the script, so the print statement "echo Error: %HADOOP_MAPRED_HOME% does not exist!" is not reachable in this case.
>    
>     I do not think it makes a big difference if I display the variable name, so I will make this change and upload a patch.
>    
>     Thank you Venkat.
>     Ahmed
>

I agree with you on HADOOP_MAPRED_HOME and HADOOP_COMMON_HOME, but not for HBASE_HOME as that is just a warning
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19557
-----------------------------------------------------------
On April 22, 2013, 3:26 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 22, 2013, 3:26 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

(Updated April 23, 2013, 4:13 a.m.)
Review request for Sqoop.
Changes
-------

Updating the scripts to print the missing variable name rather than the actual path for HADOOP_COMMON_HOME and HADOOP_MAPRED_HOME in case they refer to a path which does not exist.
Description
-------

A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
I did this in a utility script "BuildJar" which can be used for other components as well.
This change is specific to Windows scripts, Linux scripts are not affected.
This addresses bug SQOOP-954.
    https://issues.apache.org/jira/browse/SQOOP-954
Diffs (updated)
-----

  bin/configure-sqoop.cmd PRE-CREATION
  bin/sqoop.cmd PRE-CREATION
  conf/sqoop-env-template.cmd PRE-CREATION

Diff: https://reviews.apache.org/r/10055/diff/
Testing
-------
Thanks,

Ahmed El Baz

> On April 22, 2013, 11:04 p.m., Venkat Ranganathan wrote:
> > bin/configure-sqoop.cmd, line 71
> > <https://reviews.apache.org/r/10055/diff/3/?file=283030#file283030line71>
> >
> >     Ahmed
> >    
> >     If you do this test on Windows in a batch file
> >    
> >     if not exist %dir% (
> >        statements
> >     )
> >     then the statements will be executed even if %dir% is undefined.   Can you verify that
>
> Ahmed El Baz wrote:
>     Thank you Venkat,
>    
>     Yes I know this but if you look at the script you will see that in the case where if not defined HADOOP_COMMON_HOME and HADOOP_MAPRED_HOME are not defined, we print an error message and exit the script, so the print statement "echo Error: %HADOOP_MAPRED_HOME% does not exist!" is not reachable in this case.
>    
>     I do not think it makes a big difference if I display the variable name, so I will make this change and upload a patch.
>    
>     Thank you Venkat.
>     Ahmed
>
>
> Venkat Ranganathan wrote:
>     I agree with you on HADOOP_MAPRED_HOME and HADOOP_COMMON_HOME, but not for HBASE_HOME as that is just a warning

I have uploaded a patch with this change.
Please let me know if you have further comments.

Thanks,
Ahmed
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19557
-----------------------------------------------------------
On April 23, 2013, 4:13 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 23, 2013, 4:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

Ship it!
Looks good.  Thanks for the contribution Ahmed.

- Venkat Ranganathan
On April 23, 2013, 4:13 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 23, 2013, 4:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

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

Ship it!
Hi Ahmed, please attach the final patch version to the JIRA and I'll commit it.

- Jarek Cecho
On April 23, 2013, 4:13 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 23, 2013, 4:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>

> On April 23, 2013, 6:38 p.m., Jarek Cecho wrote:
> > Hi Ahmed, please attach the final patch version to the JIRA and I'll commit it.

Done.
Thank you.
- Ahmed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10055/#review19589
-----------------------------------------------------------
On April 23, 2013, 4:13 a.m., Ahmed El Baz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10055/
> -----------------------------------------------------------
>
> (Updated April 23, 2013, 4:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> A patch implementing the Windows version of Sqoop run scripts. The scripts follow the same logic as there .sh counterparts.
> One difference is to create a Jar which references all classpath elements in its Manifest, and provide that jar as the single jar needed for Sqoop. The reason here is that in some cases if the number of classpath elements is large, HADOOP_CLASSPATH gets very long which causes failures in Windows since there is a limit to command lines.
> As a workaround, I added a step to wrap all jars in the classpath in a single jar, and then use that generated jar (this is also done in hadoop for Windows to handle similar issues)
> I did this in a utility script "BuildJar" which can be used for other components as well.
> This change is specific to Windows scripts, Linux scripts are not affected.
>
>
> This addresses bug SQOOP-954.
>     https://issues.apache.org/jira/browse/SQOOP-954
>
>
> Diffs
> -----
>
>   bin/configure-sqoop.cmd PRE-CREATION
>   bin/sqoop.cmd PRE-CREATION
>   conf/sqoop-env-template.cmd PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10055/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ahmed El Baz
>
>