Introduce a flag to control the path to Swift headers installation.

[stevapple]

The Swift compiler regards libdispatch headers as runtime resources, located relative to the compiler executable. If we want to distribute a standalone SDK, library and header install path should diverge.

This PR introduces a flag to manually specify the path to Swift runtime resource directory, which is used to install headers for Swift. Notice that the flag is only effective under ENABLE_SWIFT, and the current behavior is preserved unless the flag is provided.

[edymtt]

The change looks reasonable to me -- I would have some feedback to make this change more readable and close to where this is used.

[edymtt]

We could use SWIFT_RUNTIME_RESOURCE_INSTALL_DIR in this line as well

Suggested change

	set(INSTALL_TARGET_DIR "${CMAKE_INSTALL_LIBDIR}/swift$<$<NOT:$<BOOL:${BUILD_SHARED_LIBS}>>:_static>/$<LOWER_CASE:${CMAKE_SYSTEM_NAME}>" CACHE PATH "Path where the libraries will be installed")
	set(INSTALL_TARGET_DIR "${SWIFT_RUNTIME_RESOURCE_INSTALL_DIR}/$<LOWER_CASE:${CMAKE_SYSTEM_NAME}>" CACHE PATH "Path where the libraries will be installed")

[stevapple]

This is intentional — the change proposes to diverge INSTALL_TARGET_DIR from INSTALL_*_HEADERS_DIR, which falls separately into SDK and toolchain part. INSTALL_TARGET_DIR should always be where it is now.

[edymtt]

What do you think about folding the initialization in line 130-132 into the set invocation, and moving such invocation above line 295?
This way the declaration is still protected by the ENABLE_SWIFT condition, and it will be near its usage.

[edymtt]

Suggested change

	set(SWIFT_RUNTIME_RESOURCE_INSTALL_DIR "" CACHE PATH "path to install swift compiler runtime resources")
	set(SWIFT_RUNTIME_RESOURCE_INSTALL_DIR "${CMAKE_INSTALL_LIBDIR}/swift$<$<NOT:$<BOOL:${BUILD_SHARED_LIBS}>>:_static>" CACHE PATH "path to install swift compiler runtime resources")

[stevapple]

Maybe we can replace L130-132 with the line? I was hesitant about placing it into L295 because the install paths set there are identical with/without ENABLE_SWIFT, and SWIFT_RUNTIME_RESOURCE_INSTALL_DIR is just used to compute those real INSTALL_DIRs, which I think should be defined beforehand.

[gottesmm]

Hey @stevapple. I just want to understand what you are trying to do here. A few questions:

1. Are you trying to install the dispatch headers into the swift install dir instead of the dispatch install dir? Wouldn't this allow for the dispatch installation to potentially stomp inadvertently on swift install dir contents? Couldn't you just script fishing this out of the install dir (maybe using mv -n)? Or am I misunderstanding?

2. Are you trying to in this hypothetical standalone SDK make the dispatch API public API?

[stevapple]

Hello @gottesmm, I’ll try to elaborate on what this pitch is going to solve:

When we build a Swift toolchain, a typical build doesn’t split the SDK off the toolchain, so the old Dispatch build script simply installs both the libraries and headers into the same /usr/lib/swift directory. However, these two are actually different. The Swift compiler treats Dispatch headers as runtime resources that always lie beside the compiler (toolchain), while the libraries fall into SDK components that can be moved around and separately packaged.

This causes some troubles for the Windows build. The Dispatch headers installed in SDK cannot be directly used by the compiler, so we now collect them from $(SDKROOT)/usr/lib/swift and install them into $(SDKROOT)/usr/include which makes them public. This will accidentally leak Dispatch and other stuffs to Clang, which is, at least so far, not the case for Linux.

Such workaround may cause some other problem. Since it changed the SDK/toolchain layout, the toolchain tests may not correctly reflect the actual case for users. Layout changes have already cause some unexpected regressions with other compiler resources, and I’m not sure if we will meet it in Dispatch.

As for answering your questions:
1.

Are you trying to install the dispatch headers into the swift install dir instead of the dispatch install dir?

Yes, and I believe that’s the real design. Headers and libraries are separate parts that happens to have the same install dir suffix.

Wouldn't this allow for the dispatch installation to potentially stomp inadvertently on swift install dir contents?

With the current implementation where we don’t correctly separate toolchain and SDK, there‘s larger possibility we make such mistake because we’re installing everything into swift install dir.

Couldn't you just script fishing this out of the install dir (maybe using mv -n)?

Of course yes, but this will diverge build and install layout, which may cause regressions that can’t be detected with tests.

Are you trying to in this hypothetical standalone SDK make the dispatch API public API?

Actually that’s the reverse side — by this fix we’ll be able to hide the unintentionally public Dispatch API on Windows.