Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8202476: ImageLib is broken in 32 bit Windows
Thank you! -- Alexey On 04/05/2018 18:53, Phil Race wrote: Yes, your confirmation on the testing was all that was needed. -phil. On 5/4/2018 10:45 AM, Alexey Ivanov wrote: Hi Phil, Just to confirm: do you approve the change? Thank you, Alexey On 02/05/2018 19:24, Alexey Ivanov wrote: Hi Phil, Thank you for your review. On 02/05/2018 17:28, Phil Race wrote: So ... the original change that removed the mapfiles broke the 32 bit build because of inconsistency between declarations + definitions of some functions. It did not affect 64 bit build because JNICALL is a no-op there. The next change (8201226) added JNICALL to make it consistent, but was not reviewed on 2d and was pushed without such review and may have fixed build issues but did NOT fix the product. Yes, you're right. It made the product buildable but it did not fix the product. This change now removes JNICALL where it is not needed and fixes the product and does not break any builds! Please confirm that you've run 64+32 bit builds through the build system AND run imaging tests on both of those. Yes, I ran 64 builds through the build system and ran 32 bit Windows build on my laptop. I ran regression tests from test/jdk/java/awt/image with both 64 and 32 bit builds on Windows. No failures. Regards, Alexey After that is confirmed you have my OK. -phil. On 05/02/2018 05:03 AM, Magnus Ihse Bursie wrote: Looks good to me, but you should have a reviewer from the client team as well. /Magnus 2 maj 2018 kl. 11:52 skrev Alexey Ivanov: Hi, Could you please review the following fix for jdk11? bug: https://bugs.openjdk.java.net/browse/JDK-8202476 webrev: http://cr.openjdk.java.net/~aivanov/8202476/jdk11/webrev.0/ This is a follow-up fix for JDK-8201226 which enabled building JDK for 32 bit Windows, its code review: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html I found this issue: http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009150.html This fix removes JNICALL modifiers from exported functions in medialib. After the fix, the failing tests in test/jdk/java/awt/image pass. Thank you in advance. Regards, Alexey
Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8202476: ImageLib is broken in 32 bit Windows
Yes, your confirmation on the testing was all that was needed. -phil. On 5/4/2018 10:45 AM, Alexey Ivanov wrote: Hi Phil, Just to confirm: do you approve the change? Thank you, Alexey On 02/05/2018 19:24, Alexey Ivanov wrote: Hi Phil, Thank you for your review. On 02/05/2018 17:28, Phil Race wrote: So ... the original change that removed the mapfiles broke the 32 bit build because of inconsistency between declarations + definitions of some functions. It did not affect 64 bit build because JNICALL is a no-op there. The next change (8201226) added JNICALL to make it consistent, but was not reviewed on 2d and was pushed without such review and may have fixed build issues but did NOT fix the product. Yes, you're right. It made the product buildable but it did not fix the product. This change now removes JNICALL where it is not needed and fixes the product and does not break any builds! Please confirm that you've run 64+32 bit builds through the build system AND run imaging tests on both of those. Yes, I ran 64 builds through the build system and ran 32 bit Windows build on my laptop. I ran regression tests from test/jdk/java/awt/image with both 64 and 32 bit builds on Windows. No failures. Regards, Alexey After that is confirmed you have my OK. -phil. On 05/02/2018 05:03 AM, Magnus Ihse Bursie wrote: Looks good to me, but you should have a reviewer from the client team as well. /Magnus 2 maj 2018 kl. 11:52 skrev Alexey Ivanov: Hi, Could you please review the following fix for jdk11? bug: https://bugs.openjdk.java.net/browse/JDK-8202476 webrev: http://cr.openjdk.java.net/~aivanov/8202476/jdk11/webrev.0/ This is a follow-up fix for JDK-8201226 which enabled building JDK for 32 bit Windows, its code review: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html I found this issue: http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009150.html This fix removes JNICALL modifiers from exported functions in medialib. After the fix, the failing tests in test/jdk/java/awt/image pass. Thank you in advance. Regards, Alexey
Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8202476: ImageLib is broken in 32 bit Windows
Hi Phil, Just to confirm: do you approve the change? Thank you, Alexey On 02/05/2018 19:24, Alexey Ivanov wrote: Hi Phil, Thank you for your review. On 02/05/2018 17:28, Phil Race wrote: So ... the original change that removed the mapfiles broke the 32 bit build because of inconsistency between declarations + definitions of some functions. It did not affect 64 bit build because JNICALL is a no-op there. The next change (8201226) added JNICALL to make it consistent, but was not reviewed on 2d and was pushed without such review and may have fixed build issues but did NOT fix the product. Yes, you're right. It made the product buildable but it did not fix the product. This change now removes JNICALL where it is not needed and fixes the product and does not break any builds! Please confirm that you've run 64+32 bit builds through the build system AND run imaging tests on both of those. Yes, I ran 64 builds through the build system and ran 32 bit Windows build on my laptop. I ran regression tests from test/jdk/java/awt/image with both 64 and 32 bit builds on Windows. No failures. Regards, Alexey After that is confirmed you have my OK. -phil. On 05/02/2018 05:03 AM, Magnus Ihse Bursie wrote: Looks good to me, but you should have a reviewer from the client team as well. /Magnus 2 maj 2018 kl. 11:52 skrev Alexey Ivanov: Hi, Could you please review the following fix for jdk11? bug: https://bugs.openjdk.java.net/browse/JDK-8202476 webrev: http://cr.openjdk.java.net/~aivanov/8202476/jdk11/webrev.0/ This is a follow-up fix for JDK-8201226 which enabled building JDK for 32 bit Windows, its code review: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html I found this issue: http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009150.html This fix removes JNICALL modifiers from exported functions in medialib. After the fix, the failing tests in test/jdk/java/awt/image pass. Thank you in advance. Regards, Alexey
Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8202476: ImageLib is broken in 32 bit Windows
Hi Phil, Thank you for your review. On 02/05/2018 17:28, Phil Race wrote: So ... the original change that removed the mapfiles broke the 32 bit build because of inconsistency between declarations + definitions of some functions. It did not affect 64 bit build because JNICALL is a no-op there. The next change (8201226) added JNICALL to make it consistent, but was not reviewed on 2d and was pushed without such review and may have fixed build issues but did NOT fix the product. Yes, you're right. It made the product buildable but it did not fix the product. This change now removes JNICALL where it is not needed and fixes the product and does not break any builds! Please confirm that you've run 64+32 bit builds through the build system AND run imaging tests on both of those. Yes, I ran 64 builds through the build system and ran 32 bit Windows build on my laptop. I ran regression tests from test/jdk/java/awt/image with both 64 and 32 bit builds on Windows. No failures. Regards, Alexey After that is confirmed you have my OK. -phil. On 05/02/2018 05:03 AM, Magnus Ihse Bursie wrote: Looks good to me, but you should have a reviewer from the client team as well. /Magnus 2 maj 2018 kl. 11:52 skrev Alexey Ivanov: Hi, Could you please review the following fix for jdk11? bug: https://bugs.openjdk.java.net/browse/JDK-8202476 webrev: http://cr.openjdk.java.net/~aivanov/8202476/jdk11/webrev.0/ This is a follow-up fix for JDK-8201226 which enabled building JDK for 32 bit Windows, its code review: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html I found this issue: http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009150.html This fix removes JNICALL modifiers from exported functions in medialib. After the fix, the failing tests in test/jdk/java/awt/image pass. Thank you in advance. Regards, Alexey
Re: [OpenJDK 2D-Dev] [11] RFR for JDK-8202476: ImageLib is broken in 32 bit Windows
So ... the original change that removed the mapfiles broke the 32 bit build because of inconsistency between declarations + definitions of some functions. It did not affect 64 bit build because JNICALL is a no-op there. The next change (8201226) added JNICALL to make it consistent, but was not reviewed on 2d and was pushed without such review and may have fixed build issues but did NOT fix the product. This change now removes JNICALL where it is not needed and fixes the product and does not break any builds! Please confirm that you've run 64+32 bit builds through the build system AND run imaging tests on both of those. After that is confirmed you have my OK. -phil. On 05/02/2018 05:03 AM, Magnus Ihse Bursie wrote: Looks good to me, but you should have a reviewer from the client team as well. /Magnus 2 maj 2018 kl. 11:52 skrev Alexey Ivanov: Hi, Could you please review the following fix for jdk11? bug: https://bugs.openjdk.java.net/browse/JDK-8202476 webrev: http://cr.openjdk.java.net/~aivanov/8202476/jdk11/webrev.0/ This is a follow-up fix for JDK-8201226 which enabled building JDK for 32 bit Windows, its code review: http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021553.html I found this issue: http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009150.html This fix removes JNICALL modifiers from exported functions in medialib. After the fix, the failing tests in test/jdk/java/awt/image pass. Thank you in advance. Regards, Alexey