Skip to content

build: adjust the build to support non-Apple environments #68258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 7, 2023
Merged

build: adjust the build to support non-Apple environments #68258

merged 1 commit into from
Sep 7, 2023

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Sep 1, 2023 •
edited
Loading

Account for import libraries and the associated layout difference on
platforms (e.g. DLLs are placed in bin). This is required to enable
building the macro path on Windows.

@compnerd
Copy link
Member Author

compnerd commented Sep 1, 2023

CC: @DougGregor @rintaro @bnbarham

@compnerd compnerd mentioned this pull request Sep 4, 2023
Closed
9 tasks
@compnerd compnerd force-pushed the how-do-you-do branch 2 times, most recently from 60afbb7 to ee3273c Compare September 6, 2023 15:53
@compnerd compnerd changed the title DNM/build: identify points of incorrectness build: adjust the build to support non-Apple environments Sep 6, 2023
bnbarham
bnbarham reviewed Sep 6, 2023
View reviewed changes
cmake/modules/AddSwift.cmake
@@ -573,6 +573,9 @@ function(_add_swift_runtime_link_flags target relpath_to_lib_dir bootstrapping)
else()
message(FATAL_ERROR "Unknown BOOTSTRAPPING_MODE '${ASRLF_BOOTSTRAPPING_MODE}'")
endif()
else()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elseif(CMAKE_SYSTEM_NAME MATCHES Windows)?

tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake
@@ -161,8 +161,11 @@ function(add_sourcekit_swift_runtime_link_flags target path HAS_SWIFT_MODULES)
# Add rpath to the host Swift libraries.
file(RELATIVE_PATH relative_hostlib_path "${path}" "${SWIFTLIB_DIR}/host")
list(APPEND RPATH_LIST "$ORIGIN/${relative_hostlib_path}")
else()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMAKE_SYSTEM_NAME MATCHES Windows/SWIFT_HOST_VARIANT_SDK MATCHES WINDOWS

tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake Outdated
Comment on lines 165 to 166
target_link_directories(${target} PRIVATE
${SWIFT_PATH_TO_SWIFT_SDK}/usr/lib/swift/Windows/x86_64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in lib/CMakeLists.txt, ie.

target_link_directories(${target} PRIVATE
      ${SWIFT_PATH_TO_SWIFT_SDK}/usr/lib/swift/${SWIFT_SDK_${SWIFT_HOST_VARIANT_SDK}_LIB_SUBDIR})

tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake Outdated
endif()

else()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block was only added for SWIFT_SWIFT_PARSER. BOOTSTRAPPING= is handled above, it was copy pasted so the comments don't really make much sense. TLDR: no else here please.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed the else case otherwise it didn't build IIRC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where? The block below only runs on Darwin and needs to keep running without the else. Is the set(RPATH_LIST ${RPATH_LIST} PARENT_SCOPE) the issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

FAILED: bin/sourcekitdInProc.dll lib/sourcekitdInProc.lib
cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_dll --intdir=tools\swift\tools\SourceKit\tools\sourcekitd\bin\InProc\CMakeFiles\sourcekitdInProc.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1437~1.328\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\sourcekitdInProc.rsp  /out:bin\sourcekitdInProc.dll /implib:lib\sourcekitdInProc.lib /pdb:bin\sourcekitdInProc.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO  /DEF:"S:/b/1/tools/swift/tools/SourceKit/tools/sourcekitd/bin/InProc/sourcekitdInProc.def"  --target=x86_64-unknown-windows-msvc  && cd ."
LINK: command "C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1437~1.328\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\sourcekitdInProc.rsp /out:bin\sourcekitdInProc.dll /implib:lib\sourcekitdInProc.lib /pdb:bin\sourcekitdInProc.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO /DEF:S:/b/1/tools/swift/tools/SourceKit/tools/sourcekitd/bin/InProc/sourcekitdInProc.def --target=x86_64-unknown-windows-msvc /MANIFEST /MANIFESTFILE:bin\sourcekitdInProc.dll.manifest" failed (exit code 1104) with the following output:
LINK : warning LNK4044: unrecognized option '/-target=x86_64-unknown-windows-msvc'; ignored
LINK : fatal error LNK1104: cannot open file 'swift_Concurrency.lib'

tools/driver/CMakeLists.txt
@@ -5,6 +5,8 @@
# parser is built in.
function(add_swift_parser_link_libraries target)
Copy link
Contributor

@bnbarham bnbarham Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I've missed this function in the past.

    target_link_libraries(${target}
                          PRIVATE swiftCore)

I have no idea how all the other tools are working, but we should fix this later (don't worry about it for this PR/Windows, we'll fix up on main after). This should be being added by _add_swift_runtime_link_flags from add_swift_host_tool but apparently isn't. Maybe autolinking is kicking in on Darwin?

Anyway, TLDR: target_link_directories is already done in _add_swift_runtime_link_flags, so this change shouldn't be needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was needed as otherwise I ended up with references to libraries that are not available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how that could be, are you sure that wasn't before updating _add_swift_runtime_link_flags? add_swift_host_tool has:

  # Once the new Swift parser is linked in, every host tool has Swift modules.
  if (SWIFT_SWIFT_PARSER AND NOT ASHT_DOES_NOT_USE_SWIFT)
    set(ASHT_HAS_SWIFT_MODULES ON)
  endif()

  if (ASHT_HAS_SWIFT_MODULES)
    _add_swift_runtime_link_flags(${executable} "../lib" "${ASHT_BOOTSTRAPPING}")
  endif()

And you've added target_link_libraries there.

@compnerd compnerd force-pushed the how-do-you-do branch 2 times, most recently from 45f1d78 to 47c56bd Compare September 6, 2023 18:45
@compnerd
Copy link
Member Author

compnerd commented Sep 6, 2023

@swift-ci please test

@compnerd
build: adjust the build to support non-Apple environments
00d4879 
Account for import libraries and the associated layout difference on
platforms (e.g. DLLs are placed in `bin`).  This is required to enable
building the macro path on Windows.
@compnerd
Copy link
Member Author

compnerd commented Sep 6, 2023

@swift-ci please test

bnbarham
bnbarham approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could still clean up a bit, but we can do that with all the other cleanup we need to do for Linux anyway :).

@compnerd
Copy link
Member Author

compnerd commented Sep 7, 2023

Going to merge this for now to unblock macros on main at the very least.

@compnerd compnerd merged commit b6764c3 into swiftlang:main Sep 7, 2023
@compnerd compnerd deleted the how-do-you-do branch September 7, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnbarham bnbarham bnbarham approved these changes

@ahoppen ahoppen Awaiting requested review from ahoppen

@rintaro rintaro Awaiting requested review from rintaro rintaro is a code owner

@hamishknight hamishknight Awaiting requested review from hamishknight hamishknight is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@compnerd @bnbarham