Support Local Swift Package by `packages:` #808

freddi-kit · 2020-03-18T12:57:40Z

Summary

Add Support Local Swift Package as Dependency by packages
Related: #796
(Refer: #796 (comment))

What Changed?

Add Support packages: for Local Swift Package as Dependency
- I added new format of packages: for local swift packages

So we can use local Swift Package by ...

name: GennedProj
...
packages:
  MyLibrary: 
    path: Packages/MyLibrary
targets:
  GennedProj:
    ...
    dependencies:
      - package: MyLibrary

note) This PR still keeps old formats of localPackages

Motivation

As we discussed at here, it might be better to support local packages not only by localPackages but also by packages.

So I added the new format for packages.

New format of `package`

As I mentioned, I create new format of packages like ...

name: GennedProj
...
packages:
  MyLibrary: 
    path: Packages/MyLibrary
targets:
  GennedProj:
    ...
    dependencies:
      - package: Packages/MyLibrary

For Testing

You can test this Pull Request by it -> https://github.com/freddi-kit/TestRepoForXcodeGenPR

freddi-kit · 2020-03-18T13:08:51Z

@yonaskolb , Thank you for discussing new format of packages before. I have question about it.

[Q] Do we need to set old localPackages: as deprecated?

I asked because this new format will be replacement of localPackages.
It can be removed in the future. I also designed this PR as easy to remove decoding localPackages part.

yonaskolb · 2020-03-19T01:16:31Z

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Next Version

 #### Added
