Re: RFR: 8295231: Move all linking of native libraries to make [v6]
On Mon, 17 Oct 2022 17:37:11 GMT, Erik Joelsson wrote: > I believe this is part of the effort for > https://bugs.openjdk.org/browse/JDK-8288293. Agree. I'd prefer to have a different description of the bug though to make it clear that this is necessary for decoupling a compiler and an OS. - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v6]
On Mon, 17 Oct 2022 17:16:21 GMT, Alexey Semenyuk wrote: > The change looks harmless. Howevere I don't understand how searching for the > standard Windows libs can then become frustrating. I believe this is part of the effort for https://bugs.openjdk.org/browse/JDK-8288293. - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v6]
On Mon, 17 Oct 2022 14:41:06 GMT, Julian Waters wrote: >> Some external libraries required by native code are linked via linker >> comments embedded in pragmas. Searching for which libraries are linked can >> then become frustrating and confusing since they may be included in an >> obscure place, and for all relevant compilers there is no difference between >> specifying them from make and in a source file. The easiest solution is to >> just always link them from make and remove any source level linkage. > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Formatting Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v6]
On Mon, 17 Oct 2022 14:41:06 GMT, Julian Waters wrote: >> Some external libraries required by native code are linked via linker >> comments embedded in pragmas. Searching for which libraries are linked can >> then become frustrating and confusing since they may be included in an >> obscure place, and for all relevant compilers there is no difference between >> specifying them from make and in a source file. The easiest solution is to >> just always link them from make and remove any source level linkage. > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Formatting The change looks harmless. Howevere I don't understand how searching for the standard Windows libs can then become frustrating. - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v6]
> Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Formatting - Changes: - all: https://git.openjdk.org/jdk/pull/10633/files - new: https://git.openjdk.org/jdk/pull/10633/files/0779d1fa..4eb2eb7b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10633=05 - incr: https://webrevs.openjdk.org/?repo=jdk=10633=04-05 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v4]
On Mon, 17 Oct 2022 14:31:46 GMT, Julian Waters wrote: >> Some external libraries required by native code are linked via linker >> comments embedded in pragmas. Searching for which libraries are linked can >> then become frustrating and confusing since they may be included in an >> obscure place, and for all relevant compilers there is no difference between >> specifying them from make and in a source file. The easiest solution is to >> just always link them from make and remove any source level linkage. > > Julian Waters has updated the pull request incrementally with five additional > commits since the last revision: > > - Update WinSysInfo.cpp > - Update WinApp.cpp > - Update MsiUtils.cpp > - Update MsiDb.cpp > - Update MsiCA.cpp Thanks all for the reviews, I placed comments where the pragmas used to be detailing the library required by the source file to address David's concerns - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v5]
> Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Update WindowsRegistry.cpp - Changes: - all: https://git.openjdk.org/jdk/pull/10633/files - new: https://git.openjdk.org/jdk/pull/10633/files/8afaf85d..0779d1fa Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10633=04 - incr: https://webrevs.openjdk.org/?repo=jdk=10633=03-04 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v4]
> Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Julian Waters has updated the pull request incrementally with five additional commits since the last revision: - Update WinSysInfo.cpp - Update WinApp.cpp - Update MsiUtils.cpp - Update MsiDb.cpp - Update MsiCA.cpp - Changes: - all: https://git.openjdk.org/jdk/pull/10633/files - new: https://git.openjdk.org/jdk/pull/10633/files/fe15f907..8afaf85d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10633=03 - incr: https://webrevs.openjdk.org/?repo=jdk=10633=02-03 Stats: 12 lines in 5 files changed: 11 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v3]
> Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Update Guid.cpp - Changes: - all: https://git.openjdk.org/jdk/pull/10633/files - new: https://git.openjdk.org/jdk/pull/10633/files/54e6ec10..fe15f907 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10633=02 - incr: https://webrevs.openjdk.org/?repo=jdk=10633=01-02 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v3]
On Sun, 16 Oct 2022 13:18:14 GMT, Alan Bateman wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update Guid.cpp > > src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 38: > >> 36: >> 37: #include >> 38: #pragma comment(lib, "Mswsock.lib") > > I think this came about with one of the early Microsoft contributions to have > transferTo optionally use TransmitFile on Windows. This created the > dependency on Mswsock. It's not clear why a pragma was used though. I believe it may have had to do with explicitly showing the dependency as David suggests, I added a comment explaining this just in case - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make [v2]
> Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Julian Waters has updated the pull request incrementally with two additional commits since the last revision: - user32 - Mswsock.lib requirement - Changes: - all: https://git.openjdk.org/jdk/pull/10633/files - new: https://git.openjdk.org/jdk/pull/10633/files/cfa80528..54e6ec10 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10633=01 - incr: https://webrevs.openjdk.org/?repo=jdk=10633=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make
On Mon, 17 Oct 2022 09:24:58 GMT, Magnus Ihse Bursie wrote: > @TheShermanTanker Question: is this a Windows-specific thing, or are there > pragma-loaded libraries for other compilers as well? To my knowledge only Visual C++ has the ability to perform linking through pragmas, the comment pragma works by leaving a linker comment in the object file (hence the name), meaning this only works with the Visual C++ linker, so while clang does support this pragma with clang-cl, for the use cases in the JDK it wouldn't matter. gcc does not have an equivalent pragma (or provide support for linking from inside source files at all, for that matter) > If the methods are equivalent, I prefer linking via make file. There isn't any difference between the 2 unless the library is tampered with by `-nodefaultlib`, but I can only find that specified in https://github.com/openjdk/jdk/blob/a033aa5a3d9c63d72d11af218b9896b037fbd8de/make/autoconf/flags-other.m4#L38 and https://github.com/openjdk/jdk/blob/392f35df4be1a9a8d7a67a25ae01230c7dd060ac/make/autoconf/lib-hsdis.m4#L45, neither of which have an effect on the libraries in this changeset - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make
On Mon, 10 Oct 2022 14:15:37 GMT, Julian Waters wrote: > Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. Marked as reviewed by ihse (Reviewer). Wow. I did not even know this was possible. Thank you for fixing this! I would have been mighty surprised if I were to learn that a library has more dependencies than the one in the makefile. @dholmes-ora The point is that we need to be consistent. If we would go that route, then *all* libraries should be loaded by pragmas. That could of course be an alternative, but I really see no upside to it. To do it like we currently do, load 99% of the libraries via make files, and then have few scattered pragmas, that's just bad. @TheShermanTanker Question: is this a Windows-specific thing, or are there pragma-loaded libraries for other compilers as well? - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make
On Mon, 10 Oct 2022 14:15:37 GMT, Julian Waters wrote: > Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. I like this change. I am pretty sure not many know about this feature at all, we don't have that many knowledgeable Windows developers. If the methods are equivalent, I prefer linking via make file. Pinging @magicus, maybe he can chime in. Cheers, Thomas - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make
On Mon, 10 Oct 2022 14:15:37 GMT, Julian Waters wrote: > Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. I don't agree with the justification here. This seems very windows specific but I like the idea that the source code tracks its own library dependencies. If I am writing some windows code that uses a particular Windows API which in turn requires a specific windows library, then these pragma comments seem an ideal way to express that dependency. This has the advantage that (a) the developer doesn't require detailed knowledge of the build system to make things work; and (b) there is more chance that if the code is later removed then removing the linking of the library will not get overlooked. YMMV. - PR: https://git.openjdk.org/jdk/pull/10633
Re: RFR: 8295231: Move all linking of native libraries to make
On Mon, 10 Oct 2022 14:15:37 GMT, Julian Waters wrote: > Some external libraries required by native code are linked via linker > comments embedded in pragmas. Searching for which libraries are linked can > then become frustrating and confusing since they may be included in an > obscure place, and for all relevant compilers there is no difference between > specifying them from make and in a source file. The easiest solution is to > just always link them from make and remove any source level linkage. src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 38: > 36: > 37: #include > 38: #pragma comment(lib, "Mswsock.lib") I think this came about with one of the early Microsoft contributions to have transferTo optionally use TransmitFile on Windows. This create the dependency on Mswsock. - PR: https://git.openjdk.org/jdk/pull/10633
RFR: 8295231: Move all linking of native libraries to make
Some external libraries required by native code are linked via linker comments embedded in pragmas. Searching for which libraries are linked can then become frustrating and confusing since they may be included in an obscure place, and for all relevant compilers there is no difference between specifying them from make and in a source file. The easiest solution is to just always link them from make and remove any source level linkage. - Commit messages: - Commit remaining libraries - Merge branch 'openjdk:master' into patch-2 - Comment change, force link failures to determine libraries to include in make - Update WinSysInfo.cpp - Update WinFileUtils.cpp - Update WinApp.cpp - Update MsiUtils.cpp - Update MsiDb.cpp - Update MsiCA.cpp - Update Guid.cpp - ... and 12 more: https://git.openjdk.org/jdk/compare/0043d58c...cfa80528 Changes: https://git.openjdk.org/jdk/pull/10633/files Webrev: https://webrevs.openjdk.org/?repo=jdk=10633=00 Issue: https://bugs.openjdk.org/browse/JDK-8295231 Stats: 35 lines in 14 files changed: 3 ins; 22 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/10633.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10633/head:pull/10633 PR: https://git.openjdk.org/jdk/pull/10633
