Improve globbing performance by using parallelized Swift glob impleme…

…ntation (tuist#6957) * Improve globbing performance by using parallelized Swift glob implementation * Fix test_loadWorkspace_withProjects * Update FileSystem * Move Alamofire to its own fixture from the monolith app_with_spm_dependencies * Remove Alamofire dependency in app_with_spm_dependencies fixture * Update FileSystem
Progressive-Insurance · Oct 30, 2024 · a074f26 · a074f26
1 parent 30bda6c
commit a074f26
Show file tree

Hide file tree

Showing 27 changed files with 243 additions and 79 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -1,5 +1,5 @@
 {
-  "originHash" : "f8c61950711e5128d86d58358ee865040f1c7a5813155aaef7799e161ec9fb3a",
+  "originHash" : "de2c05e5b2583fd20c9ac0362c356c78df4ddffaee8430d3c04ad279851ae24c",
   "pins" : [
     {
       "identity" : "aexml",
@@ -51,8 +51,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/tuist/FileSystem.git",
       "state" : {
-        "revision" : "0cda08ddcb9c9ac8c83891d83f26387cc1104029",
-        "version" : "0.5.0"
+        "revision" : "f4ea155e26262f340b1cabc8580ccaa445f690bc",
+        "version" : "0.6.0"
       }
     },
     {
@@ -204,8 +204,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/tuist/swift-glob",
       "state" : {
-        "revision" : "a0bc22cc5e5113356d31540dbe66da0b653cd6d8",
-        "version" : "0.2.2"
+        "revision" : "a32f83e851593b4bec436ebac910d70af6d77f6a",
+        "version" : "0.3.4"
       }
     },
     {

diff --git a/Package.swift b/Package.swift
@@ -512,7 +512,7 @@ let package = Package(
         .package(
             url: "https://github.com/tuist/XcodeGraph.git", .upToNextMajor(from: "0.16.0")
         ),
-        .package(url: "https://github.com/tuist/FileSystem.git", .upToNextMajor(from: "0.5.0")),
+        .package(url: "https://github.com/tuist/FileSystem.git", .upToNextMajor(from: "0.6.0")),
         .package(url: "https://github.com/tuist/Command.git", exact: "0.8.0"),
     ],
     targets: targets

diff --git a/Sources/TuistAcceptanceTesting/TuistAcceptanceFixtures.swift b/Sources/TuistAcceptanceTesting/TuistAcceptanceFixtures.swift
@@ -2,6 +2,7 @@ import Foundation
 
 public enum TuistAcceptanceFixtures {
     case appWithAirshipSDK
+    case appWithAlamofire
     case appWithBuildRules
     case appWithComposableArchitecture
     case appWithCustomDefaultConfiguration
@@ -90,6 +91,8 @@ public enum TuistAcceptanceFixtures {
         switch self {
         case .appWithAirshipSDK:
             return "app_with_airship_sdk"
+        case .appWithAlamofire:
+            return "app_with_alamofire"
         case .appWithBuildRules:
             return "app_with_build_rules"
         case .appWithComposableArchitecture:

diff --git a/Sources/TuistLoader/Loaders/RecursiveManifestLoader.swift b/Sources/TuistLoader/Loaders/RecursiveManifestLoader.swift
@@ -69,15 +69,17 @@ public struct RecursiveManifestLoader: RecursiveManifestLoading {
             rootDirectory: rootDirectory
         )
         let projectSearchPaths = (loadedWorkspace?.projects ?? ["."])
-        let manifestLoader = manifestLoader
         let projectPaths = try await projectSearchPaths.map {
             try generatorPaths.resolve(path: $0)
         }.concurrentFlatMap {
-            try await fileSystem.glob(directory: $0, include: [""]).collect()
-        }.filter {
-            fileHandler.isFolder($0)
-        }.concurrentFilter {
-            try await manifestLoader.manifests(at: $0).contains(.project)
+            try await fileSystem.glob(
+                directory: AbsolutePath.root,
+                include: [
+                    String($0.appending(component: Manifest.project.fileName($0)).pathString.dropFirst()),
+                ]
+            )
+            .collect()
+            .map(\.parentDirectory)
         }
 
         let projects = await LoadedProjects(projects: try loadProjects(paths: projectPaths).projects)

diff --git a/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/ResourceFileElement+ManifestMapper.swift
@@ -23,8 +23,8 @@ extension XcodeGraph.ResourceFileElement {
             for path in excluding {
                 let absolute = try AbsolutePath(validating: path)
                 let globs = try await fileSystem.glob(
-                    directory: AbsolutePath(validating: absolute.dirname),
-                    include: [absolute.basename]
+                    directory: .root,
+                    include: [String(absolute.pathString.dropFirst())]
                 )
                 .collect()
                 excluded.formUnion(globs)

diff --git a/Sources/TuistLoader/Models+ManifestMappers/Workspace+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/Workspace+ManifestMapper.swift
@@ -15,21 +15,22 @@ extension XcodeGraph.Workspace {
         manifest: ProjectDescription.Workspace,
         path: AbsolutePath,
         generatorPaths: GeneratorPaths,
-        manifestLoader: ManifestLoading,
+        manifestLoader _: ManifestLoading,
         fileSystem: FileSysteming
     ) async throws -> XcodeGraph.Workspace {
         func globProjects(_ path: Path) async throws -> [AbsolutePath] {
             let resolvedPath = try generatorPaths.resolve(path: path)
             let projects = try await fileSystem.glob(
                 directory: AbsolutePath.root,
-                include: [String(resolvedPath.pathString.dropFirst())]
+                include: [
+                    String(resolvedPath.appending(component: Manifest.package.fileName(resolvedPath)).pathString.dropFirst()),
+                    String(resolvedPath.appending(component: Manifest.project.fileName(resolvedPath)).pathString.dropFirst()),
+                ]
             )
             .collect()
-            .filter(FileHandler.shared.isFolder)
+            .map(\.parentDirectory)
             .filter { $0.basename != Constants.tuistDirectoryName && !$0.pathString.contains(".build/checkouts") }
-            .concurrentFilter {
-                try await manifestLoader.manifests(at: $0).contains(where: { $0 == .package || $0 == .project })
-            }
+            .uniqued()
 
             if projects.isEmpty {
                 // FIXME: This should be done in a linter.

diff --git a/Sources/TuistLoader/Models/FileListGlob+Unfold.swift b/Sources/TuistLoader/Models/FileListGlob+Unfold.swift
@@ -42,8 +42,8 @@ extension FileListGlob {
         for path in excluding {
             let resolved = try generatorPaths.resolve(path: path)
             let globs = try await fileSystem.glob(
-                directory: AbsolutePath(validating: resolved.dirname),
-                include: [resolved.basename]
+                directory: .root,
+                include: [String(resolved.pathString.dropFirst())]
             )
             .collect()
             result.formUnion(globs)

diff --git a/Tests/TuistDependenciesAcceptanceTests/DependenciesAcceptanceTests.swift b/Tests/TuistDependenciesAcceptanceTests/DependenciesAcceptanceTests.swift
@@ -33,6 +33,15 @@ final class DependenciesAcceptanceTestAppWithSPMDependenciesWithoutInstall: Tuis
     }
 }
 
+final class DependenciesAcceptanceTestAppAlamofire: TuistAcceptanceTestCase {
+    func test_app_with_alamofire() async throws {
+        try await setUpFixture(.appWithAlamofire)
+        try await run(InstallCommand.self)
+        try await run(GenerateCommand.self)
+        try await run(BuildCommand.self, "App")
+    }
+}
+
 final class DependenciesAcceptanceTestIosAppWithSPMDependencies: TuistAcceptanceTestCase {
     func test_ios_app_spm_dependencies() async throws {
         try await setUpFixture(.iosAppWithSpmDependencies)

diff --git a/Tests/TuistLoaderTests/Loaders/ManifestModelConverterTests.swift b/Tests/TuistLoaderTests/Loaders/ManifestModelConverterTests.swift
@@ -224,6 +224,7 @@ final class ManifestModelConverterTests: TuistUnitTestCase {
             "A",
             "B",
         ])
+        try await createFiles(["A/Project.swift", "B/Project.swift"])
 
         let manifest = WorkspaceManifest.test(name: "SomeWorkspace", projects: ["A", "B"])
         let manifests = [

diff --git a/Tests/TuistLoaderTests/Loaders/RecursiveManifestLoaderTests.swift b/Tests/TuistLoaderTests/Loaders/RecursiveManifestLoaderTests.swift
@@ -57,7 +57,7 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
     func test_loadProject_loadingSingleProject() async throws {
         // Given
         let projectA = createProject(name: "ProjectA")
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
 
         // When
         let manifests = try await subject.loadWorkspace(at: path.appending(try RelativePath(validating: "Some/Path/A")))
@@ -91,9 +91,9 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
                 "TargetC": [],
             ]
         )
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
-        try stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
-        try stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
+        try await stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
 
         // When
         let manifests = try await subject.loadWorkspace(at: path.appending(try RelativePath(validating: "Some/Path/A")))
@@ -141,11 +141,11 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
                 "TargetE": [],
             ]
         )
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
-        try stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
-        try stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
-        try stub(manifest: projectD, at: try RelativePath(validating: "Some/Path/D"))
-        try stub(manifest: projectE, at: try RelativePath(validating: "Some/Path/E"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
+        try await stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
+        try await stub(manifest: projectD, at: try RelativePath(validating: "Some/Path/D"))
+        try await stub(manifest: projectE, at: try RelativePath(validating: "Some/Path/E"))
 
         // When
         let manifests = try await subject.loadWorkspace(at: path.appending(try RelativePath(validating: "Some/Path/A")))
@@ -170,7 +170,7 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
                 ],
             ]
         )
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
 
         // When / Then
         await XCTAssertThrowsSpecific(
@@ -208,9 +208,9 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
             ]
         )
 
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
-        try stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
-        try stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
+        try await stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
         try stub(manifest: workspace, at: try RelativePath(validating: "Some/Path"))
 
         // When
@@ -254,9 +254,9 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
             ]
         )
 
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
-        try stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
-        try stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path/A"))
+        try await stub(manifest: projectB, at: try RelativePath(validating: "Some/Path/B"))
+        try await stub(manifest: projectC, at: try RelativePath(validating: "Some/Path/C"))
         try stub(manifest: workspace, at: try RelativePath(validating: "Some/Path"))
 
         // When
@@ -291,7 +291,7 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
             ]
         )
 
-        try stub(manifest: projectA, at: try RelativePath(validating: "Some/Path"))
+        try await stub(manifest: projectA, at: try RelativePath(validating: "Some/Path"))
         try stub(manifest: workspace, at: try RelativePath(validating: "Some/Path"))
 
         // When
@@ -371,11 +371,12 @@ final class RecursiveManifestLoaderTests: TuistUnitTestCase {
     private func stub(
         manifest: Project,
         at relativePath: RelativePath
-    ) throws {
+    ) async throws {
         let manifestPath = path
             .appending(relativePath)
             .appending(component: Manifest.project.fileName(path.appending(relativePath)))
-        try fileHandler.touch(manifestPath)
+        try await fileSystem.makeDirectory(at: manifestPath.parentDirectory)
+        try await fileSystem.touch(manifestPath)
         projectManifests[manifestPath.parentDirectory] = manifest
     }
 

diff --git a/Tests/TuistLoaderTests/Models+ManifestMappers/ResourceFileElementManifestMapperTests.swift b/Tests/TuistLoaderTests/Models+ManifestMappers/ResourceFileElementManifestMapperTests.swift
@@ -141,7 +141,6 @@ final class ResourceFileElementManifestMapperTests: TuistUnitTestCase {
         XCTAssertEqual(
             got,
             [
-                .file(path: resourcesFolder, tags: []),
                 .file(path: includedResource, tags: []),
             ]
         )
@@ -174,7 +173,6 @@ final class ResourceFileElementManifestMapperTests: TuistUnitTestCase {
         XCTAssertEqual(
             got,
             [
-                .file(path: resourcesFolder, tags: []),
                 .file(path: includedResource, tags: []),
             ]
         )
@@ -204,10 +202,9 @@ final class ResourceFileElementManifestMapperTests: TuistUnitTestCase {
         )
 
         // Then
-        XCTAssertEqual(
+        XCTAssertBetterEqual(
             got,
             [
-                .file(path: resourcesFolder, tags: []),
                 .file(path: includedResource, tags: []),
             ]
         )

diff --git a/Tests/TuistLoaderTests/Models+ManifestMappers/Workspace+ManifestMapperTests.swift b/Tests/TuistLoaderTests/Models+ManifestMappers/Workspace+ManifestMapperTests.swift
@@ -37,6 +37,7 @@ final class WorkspaceManifestMapperTests: TuistUnitTestCase {
             .locate(from: .any)
             .willReturn(workspacePath)
 
+        try await fileSystem.touch(workspacePath.appending(component: "Project.swift"))
         try fileHandler.createFolder(workspacePath.appending(components: ".build", "checkouts"))
 
         // When

diff --git a/fixtures/app_with_alamofire/.gitignore b/fixtures/app_with_alamofire/.gitignore
@@ -0,0 +1,70 @@
+### macOS ###
+# General
+.DS_Store
+.AppleDouble
+.LSOverride
+
+# Icon must end with two
+Icon
+
+# Thumbnails
+._*
+
+# Files that might appear in the root of a volume
+.DocumentRevisions-V100
+.fseventsd
+.Spotlight-V100
+.TemporaryItems
+.Trashes
+.VolumeIcon.icns
+.com.apple.timemachine.donotpresent
+
+# Directories potentially created on remote AFP share
+.AppleDB
+.AppleDesktop
+Network Trash Folder
+Temporary Items
+.apdisk
+
+### Xcode ###
+# Xcode
+#
+# gitignore contributors: remember to update Global/Xcode.gitignore, Objective-C.gitignore & Swift.gitignore
+
+## User settings
+xcuserdata/
+
+## compatibility with Xcode 8 and earlier (ignoring not required starting Xcode 9)
+*.xcscmblueprint
+*.xccheckout
+
+## compatibility with Xcode 3 and earlier (ignoring not required starting Xcode 4)
+build/
+DerivedData/
+*.moved-aside
+*.pbxuser
+!default.pbxuser
+*.mode1v3
+!default.mode1v3
+*.mode2v3
+!default.mode2v3
+*.perspectivev3
+!default.perspectivev3
+
+### Xcode Patch ###
+*.xcodeproj/*
+!*.xcodeproj/project.pbxproj
+!*.xcodeproj/xcshareddata/
+!*.xcworkspace/contents.xcworkspacedata
+/*.gcno
+
+### Projects ###
+*.xcodeproj
+*.xcworkspace
+
+### Tuist derived files ###
+graph.dot
+Derived/
+
+### Tuist managed dependencies ###
+Tuist/.build
diff --git a/fixtures/app_with_alamofire/.mise.toml b/fixtures/app_with_alamofire/.mise.toml
@@ -0,0 +1,2 @@
+[tools]
+tuist = "4.31.0"
diff --git a/fixtures/app_with_alamofire/App/Sources/AppApp.swift b/fixtures/app_with_alamofire/App/Sources/AppApp.swift
@@ -0,0 +1,10 @@
+import SwiftUI
+
+@main
+struct MyApp: App {
+    var body: some Scene {
+        WindowGroup {
+            ContentView()
+        }
+    }
+}
diff --git a/fixtures/app_with_alamofire/App/Sources/ContentView.swift b/fixtures/app_with_alamofire/App/Sources/ContentView.swift
@@ -0,0 +1,20 @@
+import Alamofire
+import SwiftUI
+
+public struct ContentView: View {
+    public init() {
+        // Use Alamofire to make sure it links fine
+        _ = AF.download("http://www.tuist.io")
+    }
+
+    public var body: some View {
+        Text("Hello, World!")
+            .padding()
+    }
+}
+
+struct ContentView_Previews: PreviewProvider {
+    static var previews: some View {
+        ContentView()
+    }
+}
-Original file line number
+Diff line change
@@ Expand Up @@
                 "A",
                 "B",
             ])
+            try await createFiles(["A/Project.swift", "B/Project.swift"])
             let manifest = WorkspaceManifest.test(name: "SomeWorkspace", projects: ["A", "B"])
             let manifests = [
@@ Expand Down @@