Skip to content

Rebase swift's component system ontop of LLVM's component system. #20910

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

Conversation

gottesmm
Copy link
Contributor

LLVM's component system works by all installable things having individual
install-* targets. Swift's component system is intended to categorize
installable things with high level categories like a package maintainer would
want (e.x. compiler, editor-integration, etc). What this commit does is:

  1. If a swift component is supposed to be installed, rather than just setting a
    variable, we also define an install-${swift_component} custom target.

  2. When we call swift_install_in_category, we now require a 2nd argument which
    is a custom target install name. This must be a unique name since we will define
    a rule based off of the provided name (install-${target_install_name}). If one
    is trying to install a target, please use the target name as the target install
    name. If one is installing a file or directory, this must just be a unique name
    among all targets.

This allows for us to plug into LLVM's cmake install component system without
giving up the nice high level components for swift.

@gottesmm gottesmm force-pushed the swift-component-ontop-llvm-component branch 3 times, most recently from 0e4fee9 to a4ba363 Compare November 30, 2018 08:27
LLVM's component system works by all installable things having individual
install-* targets. Swift's component system is intended to categorize
installable things with high level categories like a package maintainer would
want (e.x. compiler, editor-integration, etc). What this commit does is:

1. If a swift component is supposed to be installed, rather than just setting a
variable, we also define an install-${swift_component} custom target.

2. When we call swift_install_in_category, we now require a 2nd argument which
is a custom target install name. This must be a unique name since we will define
a rule based off of the provided name (install-${target_install_name}). If one
is trying to install a target, please use the target name as the target install
name. If one is installing a file or directory, this must just be a unique name
among all targets.

This allows for us to plug into LLVM's cmake install component system without
giving up the nice high level components for swift.
@gottesmm gottesmm force-pushed the swift-component-ontop-llvm-component branch from a4ba363 to d8bd9b1 Compare November 30, 2018 08:29
@gottesmm
Copy link
Contributor Author

@swift-ci build toolchain

FILES "${UNIVERSAL_LIBRARY_NAME}"
DESTINATION "lib${LLVM_LIBDIR_SUFFIX}/${resource_dir}/${resource_dir_sdk_subdir}"
PERMISSIONS ${file_permissions})
if(sdk STREQUAL WINDOWS)
foreach(arch ${SWIFT_SDK_WINDOWS_ARCHITECTURES})
if(TARGET ${name}-windows-${arch}_IMPLIB)
get_target_property(import_library ${name}-windows-${arch}_IMPLIB IMPORTED_LOCATION)
swift_install_in_component(${SWIFTLIB_INSTALL_IN_COMPONENT}
swift_install_in_component(${SWIFTLIB_INSTALL_IN_COMPONENT} ${import_library}
Copy link
Member

Choose a reason for hiding this comment

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

This really should be part of the component itself. The import library is needed to be built against, and normally install(TARGET) will do the work for you.

@@ -348,7 +348,7 @@ function(_compile_swift_files
list(APPEND module_outputs "${interface_file}")
endif()

swift_install_in_component("${SWIFTFILE_INSTALL_IN_COMPONENT}"
swift_install_in_component("${SWIFTFILE_INSTALL_IN_COMPONENT}" ${module_base}-module
Copy link
Member

Choose a reason for hiding this comment

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

I think that it would be better to install the module with the module itself. You cannot use the module for development otherwise.

@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

A huge amount of stuff has changed in the mean time. Do you still want this @gottesmm?

@CodaFi CodaFi closed this Nov 18, 2019
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.

3 participants