Add support for defining `PRODUCT_NAME` in the manifest #435

ollieatkinson · 2019-07-03T19:59:56Z

Short description 📝

Specifying the product name will allow you to have a different target and product name. This is particularly useful when you want to support different versions of the same product. E.g. different platforms.

Solution 📦

Expose productName in the manifest.

Implementation 👩‍💻👨‍💻

Add productName to manifest
Update TuistGenerator to use new variable
Update Documentation
Update CHANGELOG.md

Test Plan 🛠

Generate fixtures/ios_app_with_frameworks via tuist generate
Run the following:

xcrun xcodebuild -workspace Workspace.xcworkspace -scheme Framework2-macOS -sdk macosx
xcrun xcodebuild -workspace Workspace.xcworkspace -scheme Framework2-iOS -sdk iphoneos
xcrun xcodebuild -workspace Workspace.xcworkspace -scheme Framework1 -sdk iphoneos
xcrun xcodebuild -workspace Workspace.xcworkspace -scheme Framework1 -sdk iphonesimulator
xcrun xcodebuild -workspace Workspace.xcworkspace -scheme App -sdk iphoneos 
xcrun xcodebuild -workspace Workspace.xcworkspace -scheme App -sdk iphonesimulator

Ensure all builds succeed
optional:
Open the project in Xcode
Play with it

tuistbot · 2019-07-03T20:19:21Z

	1 Warning
⚠️	Have you introduced any user-facing changes? If so, please take some time to update the documentation. Keeping the documentation up to date makes it easier for users to learn how to use Tuist.

Generated by 🚫 Danger

kwridan

👍

I was testing out fixtures/ios_app_with_frameworks - attempting to build the App, Xcode didn't build the dependencies oddly, I had to manually building each framework individually - couldn't spot the issue just yet, will take another look.

Sources/TuistKit/Generator/ManifestTargetGenerator.swift

ollieatkinson · 2019-07-04T08:01:17Z

Ah, I must have missed that as my build system is set to legacy - I see the problem when switching to the new build system. I will check the build product references there's likely one broken somewhere (using target name rather than product name).

ollieatkinson · 2019-07-04T12:32:23Z

@kwridan So it turns out there was a bug. Previously the products were store in a dictionary products which were indexed by the product name. This obviously caused problems when two product names were the same. So to fix this I instead uniqued on the target name the same way Xcode does.

Interestingly enough the Legacy build system still worked which is why I didn't see it originally, the new build system complained. Thanks for raising the issue!

codecov · 2019-07-04T12:39:53Z

Codecov Report

Merging #435 into master will increase coverage by 0.08%.
The diff coverage is 98.12%.

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   92.19%   92.28%   +0.08%     
==========================================
  Files         316      316              
  Lines       15891    15949      +58     
==========================================
+ Hits        14651    14718      +67     
+ Misses       1240     1231       -9

Impacted Files	Coverage Δ
Sources/ProjectDescription/Target.swift	`100% <100%> (ø)`	⬆️
Sources/TuistGenerator/Linter/TargetLinter.swift	`96.77% <100%> (+0.38%)`	⬆️
...es/TuistGenerator/Generator/SchemesGenerator.swift	`99.6% <100%> (ø)`	⬆️
...neratorTests/Models/TestData/Target+TestData.swift	`100% <100%> (ø)`	⬆️
Tests/TuistGeneratorTests/Graph/GraphTests.swift	`100% <100%> (ø)`	⬆️
...ts/Generator/StableStructureIntegrationTests.swift	`98.97% <100%> (+0.01%)`	⬆️
...tGeneratorTests/Generator/LinkGeneratorTests.swift	`100% <100%> (ø)`	⬆️
...rces/TuistKit/Generator/GeneratorModelLoader.swift	`89.58% <100%> (+0.09%)`	⬆️
...s/TuistKit/Generator/ManifestTargetGenerator.swift	`100% <100%> (ø)`	⬆️
...atorTests/Generator/ProjectFileElementsTests.swift	`99.63% <100%> (ø)`	⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95f7422...3907b63. Read the comment docs.

codecov · 2019-07-04T12:39:54Z

Codecov Report

Merging #435 into master will increase coverage by 0.01%.
The diff coverage is 98.07%.

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   92.19%   92.21%   +0.01%     
==========================================
  Files         316      316              
  Lines       15891    15921      +30     
==========================================
+ Hits        14651    14681      +30     
  Misses       1240     1240

Impacted Files	Coverage Δ
...rces/TuistKit/Generator/GeneratorModelLoader.swift	`89.58% <100%> (+0.09%)`	⬆️
...atorTests/Generator/ProjectFileElementsTests.swift	`99.63% <100%> (ø)`	⬆️
Sources/ProjectDescription/Target.swift	`100% <100%> (ø)`	⬆️
...s/TuistKit/Generator/ManifestTargetGenerator.swift	`100% <100%> (ø)`	⬆️
...atorTests/Generator/BuildPhaseGeneratorTests.swift	`100% <100%> (ø)`	⬆️
...es/TuistGenerator/Generator/SchemesGenerator.swift	`99.6% <100%> (ø)`	⬆️
...rator/MultipleConfigurationsIntegrationTests.swift	`96.47% <100%> (ø)`	⬆️
Tests/TuistGeneratorTests/Graph/GraphTests.swift	`100% <100%> (ø)`	⬆️
...ts/Generator/StableStructureIntegrationTests.swift	`98.97% <100%> (+0.01%)`	⬆️
Tests/ProjectDescriptionTests/TargetTests.swift	`100% <100%> (ø)`	⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f584917...7a4bc41. Read the comment docs.

