Merge clang/swift modules into one `direct_module` in SwiftInfo for a framework #916

gyfelton · 2024-09-16T21:17:24Z

What changed:

Rework of _get_merged_swift_info by bubble up the creation of clang module to _apple_framework_packaging_impl level, where it determine how to populate the clang module based on VFS feature on/off.
Remove the need to pass virtualize_frameworks flag to the private method _get_merged_swift_info

Why this change

add fail(swift_info.direct_modules) after creation of it at the end of the packging impl method when running bazel build //tests/ios/frameworks/testonly/... --config=ios --config=vfs, you get

[
struct(
clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = None, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", 
swift = None),

 struct(
 clang = None, compilation_context = None, is_system = False, name = "SwiftLibrary",
 swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]

with the current impl in master branch. Note that for the same module SwiftLibrary, there are two structs representing it, one has clang and no swift, and one has swift and nothing inside clang.

So this PR effectively merge two, but the clang module generation differ based on VFS turn on or not:
with VFS on:

[struct(
    clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = None, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", swift = None), struct(clang = None, compilation_context = None, is_system = False, name = "SwiftLibrary", 
 
   swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]

with VFS off:

[struct(

clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/module.modulemap>, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", 

swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/SwiftLibrary.swiftmodule/arm64.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/SwiftLibrary.swiftmodule/arm64.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]

This way we now have one direct_module instead of two representing the same SwiftLIbrary module. This is important when trying to collect providers for docc_archive rule on rules_apple side as otherwise we face in dep attribute of docc_archive rule <some_target_docc>: '<some_target>' does not have mandatory providers: 'DocCBundleInfo' or 'DocCSymbolGraphsInfo'. error.

Tests done

tested with downstream big repo's full CI job (which has VFS on + bazel version 7.2.0)
CI job itself being green

This reverts commit 0a19e1c.

luispadron

This looks more correct to me, i tested this on Cash App and all builds / Xcode integration is good.

I still am not able to rules_ios targets with docc_archive rules from rules_apple an am getting this error:

ERROR: Code/iOSAdditions/NonEmpty/BUILD.bazel:7:14: Extracting symbol graph for NonEmpty_public_hmap failed: (Exit 1): worker failed: error executing SwiftSymbolGraphExtract command (from target //Code/iOSAdditions/NonEmpty:NonEmpty_public_hmap) 
  (cd /private/var/tmp/_bazel_lpadron/9ae9416857eb79bb978de35a53d54970/execroot/_main && \
  exec env - \
    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=14.5 \
    CACHE_EPOCH=8 \
    PATH=/bin:/usr/bin:/usr/local/bin \
    XCODE_VERSION_OVERRIDE=15.4.0.15F31d \
  bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_swift~/tools/worker/worker swift-symbolgraph-extract @bazel-out/darwin_arm64-dbg/bin/Code/iOSAdditions/NonEmpty/NonEmpty_public_hmap.symbolgraphs-0.params)
# Configuration: a5ebcedd7f39e9905d3f1d383fba5c7158c026b68aac0efa518bccdec7476580
# Execution platform: @@platforms//host:host
Couldn't load module 'NonEmpty_public_hmap' in the current SDK and search paths.
Current visible modules:
AGL
AVFAudio
AVFoundation
AVKit
AVRouting
Accelerate
Accessibility
Accounts
AdServices
AdSupport
AddressBook
AppIntents
AppKit
AppTrackingTransparency
AppleArchive
AppleScriptObjC
AppleTextureEncoder
ApplicationServices
AudioToolbox
AudioUnit
AudioVideoBridging
AuthenticationServices
AutomaticAssessmentConfiguration
Automator
BackgroundAssets
BackgroundTasks
Bootstrap
BrowserEngineCore
BrowserEngineKit
BusinessChat
CFNetwork
CUPS
CallKit
CarKey
Carbon
Charts
Cinematic
ClassKit
CloudKit
Cocoa
Collaboration
ColorSync
Combine
CommonCrypto
Compression
Contacts
ContactsUI
CoreAudio
CoreAudioKit
CoreAudioTypes
CoreBluetooth
CoreData
CoreFoundation
CoreGraphics
CoreHaptics
CoreImage
CoreLocation
CoreMIDI
CoreML
CoreMedia
CoreMediaIO
CoreMotion
CoreServices
CoreSpotlight
CoreTelephony
CoreText
CoreTransferable
CoreVideo
CoreWLAN
CreateML
CreateMLComponents
CryptoKit
CryptoTokenKit
Cxx
CxxStdlib
DVDPlayback
Darwin
DataDetection
DeveloperToolsSupport
DeviceActivity
DeviceCheck
DirectoryService
DiscRecording
DiscRecordingUI
DiskArbitration
Dispatch
DispatchIntrospection
Distributed
DockKit
EditLine
EndpointSecurity
EventKit
ExceptionHandling
ExecutionPolicy
ExtensionFoundation
ExtensionKit
ExternalAccessory
FFI
FamilyControls
FileProvider
FileProviderUI
FinanceKit
FinanceKitUI
FinderSync
ForceFeedback
Foundation
GLKit
GLUT
GSS
GameController
GameKit
GameplayKit
GroupActivities
HealthKit
Hypervisor
ICADevices
ICU
IOBluetooth
IOBluetoothUI
IOKit
IOSurface
IOUSBHost
IdentityLookup
ImageCaptureCore
ImageIO
InputMethodKit
Intents
IntentsUI
JavaRuntimeSupport
JavaScriptCore
KernelManagement
LatentSemanticMapping
LightweightCodeRequirements
LinkPresentation
LocalAuthentication
LocalAuthenticationEmbeddedUI
MLCompute
MachO
MailKit
ManagedAppDistribution
ManagedSettings
MapKit
Matter
MatterSupport
MediaAccessibility
MediaLibrary
MediaPlayer
MediaToolbox
Metal
MetalFX
MetalKit
MetalPerformanceShaders
MetalPerformanceShadersGraph
MetricKit
ModelIO
MultipeerConnectivity
MusicKit
NaturalLanguage
NearbyInteraction
NetFS
Network
NetworkExtension
NotificationCenter
OSAKit
OSLog
ObjectiveC
Observation
OpenAL
OpenCL
OpenDirectory
OpenGL
PDFKit
PHASE
ParavirtualizedGraphics
PassKit
PencilKit
Photos
PhotosUI
PreferencePanes
PushKit
PushToTalk
Quartz
QuartzCore
QuickLook
QuickLookThumbnailing
QuickLookUI
RealityFoundation
RealityKit
RegexBuilder
ReplayKit
SQLite3
SafariServices
SafetyKit
SceneKit
ScreenCaptureKit
ScreenSaver
ScreenTime
ScriptingBridge
Security
SecurityFoundation
SecurityInterface
SensitiveContentAnalysis
SensorKit
ServiceManagement
SharedWithYou
SharedWithYouCore
ShazamKit
Social
SoundAnalysis
Spatial
Speech
SpriteKit
StoreKit
Swift
SwiftBridging
SwiftData
SwiftOnoneSupport
SwiftOverlayShims
SwiftShims
SwiftUI
Symbols
System
SystemConfiguration
SystemExtensions
TWAIN
TabularData
Tcl
ThreadNetwork
TipKit
Translation
UniformTypeIdentifiers
UserNotifications
UserNotificationsUI
VideoSubscriberAccount
VideoToolbox
Virtualization
Vision
VisionKit
WeatherKit
WebKit
WidgetKit
XPC
XPCOverlay
_AVKit_SwiftUI
_AppIntents_AppKit
_AppIntents_SwiftUI
_AuthenticationServices_SwiftUI
_Backtracing
_Builtin_intrinsics
_Builtin_stddef_max_align_t
_Concurrency
_CoreData_CloudKit
_DeviceActivity_SwiftUI
_GroupActivities_AppKit
_InternalSwiftScan
_LocalAuthentication_SwiftUI
_ManagedAppDistribution_SwiftUI
_MapKit_SwiftUI
_MusicKit_SwiftUI
_PassKit_SwiftUI
_PhotosUI_SwiftUI
_QuickLook_SwiftUI
_SceneKit_SwiftUI
_SpriteKit_SwiftUI
_StoreKit_SwiftUI
_StringProcessing
_SwiftConcurrencyShims
_SwiftData_CoreData
_SwiftData_SwiftUI
_SwiftOSOverlayShims
_SwiftXPCOverlayShims
_System_Foundation
_Translation_SwiftUI
asl
dnssd
iTunesLibrary
kcdata
launch
ldap
libDER
libexslt
libkern
libunwind
libxml2
libxslt
networkext
notify
os
os_object
os_workgroup
ptrauth
rpc
simd
tidy
unwind
vmnet
vproc
xcselect
zlib
ERROR: Code/iOSAdditions/NonEmpty/BUILD.bazel:7:14: Extracting symbol graph for NonEmpty failed: (Exit 1): worker failed: error executing SwiftSymbolGraphExtract command (from target //Code/iOSAdditions/NonEmpty:NonEmpty_swift) 
  (cd /private/var/tmp/_bazel_lpadron/9ae9416857eb79bb978de35a53d54970/execroot/_main && \
  exec env - \
    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=14.5 \
    CACHE_EPOCH=8 \
    PATH=/bin:/usr/bin:/usr/local/bin \
    XCODE_VERSION_OVERRIDE=15.4.0.15F31d \
  bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_swift~/tools/worker/worker swift-symbolgraph-extract @bazel-out/darwin_arm64-dbg/bin/Code/iOSAdditions/NonEmpty/NonEmpty_swift.symbolgraphs-0.params)
# Configuration: a5ebcedd7f39e9905d3f1d383fba5c7158c026b68aac0efa518bccdec7476580
# Execution platform: @@platforms//host:host
<unknown>:0: error: cannot load underlying module for 'NonEmpty'
Error: Failed to load the module 'NonEmpty'. Are you missing build dependencies or include/framework directories?
See the previous error messages for details. Aborting.

I wonder if we need to stop forwarding SwiftInfo and stuff from the hmap/vfs rules, not sure why it provides those anyway

gyfelton · 2024-09-17T19:13:07Z

@luispadron good catch i can repro on my side as well. Let me try to see how easy to fix it

gyfelton · 2024-09-17T19:28:47Z

@luispadron also i was wrong that it works with latest rules swift/apple, it did not in downstream repo and it also failed in current CI. need to rework a bit

gyfelton · 2024-09-18T02:50:15Z

the error only happens with latest rule but latest rule already failed on docc_archive rule, so will. address it in a sep. PR

Last place that touched this code is on https://github.com/bazel-ios/rules_ios/pull/906where I thought it's important to construct SwiftInfo provider for hmap and add compilation context to pass the failing test. However that leads to a new [issue](#916 (review)) with `docc_archive` rule with latest `rules_apple/rules_swift`. We realized it's actually not needed to provide any SwiftInfo provider at all or it will "confuse" rules_swift/apple (since newer version starts to rely more on clang info from Swift info itself instead `apple_common.Objc`). Removing this also have the original failing test passing. Tests done: - [x] my own downstream repo still builds fine. - [x] CI is green Will squash before merge as it contains already merged commits

gyfelton added 3 commits September 16, 2024 17:17

Remove allocation of extra swift module

0a19e1c

Revert "Remove allocation of extra swift module"

722999d

This reverts commit 0a19e1c.

Move clang module creation to top level

f61af30

gyfelton changed the title ~~[WIP] Remove allocation of extra swift module~~ Merge clang/swift modules inside SwiftInfo Sep 17, 2024

gyfelton changed the title ~~Merge clang/swift modules inside SwiftInfo~~ Merge clang/swift modules into one struct in SwiftInfo Sep 17, 2024

gyfelton requested a review from luispadron September 17, 2024 16:31

gyfelton marked this pull request as ready for review September 17, 2024 16:31

gyfelton changed the title ~~Merge clang/swift modules into one struct in SwiftInfo~~ Merge clang/swift modules into one direct_module in SwiftInfo for a framework Sep 17, 2024

gyfelton changed the title ~~Merge clang/swift modules into one direct_module in SwiftInfo for a framework~~ Merge clang/swift modules into one direct_module in SwiftInfo for a framework Sep 17, 2024

gyfelton requested a review from thiagohmcruz September 17, 2024 16:33

luispadron approved these changes Sep 17, 2024

View reviewed changes

thiagohmcruz approved these changes Sep 17, 2024

View reviewed changes

gyfelton enabled auto-merge (squash) September 18, 2024 02:50

gyfelton disabled auto-merge September 18, 2024 13:25

gyfelton enabled auto-merge (squash) September 18, 2024 13:25

gyfelton merged commit e74047f into master Sep 18, 2024
23 checks passed

gyfelton deleted the yuanfeng/fix-missing-docc-provider branch September 18, 2024 13:26

gyfelton mentioned this pull request Sep 18, 2024

Remove swift info provider creation inside hmap rule #919

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge clang/swift modules into one `direct_module` in SwiftInfo for a framework #916

Merge clang/swift modules into one `direct_module` in SwiftInfo for a framework #916

gyfelton commented Sep 16, 2024 •

edited

Loading

luispadron left a comment

gyfelton commented Sep 17, 2024

gyfelton commented Sep 17, 2024

gyfelton commented Sep 18, 2024

Merge clang/swift modules into one direct_module in SwiftInfo for a framework #916

Merge clang/swift modules into one direct_module in SwiftInfo for a framework #916

Conversation

gyfelton commented Sep 16, 2024 • edited Loading

What changed:

Why this change

Tests done

luispadron left a comment

Choose a reason for hiding this comment

gyfelton commented Sep 17, 2024

gyfelton commented Sep 17, 2024

gyfelton commented Sep 18, 2024

Merge clang/swift modules into one `direct_module` in SwiftInfo for a framework #916

Merge clang/swift modules into one `direct_module` in SwiftInfo for a framework #916

gyfelton commented Sep 16, 2024 •

edited

Loading