Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Thu, 12 May 2022 04:25:24 GMT, David Holmes wrote: >> This change seem to have made this group of declarations obsolete: >> `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow >> the [link]( >> https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). >> Does anyone have plans to drop those? Have any bugs been filed for that? If >> not, I'll attend to that myself. > > @mkartashev all references to WINVER in the AWT code seem obsolete. Possibly > most of the IS_WINxxx usages could also be replaced. @dholmes-ora @MBaesken Thank you! Filed [JDK-8286634](https://bugs.openjdk.java.net/browse/JDK-8286634). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 11 May 2022 16:00:32 GMT, Maxim Kartashev wrote: > This change seem to have made this group of declarations obsolete: > `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow > the > [link](https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). > Does anyone have plans to drop those? Have any bugs been filed for that? If > not, I'll attend to that myself. Hi Maxim, no plans from me, so feel free to do further cleanups. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 11 May 2022 16:00:32 GMT, Maxim Kartashev wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> adjust API level to Windows 8 for security.cpp and do some cleanup > > This change seem to have made this group of declarations obsolete: > `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow > the [link]( > https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). > Does anyone have plans to drop those? Have any bugs been filed for that? If > not, I'll attend to that myself. @mkartashev all references to WINVER in the AWT code seem obsolete. Possibly most of the IS_WINxxx usages could also be replaced. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This change seem to have made this group of declarations obsolete: `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow the [link]( https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). Does anyone have plans to drop those? Have any bugs been filed for that? If not, I'll attend to that myself. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Looks good, thanks for doing this cleanup. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Hi David, thanks for the review ! - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This seems okay to me in this form. I agree that explicitly setting this is better than unknowingly using API's that might not be available on supported platforms. Thanks. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 09:08:28 GMT, Matthias Baesken wrote:
> Does this mean that not setting _WIN32_WINNT means :any API is allowed" ?
Hi David , I did one more try with my current setup (VS2017 on a Win10
notebook). I did not set _WIN32_WINNT.
My little test program
#include
#include
int main() {
#ifdef _WIN32_WINNT
printf("_WIN32_WINNT is defined .\n");
#if (_WIN32_WINNT == 0x0600)
printf("Vista API setting\n");
#endif
#if (_WIN32_WINNT == 0x0601)
printf("Win 7 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0602)
printf("Win 8 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0603)
printf("Win 8.1 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0A00)
printf("Win 10 API setting\n");
#endif
#endif
return 0;
}
shows me
_WIN32_WINNT is defined .
Win 10 API setting
So I think with our current compilers in use like VS2017 / VS2019 we allow
Win10 APIs in most of our code except a few places where we set _WIN32_WINNT
and go back to some mixture of older APIs.
Not sure if this is a good thing, we could break for example Win 8.1/Win2012
compatibility easily this way.
-
PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:34:43 GMT, David Holmes wrote: > I'm confused. > `src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp` doesn't set > _WIN32_WINNT so how is that later API being enabled? Does this mean that not > setting _WIN32_WINNT means :any API is allowed" ? I found this info here https://stackoverflow.com/questions/37668692/visual-studio-setting-winver-win32-winnt-to-windows-8-on-windows-7 "VS 2012 uses the Windows 8.0 SDK which defaults to _WIN32_WINNT=0x0602 (Windows 8). VS 2013 / 2015 uses the Windows 8.1 SDK which defaults to _WIN32_WINNT=0x0603 (Windows 8.1). If you use VS 2015 and the Windows 10 SDK, it defaults to _WIN32_WINNT=0x0A00 (Windows 10)." So it seems you get some more or less recent default when working with a not too old compiler/SDK. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup I'm confused. `src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp` doesn't set _WIN32_WINNT so how is that later API being enabled? Does this mean that not setting _WIN32_WINNT means :any API is allowed" ? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
> Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust API level to Windows 8 for security.cpp and do some cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/8428/files - new: https://git.openjdk.java.net/jdk/pull/8428/files/23b63c5b..721528c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8428=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8428=02-03 Stats: 31 lines in 7 files changed: 1 ins; 26 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 13:27:41 GMT, David Holmes wrote: > I agree with Erik - the source locations need to be modified to not define > it. If we want to keep a record perhaps add an assertion that the value is > what was expected? > > I still feel we have a disconnect between this and an actual check for what > the build and runtime platform version is ... > > and it isn't at all clear how someone using an API only in a later Windows > version, and so setting _WIN32_WINNT to a higher value, will know that this > is defined down in the build files ? Hi David, I agree the other locations as long as they are not 3rd party code should be cleaned up. Someone using an API that is only available in later Windows versions would get a redefinition warning as long as no undefine is done. But we have to set _WIN32_WINNT anyway to a higher value (windows8, I think that's 0x0602) to compile src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp . - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 07:10:58 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Copyright year in wepoll.c Interesting fact : we run now into this compile error : d:\a\jdk\jdk\jdk\src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp(1262): error C2065: 'NCRYPT_CIPHER_KEY_BLOB': undeclared identifier d:\a\jdk\jdk\jdk\src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp(1280): error C2065: 'NCRYPT_PROTECTED_KEY_BLOB': undeclared identifier Reason seems that already for some time ( at least OpenJDK 11 (!) ) we have an implicit minimum requirement of Windows 8 / Windows 2012 APIs for this code, so enforcing Win 7 is too old : https://docs.microsoft.com/en-us/windows/win32/api/ncrypt/nf-ncrypt-ncryptexportkey NCRYPT_PROTECTED_KEY_BLOB Export a protected key in a NCRYPT_KEY_BLOB_HEADER structure. Windows 8 and Windows Server 2012: Support for this value begins. NCRYPT_CIPHER_KEY_BLOB Export a cipher key in a NCRYPT_KEY_BLOB_HEADER structure. Windows 8 and Windows Server 2012: Support for this value begins. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 08:23:52 GMT, Alan Bateman wrote: >> Hi Alan, I agree (thats why I did not change the define in harfbuzz, but I >> missed that wepoll.c is 3rd party code as well). > > I assume you can revert the update to the copyright header in the latest > version as there aren't any changes to this source file now. Yes sure I did it . - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 07:10:58 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Copyright year in wepoll.c I agree with Erik - the source locations need to be modified to not define it. If we want to keep a record perhaps add an assertion that the value is what was expected? I still feel we have a disconnect between this and an actual check for what the build and runtime platform version is ... and it isn't at all clear how someone using an API only in a later Windows version, and so setting _WIN32_WINNT to a higher value, will know that this is defined down in the build files ? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Tue, 3 May 2022 07:10:58 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Copyright year in wepoll.c If we define this centrally using compiler flags, then we should not also update each location in the source. Those need to either be removed or left alone. In the cases where the source definition is guarded with an ifndef and there is a comment explaining why a particular version of windows is required, keeping it around could make sense. But on the other hand, since those defines are overridden using flags, they are likely to bit rot over time so we might as well just remove all of them. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
On Thu, 28 Apr 2022 07:13:24 GMT, Matthias Baesken wrote: >> src/java.base/windows/native/libnio/ch/wepoll.c line 159: >> >>> 157: >>> 158: #undef _WIN32_WINNT >>> 159: #define _WIN32_WINNT 0x0601 >> >> This is 3rd party code and would prefer not change it if possible. > > Hi Alan, I agree (thats why I did not change the define in harfbuzz, but I > missed that wepoll.c is 3rd party code as well). I assume you can revert the update to the copyright header in the latest version as there aren't any changes to this source file now. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v3]
> Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust Copyright year in wepoll.c - Changes: - all: https://git.openjdk.java.net/jdk/pull/8428/files - new: https://git.openjdk.java.net/jdk/pull/8428/files/dff223c5..23b63c5b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8428=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8428=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v2]
> Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: set _WIN32_WINNT=0x0601 centrally in flags-cflags.m4 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8428/files - new: https://git.openjdk.java.net/jdk/pull/8428/files/aef2486f..dff223c5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8428=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8428=00-01 Stats: 4 lines in 2 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Fri, 29 Apr 2022 05:09:35 GMT, David Holmes wrote: > That only seems to be half of the issue though. If we are defining > _WIN32_WINNT=0x0601 because the minimum required OS API support level is > Windows 7, then don't we need a check that the build platform is also at > least Windows 7? Hi David, on older OS than Windows 7 we wouldn't be able to build OJDK anyway currently. Our oldest VS we still support (see toolchain_microsoft.m4) is VS2017. VS2017 has the following system requirements https://docs.microsoft.com/en-us/visualstudio/releases/2017/vs2017-system-requirements-vs see supported OS , there the oldest supported OS is Windows Server 2012 R2 and Windows 7 SP1. So on older than Win7 we would not even have a compiler anymore that passes configure. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). That only seems to be half of the issue though. If we are defining _WIN32_WINNT=0x0601 because the minimum required OS API support level is Windows 7, then don't we need a check that the build platform is also at least Windows 7? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Thu, 28 Apr 2022 07:16:30 GMT, Matthias Baesken wrote: > Hi Erik/David/Phil, we already have a good central place where we set the > definition of WIN32_LEAN_AND_MEAN > > autoconf/flags-cflags.m4:460: ALWAYS_DEFINES_JDK="-DWIN32_LEAN_AND_MEAN > -D_CRT_SECURE_NO_DEPRECATE autoconf/flags-cflags.m4:462: > ALWAYS_DEFINES_JVM="-DNOMINMAX -DWIN32_LEAN_AND_MEAN" > > should we add there the _WIN32_WINNT=0x0601 define (and remove the differing > defines instead at the other places) ? (defines of _WIN32_WINNT in 3rd party > code would stay like wepoll.c and harfbuzz). Seems reasonable to me at least. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Hi Erik/David/Phil, we already have a good central place where we set the definition of WIN32_LEAN_AND_MEAN autoconf/flags-cflags.m4:460:ALWAYS_DEFINES_JDK="-DWIN32_LEAN_AND_MEAN -D_CRT_SECURE_NO_DEPRECATE \ autoconf/flags-cflags.m4:462:ALWAYS_DEFINES_JVM="-DNOMINMAX -DWIN32_LEAN_AND_MEAN" should we add there the _WIN32_WINNT=0x0601 define (and remove the differing defines instead at the other places) ? (defines of _WIN32_WINNT in 3rd party code would stay like wepoll.c and harfbuzz). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 15:10:51 GMT, Alan Bateman wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > src/java.base/windows/native/libnio/ch/wepoll.c line 159: > >> 157: >> 158: #undef _WIN32_WINNT >> 159: #define _WIN32_WINNT 0x0601 > > This is 3rd party code and would prefer not change it if possible. Hi Alan, I agree (thats why I did not change the define in harfbuzz, but I missed that wepoll.c is 3rd party code as well). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). There is obviously a lot of history here and different parts of the codebase had hit different minimum OS version requirements at different times. While at one time we would have had to cater for building on systems with and without a given API those days are long gone for the code being touched here (AFAICS). As Erik states we should be able to set a minimum _WIN32_WINNT_ value needed to build all the codebase and simply have that checked and set at configure time. The code itself would not need to define _WIN32_WINNT. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Requiring different windows versions in different parts of the product doesn't make much sense, but I think it's even worse trying to keep all these different locations in sync. At least most of these have a comment explaining why that particular Windows version is required there. Changing these values invalidates those comments. If we are to do something about this, then we need to define this macro in a central location, and verify the value in configure. Then we would provide it either on compile command lines, or in a globally included config.h. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). This is OK by me, but I wonder if the build folks might want to think about this and whether it should be centralised somehow since it seems odd to mandate different versions of windows in different places. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings
On Wed, 27 Apr 2022 14:57:41 GMT, Matthias Baesken wrote: > Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). src/java.base/windows/native/libnio/ch/wepoll.c line 159: > 157: > 158: #undef _WIN32_WINNT > 159: #define _WIN32_WINNT 0x0601 This is 3rd party code and would prefer not change it if possible. - PR: https://git.openjdk.java.net/jdk/pull/8428
RFR: JDK-8285730: unify _WIN32_WINNT settings
Currently we set _WIN32_WINNT at various places in the codebase; this is used to target a minimum Windows version we want to support. See also for more detailled information : https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt Macros for Conditional Declarations "Certain functions that depend on a particular version of Windows are declared using conditional code. This enables you to use the compiler to detect whether your application uses functions that are not supported on its target version(s) of Windows." However currently we have quite a lot of differing settings of _WIN32_WINNT in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible would make sense because we have this setting already at java_props_md.c (so targeting older Windows versions at other places is most likely not useful). - Commit messages: - JDK-8285730 Changes: https://git.openjdk.java.net/jdk/pull/8428/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8428=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285730 Stats: 14 lines in 7 files changed: 1 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
