[ 
https://issues.apache.org/jira/browse/HADOOP-8921?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13475894#comment-13475894
 ] 

Brandon Li commented on HADOOP-8921:
------------------------------------

The patch looks good. Even though "-Dcompile.native=true" is redundant in the 
command "ant compile-native -Dcompile.native=true"("ant compile-native" can do 
the job itself), it's still good to be consistent.

One minor thing is the os-arch check. Original code doesn't check the 
x86_64/x86 availability, native code compilation can fail on certain 
platforms(such as MacOS). At lease the developer will notice the failure. With 
the os-arch check in the patch, the "ant compile-native" command will report 
success but actually didn't do the work. The original behavior seems more 
intuitive to me.


                
> ant build.xml in branch-1 ignores -Dcompile.native
> --------------------------------------------------
>
>                 Key: HADOOP-8921
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8921
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: build
>    Affects Versions: 1.2.0
>         Environment: Mac OS X 10.7.4
>            Reporter: Gopal V
>            Priority: Trivial
>              Labels: ant, autoconf, patch
>         Attachments: HADOOP-8921.4.patch
>
>
> ant -Dcompile.native=false still runs autoconf and libtoolize
> According to ant 1.8 manual, any <target if> conditions are checked only 
> after the dependencies are run through. The current if condition in code 
> fails to prevent the autoconf/libtool components from running.
> Fixing it by moving the if condition up into the "compile-native" target and 
> changing it to a param substitution instead of being evaluated as a condition.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to