+- Add support non-mirrored local Swift Packages in `packages`. [#808](https://github.com/yonaskolb/XcodeGen/pull/808) @freddi-kit


Let's merge these 2 entries as they will be released together, with references to both PRs
Add support for local Swift Packages

Sounds good! I'll do it.

However, it might be better to refer only #808 here. It is because localPackages:'s support will be removed in this PR and I want to prevent user's from being confused by not refering #796 .

yonaskolb · 2020-03-19T01:18:41Z

Docs/ProjectSpec.md

- [ ] **packages**: **[String: [Swift Package](#swift-package)]** - a map of Swift packages by name
- [ ] **localPackages**: **[String: [Local Swift Package](#local-swift-package)]** -  a map of local Swift packages by name. The paths must be directories with a `Package.swift` file in them. If same name remote repo is listed in `packages`, remote repo will be overridden to a local version in `localPackages` for development purposes. 
+- [ ] **packages**: **[String: [Swift Package](#swift-package)]** - a map of Swift packages by name. The paths must be directories with a `Package.swift` file if specified as local package by `path`.
+- [ ] **localPackages**: **[String: [Local Swift Package](#local-swift-package)]** -  a map of local Swift packages by name. This is same with local package at **packages** by `path`.  The paths must be directories with a `Package.swift` file in them. If same name remote repo is listed in `packages`, the repo will be overridden to a package in `localPackages` for development purposes. 


Let's remove this localPackages section, and the LocalPackage struct itself. When parsing we can still support the [String] from the existing version. No need to support LocalPackage though.

yonaskolb · 2020-03-19T01:19:23Z

Docs/ProjectSpec.md

@@ -49,8 +49,8 @@ You can also use environment variables in your configuration file, by using `${S
 - [ ] **fileGroups**: **[String]** - A list of paths to add to the root of the project. These aren't files that will be included in your targets, but that you'd like to include in the project hierachy anyway. For example a folder of xcconfig files that aren't already added by any target sources, or a Readme file.
 - [ ] **schemes**: **[Scheme](#scheme)** - A list of schemes by name. This allows more control over what is found in [Target Scheme](#target-scheme)
 - [ ] **targetTemplates**: **[String: [Target Template](#target-template)]** - a list of targets that can be used as templates for actual targets which reference them via a `template` property. They can be used to extract common target settings. Works great in combination with `include`.
- [ ] **packages**: **[String: [Swift Package](#swift-package)]** - a map of Swift packages by name
- [ ] **localPackages**: **[String: [Local Swift Package](#local-swift-package)]** -  a map of local Swift packages by name. The paths must be directories with a `Package.swift` file in them. If same name remote repo is listed in `packages`, remote repo will be overridden to a local version in `localPackages` for development purposes. 
+- [ ] **packages**: **[String: [Swift Package](#swift-package)]** - a map of Swift packages by name. The paths must be directories with a `Package.swift` file if specified as local package by `path`.


We can leave this comment to the #swift-package section

yonaskolb · 2020-03-19T01:20:03Z

Docs/ProjectSpec.md

 targets:
  App:
    dependencies:
      - package: Yams
+      - package: RxClient
 ```

 ## Local Swift Package


Let's remove this whole section

yonaskolb · 2020-03-19T01:21:07Z

Docs/Usage.md

 targets:
  App:
    dependencies:
      # by default the package product that is linked to is the same as the package name
      - package: Yams
      - package: SwiftPM
+      - package: RxClient


I can't add a comment but lets remove the You can also include local Swift Packages by referencing them by paths... section below

yonaskolb · 2020-03-19T01:23:21Z

Sources/ProjectSpec/Project.swift

-            self.localPackages = localPackages.reduce(into: [String: LocalSwiftPackage](), { $0[$1] = LocalSwiftPackage(path: $1) })
-        } else {
-            self.localPackages = [:]
+            packages.merge(localPackages.reduce(into: [String: SwiftPackage](), { $0[$1] = SwiftPackage(kind: .local(path: $1)) }))


Lets use the last path component as the name

yonaskolb · 2020-03-19T01:26:02Z

Sources/ProjectSpec/SwiftPackage.swift

@@ -7,24 +7,38 @@ public struct SwiftPackage: Equatable {

    public typealias VersionRequirement = XCRemoteSwiftPackageReference.VersionRequirement

-    public let url: String
-    public let versionRequirement: VersionRequirement
+    public enum Kind: Equatable {


We can just make the whole SwiftPackage the enum, without a need for a Kind

yonaskolb · 2020-03-19T01:42:48Z

Sources/XcodeGenKit/PBXProjGenerator.swift

@@ -802,7 +803,7 @@ public class PBXProjGenerator {

                // If package's reference is none and there is no specified package in localPackages,
                // then ignore the package specified as dependency.
-                if packageReference == nil, !project.localPackages.keys.contains(dependency.reference) {
+                if packageReference == nil, !localPackageReferences.contains(dependency.reference) {


Does this still work? Can you have just a local Package with a path, without having an actual remove and then a mirrored local package

I think this will work as is. In the first place, this validation should always be false because of project validation at SpecValidation.swift ↓.

This validation is insurance to not generate invalid projects, I think.

XcodeGen/Sources/ProjectSpec/SpecValidation.swift

Lines 176 to 178 in 59ab223

if packages[dependency.reference] == nil, localPackages[dependency.reference] == nil {

errors.append(.invalidSwiftPackage(name: dependency.reference, target: target.name))

}

note) localPackages is merged into packages at this PR

yonaskolb · 2020-03-19T01:44:24Z

Tests/ProjectSpecTests/ProjectSpecTests.swift

@@ -97,7 +97,7 @@ class ProjectSpecTests: XCTestCase {
                project.settings = invalidSettings
                project.configFiles = ["invalidConfig": "invalidConfigFile"]
                project.fileGroups = ["invalidFileGroup"]
-                project.localPackages = ["invalidLocalPackage" : LocalSwiftPackage(path: "invalidLocalPackage")]
+                project.packages = ["invalidLocalPackage" : SwiftPackage(kind: .local(path: "invalidLocalPackage"))]


Once SwiftPackage.Kind is removed and SwiftPackage becomes the enum, this because simpler throughout:

Suggested change

project.packages = ["invalidLocalPackage" : SwiftPackage(kind: .local(path: "invalidLocalPackage"))]

project.packages = ["invalidLocalPackage" : .local(path: "invalidLocalPackage")]

yonaskolb · 2020-03-19T01:45:29Z

Tests/ProjectSpecTests/SpecLoadingTests.swift

                    ],
-                    "localPackages": ["../../Package"],
+                    "localPackages": [
+                        "package10": ["path": "../XcodeGen"]


We can remove this way of specifying local packages

yonaskolb · 2020-03-19T01:56:11Z

Sources/ProjectSpec/SwiftPackage.swift

-        url = try jsonDictionary.json(atKeyPath: "url")
-        versionRequirement = try VersionRequirement(jsonDictionary: jsonDictionary)
-        try validateVersion()
+        if let url: String = jsonDictionary.json(atKeyPath: "url") {


Let's reverse this so if path is present, parse as local, otherwise remote. This let's people simply add a path to a remote definition to temporarily make it local, without having to remove url and version

…easy

- Also, added Comment

freddi-kit · 2020-03-19T16:02:52Z

@yonaskolb Thank you for your reviewing, I added comments and fixes. I checked but please tell me if I miss something.

yonaskolb

Looks good @freddi-kit! I'll merge and do a release once we fix the last very minor copy change above

Docs/ProjectSpec.md

Co-Authored-By: Yonas Kolb <yonaskolb@users.noreply.github.com>

freddi-kit · 2020-03-25T00:21:43Z

Thank you, @yonaskolb !

yonaskolb · 2020-03-25T03:02:53Z

Released in 2.15.0

freddi-kit added 16 commits March 5, 2020 20:40

Added support local package on packages:

81c2f4e

Updated Test

80fdc18

Changed validateVersion as static

77a3cf4

Updated Usage

65fd3a3

Updated Project Spec

0fc7c5a

Added validation of local package

1985392

Merge branch 'support-local-sp' into support-local-sp-with-packages

e4989a6

Fixed local packages is not linked as framework

105367b

Changed local package format of SPM project in Fixtures

80f95b5

Merge branch 'support-local-sp' into support-local-sp-with-packages

5eb4c33

Removed localPackages variable

ca8136b

Added old local package format test

31aa708

Updated Project Spec

bbbc90c

Fixed English Error

e92e8a2

Removed unnecessary space

e61dafd

Updated CHANGELOG

a05ec92

Removed LocalSwiftPackage

defd73f

brentleyjones requested a review from yonaskolb March 18, 2020 14:27

yonaskolb reviewed Mar 19, 2020

View reviewed changes

yonaskolb mentioned this pull request Mar 19, 2020

Support Local Swift Package as Dependency #796

Merged

yonaskolb reviewed Mar 19, 2020

View reviewed changes

freddi-kit added 7 commits March 19, 2020 23:29

Make SwiftPackage itself as enum

550df76

Reverted parsing order of path and url for making switch package …

a3ddb48

…easy

Removed new style of localPackages implemented at #796

6bbd90c

Removed old localPackages things from Documents

5c380bf

Resolve local package name from abstract path

af3a7e0

- Also, added Comment

Fixed description of Swift Package

c3d9e67

Update CHANGELOG

c875775

Merge branch 'master' into support-local-sp-with-packages

18efe96

freddi-kit requested a review from yonaskolb March 23, 2020 09:59

yonaskolb reviewed Mar 24, 2020

View reviewed changes

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

Update Docs/ProjectSpec.md

f024fc3

Co-Authored-By: Yonas Kolb <yonaskolb@users.noreply.github.com>

yonaskolb approved these changes Mar 25, 2020

View reviewed changes

yonaskolb merged commit e784ed7 into yonaskolb:master Mar 25, 2020

freddi-kit deleted the support-local-sp-with-packages branch March 25, 2020 00:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Local Swift Package by `packages:` #808

Support Local Swift Package by `packages:` #808

freddi-kit commented Mar 18, 2020 •

edited

Loading

freddi-kit commented Mar 18, 2020 •

edited

Loading

yonaskolb Mar 19, 2020

freddi-kit Mar 19, 2020 •

edited

Loading

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

freddi-kit Mar 19, 2020 •

edited

Loading

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

yonaskolb Mar 19, 2020

freddi-kit Mar 19, 2020

freddi-kit commented Mar 19, 2020

yonaskolb left a comment

freddi-kit commented Mar 25, 2020

yonaskolb commented Mar 25, 2020

	if packages[dependency.reference] == nil, localPackages[dependency.reference] == nil {
	errors.append(.invalidSwiftPackage(name: dependency.reference, target: target.name))
	}

	project.packages = ["invalidLocalPackage" : SwiftPackage(kind: .local(path: "invalidLocalPackage"))]
	project.packages = ["invalidLocalPackage" : .local(path: "invalidLocalPackage")]

Support Local Swift Package by packages: #808

Support Local Swift Package by packages: #808

Conversation

freddi-kit commented Mar 18, 2020 • edited Loading

Summary

What Changed?

Motivation

New format of package

For Testing

freddi-kit commented Mar 18, 2020 • edited Loading

Choose a reason for hiding this comment

freddi-kit Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freddi-kit Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freddi-kit commented Mar 19, 2020

yonaskolb left a comment

Choose a reason for hiding this comment

freddi-kit commented Mar 25, 2020

yonaskolb commented Mar 25, 2020

Support Local Swift Package by `packages:` #808

Support Local Swift Package by `packages:` #808

freddi-kit commented Mar 18, 2020 •

edited

Loading

New format of `package`

freddi-kit commented Mar 18, 2020 •

edited

Loading

freddi-kit Mar 19, 2020 •

edited

Loading

freddi-kit Mar 19, 2020 •

edited

Loading