Fix build phases not being generated in the right positions (#506)

* Fix build phases not being generated in the right positions

* Remove extra space

* Add CHANGELOG entry

* Fix step

* Fix tests

* Address comments

* Add missing line

CHANGELOG.md

@@ -16,6 +16,7 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/
 - Prevent embedding static frameworks https://github.com/tuist/tuist/pull/490 by @kwridan
 - Output losing its format when tuist is run through `tuistenv` https://github.com/tuist/tuist/pull/493 by @pepibumur
 - Product name linting failing when it contains variables https://github.com/tuist/tuist/pull/494 by @dcvz
+- Build phases not generated in the right position https://github.com/tuist/tuist/pull/506 by @pepibumur
 
 ## 0.17.0
 

Gemfile

@@ -16,4 +16,5 @@ gem "encrypted-environment", "~> 0.2.0"
 gem "google-cloud-storage", "~> 1.21"
 gem "colorize", "~> 0.8.1"
 gem "cocoapods", "~> 1.7"
+gem "xcodeproj", "~> 1.12"
 gem "highline", "~> 2.0"

Gemfile.lock

@@ -235,6 +235,7 @@ DEPENDENCIES
   rake (~> 12.3)
   rubocop (~> 0.74.0)
   simctl (~> 1.6)
+  xcodeproj (~> 1.12)
 
 BUNDLED WITH
    1.17.3

Sources/TuistGenerator/Generator/BuildPhaseGenerator.swift

@@ -36,6 +36,19 @@ protocol BuildPhaseGenerating: AnyObject {
                              fileElements: ProjectFileElements,
                              pbxproj: PBXProj,
                              sourceRootPath: AbsolutePath) throws
+
+    /// Generates target actions
+    ///
+    /// - Parameters:
+    ///   - actions: Actions to be generated as script build phases.
+    ///   - pbxTarget: PBXTarget from the Xcode project.
+    ///   - pbxproj: PBXProj instance.
+    ///   - sourceRootPath: Path to the directory that will contain the generated project.
+    /// - Throws: An error if the script phase can't be generated.
+    func generateActions(actions: [TargetAction],
+                         pbxTarget: PBXTarget,
+                         pbxproj: PBXProj,
+                         sourceRootPath: AbsolutePath) throws
 }
 
 final class BuildPhaseGenerator: BuildPhaseGenerating {
@@ -47,12 +60,7 @@ final class BuildPhaseGenerator: BuildPhaseGenerating {
                              pbxTarget: PBXTarget,
                              fileElements: ProjectFileElements,
                              pbxproj: PBXProj,
-                             sourceRootPath: AbsolutePath) throws {
-        try generateActions(actions: target.actions.preActions,
-                            pbxTarget: pbxTarget,
-                            pbxproj: pbxproj,
-                            sourceRootPath: sourceRootPath)
-
+                             sourceRootPath _: AbsolutePath) throws {
         if let headers = target.headers {
             try generateHeadersBuildPhase(headers: headers,
                                           pbxTarget: pbxTarget,
@@ -71,11 +79,6 @@ final class BuildPhaseGenerator: BuildPhaseGenerating {
                                         pbxTarget: pbxTarget,
                                         fileElements: fileElements,
                                         pbxproj: pbxproj)
-
-        try generateActions(actions: target.actions.postActions,
-                            pbxTarget: pbxTarget,
-                            pbxproj: pbxproj,
-                            sourceRootPath: sourceRootPath)
     }
 
     func generateActions(actions: [TargetAction],

Sources/TuistGenerator/Generator/TargetGenerator.swift

@@ -67,6 +67,12 @@ final class TargetGenerator: TargetGenerating {
         pbxproj.add(object: pbxTarget)
         pbxProject.targets.append(pbxTarget)
 
+        /// Pre actions
+        try buildPhaseGenerator.generateActions(actions: target.actions.preActions,
+                                                pbxTarget: pbxTarget,
+                                                pbxproj: pbxproj,
+                                                sourceRootPath: sourceRootPath)
+
         /// Build configuration
         try configGenerator.generateTargetConfig(target,
                                                  pbxTarget: pbxTarget,
@@ -95,6 +101,12 @@ final class TargetGenerator: TargetGenerating {
                                         sourceRootPath: sourceRootPath,
                                         graph: graph,
                                         system: system)
+
+        /// Post actions
+        try buildPhaseGenerator.generateActions(actions: target.actions.postActions,
+                                                pbxTarget: pbxTarget,
+                                                pbxproj: pbxproj,
+                                                sourceRootPath: sourceRootPath)
         return pbxTarget
     }
 

Tests/TuistGeneratorTests/Generator/BuildPhaseGeneratorTests.swift

@@ -29,39 +29,6 @@ final class BuildPhaseGeneratorTests: XCTestCase {
         graph = Graph.test()
     }
 
-    func test_generateBuildPhases_generatesActions() throws {
-        let tmpDir = try TemporaryDirectory(removeTreeOnDeinit: true)
-        let pbxTarget = PBXNativeTarget(name: "Test")
-        let pbxproj = PBXProj()
-        let fileElements = ProjectFileElements()
-        pbxproj.add(object: pbxTarget)
-
-        let target = Target.test(sources: [],
-                                 resources: [],
-                                 actions: [
-                                     TargetAction(name: "post", order: .post, path: tmpDir.path.appending(component: "script.sh"), arguments: ["arg"]),
-                                     TargetAction(name: "pre", order: .pre, path: tmpDir.path.appending(component: "script.sh"), arguments: ["arg"]),
-                                 ])
-
-        try subject.generateBuildPhases(path: tmpDir.path,
-                                        target: target,
-                                        graph: Graph.test(),
-                                        pbxTarget: pbxTarget,
-                                        fileElements: fileElements,
-                                        pbxproj: pbxproj,
-                                        sourceRootPath: tmpDir.path)
-
-        let preBuildPhase = pbxTarget.buildPhases.first as? PBXShellScriptBuildPhase
-        XCTAssertEqual(preBuildPhase?.name, "pre")
-        XCTAssertEqual(preBuildPhase?.shellPath, "/bin/sh")
-        XCTAssertEqual(preBuildPhase?.shellScript, "script.sh arg")
-
-        let postBuildPhase = pbxTarget.buildPhases.last as? PBXShellScriptBuildPhase
-        XCTAssertEqual(postBuildPhase?.name, "post")
-        XCTAssertEqual(postBuildPhase?.shellPath, "/bin/sh")
-        XCTAssertEqual(postBuildPhase?.shellScript, "script.sh arg")
-    }
-
     func test_generateSourcesBuildPhase() throws {
         // Given
         let target = PBXNativeTarget(name: "Test")

Tests/TuistGeneratorTests/Generator/TargetGeneratorTests.swift

@@ -107,6 +107,48 @@ final class TargetGeneratorTests: XCTestCase {
         ])
     }
 
+    func test_generateTarget_actions() throws {
+        // Given
+        let graph = Graph.test()
+        let target = Target.test(sources: [],
+                                 resources: [],
+                                 actions: [
+                                     TargetAction(name: "post", order: .post, path: path.appending(component: "script.sh"), arguments: ["arg"]),
+                                     TargetAction(name: "pre", order: .pre, path: path.appending(component: "script.sh"), arguments: ["arg"]),
+                                 ])
+        let project = Project.test(path: path, targets: [target])
+        let groups = ProjectGroups.generate(project: project,
+                                            pbxproj: pbxproj,
+                                            sourceRootPath: path,
+                                            playgrounds: MockPlaygrounds())
+        try fileElements.generateProjectFiles(project: project,
+                                              graph: graph,
+                                              groups: groups,
+                                              pbxproj: pbxproj,
+                                              sourceRootPath: path)
+
+        // When
+        let pbxTarget = try subject.generateTarget(target: target,
+                                                   pbxproj: pbxproj,
+                                                   pbxProject: pbxProject,
+                                                   projectSettings: Settings.test(),
+                                                   fileElements: fileElements,
+                                                   path: path,
+                                                   sourceRootPath: path,
+                                                   graph: graph)
+
+        // Then
+        let preBuildPhase = pbxTarget.buildPhases.first as? PBXShellScriptBuildPhase
+        XCTAssertEqual(preBuildPhase?.name, "pre")
+        XCTAssertEqual(preBuildPhase?.shellPath, "/bin/sh")
+        XCTAssertEqual(preBuildPhase?.shellScript, "script.sh arg")
+
+        let postBuildPhase = pbxTarget.buildPhases.last as? PBXShellScriptBuildPhase
+        XCTAssertEqual(postBuildPhase?.name, "post")
+        XCTAssertEqual(postBuildPhase?.shellPath, "/bin/sh")
+        XCTAssertEqual(postBuildPhase?.shellScript, "script.sh arg")
+    }
+
     // MARK: - Helpers
 
     private func createTargetNodes(project: Project,

features/generate.feature

@@ -44,7 +44,7 @@ Feature: Generate a new project using Tuist
     Given that tuist is available
     And I have a working directory
     Then I copy the fixture invalid_workspace_manifest_name into the working directory
-    Then tuist generates yields error "Error: Manifest not found at path ${ARG_PATH}"
+    Then tuist generate yields error "Error: Manifest not found at path ${ARG_PATH}"
 
   Scenario: The project is an iOS application with frameworks and tests (ios_app_with_static_libraries)
     Given that tuist is available
@@ -174,4 +174,12 @@ Scenario: The project is an iOS application with an incompatible Xcode version (
     Given that tuist is available
     And I have a working directory
     Then I copy the fixture ios_app_with_incompatible_xcode into the working directory
-    Then tuist generates yields error "The project, which only supports the versions of Xcode 3.2.1, is not compatible with your selected version of Xcode"
+    Then tuist generate yields error "The project, which only supports the versions of Xcode 3.2.1, is not compatible with your selected version of Xcode"
+
+Scenario: The project is an iOS application with target actions
+    Given that tuist is available
+    And I have a working directory
+    Then I copy the fixture ios_app_with_actions into the working directory
+    Then tuist generates the project
+    Then the target App should have the build phase Tuist in the first position
+    Then the target App should have the build phase Rocks in the last position

features/step_definitions/shared/tuist.rb

@@ -7,16 +7,17 @@
 Then(/tuist generates the project/) do
   system("swift", "run", "tuist", "generate", "--path", @dir)
   @workspace_path = Dir.glob(File.join(@dir, "*.xcworkspace")).first
+  @xcodeproj_path = Dir.glob(File.join(@dir, "*.xcodeproj")).first
 end
 
 Then(/tuist sets up the project/) do
   system("swift", "run", "tuist", "up", "--path", @dir)
   @workspace_path = Dir.glob(File.join(@dir, "*.xcworkspace")).first
+  @xcodeproj_path = Dir.glob(File.join(@dir, "*.xcodeproj")).first
 end
 
-Then(/tuist generates yields error "(.+)"/) do |error|
-  expected_msg = error.sub!("${ARG_PATH}", @dir)
-  system("swift", "build")
+Then(/tuist generate yields error "(.+)"/) do |error|
+  expected_msg = error.gsub("${ARG_PATH}", @dir)
   _, stderr, status = Open3.capture3("swift", "run", "--skip-build", "tuist", "generate", "--path", @dir)
   actual_msg = stderr.strip
 
@@ -27,7 +28,7 @@
     Does not contain the expected:
       #{error}
   EOD
-  assert actual_msg.include?(error), error_message
+  assert actual_msg.include?(expected_msg), error_message
   refute status.success?
 end
 

features/step_definitions/shared/xcode.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'simctl'
+require 'xcodeproj'
 
 Then(/I should be able to (.+) the scheme (.+)/) do |action, scheme|
   @derived_data_path = File.join(@dir, "DerivedData")
@@ -37,3 +38,23 @@
   search_for = "#{key} = #{value}"
   assert(out.include?(search_for), "Couldn't find '#{search_for}'")
 end
+
+Then(/the target (.+) should have the build phase (.+) in the first position/) do |target_name, phase_name|
+  project = Xcodeproj::Project.open(@xcodeproj_path)
+  targets = project.targets
+  target = targets.detect { |t| t.name == target_name }
+  flunk("Target #{target_name} not found in the project") if target.nil?
+  build_phase = target.build_phases.first
+  flunk("The target #{target_name} doesn't have build phases") if build_phase.nil?
+  assert_equal phase_name, build_phase.name
+end
+
+Then(/the target (.+) should have the build phase (.+) in the last position/) do |target_name, phase_name|
+  project = Xcodeproj::Project.open(@xcodeproj_path)
+  targets = project.targets
+  target = targets.detect { |t| t.name == target_name }
+  flunk("Target #{target_name} not found in the project") if target.nil?
+  build_phase = target.build_phases.last
+  flunk("The target #{target_name} doesn't have build phases") if build_phase.nil?
+  assert_equal phase_name, build_phase.name
+end

fixtures/README.md

@@ -229,3 +229,7 @@ An iOS application with CocoaPods dependencies
 ## ios_app_with_incompatible_xcode
 
 An iOS app whose TuistConfig file requires an Xcode version that is not available in the system.
+
+## ios_app_with_actions
+
+An iOS app with a target that has pre and post actions.

fixtures/ios_app_with_actions/.gitignore

@@ -0,0 +1,64 @@
+### 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
+Pods/

fixtures/ios_app_with_actions/Info.plist

@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>CFBundleDevelopmentRegion</key>
+	<string>$(DEVELOPMENT_LANGUAGE)</string>
+	<key>CFBundleExecutable</key>
+	<string>$(EXECUTABLE_NAME)</string>
+	<key>CFBundleIdentifier</key>
+	<string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
+	<key>CFBundleInfoDictionaryVersion</key>
+	<string>6.0</string>
+	<key>CFBundleName</key>
+	<string>$(PRODUCT_NAME)</string>
+	<key>CFBundlePackageType</key>
+	<string>APPL</string>
+	<key>CFBundleShortVersionString</key>
+	<string>1.0</string>
+	<key>CFBundleVersion</key>
+	<string>1</string>
+	<key>LSRequiresIPhoneOS</key>
+	<true/>
+	<key>NSHumanReadableCopyright</key>
+	<string>Copyright ©. All rights reserved.</string>
+	<key>UIRequiredDeviceCapabilities</key>
+	<array>
+		<string>armv7</string>
+	</array>
+	<key>UISupportedInterfaceOrientations</key>
+	<array>
+		<string>UIInterfaceOrientationPortrait</string>
+		<string>UIInterfaceOrientationLandscapeLeft</string>
+		<string>UIInterfaceOrientationLandscapeRight</string>
+	</array>
+	<key>UISupportedInterfaceOrientations~ipad</key>
+	<array>
+		<string>UIInterfaceOrientationPortrait</string>
+		<string>UIInterfaceOrientationPortraitUpsideDown</string>
+		<string>UIInterfaceOrientationLandscapeLeft</string>
+		<string>UIInterfaceOrientationLandscapeRight</string>
+	</array>
+</dict>
+</plist>