Skip to content
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

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

Merged
merged 8 commits into from Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions Gemfile
Expand Up @@ -16,3 +16,4 @@ 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"
1 change: 1 addition & 0 deletions Gemfile.lock
Expand Up @@ -233,6 +233,7 @@ DEPENDENCIES
rake (~> 12.3)
rubocop (~> 0.74.0)
simctl (~> 1.6)
xcodeproj (~> 1.12)

BUNDLED WITH
1.17.3
25 changes: 14 additions & 11 deletions Sources/TuistGenerator/Generator/BuildPhaseGenerator.swift
Expand Up @@ -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 genreated project.
pepicrft marked this conversation as resolved.
Show resolved Hide resolved
/// - 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 {
Expand All @@ -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,
Expand All @@ -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],
Expand Down
12 changes: 12 additions & 0 deletions Sources/TuistGenerator/Generator/TargetGenerator.swift
Expand Up @@ -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,
Expand Down Expand Up @@ -95,6 +101,12 @@ final class TargetGenerator: TargetGenerating {
sourceRootPath: sourceRootPath,
graph: graph,
system: system)

/// Pos actions
pepicrft marked this conversation as resolved.
Show resolved Hide resolved
try buildPhaseGenerator.generateActions(actions: target.actions.postActions,
pbxTarget: pbxTarget,
pbxproj: pbxproj,
sourceRootPath: sourceRootPath)
return pbxTarget
}

Expand Down
33 changes: 0 additions & 33 deletions Tests/TuistGeneratorTests/Generator/BuildPhaseGeneratorTests.swift
Expand Up @@ -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")
Expand Down
42 changes: 42 additions & 0 deletions Tests/TuistGeneratorTests/Generator/TargetGeneratorTests.swift
Expand Up @@ -107,6 +107,48 @@ final class TargetGeneratorTests: XCTestCase {
])
}

func test_generateTarget_actions() throws {
/// Given
pepicrft marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down
12 changes: 10 additions & 2 deletions features/generate.feature
Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parse am I right defining in this test what you expect in the generated project?

Copy link

Choose a reason for hiding this comment

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

Correct!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! With this test in place we'll make sure we don't introduce regressions in the future.

Then the target App should have the build phase Rocks in the last position
9 changes: 5 additions & 4 deletions features/step_definitions/shared/tuist.rb
Expand Up @@ -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

Expand All @@ -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

Expand Down
21 changes: 21 additions & 0 deletions 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")
Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions fixtures/README.md
Expand Up @@ -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.
64 changes: 64 additions & 0 deletions 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/
43 changes: 43 additions & 0 deletions 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>