Skip to content

[build] Fix bootstrapping builds that disable back-deployment #73619

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.

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

Merged
merged 1 commit into from
May 22, 2024

Conversation

egorzhdan
Copy link
Contributor

This fixes a build that passes -D SWIFT_STDLIB_SUPPORT_BACK_DEPLOYMENT=NO and -D BOOTSTRAPPING_MODE=BOOTSTRAPPING-WITH-HOSTLIBS.

Previously a few CMake errors would be emitted:

CMake Error at cmake/modules/AddSwift.cmake:478 (get_property):
  get_property could not find TARGET HostCompatibilityLibs.  Perhaps it has
  not yet been created.

CMake Error at cmake/modules/AddSwift.cmake:527 (add_dependencies):
  add_dependencies called with incorrect number of arguments
Call Stack (most recent call first):
  cmake/modules/AddSwift.cmake:969 (_add_swift_runtime_link_flags)
  tools/driver/CMakeLists.txt:23 (add_swift_host_tool)

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@edymtt edymtt left a comment

Choose a reason for hiding this comment

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

Thanks for catching this (and sorry for affecting your workflow) -- looks like my comment in #73383 has aged poorly.

LGTM with a couple of optional remarks.

# only if needed
target_link_directories(${target} PRIVATE "${compatibility_libs_path}")
add_dependencies(${target} ${compatibility_libs})
if(NOT "${compatibility_libs_path}" STREQUAL "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: there is a more idiomatic way to express this

Suggested change
if(NOT "${compatibility_libs_path}" STREQUAL "")
if(compatibility_libs_path)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have cycles, could you surrounds the lines for the BOOTSTRAPPING case (lines 544-545 after the changes) with this very check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, fixed this, thanks!

This fixes a build that passes `-D SWIFT_STDLIB_SUPPORT_BACK_DEPLOYMENT=NO` and `-D BOOTSTRAPPING_MODE=BOOTSTRAPPING-WITH-HOSTLIBS`.

Previously a few CMake errors would be emitted:
```
CMake Error at cmake/modules/AddSwift.cmake:478 (get_property):
  get_property could not find TARGET HostCompatibilityLibs.  Perhaps it has
  not yet been created.

CMake Error at cmake/modules/AddSwift.cmake:527 (add_dependencies):
  add_dependencies called with incorrect number of arguments
Call Stack (most recent call first):
  cmake/modules/AddSwift.cmake:969 (_add_swift_runtime_link_flags)
  tools/driver/CMakeLists.txt:23 (add_swift_host_tool)
```
@egorzhdan egorzhdan force-pushed the egorzhdan/bootstrapping-without-backdeployment branch from 0edff62 to 421ee98 Compare May 20, 2024 13:32
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan egorzhdan merged commit 1d54816 into main May 22, 2024
5 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/bootstrapping-without-backdeployment branch May 22, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants