Skip to content

Commit

Permalink
fix: Gh425 add swift src info to index (#431)
Browse files Browse the repository at this point in the history
- Move `swift.FileInfo` to `swiftpkg.SwiftFileInfo`.
- Update `swiftpkg.SwiftFileInfo` to check for `@objc` directive.
- Add `ModulemapLabel` to `swift.Module`. For Swift source modules, this
field is populated if a modulemap target is required (e.g., has `@objc`
directives).
- Add `modulemap_label` to `deps_indexes.new_module`.
- Update `deps_indexes.labels_for_module` to check for `modulemap_label`
on the module instead of assuming that all Swift targets that are a
dependency for an Objc target should have one.

Related to #425.
  • Loading branch information
cgrindel authored Jun 28, 2023
1 parent 6ac13d6 commit c3560e9
Show file tree
Hide file tree
Showing 17 changed files with 282 additions and 64 deletions.
18 changes: 14 additions & 4 deletions examples/firebase_example/swift_deps_index.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
"name": "FirebaseAnalyticsWithoutAdIdSupportWrapper",
"c99name": "FirebaseAnalyticsWithoutAdIdSupportWrapper",
"src_type": "objc",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWithoutAdIdSupportWrapper",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWithoutAdIdSupportWrapper_FirebaseAnalyticsWithoutAdIdSupportWrapper",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseAnalyticsWithoutAdIdSupport"
Expand All @@ -116,7 +116,7 @@
"name": "FirebaseAnalyticsWrapper",
"c99name": "FirebaseAnalyticsWrapper",
"src_type": "objc",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWrapper",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseAnalyticsWrapper_FirebaseAnalyticsWrapper",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseAnalytics",
Expand Down Expand Up @@ -185,6 +185,7 @@
"c99name": "FirebaseAuthCombineSwift",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Auth_FirebaseAuthCombineSwift",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Auth_FirebaseAuthCombineSwift_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseAuthCombine-Community"
Expand All @@ -195,6 +196,7 @@
"c99name": "FirebaseFirestoreCombineSwift",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Firestore_FirebaseFirestoreCombineSwift",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Firestore_FirebaseFirestoreCombineSwift_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseFirestoreCombine-Community"
Expand All @@ -205,6 +207,7 @@
"c99name": "FirebaseFunctionsCombineSwift",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Functions_FirebaseFunctionsCombineSwift",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Functions_FirebaseFunctionsCombineSwift_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseFunctionsCombine-Community"
Expand All @@ -215,6 +218,7 @@
"c99name": "FirebaseStorageCombineSwift",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Storage_FirebaseStorageCombineSwift",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCombineSwift_Sources_Storage_FirebaseStorageCombineSwift_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseStorageCombine-Community"
Expand All @@ -240,6 +244,7 @@
"c99name": "FirebaseCoreInternal",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseCore_Internal_Sources_FirebaseCoreInternal",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseCore_Internal_Sources_FirebaseCoreInternal_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseAnalytics",
Expand Down Expand Up @@ -341,6 +346,7 @@
"c99name": "FirebaseFunctions",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseFunctions_Sources_FirebaseFunctions",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseFunctions_Sources_FirebaseFunctions_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseFunctionsCombine-Community",
Expand Down Expand Up @@ -406,6 +412,7 @@
"c99name": "FirebaseMLModelDownloader",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseMLModelDownloader_Sources_FirebaseMLModelDownloader",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseMLModelDownloader_Sources_FirebaseMLModelDownloader_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseMLModelDownloader"
Expand Down Expand Up @@ -447,6 +454,7 @@
"c99name": "FirebaseRemoteConfigSwift",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseRemoteConfigSwift_Sources_FirebaseRemoteConfigSwift",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseRemoteConfigSwift_Sources_FirebaseRemoteConfigSwift_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseRemoteConfigSwift"
Expand Down Expand Up @@ -484,6 +492,7 @@
"c99name": "FirebaseStorage",
"src_type": "swift",
"label": "@swiftpkg_firebase_ios_sdk//:FirebaseStorage_Sources_FirebaseStorage",
"modulemap_label": "@swiftpkg_firebase_ios_sdk//:FirebaseStorage_Sources_FirebaseStorage_modulemap",
"package_identity": "firebase-ios-sdk",
"product_memberships": [
"FirebaseStorageCombine-Community",
Expand Down Expand Up @@ -680,7 +689,7 @@
"name": "GoogleDataTransport",
"c99name": "GoogleDataTransport",
"src_type": "objc",
"label": "@swiftpkg_googledatatransport//:GoogleDataTransport",
"label": "@swiftpkg_googledatatransport//:GoogleDataTransport_GoogleDataTransport",
"package_identity": "googledatatransport",
"product_memberships": [
"GoogleDataTransport"
Expand Down Expand Up @@ -921,6 +930,7 @@
"c99name": "PromisesTestHelpers",
"src_type": "swift",
"label": "@swiftpkg_promises//:Sources_PromisesTestHelpers",
"modulemap_label": "@swiftpkg_promises//:Sources_PromisesTestHelpers_modulemap",
"package_identity": "promises",
"product_memberships": [
"PromisesTestHelpers"
Expand Down Expand Up @@ -1235,7 +1245,7 @@
"name": "GoogleDataTransport",
"type": "library",
"target_labels": [
"@swiftpkg_googledatatransport//:GoogleDataTransport"
"@swiftpkg_googledatatransport//:GoogleDataTransport_GoogleDataTransport"
]
},
{
Expand Down
1 change: 1 addition & 0 deletions gazelle/internal/reslog/rule_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func TestRuleResolution(t *testing.T) {
"Custom",
swiftpkg.SwiftSourceType,
newLabel("com_github_example_custom", "", "Custom"),
nil,
swift.HTTPArchivePkgIdentity,
nil,
),
Expand Down
2 changes: 0 additions & 2 deletions gazelle/internal/swift/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ go_library(
"code_dir.go",
"dependency_index.go",
"doc.go",
"file_info.go",
"files.go",
"find_rules.go",
"http_archive.go",
Expand Down Expand Up @@ -63,7 +62,6 @@ go_test(
"bzlmod_test.go",
"code_dir_test.go",
"dependency_index_test.go",
"file_info_test.go",
"files_test.go",
"http_archive_test.go",
"is_builtin_module_test.go",
Expand Down
9 changes: 9 additions & 0 deletions gazelle/internal/swift/bazel_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swiftpkg"
)

const modulemapLabelNameSuffix = "_modulemap"

// BazelLabelFromTarget creates a Bazel label from a Swift target.
// The logic in this function must stay in sync with
// pkginfo_targets.bazel_label_name_from_parts() in the Starlark code.
Expand All @@ -24,3 +26,10 @@ func BazelLabelFromTarget(repoName string, target *swiftpkg.Target) *label.Label
lbl := label.New(repoName, "", name)
return &lbl
}

// ModulemapBazelLabelFromTargetLabel creates a Bazel label for a modulemap target from the
// corresponding target label.
func ModulemapBazelLabelFromTargetLabel(lbl *label.Label) *label.Label {
mml := label.New(lbl.Repo, lbl.Pkg, lbl.Name+modulemapLabelNameSuffix)
return &mml
}
7 changes: 7 additions & 0 deletions gazelle/internal/swift/bazel_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,10 @@ func TestBazelLabelFromTarget(t *testing.T) {
assert.Equal(t, &expected, actual, tt.msg)
}
}

func TestModulemapBazelLabelFromTargetLabel(t *testing.T) {
targetLabel := label.New("example_cool_repo", "bzlpkg", "foo")
actual := swift.ModulemapBazelLabelFromTargetLabel(&targetLabel)
expected := label.New("example_cool_repo", "bzlpkg", "foo_modulemap")
assert.Equal(t, &expected, actual)
}
2 changes: 1 addition & 1 deletion gazelle/internal/swift/http_archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func NewHTTPArchiveFromRule(r *rule.Rule, repoRoot string) (*HTTPArchive, error)
moduleName := ModuleName(br)
l := label.New(repoName, "", br.Name())
m := NewModule(moduleName, moduleName, swiftpkg.SwiftSourceType, &l,
HTTPArchivePkgIdentity, nil)
nil, HTTPArchivePkgIdentity, nil)
modules = append(modules, m)
}

Expand Down
36 changes: 33 additions & 3 deletions gazelle/internal/swift/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ type Module struct {
C99name string
SrcType swiftpkg.SourceType
Label *label.Label
ModulemapLabel *label.Label
PkgIdentity string
ProductMemberships []string
}

// NewModule creates a new module.
func NewModule(
name, c99name string,
srcType swiftpkg.SourceType,
bzlLabel *label.Label,
modulemapLabel *label.Label,
pkgIdentity string,
pms []string,
) *Module {
Expand All @@ -33,6 +36,7 @@ func NewModule(
Label: bzlLabel,
PkgIdentity: pkgIdentity,
ProductMemberships: pms,
ModulemapLabel: modulemapLabel,
}
}

Expand All @@ -45,13 +49,25 @@ func NewModuleFromLabelStruct(
pkgIdentity string,
pms []string,
) *Module {
return NewModule(name, c99name, srcType, &bzlLabel, pkgIdentity, pms)
return NewModule(name, c99name, srcType, &bzlLabel, nil, pkgIdentity, pms)
}

// NewModuleFromTarget returns a module from the specified Swift target.
func NewModuleFromTarget(repoName, pkgIdentity string, t *swiftpkg.Target) (*Module, error) {
lbl := BazelLabelFromTarget(repoName, t)
return NewModule(t.Name, t.C99name, t.SrcType, lbl, pkgIdentity, t.ProductMemberships), nil
var mml *label.Label
if t.SwiftFileInfos.RequiresModulemap() {
mml = ModulemapBazelLabelFromTargetLabel(lbl)
}
return NewModule(
t.Name,
t.C99name,
t.SrcType,
lbl,
mml,
pkgIdentity,
t.ProductMemberships,
), nil
}

// LabelStr returns the label string for module.
Expand All @@ -78,10 +94,12 @@ type moduleJSONData struct {
C99name string `json:"c99name"`
SrcType swiftpkg.SourceType `json:"src_type"`
Label string `json:"label"`
ModulemapLabel string `json:"modulemap_label,omitempty"`
PkgIdentity string `json:"package_identity"`
ProductMemberships []string `json:"product_memberships"`
}

// MarshalJSON customizes the marshalling of a module to JSON.
func (m *Module) MarshalJSON() ([]byte, error) {
var pms []string
if len(m.ProductMemberships) > 0 {
Expand All @@ -97,9 +115,13 @@ func (m *Module) MarshalJSON() ([]byte, error) {
PkgIdentity: m.PkgIdentity,
ProductMemberships: pms,
}
if m.ModulemapLabel != nil {
jd.ModulemapLabel = m.ModulemapLabel.String()
}
return json.Marshal(&jd)
}

// UnmarshalJSON customizes the unmarshalling of a module from JSON.
func (m *Module) UnmarshalJSON(b []byte) error {
var jd moduleJSONData
if err := json.Unmarshal(b, &jd); err != nil {
Expand All @@ -109,7 +131,15 @@ func (m *Module) UnmarshalJSON(b []byte) error {
if err != nil {
return err
}
newm := NewModule(jd.Name, jd.C99name, jd.SrcType, &l, jd.PkgIdentity, jd.ProductMemberships)
var mml *label.Label
if jd.ModulemapLabel != "" {
mmLabel, err := label.Parse(jd.ModulemapLabel)
if err != nil {
return err
}
mml = &mmLabel
}
newm := NewModule(jd.Name, jd.C99name, jd.SrcType, &l, mml, jd.PkgIdentity, jd.ProductMemberships)
*m = *newm
return nil
}
2 changes: 1 addition & 1 deletion gazelle/internal/swift/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func TestModule(t *testing.T) {
t.Run("label string", func(t *testing.T) {
l := label.New("my_repo", "path/to/pkg", "Foo")
m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &l, "my-repo", nil)
m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &l, nil, "my-repo", nil)
actual := m.LabelStr()
expected := swift.NewLabelStr(&l)
assert.Equal(t, expected, actual)
Expand Down
7 changes: 4 additions & 3 deletions gazelle/internal/swift/rules_from_srcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/bazelbuild/bazel-gazelle/language"
"github.com/bazelbuild/bazel-gazelle/rule"
"github.com/cgrindel/rules_swift_package_manager/gazelle/internal/swiftpkg"
mapset "github.com/deckarep/golang-set/v2"
)

Expand All @@ -14,7 +15,7 @@ func RulesFromSrcs(
srcs []string,
defaultModuleName string,
) []*rule.Rule {
fileInfos := NewFileInfosFromRelPaths(args.Dir, srcs)
fileInfos := swiftpkg.NewSwiftFileInfosFromRelPaths(args.Dir, srcs)
swiftImports, moduleType := collectSwiftInfo(fileInfos)

shouldSetVis := shouldSetVisibility(args)
Expand All @@ -31,10 +32,10 @@ func RulesFromSrcs(
return rules
}

var guiModules = mapset.NewSet[string]("AppKit", "UIKit", "SwiftUI")
var guiModules = mapset.NewSet("AppKit", "UIKit", "SwiftUI")

// Returns the imports and the module typ
func collectSwiftInfo(fileInfos []*FileInfo) ([]string, ModuleType) {
func collectSwiftInfo(fileInfos []*swiftpkg.SwiftFileInfo) ([]string, ModuleType) {
importsGUIModules := false
hasTestFiles := false
hasMain := false
Expand Down
3 changes: 2 additions & 1 deletion gazelle/internal/swiftcfg/swift_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func TestWriteAndReadDependencyIndex(t *testing.T) {
origsc.DependencyIndexPath = filepath.Join(dir, swiftcfg.DefaultDependencyIndexBasename)

lbl := label.New("cool_repo", "Sources/Foo", "Foo")
m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &lbl, "", []string{"ProductA"})
m := swift.NewModule("Foo", "Foo", swiftpkg.SwiftSourceType, &lbl, nil, "",
[]string{"ProductA"})
origsc.DependencyIndex.AddModule(m)

// Write the index
Expand Down
2 changes: 2 additions & 0 deletions gazelle/internal/swiftpkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
"platform.go",
"product.go",
"source_type.go",
"swift_file_info.go",
"target.go",
"target_dependency.go",
],
Expand All @@ -33,6 +34,7 @@ go_test(
"module_type_test.go",
"package_info_test.go",
"source_type_test.go",
"swift_file_info_test.go",
],
# The PackageInfo tests use SwiftBin to create an actual Swift package. To use Swift, the test
# needs to be executed outside of the sandbox.
Expand Down
2 changes: 1 addition & 1 deletion gazelle/internal/swiftpkg/package_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewPackageInfo(sw swiftbin.Executor, dir string) (*PackageInfo, error) {
// Ignore phantom targets (i.e., appear in description but not in the dump)
continue
}
t, err := NewTargetFromManifestInfo(&descT, dumpT, prodNames)
t, err := NewTargetFromManifestInfo(dir, &descT, dumpT, prodNames)
if err != nil {
return nil, fmt.Errorf("failed to create target for %s: %w", descT.Name, err)
}
Expand Down
Loading

0 comments on commit c3560e9

Please sign in to comment.