RFR(S): 8174202 : jtreg AOT tests should not assume library extension of .so

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(S): 8174202 : jtreg AOT tests should not assume library extension of .so

Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
> 67 lines changed: 47 ins; 11 del; 9 mod;

Hi all,

could you please review this small fix?

problem: some of aot tests assumed that output file is named unnamed.so regardless the platform. the problem was worked around in jaotc tool.
fix: the workaround was removed and now we use platform-specific extension for default output name. it's unnamed.dll on windows, unnamed.dylib on mac and unnamed.so on all other platforms.
the tests have been updated correspondingly.

webrev: http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
jbs: https://bugs.openjdk.java.net/browse/JDK-8174202
testing: hotspot/compiler/aot (w/ 8183337[1] applied) on all supported platforms

[1] http://cr.openjdk.java.net/~iignatyev//8183337/webrev.00/index.html

Thanks,
-- Igor
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8174202 : jtreg AOT tests should not assume library extension of .so

Vladimir Kozlov
Good.

Thanks,
Vladimir

On 8/10/17 4:33 PM, Igor Ignatyev wrote:

> http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
>> 67 lines changed: 47 ins; 11 del; 9 mod;
>
> Hi all,
>
> could you please review this small fix?
>
> problem: some of aot tests assumed that output file is named unnamed.so regardless the platform. the problem was worked around in jaotc tool.
> fix: the workaround was removed and now we use platform-specific extension for default output name. it's unnamed.dll on windows, unnamed.dylib on mac and unnamed.so on all other platforms.
> the tests have been updated correspondingly.
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
> jbs: https://bugs.openjdk.java.net/browse/JDK-8174202
> testing: hotspot/compiler/aot (w/ 8183337[1] applied) on all supported platforms
>
> [1] http://cr.openjdk.java.net/~iignatyev//8183337/webrev.00/index.html
>
> Thanks,
> -- Igor
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8174202 : jtreg AOT tests should not assume library extension of .so

Igor Ignatyev
Thank you Vladimir.
I've found a couple of copy-paste errors in jdk/tools/jaotc/Main.java, I always used '.so' substringing output filename to object filename.

> --- a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Main.java Thu Aug 10 14:23:51 2017 -0700
> +++ b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Main.java Thu Aug 10 19:07:31 2017 -0700
> @@ -577,7 +577,7 @@
>                      break;
>                  case "Mac OS X":
>                      if (name.endsWith(".dylib")) {
> -                        objectFileName = name.substring(0, name.length() - ".so".length());
> +                        objectFileName = name.substring(0, name.length() - ".dylib".length());
>                      }
>                      objectFileName = objectFileName + ".o";
>                      linkerPath = (options.linkerpath != null) ? options.linkerpath : "ld";
> @@ -586,7 +586,7 @@
>                  default:
>                      if (osName.startsWith("Windows")) {
>                          if (name.endsWith(".dll")) {
> -                            objectFileName = name.substring(0, name.length() - ".so".length());
> +                            objectFileName = name.substring(0, name.length() - ".dll".length());
>                          }
>                          objectFileName = objectFileName + ".obj";
>                          linkerPath = (options.linkerpath != null) ? options.linkerpath : getWindowsLinkPath();

Thanks,
-- Igor

> On Aug 10, 2017, at 5:24 PM, Vladimir Kozlov <[hidden email]> wrote:
>
> Good.
>
> Thanks,
> Vladimir
>
> On 8/10/17 4:33 PM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
>>> 67 lines changed: 47 ins; 11 del; 9 mod;
>> Hi all,
>> could you please review this small fix?
>> problem: some of aot tests assumed that output file is named unnamed.so regardless the platform. the problem was worked around in jaotc tool.
>> fix: the workaround was removed and now we use platform-specific extension for default output name. it's unnamed.dll on windows, unnamed.dylib on mac and unnamed.so on all other platforms.
>> the tests have been updated correspondingly.
>> webrev: http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8174202
>> testing: hotspot/compiler/aot (w/ 8183337[1] applied) on all supported platforms
>> [1] http://cr.openjdk.java.net/~iignatyev//8183337/webrev.00/index.html
>> Thanks,
>> -- Igor

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S): 8174202 : jtreg AOT tests should not assume library extension of .so

Vladimir Kozlov
On 8/10/17 7:11 PM, Igor Ignatyev wrote:

> Thank you Vladimir.
> I've found a couple of copy-paste errors in jdk/tools/jaotc/Main.java, I always used '.so' substringing output filename to object filename.
>
>> --- a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Main.java Thu Aug 10 14:23:51 2017 -0700
>> +++ b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/Main.java Thu Aug 10 19:07:31 2017 -0700
>> @@ -577,7 +577,7 @@
>>                       break;
>>                   case "Mac OS X":
>>                       if (name.endsWith(".dylib")) {
>> -                        objectFileName = name.substring(0, name.length() - ".so".length());
>> +                        objectFileName = name.substring(0, name.length() - ".dylib".length());
>>                       }
>>                       objectFileName = objectFileName + ".o";
>>                       linkerPath = (options.linkerpath != null) ? options.linkerpath : "ld";
>> @@ -586,7 +586,7 @@
>>                   default:
>>                       if (osName.startsWith("Windows")) {
>>                           if (name.endsWith(".dll")) {
>> -                            objectFileName = name.substring(0, name.length() - ".so".length());
>> +                            objectFileName = name.substring(0, name.length() - ".dll".length());
>>                           }
>>                           objectFileName = objectFileName + ".obj";
>>                           linkerPath = (options.linkerpath != null) ? options.linkerpath : getWindowsLinkPath();

Good. I thought it was intentional :)

Vladimir

>
> Thanks,
> -- Igor
>
>> On Aug 10, 2017, at 5:24 PM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> Good.
>>
>> Thanks,
>> Vladimir
>>
>> On 8/10/17 4:33 PM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
>>>> 67 lines changed: 47 ins; 11 del; 9 mod;
>>> Hi all,
>>> could you please review this small fix?
>>> problem: some of aot tests assumed that output file is named unnamed.so regardless the platform. the problem was worked around in jaotc tool.
>>> fix: the workaround was removed and now we use platform-specific extension for default output name. it's unnamed.dll on windows, unnamed.dylib on mac and unnamed.so on all other platforms.
>>> the tests have been updated correspondingly.
>>> webrev: http://cr.openjdk.java.net/~iignatyev//8174202/webrev.00/index.html
>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8174202
>>> testing: hotspot/compiler/aot (w/ 8183337[1] applied) on all supported platforms
>>> [1] http://cr.openjdk.java.net/~iignatyev//8183337/webrev.00/index.html
>>> Thanks,
>>> -- Igor
>
Loading...