Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin
Hi Thomas, thanks for offering a sponsoring. @Matthias: can you please test this patch, so we can commit? On 11/12/18 7:16 AM, Thomas Stüfe wrote: Hi Michal, I can sponsor for you. @Matthias: yould you test on your Box too if this patch works? Best Regards, Thomas On Mon, Nov 12, 2018 at 5:24 AM Michal Vala wrote: Hi Thomas, thanks! I've tested on Windows 2012, vs2013. Anyone with latest Windows 10 to test this? Also I'd like to ask someone to sponsor this, as I'm just an author. On 11/9/18 7:09 PM, Thomas Stüfe wrote: Hi Michal, I tested this and it now works nicely for me (win7, vs2017, with current jdk/jdk). Change looks fine to me to. Best Regards, Thomas On Fri, Nov 9, 2018 at 7:23 PM Michal Vala wrote: I got valid idea project even with empty JT_HOME as placeholder was correctly replaced by empty string. Sure that it's not acceptable. Anyway, JT_HOME should be only variable that can be empty. new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.02/ On 11/9/18 5:42 PM, Thomas Stüfe wrote: On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe wrote: Hi Michal, does not yet work for me. I get cygpath Usage output: $ bash ./bin/idea.sh Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME... cygpath [-c HANDLE] cygpath [-ADHOPSW] cygpath [-F ID] Convert Unix and Windows format paths, or output system path information ... Cheers, Thomas add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`" seems to be the culprit. JT_HOME is empty, and I never did set that before (I usually work on Linux though). I think the problem is that in this expression: if [ "x$CYGPATH" = "x" ]; then .. else .. fi the non-windows path does not require the variables to be set. Whereas calling "cygpath -am" without an argument is an error which leads to the usage output. ..Thomas On Fri, Nov 9, 2018 at 6:09 PM Michal Vala wrote: You're right, sorry. Updated webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/ On 11/9/18 5:42 PM, Erik Joelsson wrote: Hello Michal, It looks like the "dirname" calls are omitted in the cygpath case, so BUILD_DIR ends up pointing to the spec file instead of the directory the file is in. /Erik On 2018-11-09 05:58, Michal Vala wrote: Hi, I've looked into this. Please review the patch: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/ On 11/9/18 9:29 AM, Baesken, Matthias wrote: Hello , I opened 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported https://bugs.openjdk.java.net/browse/JDK-8213591 for the reported issue . Best regards, Matthias -Original Message- From: Erik Joelsson Sent: Donnerstag, 8. November 2018 18:05 To: Baesken, Matthias ; Chris Hegarty ; 'build-dev@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin A patch fixing idea.sh so that it works on Windows would certainly be welcome. /Erik On 2018-11-08 05:12, Baesken, Matthias wrote: Hi Chris , thanks for the info . However I found out that replacing the /cygdrive/C/ with C:/ in the top-level xml/imlfiles in the ".idea" - folder makes IntelliJ happy, I could then open the project successfully from IntelliJ . So I guess a couple of"cygpath -aw" -calls at the right places in the project generation might fix the idea.sh based project file generation on Cygwin (without postprocessing). Any comments on this ? Or is there another way to get .idea/-files that open "out of the box" ? Best regards, Matthias -Original Message- From: Chris Hegarty Sent: Donnerstag, 8. November 2018 12:52 To: Baesken, Matthias ; 'build- d...@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin Matthias, On 08/11/18 11:45, Baesken, Matthias wrote: Hello, I tried to use bin/idea.sh with Cygwin to generate project files for IDEA IntelliJ Community . The project file generation seems to work and outputs the .idea - folder with lots of xml files in it . However , when opening the project from IDEA, it fails with a message : VCS root configuration problems - The directory \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea root but no hg4idea repositories were found there . C.\hg\open\jdk\jdk6 Could it be that the Cygwin-paths in the generated xml-files confuse the IDEA intelliJ IDE ? Certainly looks like it. Has anybody ever used it successfully with Cygwin/ Windows ? ( or with some other UNIX shell/toolset for Windows) ? I have not tried. I use it successfully on macOS and Linux. -Chris. -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8213622 - Windows VS2013 build failure - "'snprintf': identifier not found"
Hi, On 11/13/18 1:08 AM, JC Beyler wrote: Hi all, @Mark: good point, fixed in the new webrev @David: also good point, just because originally it was written differently and I moved the code to this format and didn't move the +1 to the "right" spot @Michal: do you mind if I take over the bug and push this fix, could you review it as well? Please, take it. I'm not jdk reviewer nor have anough C++ skills to do a reviews. I don't see any issue there. I've also tried to build it with VS2013 and it works fine. Thanks! Here is the new webrev: http://cr.openjdk.java.net/~jcbeyler/8213622/webrev.01/ Here is the bug: https://bugs.openjdk.java.net/browse/JDK-8213622 Thanks, Jc On Mon, Nov 12, 2018 at 2:08 PM David Holmes wrote: Hi Jc, This seems okay to me. Only minor query is why you do the +1 (presumably for terminating NUL) on the return_len instead of len ? Thanks, David On 13/11/2018 7:11 AM, JC Beyler wrote: Hi all, I created this change instead: http://cr.openjdk.java.net/~jcbeyler/8213622/webrev.00/ <http://cr.openjdk.java.net/%7Ejcbeyler/8213622/webrev.00/> It removes the sprintf calls for strlen and strncpy calls. I checked that the code works if I force an error on GetObjectClass for example: FATAL ERROR in native method: GetObjectClass : Return is NULL at nsk.share.gc.lock.CriticalSectionTimedLocker.criticalSection(CriticalSectionTimedLocker.java:47) at nsk.share.gc.lock.CriticalSectionLocker$1.run(CriticalSectionLocker.java:56) at nsk.share.gc.lock.jniref.JNILocalRefLocker.criticalNative(Native Method) at nsk.share.gc.lock.jniref.JNILocalRefLocker.criticalSection(JNILocalRefLocker.java:44) at nsk.share.gc.lock.CriticalSectionTimedLocker.criticalSection(CriticalSectionTimedLocker.java:47) at nsk.share.gc.lock.CriticalSectionLocker$1.run(CriticalSectionLocker.java:56) at java.lang.Thread.run(java.base@12-internal/Thread.java:835) I'll pass it through the submit repo if this looks like a better fix. Let me know what you think, Jc On Sun, Nov 11, 2018 at 10:38 PM JC Beyler mailto:jcbey...@google.com>> wrote: Hi all, If I've caught up with the conversation, it sounds like: - My code breaks VS2013 build but that soon that won't be supported - We can't just do a renaming macro due to my use of sprintf with an empty buffer - So we need to either do what was suggested by Kim: "That first snprintf call could be rewritten using several calls to strlen to calculate the needed size in some different manner. The later call could also be rewritten to use several calls to strcpy rather than snprintf." Or something to that effect I can work on a fix that will handle this if we want, I'm just not sure if that's the path that is being recommended here though. It seems that in this particular case, it is not really hard to fix the code for now; the day VS2013 is no longer build-able by other pieces of code we can come back to this solution. To be honest, the reason this is like this is due to not being able to have C++ strings when I tried initially. The day we can have those, then this code can become even simpler. Let me know what you think and would prefer, Jc On Sun, Nov 11, 2018 at 9:01 PM David Holmes mailto:david.hol...@oracle.com>> wrote: Hi Michal, On 12/11/2018 2:18 PM, Michal Vala wrote: > > > On 11/10/18 12:07 AM, David Holmes wrote: >> cc'ing JC Beyler as this was his code. >> >> On 10/11/2018 4:35 AM, Kim Barrett wrote: >>>> On Nov 9, 2018, at 11:43 AM, Michal Vala mailto:mv...@redhat.com>> wrote: >>>> >>>> Hi, >>>> >>>> please review following patch fixing build failure on Windows with >>>> VS2013 toolchain. >> >> Not sure we're still supporting VS2013 ... ?? > > Minimum accepted by configure is 2010. 2013 is 2nd in priorities after > 2017. It has `VS_SUPPORTED_2013=false` though, but why not keep it > buildable? That depends on how painful it is to achieve that. As Kim has explained the suggested fix may allow building but it isn't correct. And we may not be able to keep this ability to build with VS2013 going forward. Thanks, David >> >>>> >>>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213622/webrev.00/ < http://cr.openjdk.java.net/%7Emvala/jdk/jdk/JDK-8213622/webrev.00/> >>> >>> The failure
Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin
Hi Thomas, thanks! I've tested on Windows 2012, vs2013. Anyone with latest Windows 10 to test this? Also I'd like to ask someone to sponsor this, as I'm just an author. On 11/9/18 7:09 PM, Thomas Stüfe wrote: Hi Michal, I tested this and it now works nicely for me (win7, vs2017, with current jdk/jdk). Change looks fine to me to. Best Regards, Thomas On Fri, Nov 9, 2018 at 7:23 PM Michal Vala wrote: I got valid idea project even with empty JT_HOME as placeholder was correctly replaced by empty string. Sure that it's not acceptable. Anyway, JT_HOME should be only variable that can be empty. new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.02/ On 11/9/18 5:42 PM, Thomas Stüfe wrote: On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe wrote: Hi Michal, does not yet work for me. I get cygpath Usage output: $ bash ./bin/idea.sh Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME... cygpath [-c HANDLE] cygpath [-ADHOPSW] cygpath [-F ID] Convert Unix and Windows format paths, or output system path information ... Cheers, Thomas add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`" seems to be the culprit. JT_HOME is empty, and I never did set that before (I usually work on Linux though). I think the problem is that in this expression: if [ "x$CYGPATH" = "x" ]; then .. else .. fi the non-windows path does not require the variables to be set. Whereas calling "cygpath -am" without an argument is an error which leads to the usage output. ..Thomas On Fri, Nov 9, 2018 at 6:09 PM Michal Vala wrote: You're right, sorry. Updated webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/ On 11/9/18 5:42 PM, Erik Joelsson wrote: Hello Michal, It looks like the "dirname" calls are omitted in the cygpath case, so BUILD_DIR ends up pointing to the spec file instead of the directory the file is in. /Erik On 2018-11-09 05:58, Michal Vala wrote: Hi, I've looked into this. Please review the patch: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/ On 11/9/18 9:29 AM, Baesken, Matthias wrote: Hello , I opened 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported https://bugs.openjdk.java.net/browse/JDK-8213591 for the reported issue . Best regards, Matthias -Original Message- From: Erik Joelsson Sent: Donnerstag, 8. November 2018 18:05 To: Baesken, Matthias ; Chris Hegarty ; 'build-dev@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin A patch fixing idea.sh so that it works on Windows would certainly be welcome. /Erik On 2018-11-08 05:12, Baesken, Matthias wrote: Hi Chris , thanks for the info . However I found out that replacing the /cygdrive/C/ with C:/ in the top-level xml/imlfiles in the ".idea" - folder makes IntelliJ happy, I could then open the project successfully from IntelliJ . So I guess a couple of"cygpath -aw" -calls at the right places in the project generation might fix the idea.sh based project file generation on Cygwin (without postprocessing). Any comments on this ? Or is there another way to get .idea/-files that open "out of the box" ? Best regards, Matthias -Original Message- From: Chris Hegarty Sent: Donnerstag, 8. November 2018 12:52 To: Baesken, Matthias ; 'build- d...@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin Matthias, On 08/11/18 11:45, Baesken, Matthias wrote: Hello, I tried to use bin/idea.sh with Cygwin to generate project files for IDEA IntelliJ Community . The project file generation seems to work and outputs the .idea - folder with lots of xml files in it . However , when opening the project from IDEA, it fails with a message : VCS root configuration problems - The directory \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea root but no hg4idea repositories were found there . C.\hg\open\jdk\jdk6 Could it be that the Cygwin-paths in the generated xml-files confuse the IDEA intelliJ IDE ? Certainly looks like it. Has anybody ever used it successfully with Cygwin/ Windows ? ( or with some other UNIX shell/toolset for Windows) ? I have not tried. I use it successfully on macOS and Linux. -Chris. -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8213622 - Windows VS2013 build failure - "'snprintf': identifier not found"
On 11/10/18 12:07 AM, David Holmes wrote: cc'ing JC Beyler as this was his code. On 10/11/2018 4:35 AM, Kim Barrett wrote: On Nov 9, 2018, at 11:43 AM, Michal Vala wrote: Hi, please review following patch fixing build failure on Windows with VS2013 toolchain. Not sure we're still supporting VS2013 ... ?? Minimum accepted by configure is 2010. 2013 is 2nd in priorities after 2017. It has `VS_SUPPORTED_2013=false` though, but why not keep it buildable? http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213622/webrev.00/ The failure is in a hotspot test. It should be using os::snprintf. Tests don't have access to os::snprintf. The test is just C++ code. Cheers, David -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin
I got valid idea project even with empty JT_HOME as placeholder was correctly replaced by empty string. Sure that it's not acceptable. Anyway, JT_HOME should be only variable that can be empty. new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.02/ On 11/9/18 5:42 PM, Thomas Stüfe wrote: On Fri, Nov 9, 2018 at 5:35 PM Thomas Stüfe wrote: Hi Michal, does not yet work for me. I get cygpath Usage output: $ bash ./bin/idea.sh Usage: cygpath (-d|-m|-u|-w|-t TYPE) [-f FILE] [OPTION]... NAME... cygpath [-c HANDLE] cygpath [-ADHOPSW] cygpath [-F ID] Convert Unix and Windows format paths, or output system path information ... Cheers, Thomas add_replacement "###JTREG_HOME###" "`cygpath -am $JT_HOME`" seems to be the culprit. JT_HOME is empty, and I never did set that before (I usually work on Linux though). I think the problem is that in this expression: if [ "x$CYGPATH" = "x" ]; then .. else .. fi the non-windows path does not require the variables to be set. Whereas calling "cygpath -am" without an argument is an error which leads to the usage output. ..Thomas On Fri, Nov 9, 2018 at 6:09 PM Michal Vala wrote: You're right, sorry. Updated webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/ On 11/9/18 5:42 PM, Erik Joelsson wrote: Hello Michal, It looks like the "dirname" calls are omitted in the cygpath case, so BUILD_DIR ends up pointing to the spec file instead of the directory the file is in. /Erik On 2018-11-09 05:58, Michal Vala wrote: Hi, I've looked into this. Please review the patch: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/ On 11/9/18 9:29 AM, Baesken, Matthias wrote: Hello , I opened 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported https://bugs.openjdk.java.net/browse/JDK-8213591 for the reported issue . Best regards, Matthias -Original Message- From: Erik Joelsson Sent: Donnerstag, 8. November 2018 18:05 To: Baesken, Matthias ; Chris Hegarty ; 'build-dev@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin A patch fixing idea.sh so that it works on Windows would certainly be welcome. /Erik On 2018-11-08 05:12, Baesken, Matthias wrote: Hi Chris , thanks for the info . However I found out that replacing the /cygdrive/C/ with C:/ in the top-level xml/imlfiles in the ".idea" - folder makes IntelliJ happy, I could then open the project successfully from IntelliJ . So I guess a couple of"cygpath -aw" -calls at the right places in the project generation might fix the idea.sh based project file generation on Cygwin (without postprocessing). Any comments on this ? Or is there another way to get .idea/-files that open "out of the box" ? Best regards, Matthias -Original Message- From: Chris Hegarty Sent: Donnerstag, 8. November 2018 12:52 To: Baesken, Matthias ; 'build- d...@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin Matthias, On 08/11/18 11:45, Baesken, Matthias wrote: Hello, I tried to use bin/idea.sh with Cygwin to generate project files for IDEA IntelliJ Community . The project file generation seems to work and outputs the .idea - folder with lots of xml files in it . However , when opening the project from IDEA, it fails with a message : VCS root configuration problems - The directory \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea root but no hg4idea repositories were found there . C.\hg\open\jdk\jdk6 Could it be that the Cygwin-paths in the generated xml-files confuse the IDEA intelliJ IDE ? Certainly looks like it. Has anybody ever used it successfully with Cygwin/ Windows ? ( or with some other UNIX shell/toolset for Windows) ? I have not tried. I use it successfully on macOS and Linux. -Chris. -- Michal Vala OpenJDK QE Red Hat Czech -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin
You're right, sorry. Updated webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.01/ On 11/9/18 5:42 PM, Erik Joelsson wrote: Hello Michal, It looks like the "dirname" calls are omitted in the cygpath case, so BUILD_DIR ends up pointing to the spec file instead of the directory the file is in. /Erik On 2018-11-09 05:58, Michal Vala wrote: Hi, I've looked into this. Please review the patch: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/ On 11/9/18 9:29 AM, Baesken, Matthias wrote: Hello , I opened 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported https://bugs.openjdk.java.net/browse/JDK-8213591 for the reported issue . Best regards, Matthias -Original Message- From: Erik Joelsson Sent: Donnerstag, 8. November 2018 18:05 To: Baesken, Matthias ; Chris Hegarty ; 'build-dev@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin A patch fixing idea.sh so that it works on Windows would certainly be welcome. /Erik On 2018-11-08 05:12, Baesken, Matthias wrote: Hi Chris , thanks for the info . However I found out that replacing the /cygdrive/C/ with C:/ in the top-level xml/iml files in the ".idea" - folder makes IntelliJ happy, I could then open the project successfully from IntelliJ . So I guess a couple of "cygpath -aw" -calls at the right places in the project generation might fix the idea.sh based project file generation on Cygwin (without postprocessing). Any comments on this ? Or is there another way to get .idea/-files that open "out of the box" ? Best regards, Matthias -Original Message- From: Chris Hegarty Sent: Donnerstag, 8. November 2018 12:52 To: Baesken, Matthias ; 'build- d...@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin Matthias, On 08/11/18 11:45, Baesken, Matthias wrote: Hello, I tried to use bin/idea.sh with Cygwin to generate project files for IDEA IntelliJ Community . The project file generation seems to work and outputs the .idea - folder with lots of xml files in it . However , when opening the project from IDEA, it fails with a message : VCS root configuration problems - The directory \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea root but no hg4idea repositories were found there . C.\hg\open\jdk\jdk6 Could it be that the Cygwin-paths in the generated xml-files confuse the IDEA intelliJ IDE ? Certainly looks like it. Has anybody ever used it successfully with Cygwin/ Windows ? ( or with some other UNIX shell/toolset for Windows) ? I have not tried. I use it successfully on macOS and Linux. -Chris. -- Michal Vala OpenJDK QE Red Hat Czech
RFR: 8213622 - Windows VS2013 build failure - "'snprintf': identifier not found"
Hi, please review following patch fixing build failure on Windows with VS2013 toolchain. http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213622/webrev.00/ -- Michal Vala OpenJDK QE Red Hat Czech
RFR: 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported - was : RE: bin/idea.sh and Cygwin
Hi, I've looked into this. Please review the patch: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213591/webrev.00/ On 11/9/18 9:29 AM, Baesken, Matthias wrote: Hello , I opened 8213591 : running bin/idea.sh in Cygwin: generated project cannot be imported https://bugs.openjdk.java.net/browse/JDK-8213591 for the reported issue . Best regards, Matthias -Original Message- From: Erik Joelsson Sent: Donnerstag, 8. November 2018 18:05 To: Baesken, Matthias ; Chris Hegarty ; 'build-dev@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin A patch fixing idea.sh so that it works on Windows would certainly be welcome. /Erik On 2018-11-08 05:12, Baesken, Matthias wrote: Hi Chris , thanks for the info . However I found out that replacing the /cygdrive/C/ with C:/ in the top-level xml/imlfiles in the ".idea" - folder makes IntelliJ happy, I could then open the project successfully from IntelliJ . So I guess a couple of"cygpath -aw" -calls at the right places in the project generation might fix the idea.sh based project file generation on Cygwin (without postprocessing). Any comments on this ? Or is there another way to get .idea/-files that open "out of the box" ? Best regards, Matthias -Original Message- From: Chris Hegarty Sent: Donnerstag, 8. November 2018 12:52 To: Baesken, Matthias ; 'build- d...@openjdk.java.net' ; maurizio.cimadam...@oracle.com Subject: Re: bin/idea.sh and Cygwin Matthias, On 08/11/18 11:45, Baesken, Matthias wrote: Hello, I tried to use bin/idea.sh with Cygwin to generate project files for IDEA IntelliJ Community . The project file generation seems to work and outputs the .idea - folder with lots of xml files in it . However , when opening the project from IDEA, it fails with a message : VCS root configuration problems - The directory \cygdrive\C\hg\open\jdk\jdk6 is registered as a hg4idea root but no hg4idea repositories were found there . C.\hg\open\jdk\jdk6 Could it be that the Cygwin-paths in the generated xml-files confuse the IDEA intelliJ IDE ? Certainly looks like it. Has anybody ever used it successfully with Cygwin/ Windows ? ( or with some other UNIX shell/toolset for Windows) ? I have not tried. I use it successfully on macOS and Linux. -Chris. -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: JDK-8208084: Windows build failure - "'snprintf': identifier not found"
thanks Coleen and Kim! On 07/30/2018 08:29 PM, coleen.phillim...@oracle.com wrote: This looks good. I will sponsor it. Coleen On 7/30/18 8:00 AM, Michal Vala wrote: On 07/27/2018 06:22 PM, Kim Barrett wrote: On Jul 24, 2018, at 2:03 AM, Michal Vala wrote: On 07/23/2018 07:15 PM, Erik Joelsson wrote: Hello, On 2018-07-23 08:27, Kim Barrett wrote: On Jul 23, 2018, at 9:38 AM, Michal Vala wrote: Hi, JDK-8208084 introduced build failure on Windows, where `snprintf` function is not implemented by Visual Studio 2013 (currently latest compiler supported by OpenJDK). I believe using `sprintf` is safe here. Please see the webrev. If it's ok, I would also need a sponsor for this. bug: https://bugs.openjdk.java.net/browse/JDK-8208084 webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.00/ Thanks -- Michal Vala OpenJDK QE Red Hat Czech It seems there's some documentation that needs to be updated. JDK 11 supports building with VS2017, and I think with JDK 12 we'll be requiring it, in preparation for starting to use features from more recent C++ standards. Indeed, that's on me. I just sent out an RFR to correct this. Your changeset doesn't update minimal Visual Studio version. It's not buildable with VS2010. If it’s really necessary to be able to build JDK 12 with earlier versions of Visual Studio for now (as I said, it might become impossible later for other reasons), the proper fix is to replace the call to snprintf with os::snprintf, which is what should have been used in the first place. I missed that when I was reviewing JDK-8207359. You're right. That also solves the issue with VS2013 and is imho proper way to implement it anyway. Here's the new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.01/ -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: JDK-8208084: Windows build failure - "'snprintf': identifier not found"
On 07/27/2018 06:22 PM, Kim Barrett wrote: On Jul 24, 2018, at 2:03 AM, Michal Vala wrote: On 07/23/2018 07:15 PM, Erik Joelsson wrote: Hello, On 2018-07-23 08:27, Kim Barrett wrote: On Jul 23, 2018, at 9:38 AM, Michal Vala wrote: Hi, JDK-8208084 introduced build failure on Windows, where `snprintf` function is not implemented by Visual Studio 2013 (currently latest compiler supported by OpenJDK). I believe using `sprintf` is safe here. Please see the webrev. If it's ok, I would also need a sponsor for this. bug: https://bugs.openjdk.java.net/browse/JDK-8208084 webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.00/ Thanks -- Michal Vala OpenJDK QE Red Hat Czech It seems there's some documentation that needs to be updated. JDK 11 supports building with VS2017, and I think with JDK 12 we'll be requiring it, in preparation for starting to use features from more recent C++ standards. Indeed, that's on me. I just sent out an RFR to correct this. Your changeset doesn't update minimal Visual Studio version. It's not buildable with VS2010. If it’s really necessary to be able to build JDK 12 with earlier versions of Visual Studio for now (as I said, it might become impossible later for other reasons), the proper fix is to replace the call to snprintf with os::snprintf, which is what should have been used in the first place. I missed that when I was reviewing JDK-8207359. You're right. That also solves the issue with VS2013 and is imho proper way to implement it anyway. Here's the new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.01/ -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: Update build documentation to reflect compiler upgrades at Oracle
Hi Erik, yes, I understand that. I'm pointing out to current inconsistency between documentation and code. There is currently no overlap (2010-2013 in doc vs 2015+ in code). I understand that VS version will go up, but it's not the case yet. Also keep the code backward compatible is not a bad thing unless it rise above some maintenance cost, which is now just one function call in test (JDK-8208084). Right? I wouldn't force backward compatibility neither, but this is too easy to fix to throw away whole feature. This discussion should probably go to JDK-8208084 RFR, as this one is solved I guess. On 07/24/2018 06:05 PM, Erik Joelsson wrote: Hello, We will most likely need to drop support for older VS in JDK 12 as there is much interest to move to a later C++ standard in Hotspot. That change has not happened yet though and will certainly require a JEP (the C++ standard change part). In the meantime, if you need JDK 12 to work on VS2013, I think your change makes sense (unless it's a step backwards from a standards perspective, hopefully Kim or someone better versed in language and standard libs can answer that). I would recommend upgrading your build environment to VS2017 instead if possible. You only need the "BuildTools" distribution to build OpenJDK. The community edition also works. Installing it in parallel with VS2013 works fine. JDK 11/12 builds will automatically pick 2017 and JDK 8/9/10 will automatically pick 2013. /Erik On 2018-07-23 23:06, Michal Vala wrote: Aha, this is for 11. It's ok then. On 07/24/2018 08:02 AM, Michal Vala wrote: Hi, On 07/23/2018 07:13 PM, Erik Joelsson wrote: The build documentation needs to be updated to reflect the compiler updates that took place at Oracle for JDK 11. Bug: https://bugs.openjdk.java.net/browse/JDK-8208096 Webrev: http://cr.openjdk.java.net/~erikj/8208096/webrev.01/ /Erik OpenJDK is currently not buildable with Visual Studio <2015 due to missing `snprintf`[1]. We should either update the doc to minimal version 2015, or commit JDK-8208084 from another RFR[2]. [1] - https://bugs.openjdk.java.net/browse/JDK-8208084 [2] - http://mail.openjdk.java.net/pipermail/build-dev/2018-July/022748.html -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: Update build documentation to reflect compiler upgrades at Oracle
Aha, this is for 11. It's ok then. On 07/24/2018 08:02 AM, Michal Vala wrote: Hi, On 07/23/2018 07:13 PM, Erik Joelsson wrote: The build documentation needs to be updated to reflect the compiler updates that took place at Oracle for JDK 11. Bug: https://bugs.openjdk.java.net/browse/JDK-8208096 Webrev: http://cr.openjdk.java.net/~erikj/8208096/webrev.01/ /Erik OpenJDK is currently not buildable with Visual Studio <2015 due to missing `snprintf`[1]. We should either update the doc to minimal version 2015, or commit JDK-8208084 from another RFR[2]. [1] - https://bugs.openjdk.java.net/browse/JDK-8208084 [2] - http://mail.openjdk.java.net/pipermail/build-dev/2018-July/022748.html -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: JDK-8208084: Windows build failure - "'snprintf': identifier not found"
On 07/23/2018 07:15 PM, Erik Joelsson wrote: Hello, On 2018-07-23 08:27, Kim Barrett wrote: On Jul 23, 2018, at 9:38 AM, Michal Vala wrote: Hi, JDK-8208084 introduced build failure on Windows, where `snprintf` function is not implemented by Visual Studio 2013 (currently latest compiler supported by OpenJDK). I believe using `sprintf` is safe here. Please see the webrev. If it's ok, I would also need a sponsor for this. bug: https://bugs.openjdk.java.net/browse/JDK-8208084 webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.00/ Thanks -- Michal Vala OpenJDK QE Red Hat Czech It seems there's some documentation that needs to be updated. JDK 11 supports building with VS2017, and I think with JDK 12 we'll be requiring it, in preparation for starting to use features from more recent C++ standards. Indeed, that's on me. I just sent out an RFR to correct this. Your changeset doesn't update minimal Visual Studio version. It's not buildable with VS2010. -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: Update build documentation to reflect compiler upgrades at Oracle
Hi, On 07/23/2018 07:13 PM, Erik Joelsson wrote: The build documentation needs to be updated to reflect the compiler updates that took place at Oracle for JDK 11. Bug: https://bugs.openjdk.java.net/browse/JDK-8208096 Webrev: http://cr.openjdk.java.net/~erikj/8208096/webrev.01/ /Erik OpenJDK is currently not buildable with Visual Studio <2015 due to missing `snprintf`[1]. We should either update the doc to minimal version 2015, or commit JDK-8208084 from another RFR[2]. [1] - https://bugs.openjdk.java.net/browse/JDK-8208084 [2] - http://mail.openjdk.java.net/pipermail/build-dev/2018-July/022748.html -- Michal Vala OpenJDK QE Red Hat Czech
RFR: JDK-8208084: Windows build failure - "'snprintf': identifier not found"
Hi, JDK-8208084 introduced build failure on Windows, where `snprintf` function is not implemented by Visual Studio 2013 (currently latest compiler supported by OpenJDK). I believe using `sprintf` is safe here. Please see the webrev. If it's ok, I would also need a sponsor for this. bug: https://bugs.openjdk.java.net/browse/JDK-8208084 webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.00/ Thanks -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
On 05/01/2018 07:59 PM, Kim Barrett wrote:
On Apr 27, 2018, at 4:26 PM, Michal Vala <mv...@redhat.com> wrote:
For now, proposed patch looks like this:
--- old/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.498343185 +0200
+++ new/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.214342670 +0200
@@ -98,26 +98,8 @@
inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
{
-// readdir_r has been deprecated since glibc 2.24.
-// See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
- dirent* p;
- int status;
assert(dirp != NULL, "just checking");
-
- // NOTE: Linux readdir_r (on RH 6.2 and 7.2 at least) is NOT like the POSIX
- // version. Here is the doc for this function:
- // http://www.gnu.org/manual/glibc-2.2.3/html_node/libc_262.html
-
- if((status = ::readdir_r(dirp, dbuf, )) != 0) {
-errno = status;
-return NULL;
- } else
-return p;
-
-#pragma GCC diagnostic pop
+ return ::readdir(dirp);
}
inline int os::closedir(DIR *dirp) {
This looks good.
Thanks!
Someone to sponsor this please?
Do you have a sponsor yet? If not, I’ll do it.
No, I don't. I'd really appreciate if you sponsor it.
--
Michal Vala
OpenJDK QE
Red Hat Czech
Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
On 04/27/2018 06:50 PM, Kim Barrett wrote:
On Apr 27, 2018, at 4:56 AM, Michal Vala <mv...@redhat.com> wrote:
On 04/27/2018 12:55 AM, Kim Barrett wrote:
On Apr 25, 2018, at 10:51 AM, Andrew Hughes <gnu.and...@redhat.com> wrote:
On 24 April 2018 at 20:17, Kim Barrett <kim.barr...@oracle.com> wrote:
On Apr 23, 2018, at 3:51 AM, Michal Vala <mv...@redhat.com> wrote:
Hi,
following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting
updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2].
webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/
I'm asking for the review and eventually sponsorship.
The patch looks good to me.
The change to os::readdir in os_linux.inline.hpp looks fine.
I was going to suggest that rather than changing perfMemory_linux.cpp to use
os::readdir in an
unusual and platform-specific way, it should instead just call ::readdir
directly. However, neither
of those is right, and that part of the change should not be made; see
https://bugs.openjdk.java.net/browse/JDK-8134540
Much nearly duplicated code for PerfMemory support
I think, in the circumstances, Michal's solution is the best option at
this point.
8134540 looks like a more long-term change currently scheduled for 12 (is
anyone currently working on it?), whereas this fix could go into 11 and remove
a couple of unneeded memory allocations.
Looking a bit deeper, there might be some additional follow-on that could be
done, eliminating
the second argument to os::readdir.
windows: unused
bsd: freebsd also deprecated readdir_r, I think for similar reasons.
solaris: doc is clear about thread safety issue being about sharing the DIR*
aix: I haven’t looked at it, but would guess it’s similar.
In other words, don’t operate on the DIR* from multiple threads simultaneously,
and just use
::readdir on non-Windows. That would all be for another RFE though.
This also seems like a more in-depth separate change and not one that
can be performed
by any of us alone, as we don't test all these platforms. As it
stands, Michal's change affects
Linux and Linux alone, and the addition of the use of NULL will make
it clearer that moving
to an os:readdir is feasible on that platform.
I disagree, and still think the perfMemory_linux.cpp change should be
removed.
(1) The change to perfMemory_linux.cpp is entirely unnecessary to
address the problem this bug is about.
(2) It violates the (implied) protocol for os::readdir. If
Linux-specific code wants to invoke Linux-specific behavior, it should
do so by calling a Linux-specific API, not abuse an intended to be
portable API.
(3) It minorly interferes with some desirable future work. If there
were some significant benefit to doing so, I wouldn't give this much
weight, but I don't see a significant benefit.
(4) The only benefit is saving some rare short-term memory
allocations. I don't think that's worth the above costs.
Note that the Windows version of os::readdir also ignores the second
argument, but all callers provide it anyway.
I've opened a new CR for general os::readdir cleanup.
https://bugs.openjdk.java.net/browse/JDK-8202353
Ok, if we agree on os_linux.inline.hpp part, I think it should go in. Rest can
be solved in JDK-8202353, which should imho include also removal of second
argument of os::readdir, once it's unused.
For now, proposed patch looks like this:
--- old/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.498343185 +0200
+++ new/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.214342670 +0200
@@ -98,26 +98,8 @@
inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
{
-// readdir_r has been deprecated since glibc 2.24.
-// See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
- dirent* p;
- int status;
assert(dirp != NULL, "just checking");
-
- // NOTE: Linux readdir_r (on RH 6.2 and 7.2 at least) is NOT like the POSIX
- // version. Here is the doc for this function:
- // http://www.gnu.org/manual/glibc-2.2.3/html_node/libc_262.html
-
- if((status = ::readdir_r(dirp, dbuf, )) != 0) {
-errno = status;
-return NULL;
- } else
-return p;
-
-#pragma GCC diagnostic pop
+ return ::readdir(dirp);
}
inline int os::closedir(DIR *dirp) {
This looks good.
Thanks!
Someone to sponsor this please?
--
Michal Vala
OpenJDK QE
Red Hat Czech
Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
On 04/27/2018 12:55 AM, Kim Barrett wrote:
On Apr 25, 2018, at 10:51 AM, Andrew Hughes <gnu.and...@redhat.com> wrote:
On 24 April 2018 at 20:17, Kim Barrett <kim.barr...@oracle.com> wrote:
On Apr 23, 2018, at 3:51 AM, Michal Vala <mv...@redhat.com> wrote:
Hi,
following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting
updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2].
webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/
I'm asking for the review and eventually sponsorship.
The patch looks good to me.
The change to os::readdir in os_linux.inline.hpp looks fine.
I was going to suggest that rather than changing perfMemory_linux.cpp to use
os::readdir in an
unusual and platform-specific way, it should instead just call ::readdir
directly. However, neither
of those is right, and that part of the change should not be made; see
https://bugs.openjdk.java.net/browse/JDK-8134540
Much nearly duplicated code for PerfMemory support
I think, in the circumstances, Michal's solution is the best option at
this point.
8134540 looks like a more long-term change currently scheduled for 12 (is
anyone currently working on it?), whereas this fix could go into 11 and remove
a couple of unneeded memory allocations.
Looking a bit deeper, there might be some additional follow-on that could be
done, eliminating
the second argument to os::readdir.
windows: unused
bsd: freebsd also deprecated readdir_r, I think for similar reasons.
solaris: doc is clear about thread safety issue being about sharing the DIR*
aix: I haven’t looked at it, but would guess it’s similar.
In other words, don’t operate on the DIR* from multiple threads simultaneously,
and just use
::readdir on non-Windows. That would all be for another RFE though.
This also seems like a more in-depth separate change and not one that
can be performed
by any of us alone, as we don't test all these platforms. As it
stands, Michal's change affects
Linux and Linux alone, and the addition of the use of NULL will make
it clearer that moving
to an os:readdir is feasible on that platform.
I disagree, and still think the perfMemory_linux.cpp change should be
removed.
(1) The change to perfMemory_linux.cpp is entirely unnecessary to
address the problem this bug is about.
(2) It violates the (implied) protocol for os::readdir. If
Linux-specific code wants to invoke Linux-specific behavior, it should
do so by calling a Linux-specific API, not abuse an intended to be
portable API.
(3) It minorly interferes with some desirable future work. If there
were some significant benefit to doing so, I wouldn't give this much
weight, but I don't see a significant benefit.
(4) The only benefit is saving some rare short-term memory
allocations. I don't think that's worth the above costs.
Note that the Windows version of os::readdir also ignores the second
argument, but all callers provide it anyway.
I've opened a new CR for general os::readdir cleanup.
https://bugs.openjdk.java.net/browse/JDK-8202353
Ok, if we agree on os_linux.inline.hpp part, I think it should go in. Rest can
be solved in JDK-8202353, which should imho include also removal of second
argument of os::readdir, once it's unused.
For now, proposed patch looks like this:
--- old/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.498343185 +0200
+++ new/src/hotspot/os/linux/os_linux.inline.hpp2018-04-20
09:16:34.214342670 +0200
@@ -98,26 +98,8 @@
inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
{
-// readdir_r has been deprecated since glibc 2.24.
-// See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
- dirent* p;
- int status;
assert(dirp != NULL, "just checking");
-
- // NOTE: Linux readdir_r (on RH 6.2 and 7.2 at least) is NOT like the POSIX
- // version. Here is the doc for this function:
- // http://www.gnu.org/manual/glibc-2.2.3/html_node/libc_262.html
-
- if((status = ::readdir_r(dirp, dbuf, )) != 0) {
-errno = status;
-return NULL;
- } else
-return p;
-
-#pragma GCC diagnostic pop
+ return ::readdir(dirp);
}
inline int os::closedir(DIR *dirp) {
--
Michal Vala
OpenJDK QE
Red Hat Czech
Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
On 04/24/2018 09:17 PM, Kim Barrett wrote: On Apr 23, 2018, at 3:51 AM, Michal Vala <mv...@redhat.com> wrote: Hi, following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2]. webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/ I'm asking for the review and eventually sponsorship. The change to os::readdir in os_linux.inline.hpp looks fine. I was going to suggest that rather than changing perfMemory_linux.cpp to use os::readdir in an unusual and platform-specific way, it should instead just call ::readdir directly. However, neither of those is right, and that part of the change should not be made; see https://bugs.openjdk.java.net/browse/JDK-8134540 Much nearly duplicated code for PerfMemory support Looking a bit deeper, there might be some additional follow-on that could be done, eliminating the second argument to os::readdir. That's what was I first doing. However, I have no resources to test all OSes. I understand that this solution is not clear. However, until we remove the second argument and eventually merge PerfMemory code, it's useless to passing it on linux. That's why I did that cleanup. It can be done for all OSes under another bug id though. windows: unused bsd: freebsd also deprecated readdir_r, I think for similar reasons. solaris: doc is clear about thread safety issue being about sharing the DIR* aix: I haven’t looked at it, but would guess it’s similar. In other words, don’t operate on the DIR* from multiple threads simultaneously, and just use ::readdir on non-Windows. That would all be for another RFE though. -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: build pragma error with gcc 4.4.7
On 04/24/2018 06:11 AM, David Holmes wrote: On 24/04/2018 1:27 PM, Andrew Hughes wrote: On 23 April 2018 at 20:19, Kim Barrett <kim.barr...@oracle.com> wrote: On Apr 21, 2018, at 11:18 AM, Andrew Hughes <gnu.and...@redhat.com> wrote: On 19 March 2018 at 23:23, Kim Barrett <kim.barr...@oracle.com> wrote: There are also problems with the patch as provided. (1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this change is being made in support of, the warning would be disabled for all following code in any translation unit that includes this file. That doesn't seem good. No, but it's really the only solution on those compilers. We have such usage already elsewhere e.g. // Silence -Wformat-security warning for fatal() PRAGMA_DIAG_PUSH PRAGMA_FORMAT_NONLITERAL_IGNORED fatal(buf); PRAGMA_DIAG_POP return true; // silence compiler warnings } in src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp If there are other warnings, then they will picked up on newer compilers, especially when building with -Werror. I don't think it's likely people are doing development on older compilers, but rather that we have to use them to build for older platforms. I would be a lot more comfortable if the possibly do-nothing push/pop and the associated code were in a .cpp file, rather than in a .hpp file where it could affect some open-ended and unexpected set of code. Ah yes, sorry, I missed that this was a .hpp, while the others were .cpp. But I think this is moot if os::readdir can be changed to call ::readdir rather than ::readdir_r, as appears to be the case, possibly with some documentation about not sharing the DIR* among multiple threads, at least not without locking. I think so too for OpenJDK 11, but I'm reluctant to backport such a change to older JDKs. I guess if we want to continue to workaround the warning there, we'll need to move the function into the .cpp file. That seemed to be what Michal was planning to do, but hasn’t gotten back to it yet. Indeed. He has a patch that does that, that I've already reviewed. Just waiting for him to post it publicly :) He already has: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated on both the mailing lists this email has gone to. yes, here: http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-April/031746.html David ----- -- Michal Vala OpenJDK QE Red Hat Czech
RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated
Hi, following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2]. webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/ I'm asking for the review and eventually sponsorship. Thanks! [1] - http://mail.openjdk.java.net/pipermail/build-dev/2018-March/021236.html [2] - https://bugs.openjdk.java.net/browse/JDK-8179887 -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: build pragma error with gcc 4.4.7
On 03/20/2018 12:23 AM, Kim Barrett wrote: Given that there seem to be no callers of os::readdir that share the DIR* among multiple threads, it would seem easier to just replace the use of ::readdir_r with ::readdir. That seems to be the intent in the deprecation decision; use ::readdir, and either don't share a DIR* among threads, or use external locking when doing so. There are also problems with the patch as provided. (1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this change is being made in support of, the warning would be disabled for all following code in any translation unit that includes this file. That doesn't seem good. (2) The default empty definition for PRAGMA_DEPRECATED_IGNORED is missing. That means the macro can't be used in shared code, in which case having defined in (shared) compilerWarnings.hpp is questionable. Thanks for the review, these are valid comments. I'll prepare new patch replacing ::readdir_r with ::readdir. -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR: build pragma error with gcc 4.4.7
On 03/16/2018 12:36 PM, Magnus Ihse Bursie wrote: On 2018-03-16 12:05, David Holmes wrote: Hi Michal, On 16/03/2018 8:48 PM, Michal Vala wrote: Hi, I've been trying to build latest jdk with gcc 4.4.7 and I hit compile error due to pragma used in function: I don't think gcc 4.4.7 is likely to work at all. Configure will complain (but continue) if you use a gcc prior to 4.7 (very recently raised to 4.8). You can try getting past this error, but you are likely to hit more issues down the road. Do you have any specific reasons for using such an old compiler? Yes, I'm targeting RHEL6 where 4.4.7 is base gcc. With patch I've posted I'm able to compile. Configure is complaining with warning, but otherwise it's ok. Few more warnings during the build but no errors. We'd like to keep it 'compilable' in RHEL6 with code as close as possible to upstream. -- Michal Vala OpenJDK QE Red Hat Czech
RFR: build pragma error with gcc 4.4.7
Hi,
I've been trying to build latest jdk with gcc 4.4.7 and I hit compile error due
to pragma used in function:
/mnt/ramdisk/openjdk/src/hotspot/os/linux/os_linux.inline.hpp:103: error:
#pragma GCC diagnostic not allowed inside functions
I'm sending little patch that fixes the issue by wrapping whole function. I've
also created a macro for ignoring deprecated declaration inside
compilerWarnings.hpp to line up with others.
Can someone please review? If it's ok, I would also need a sponsor.
diff -r 422615764e12 src/hotspot/os/linux/os_linux.inline.hpp
--- a/src/hotspot/os/linux/os_linux.inline.hpp Thu Mar 15 14:54:10 2018 -0700
+++ b/src/hotspot/os/linux/os_linux.inline.hpp Fri Mar 16 10:50:24 2018 +0100
@@ -96,13 +96,12 @@
return ::ftruncate64(fd, length);
}
-inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
-{
// readdir_r has been deprecated since glibc 2.24.
// See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
+PRAGMA_DIAG_PUSH
+PRAGMA_DEPRECATED_IGNORED
+inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
+{
dirent* p;
int status;
assert(dirp != NULL, "just checking");
@@ -114,11 +113,11 @@
if((status = ::readdir_r(dirp, dbuf, )) != 0) {
errno = status;
return NULL;
- } else
+ } else {
return p;
-
-#pragma GCC diagnostic pop
+ }
}
+PRAGMA_DIAG_POP
inline int os::closedir(DIR *dirp) {
assert(dirp != NULL, "argument is NULL");
diff -r 422615764e12 src/hotspot/share/utilities/compilerWarnings.hpp
--- a/src/hotspot/share/utilities/compilerWarnings.hpp Thu Mar 15 14:54:10 2018
-0700
+++ b/src/hotspot/share/utilities/compilerWarnings.hpp Fri Mar 16 10:50:24 2018
+0100
@@ -48,6 +48,7 @@
#define PRAGMA_FORMAT_NONLITERAL_IGNORED _Pragma("GCC diagnostic ignored
\"-Wformat-nonliteral\"") \
_Pragma("GCC diagnostic ignored
\"-Wformat-security\"")
#define PRAGMA_FORMAT_IGNORED _Pragma("GCC diagnostic ignored \"-Wformat\"")
+#define PRAGMA_DEPRECATED_IGNORED _Pragma("GCC diagnostic ignored
\"-Wdeprecated-declarations\"")
#if defined(__clang_major__) && \
(__clang_major__ >= 4 || \
Thanks!
--
Michal Vala
OpenJDK QE
Red Hat Czech
Re: jdk10 bootcycle build linux failing
Hi Erik, I've played also with --with-num-cores. It seems like it has an effect only on first build, but no effect on bootcycle build (same as --with-memory-size). Is this behavior intended or is it a bug? Or am I doing something wrong? I haven't tried "make JOBS=x" yet. I guess it may solve the issue, but I would expect that --with-num-cores should cover this. Thanks On 03/01/2018 10:59 PM, Erik Joelsson wrote: Hello Michal, The --with-memory-size option is only used to help configure pick a more reasonable concurrency level for make. If your machine still gets overwhelmed, I would recommend manually turning down concurrency until you get it to work, using "make JOBS=X". You can check the end of the configure output for how many concurrent jobs configure set for you and work down from there. /Erik On 2018-03-01 03:36, Michal Vala wrote: Hi, I have an issue with bootcycle build. First run with external boot jdk is ok, but bootcycle phase always fails. I'm building on fedora27(local) and rhel7(VM). In case of local, build consume too much memory and kill my OS. I've tried to set `with-memory-size` but it seems like it accepts this value just for first build, but not for bootcycle. I'm building with: $ bash ./configure --disable-warnings-as-errors --with-boot-jdk=/home/tester/jdk-9.0.4 $ make bootcycle-images I suspect that build eats too much memory and OS kills some process, but I'm lack of knowledge here. Anyone else meeting similar issue? end of build log inlined here. I can provide more, but I was not able to find any useful log output: collect2: error: ld terminated with signal 9 [Killed] gmake[5]: *** [/home/tester/openjdk/build/linux-x86_64-normal-server-release/bootcycle-build/hotspot/variant-server/libjvm/gtest/libjvm.so] Error 1 gmake[5]: *** Deleting file `/home/tester/openjdk/build/linux-x86_64-normal-server-release/bootcycle-build/hotspot/variant-server/libjvm/gtest/libjvm.so' gmake[5]: *** Waiting for unfinished jobs gmake[4]: *** [hotspot-server-libs] Error 1 ERROR: Build failed for target 'product-images' in configuration 'linux-x86_64-normal-server-release' (exit code 2) gmake[4]: warning: -jN forced in submake: disabling jobserver mode. Stopping sjavac server === Output from failing command(s) repeated here === * For target hotspot_variant-server_libjvm_gtest_objs_BUILD_GTEST_LIBJVM_link: collect2: error: ld terminated with signal 9 [Killed] * All command lines available in /home/tester/openjdk/build/linux-x86_64-normal-server-release/bootcycle-build/make-support/failure-logs. === End of repeated output === No indication of failed target found. Hint: Try searching the build log for '] Error'. Hint: See doc/building.html#troubleshooting for assistance. gmake[3]: *** [main] Error 1 gmake[2]: *** [bootcycle-images] Error 2 ERROR: Build failed for target 'bootcycle-images' in configuration 'linux-x86_64-normal-server-release' (exit code 2) No indication of failed target found. Hint: Try searching the build log for '] Error'. Hint: See doc/building.html#troubleshooting for assistance. make[1]: *** [main] Error 1 make: *** [bootcycle-images] Error 2 -- Michal Vala OpenJDK QE Red Hat Czech
Re: Choosing which Visual Studio installation to use
Hi, On 02/14/2018 12:24 PM, Thomas Stüfe wrote: Hi all, On Windows, if one has multiple VS installations side by side, is it possible to choose which Visual Studio installation to use? Note that I start the configure script from within the cygwin shell. OpenJDK configure is reading VS environment properties VS100COMNTOOLS/VS120COMNTOOLS/VS140COMNTOOLS etc. One option is to set just one of them. AFAIK configure takes latest supported VS (2013 for ojdk9). Another option is to use configure parameters, but I don't know how exactly use them. Try to look to configure help $ bash ./configure --help | grep toolchain --with-tools-diralias for --with-toolchain-path for backwards --with-toolchain-path prepend these directories when searching for toolchain binaries (compilers etc) --with-toolchain-type the toolchain type (or family) to use, use '--help' --with-toolchain-version the version of the toolchain to look for, use --with-build-devkit Devkit to use for the build platform toolchain [toolchain dependent] The following toolchains are available as arguments to --with-toolchain-type. -- Michal Vala OpenJDK QE Red Hat Czech
