Skip to content

Add a lit test for Xcode project generation #68713

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 3 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ def build_with_cmake(self, build_targets, build_type, build_args,
+ self.args.extra_cmake_options + [self.source_dir],
env=env)

if not self.args.skip_build or self.product_name() == "llvm":
is_llvm = self.product_name() == "llvm"
if (not is_llvm and not self.args.skip_build) or (
is_llvm and self.args._build_llvm
):
cmake_opts = [self.build_dir, "--config", build_type]

if self.args.cmake_generator == "Xcode":
Expand Down
25 changes: 12 additions & 13 deletions utils/swift_build_support/swift_build_support/products/llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,20 +216,11 @@ def build(self, host_target):
# space/time efficient than -g on that platform.
llvm_cmake_options.define('LLVM_USE_SPLIT_DWARF:BOOL', 'YES')

build_targets = ['all']

if self.args.llvm_ninja_targets_for_cross_compile_hosts and \
self.is_cross_compile_target(host_target):
build_targets = (self.args.llvm_ninja_targets_for_cross_compile_hosts)
elif self.args.llvm_ninja_targets:
build_targets = (self.args.llvm_ninja_targets)

# indicating we don't want to build LLVM should
# override any custom ninja target we specified
if not self.args._build_llvm:
build_targets = ['clean']
Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure why we used to add the clean target but it looks like we lost it. Is that intentional?

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Sep 23, 2023

Choose a reason for hiding this comment

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

Neither am I, but I doubt it makes a difference.

Is that intentional?

Yes, I changed the logic slightly to not run any build commands for LLVM when --build-llvm=0 (the "actually don’t build LLVM" option), so it no longer matters what build_targets is set to in this case. This behavior is consistent with how we handle other CMake products, and it allows the new test to not just succeed (a build command that was assembled for the Xcode generator will fail against a Ninja build directory due to incompatible adjustments to the options), but also run correctly (we don’t want to run any build commands against the symlinked LLVM build directory).

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable to me 👍🏽


if self.args.skip_build or not self.args.build_llvm:
# Indicating we don't want to build LLVM at all should
# override everything.
build_targets = []
elif self.args.skip_build or not self.args.build_llvm:
# We can't skip the build completely because the standalone
# build of Swift depends on these.
build_targets = ['llvm-tblgen', 'clang-resource-headers',
Expand All @@ -248,6 +239,14 @@ def build(self, host_target):
'llvm-nm',
'llvm-size'
])
else:
build_targets = ['all']

if self.args.llvm_ninja_targets_for_cross_compile_hosts and \
self.is_cross_compile_target(host_target):
build_targets = (self.args.llvm_ninja_targets_for_cross_compile_hosts)
elif self.args.llvm_ninja_targets:
build_targets = (self.args.llvm_ninja_targets)

if self.args.host_libtool:
llvm_cmake_options.define('CMAKE_LIBTOOL', self.args.host_libtool)
Expand Down
17 changes: 17 additions & 0 deletions validation-test/BuildSystem/generate_xcode.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# RUN: %empty-directory(%t)
# RUN: %empty-directory(%t/Xcode)

# Build system generation for 'swift' depends on several LLVM tools
# (e.g. llvm-tblgen). This is why we still build them with '--skip-build' or
# '--skip-build-llvm' set, and have an additional '--build-llvm' option to
# actually skip building that bare minimum too.
#
# To save time, borrow these dependencies from the current build directories,
# and test Xcode project generation only for 'swift'.

# RUN: ln -s %swift_obj_root/../cmark-%target-os-%target-arch %t/Xcode/cmark-%target-os-%target-arch
# RUN: ln -s %swift_obj_root/../llvm-%target-os-%target-arch %t/Xcode/llvm-%target-os-%target-arch
# RUN: SWIFT_BUILD_ROOT=%t %swift_src_root/utils/build-script --cmake %cmake --swift-darwin-supported-archs="$(uname -m)" --build-subdir="Xcode" --skip-build-cmark --build-llvm=0 --xcode

# REQUIRES: standalone_build
# REQUIRES: OS=macosx
2 changes: 1 addition & 1 deletion validation-test/BuildSystem/llvm-targets-options.test
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# RUN: mkdir -p %t
# RUN: SKIP_XCODE_VERSION_CHECK=1 SWIFT_BUILD_ROOT=%t %swift_src_root/utils/build-script --dry-run --build-llvm=0 --llvm-ninja-targets="lib/all clangDependencyScanning" --cmake %cmake 2>&1 | %FileCheck --check-prefix=BUILD-LLVM-0-CHECK %s

# BUILD-LLVM-0-CHECK: cmake --build {{.*}}/llvm-{{[^/]*}} clean
# BUILD-LLVM-0-CHECK-NOT: cmake --build {{.*}}/Ninja-DebugAssert/llvm


# RUN: %empty-directory(%t)
Expand Down
1 change: 0 additions & 1 deletion validation-test/BuildSystem/skip_build_xcode.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# RUN: %empty-directory(%t)
# RUN: mkdir -p %t
# RUN: SKIP_XCODE_VERSION_CHECK=1 SWIFT_BUILD_ROOT=%t %swift_src_root/utils/build-script --xcode --release --swift-darwin-supported-archs="$(uname -m)" --dry-run --cmake %cmake 2>&1 | %FileCheck %s

# REQUIRES: standalone_build
Expand Down