Re: RFR: 8283575: Check for GNU time fails for version >1.7 [v2]

2022-03-24 Thread Magnus Ihse Bursie
On Wed, 23 Mar 2022 22:06:49 GMT, Erik Joelsson  wrote:

>> The version output of GNU time changed from "GNU time" to "GNU Time" in 
>> version 1.8. We need to update our check for identifying GNU time to handle 
>> this.
>
> Erik Joelsson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update make/autoconf/basic_tools.m4
>   
>   Co-authored-by: Magnus Ihse Bursie 

Marked as reviewed by ihse (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7925


Re: RFR: 8283575: Check for GNU time fails for version >1.7 [v2]

2022-03-23 Thread Erik Joelsson
> The version output of GNU time changed from "GNU time" to "GNU Time" in 
> version 1.8. We need to update our check for identifying GNU time to handle 
> this.

Erik Joelsson has updated the pull request incrementally with one additional 
commit since the last revision:

  Update make/autoconf/basic_tools.m4
  
  Co-authored-by: Magnus Ihse Bursie 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7925/files
  - new: https://git.openjdk.java.net/jdk/pull/7925/files/2cf533f0..04d6bf49

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7925=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7925=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7925.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7925/head:pull/7925

PR: https://git.openjdk.java.net/jdk/pull/7925


Re: RFR: 8283575: Check for GNU time fails for version >1.7

2022-03-23 Thread Magnus Ihse Bursie
On Wed, 23 Mar 2022 15:35:44 GMT, Erik Joelsson  wrote:

> The version output of GNU time changed from "GNU time" to "GNU Time" in 
> version 1.8. We need to update our check for identifying GNU time to handle 
> this.

I usually handle cases like this by prefixing with a comment:

# Additional [] needed to keep m4 from mangling shell constructs.

and then wrap the entire statement in an outer `[ ]` so the inner, "real" 
brackets are unchanged.

make/autoconf/basic_tools.m4 line 351:

> 349:   UTIL_LOOKUP_PROGS(PATCH, gpatch patch)
> 350:   # Check if it's GNU time
> 351:   IS_GNU_TIME=`$TIME --version 2>&1 | $GREP 'GNU [[Tt]]ime'`

Suggestion:

  [ IS_GNU_TIME=`$TIME --version 2>&1 | $GREP 'GNU [Tt]ime'` ]


Like this.

-

PR: https://git.openjdk.java.net/jdk/pull/7925


Re: RFR: 8283575: Check for GNU time fails for version >1.7

2022-03-23 Thread Erik Joelsson
On Wed, 23 Mar 2022 16:22:55 GMT, Aleksey Shipilev  wrote:

> Looks okay, provided you tested with some version of GNU [Tt]ime.

Thanks! I hit this while trying to use GNU time on my mac to get LOG=profile to 
work, to profile compilation times of individual compilation units. I built 
latest (1.9) from source and it didn't detect it (same with 1.8). With this 
patch it works as expected.

> Is double square bracket some sort of escaping? Haven't seen it before.

Yes, it's escaping needed to use brackets in autoconf/m4. Sometimes we put the 
escape sequence around the complete expression, sometimes we just double up 
like I did here. You should be able to find several occurrences through out our 
autoconf scripts.

-

PR: https://git.openjdk.java.net/jdk/pull/7925


Re: RFR: 8283575: Check for GNU time fails for version >1.7

2022-03-23 Thread Aleksey Shipilev
On Wed, 23 Mar 2022 15:35:44 GMT, Erik Joelsson  wrote:

> The version output of GNU time changed from "GNU time" to "GNU Time" in 
> version 1.8. We need to update our check for identifying GNU time to handle 
> this.

Looks okay, provided you tested with some version of GNU [Tt]ime.

Is double square bracket some sort of escaping? Haven't seen it before.

-

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7925


RFR: 8283575: Check for GNU time fails for version >1.7

2022-03-23 Thread Erik Joelsson
The version output of GNU time changed from "GNU time" to "GNU Time" in version 
1.8. We need to update our check for identifying GNU time to handle this.

-

Commit messages:
 - JDK-8283575

Changes: https://git.openjdk.java.net/jdk/pull/7925/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7925=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283575
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7925.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7925/head:pull/7925

PR: https://git.openjdk.java.net/jdk/pull/7925