[GitHub] thrift pull request #1354: THRIFT-4232 ./configure does bad ant version chec...

2017-09-21 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/thrift/pull/1354


---


[GitHub] thrift pull request #1354: THRIFT-4232 ./configure does bad ant version chec...

2017-09-14 Thread magiccrafter
Github user magiccrafter commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1354#discussion_r138824699
  
--- Diff: aclocal/ax_javac_and_java.m4 ---
@@ -118,7 +118,7 @@ AC_DEFUN([AX_CHECK_JAVA_CLASS],
 AC_DEFUN([AX_CHECK_ANT_VERSION],
  [
   AC_MSG_CHECKING(for ant version > $2)
-  ANT_VALID=`expr $($1 -version 2>/dev/null | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p') \>= $2`
+  ANT_VALID=`expr "x$(printf "$2\n$($1 -version 2>/dev/null | sed 
-n 's/.*version \(@<:@0-9\.@:>@*\).*/\1/p')" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 
-g | sed -n 1p)" = "x$2"`
--- End diff --

The issue that the pull request is fixing is the version comparison part. 
The ant version extraction is working like charm and doesn't need to be changed.

@jeking3, you are not testing accurately. Thats not a bash script file, 
this is a Autoconf/m4 macro and the following:
`ant -version | sed -n 's/.*version \(@<:@0-9\.@:>@*\).*/\1/p'`
 at the time of running the bash command will have the 'Quadrigraphs'(@<:@) 
escaped:
`ant -version | sed -n 's/.*version \([0-9\.]*\).*/\1/p'`

For further details check the docs: 
https://www.gnu.org/software/autoconf/manual/autoconf.html#Quadrigraphs

I hope this clears the things a bit.
Thanks


---


[GitHub] thrift pull request #1354: THRIFT-4232 ./configure does bad ant version chec...

2017-09-13 Thread jeking3
Github user jeking3 commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1354#discussion_r138770754
  
--- Diff: aclocal/ax_javac_and_java.m4 ---
@@ -118,7 +118,7 @@ AC_DEFUN([AX_CHECK_JAVA_CLASS],
 AC_DEFUN([AX_CHECK_ANT_VERSION],
  [
   AC_MSG_CHECKING(for ant version > $2)
-  ANT_VALID=`expr $($1 -version 2>/dev/null | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p') \>= $2`
+  ANT_VALID=`expr "x$(printf "$2\n$($1 -version 2>/dev/null | sed 
-n 's/.*version \(@<:@0-9\.@:>@*\).*/\1/p')" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 
-g | sed -n 1p)" = "x$2"`
--- End diff --

I don't see this working on my bash shell:

```
jking@ubuntu:~/thrift/github/thrift$ ant -version
Apache Ant(TM) version 1.9.8 compiled on January 19 2017

jking@ubuntu:~/thrift/github/thrift$ ant -version | sed -n 's/.*version 
\(@<:@0-9\.@:>@*\).*/\1/p'
(no output)
```

Try this instead:
```
jking@ubuntu:~/thrift/github/thrift$ ant -version | cut -d' ' -f4
1.9.8
```

Test:

If I require 1.9.9, fails, if I require 1.9.7, passes:
```
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.9\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.9"
0
jking@ubuntu:~/thrift/github/thrift$ expr "x$(printf "1.9.7\n$(ant -version 
| cut -d' ' -f4)" | sort -t '.' -k 1,1 -k 2,2 -k 3,3 -g | sed -n 1p)" = "x1.9.7"
1
```


---


[GitHub] thrift pull request #1354: THRIFT-4232 ./configure does bad ant version chec...

2017-09-11 Thread magiccrafter
GitHub user magiccrafter opened a pull request:

https://github.com/apache/thrift/pull/1354

THRIFT-4232 ./configure does bad ant version check



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/magiccrafter/thrift THRIFT-4232

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1354.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1354


commit e91d115f628c33c121b82c4820656861caac2e83
Author: Nasko Vasilev 
Date:   2017-09-11T22:40:49Z

THRIFT-4232 ./configure does bad ant version check

commit 3bbc487133b89459eb8800ebc64adf95496d4559
Author: Nasko Vasilev 
Date:   2017-09-11T22:57:38Z

THRIFT-4232 ./configure does bad ant version check (inclusive comparison 
with >= instead of > only)




---