Spec file relative directory expansion

[ellneal]

Closes #385.

This PR started as a bug fix but ended up including quite a bit refactoring; so I welcome any feedback (I've tried to stay as true to the style of the existing code as I could).

The Problem

Included spec files still used the root spec file's directory as their base directory.

Root/
    project.yml
    Sources/
        ...
    Embedded/
        project.yml
        Sources/
            ...

In this structure, a target declared in Embedded/project.yml would have to use the path Embedded/Sources to reference the Sources directory adjacent to it.

The Fix

The first major change is that I've introduced a Spec struct that references the directory containing the spec file, the JSONDictionary content of the spec file, and any included spec files.

public struct Spec {
    public let relativePath: Path
    public let jsonDictionary: JSONDictionary
    public let subSpecs: [Spec]
}

This struct replaces the Project.loadDictionary(path:) function used before, and the Spec struct itself is used in the initialization of a Project, where previously a raw JSONDictionary was used.

The second major change is the introduction of a PathContainer protocol to abstract away the process of changing a path in an embedded file.

protocol PathContainer {
    static var pathProperties: [PathProperty] { get }
}

enum PathProperty {
    case string(String)
    case dictionary([PathProperty])
    case object(String, [PathProperty])
}

This is implemented by the model types (e.g. Target) to convert any dictionary keys that reference paths relative to the spec file. It is then used during initialization of a Project before collapsing the tree of Spec objects into a single JSONDictionary.

[yonaskolb]

Great work @ellneal! Sorry for the delay in getting to this. I've left some initial comments above.
Technically this would be a breaking change. I wonder if the default should be to keep the old paths, and opt in via an option or property on the include

[ellneal]

Thanks @yonaskolb. I would hazard a guess that this behaviour is what most people would expect, so I do think that any option param should be to opt into the legacy behaviour, rather than the other way around. Although I agree that it's a breaking change, so perhaps this should go out in a minor version bump rather than a patch version bump? I'll get to work implementing some of your feedback.

[ellneal]

@yonaskolb I've added a new option when including files to provide the legacy behaviour of not expanding paths.

[yonaskolb]

Could you also add a changelog entry saying:

Changed

• BREAKING All the paths within included files are now relative to that file and not the root spec. This can be disabled with a relativePaths: false on the include. See docs for more details

[yonaskolb]

Note to self that if these classes were Codable we could use a CodingKey for this

[ellneal]

I had the same thought, but not the motivation to do it. 👍

[yonaskolb]

Fantastic work @ellneal! I think this will make a lot of people happy

[ellneal]

My pleasure 😄

[rpassis]

FWIW I just updated to 2.2.0 and noticed a regression due to this change.

Our project configuration structure has a project.yml on the root folder of our project, and that file includes a bunch of targets that are all configured as separate files under xcodegen/*.yml:

include:
  - xcodegen/version.yml
  - xcodegen/base.yml
  - xcodegen/myapp.yml
  - xcodegen/myapp-dev.yml
  - xcodegen/myframework.yml
  ... etc

When running generate with this version I get a bunch of spec validation errors similar to the below:

- Target "MyFramework" has a missing source directory "/Users/user/dev/MyApp/xcodegen/MyFramework"

It is obviously expecting the source directories to be relative to where the yml file is located.
Not sure if you guys were aware of it but I am glad there's an opt-out flag :)

[yonaskolb]

Yeah it was a decision to make relative paths the default, as per the change log. Unfortunately this is a breaking change. You can opt out like this

include:
  - path: xcodegen/version.xml
    relativePaths: false

Would be great to know stats on which default people prefer.

[brentleyjones]

While it was a pain to migrate, I prefer the new way by a long shot.

[danielwilliamson]

Yeah same opinion here. It's rather a pain to migrate but I can see why it's needed.

[DrewKiino]

amazing update. thank you.