… architechtures when linking

…bedded

ollieatkinson · 2019-07-04T20:20:59Z

Updated ios_app_with_transitive_framework with the issue found during integration testing:

The problem arises when the new product name to support two targets, it will look at all targets inside a manifest and embed frameworks from each target. This caused a problem when trying to support multiple platforms in one manifest.

…gets

kwridan

Great work putting this together! 👍

Few comments / questions :) let me know if I can help with any of them.

Sources/TuistGenerator/Generator/ConfigGenerator.swift

Sources/TuistGenerator/Graph/Graph.swift

Sources/TuistGenerator/Models/Target.swift

Tests/TuistGeneratorTests/Generator/BuildPhaseGeneratorTests.swift

Tests/TuistGeneratorTests/Generator/TargetGeneratorTests.swift

Tests/TuistGeneratorTests/Graph/GraphTests.swift

fixtures/ios_app_with_transitive_framework/Framework1/Sources/Framework1File.swift

Sources/TuistGenerator/Graph/Graph.swift

…from a single embed phase

kwridan

🚀

kwridan · 2019-07-05T12:10:45Z

Tests/TuistGeneratorTests/Linter/TargetLinterTests.swift

+            XCTAssertTrue(got.contains(LintingIssue(reason: reason, severity: .error)))
+        }
+
+        let XCTAssertValidProductName: (String) -> Void = { bundleId in


ollieatkinson force-pushed the product-name branch 2 times, most recently from 3b0a960 to 958e4d2 Compare July 3, 2019 20:17

kwridan reviewed Jul 3, 2019

View reviewed changes

Sources/TuistKit/Generator/ManifestTargetGenerator.swift Outdated Show resolved Hide resolved

ollieatkinson added 7 commits July 4, 2019 13:26

Support PRODUCT_NAME in manifest

d8ce237

swiftformat .

8f2ebf9

Update Fixture

daac41c

Unique products on target name rather than product name

714abb0

swiftformat .

2e1695c

Update product name for manifest

7301e37

Update fixture to test building for the different platforms

3430c9a

ollieatkinson marked this pull request as ready for review July 4, 2019 12:27

Update CHANGELOG and docs

be27efa

ollieatkinson force-pushed the product-name branch from 7a4bc41 to be27efa Compare July 4, 2019 12:47

ollieatkinson requested a review from a team July 4, 2019 13:18

Only link direct dependencies of the target node, check for framework…

1f6b89a

… architechtures when linking

ollieatkinson force-pushed the product-name branch from fea9f62 to 1f6b89a Compare July 4, 2019 19:46

Update fixture to cover scenario causing the wrong framework to be em…

2276bc7

…bedded

ollieatkinson added 3 commits July 4, 2019 21:22

Update fixture to show problem with embedding supporting multiple tar…

ac3006d

…gets

swiftformat .

1f4f684

Update fixture to reference correct target name

cdff8e2

kwridan reviewed Jul 5, 2019

View reviewed changes

ollieatkinson added 2 commits July 5, 2019 08:38

Ensure tests are built to cover problem with multiple build products …

b671aa9

…from a single embed phase

Code Review

3907b63

kwridan approved these changes Jul 5, 2019

View reviewed changes

ollieatkinson merged commit 3453db8 into master Jul 5, 2019

ollieatkinson deleted the product-name branch July 5, 2019 19:17

moritzsternemann mentioned this pull request Sep 10, 2021

Support Reverse-Domain-Name Style Named SPM Dependencies #3432

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for defining `PRODUCT_NAME` in the manifest #435

Add support for defining `PRODUCT_NAME` in the manifest #435

ollieatkinson commented Jul 3, 2019 •

edited

tuistbot commented Jul 3, 2019 •

edited

kwridan left a comment

ollieatkinson commented Jul 4, 2019

ollieatkinson commented Jul 4, 2019

codecov bot commented Jul 4, 2019 •

edited

codecov bot commented Jul 4, 2019

ollieatkinson commented Jul 4, 2019

kwridan left a comment

kwridan left a comment

kwridan Jul 5, 2019

Add support for defining PRODUCT_NAME in the manifest #435

Add support for defining PRODUCT_NAME in the manifest #435

Conversation

ollieatkinson commented Jul 3, 2019 • edited

Short description 📝

Solution 📦

Implementation 👩‍💻👨‍💻

Test Plan 🛠

tuistbot commented Jul 3, 2019 • edited

kwridan left a comment

Choose a reason for hiding this comment

ollieatkinson commented Jul 4, 2019

ollieatkinson commented Jul 4, 2019

codecov bot commented Jul 4, 2019 • edited

Codecov Report

codecov bot commented Jul 4, 2019

Codecov Report

ollieatkinson commented Jul 4, 2019

kwridan left a comment

Choose a reason for hiding this comment

kwridan left a comment

Choose a reason for hiding this comment

kwridan Jul 5, 2019

Choose a reason for hiding this comment

Add support for defining `PRODUCT_NAME` in the manifest #435

Add support for defining `PRODUCT_NAME` in the manifest #435

ollieatkinson commented Jul 3, 2019 •

edited

tuistbot commented Jul 3, 2019 •

edited

codecov bot commented Jul 4, 2019 •

edited