-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Stop installing obsolete swift-build-tool executable #84364
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
@swift-ci build toolchain |
It's possible this change is too narrow - however, we do still need to build llbuild here to support the SwiftPM bootstrap, we just don't need to install anything it produces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's only used for bootstrapping, do we need llbuild in build-script-impl
at all? It looks like SwiftPM's bootstrap script already builds llbuild, or at least has logic for building it:
def build_llbuild(args):
"""Builds LLBuild using CMake."""
logging.info("Building llbuild")
# Set where we are going to build llbuild for future steps to find it
args.build_dirs["llbuild"] = os.path.join(args.target_dir, "llbuild")
api_dir = os.path.join(args.build_dirs["llbuild"], ".cmake/api/v1/query")
mkdir_p(api_dir)
call(["touch", "codemodel-v2"], cwd=api_dir, verbose=args.verbose)
flags = [
"-DCMAKE_C_COMPILER:=%s" % (args.clang_path),
"-DCMAKE_CXX_COMPILER:=%s" % (args.clangxx_path),
"-DCMAKE_AR:PATH=%s" % (args.ar_path),
"-DCMAKE_RANLIB:PATH=%s" % (args.ranlib_path),
"-DLLBUILD_SUPPORT_BINDINGS:=Swift",
]
cmake_env = []
if platform.system() == 'Darwin':
# On Darwin, make sure we're building for the host architecture.
flags.append("-DCMAKE_OSX_ARCHITECTURES:=%s" % (get_build_target(args).split('-')[0]))
# Inject linkage of C++ standard library
cmake_env.append("LDFLAGS=-lc++")
if args.sysroot:
flags.append("-DSQLite3_INCLUDE_DIR=%s/usr/include" % args.sysroot)
args.source_dirs["llbuild"] = get_llbuild_source_path(args)
build_with_cmake(args, flags, [], args.source_dirs["llbuild"], args.build_dirs["llbuild"], cmake_env=cmake_env)
@etcwilde I considered that, but I wasn't sure what should be responsible for running the llbuild tests if I made that change. |
b835fb9
to
f04ed6f
Compare
@swift-ci smoke test |
@swift-ci build toolchain |
@swift-ci please build toolchain windows |
Unclear what the problem on Windows actually is
|
Thanks for doing this, I've been doing the same with my native Android toolchain for a while now. Maybe dump a warning for people who are unknowingly using this flag still, and remove I thought about removing the flag altogether, but it looks like it is automatically generated in |
swiftlang/swift-llbuild#1013 removes swift-build-tool from the products of llbuild, as it's no longer used during SwiftPM builds or bootstrapping. Remove the reference in build-script-impl