Allow specifying run script and copy files phase ordering #402

brentleyjones · 2018-09-14T19:09:49Z

Fixes #400.

Allows specifying run scripts to run immediately after the Compile Sources phase (which should be used instead of postbuildScripts 99% of the time).
Allows specifying copy files phases to be run before the Compile Sources phase
- Adjusts modulemap and static library heady copy files phases to be run pre Compile Sources
- Adjusts the Carthage copy-frameworks to be run sooner

Sources/ProjectSpec/TargetSource.swift

brentleyjones · 2018-09-14T19:11:31Z

Sources/XcodeGenKit/SourceGenerator.swift

@@ -194,7 +195,8 @@ class SourceGenerator {
                guard targetType == .staticLibrary else { return nil }
                return .copyFiles(TargetSource.BuildPhase.CopyFilesSettings(
                    destination: .productsDirectory,
-                    subpath: "include/$(PRODUCT_NAME)"
+                    subpath: "include/$(PRODUCT_NAME)",
+                    phaseOrder: .preCompile


This being .preCompile is a prime example of why these changes are needed.

brentleyjones · 2018-09-14T19:58:38Z

Even with these changes the new build system is broken. That doesn't make these changes not needed though. Eventually Xcode 10 will be fixed, but being fixed will still warrant these changes.

toshi0383 · 2018-09-15T03:15:11Z

@brentleyjones

which should be used instead of postbuildScripts 99% of the time

Question: What do you expect for remaining 1% of them?

Also I think sources's buildPhase structure is getting too complicated. It should look as same as we see on Xcode.
Ideally I expect a format like this.

        buildPhase:
          copyFiles:
            destination: productsDirectory
            subpath: include
          compileSources:  // <- This simply makes the processing order clearer.
          runScript:
            path: my-script.sh

brentleyjones · 2018-09-17T11:40:42Z

Question: What do you expect for remaining 1% of them?

When you want to post-process something that has been copied into the products folder (like adjusting application icons for debug versus releases).

brentleyjones · 2018-09-17T13:21:01Z

Also I think sources's buildPhase structure is getting too complicated. It should look as same as we see on Xcode.
Ideally I expect a format like this.

With how we specify sources and run scripts, that wouldn't be possible right now.

Sources/ProjectSpec/TargetSource.swift

Sources/ProjectSpec/Target.swift

Docs/ProjectSpec.md

yonaskolb · 2018-09-19T09:58:38Z

Looks like the tests need to be updated

With Xcode 10 dependent targets won't start compiling until compilation and all run scripts are done for the target. This means that your run script should run before Copy Files (and Copy Resources, etc.) phases unless it needs to run later, to allow dependent targets to compile faster.

Will be reused similar to `generateBuildScript()` for copy files phases at different orders.

Copy Files phases don't stop dependent targets from starting to compile. If some of the files being copied are needed for the dependent target to compile then they need to happen before the Compile Sources phase. This change allows specifying that.

With Xcode 10 run scripts will prevent dependent targets from starting to compile. The Carthage copy-frameworks script should be right after compilation to speed up compilation.

Also cleanup the documentation and add more tests for the newly added `postCompileScripts`.

brentleyjones · 2018-09-19T14:25:39Z

@yonaskolb Rebased and tests fixed.

brentleyjones · 2018-09-28T17:34:03Z

@yonaskolb Care to give this another look?

yonaskolb · 2018-09-29T06:07:42Z

Sources/ProjectSpec/Target.swift

@@ -279,8 +283,9 @@ extension Target: NamedJSONDictionaryConvertible {
        directlyEmbedCarthageDependencies = jsonDictionary.json(atKeyPath: "directlyEmbedCarthageDependencies")
        requiresObjCLinking = jsonDictionary.json(atKeyPath: "requiresObjCLinking")

-        prebuildScripts = jsonDictionary.json(atKeyPath: "prebuildScripts") ?? []
-        postbuildScripts = jsonDictionary.json(atKeyPath: "postbuildScripts") ?? []
+        preBuildScripts = jsonDictionary.json(atKeyPath: "preBuildScripts") ?? jsonDictionary.json(atKeyPath: "prebuildScripts") ?? []


Makes me think the lint command in #214 could also be used to highlight deprecations

Docs/ProjectSpec.md

yonaskolb · 2018-09-29T06:09:49Z

Docs/ProjectSpec.md

@@ -275,6 +276,9 @@ A source can be provided via a string (the path) or an object of the form:
 			- `sharedSupport`
 			- `plugins`
 		- [ ] **subpath**: **String** - The path inside of the destination to copy the files.
+		- [ ] **phaseOrder**: **String** - When the Copy Files phase should execute in relation to other phases. This can be one of the following values:


Does this need to be exposed to users? Would it ever be required? I'd rather keep the options we expose low. XcodeGen is supposed to remove all those sorts of decisions.

I felt it should be exposed since a user could choose to have a specific set of resources copied before compilation. We currently have those covered, AFAIK, but others might have more exotic needs.

I'd rather leave this out for now, and only add it if it someone has a need for it 👍

@yonaskolb Coming back from WWDC I've learned that the way I'm doing module map copying currently isn't the best. While hacking through a local solution before figuring out how XcodeGen should be updated I ran into wanting to specify the phase order of the copy phase. Essentially I need to copy files before compilation. Wondering how you feel about me exposing this option now?

yonaskolb · 2018-11-02T05:36:01Z

Hi @brentleyjones. Would you be able to get this branch up to date to resolve the conflicts?

brentleyjones · 2018-11-02T14:15:24Z

@yonaskolb Yeah, I'll try to get to it today 😄.

brentleyjones · 2018-11-05T16:41:47Z

@yonaskolb Assuming tests pass, I believe this is all merged up. I also removed the ability to specify copy files phase ordering.

yonaskolb

Fantastic, thanks @brentleyjones! 😄

brentleyjones commented Sep 14, 2018

View reviewed changes

Sources/ProjectSpec/TargetSource.swift Outdated Show resolved Hide resolved

brentleyjones commented Sep 14, 2018

View reviewed changes

yonaskolb reviewed Sep 18, 2018

View reviewed changes

Sources/ProjectSpec/TargetSource.swift Outdated Show resolved Hide resolved

Sources/ProjectSpec/Target.swift Show resolved Hide resolved

Docs/ProjectSpec.md Outdated Show resolved Hide resolved

brentleyjones added 6 commits September 19, 2018 08:58

Extract copy files build phase creation

731428c

Will be reused similar to `generateBuildScript()` for copy files phases at different orders.

Update CHANGELOG.,md

6819516

Move Carthage copy-frameworks script earlier

aff1ae2

With Xcode 10 run scripts will prevent dependent targets from starting to compile. The Carthage copy-frameworks script should be right after compilation to speed up compilation.

Rename prebuildScripts and postbuildScripts

296b938

Also cleanup the documentation and add more tests for the newly added `postCompileScripts`.

brentleyjones force-pushed the copy-files-ordering branch from 61f2040 to 296b938 Compare September 19, 2018 14:25

yonaskolb reviewed Sep 29, 2018

View reviewed changes

Merge branch 'master' into copy-files-ordering

ef0c87e

brentleyjones force-pushed the copy-files-ordering branch from 88aa9e6 to ef0c87e Compare November 5, 2018 16:38

Remove ability to specify copy files phase ordering

e6288b4

brentleyjones assigned yonaskolb Nov 5, 2018

yonaskolb approved these changes Nov 8, 2018

View reviewed changes

yonaskolb merged commit 50ed421 into master Nov 8, 2018

yonaskolb deleted the copy-files-ordering branch November 8, 2018 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying run script and copy files phase ordering #402

Allow specifying run script and copy files phase ordering #402

brentleyjones commented Sep 14, 2018 •

edited

brentleyjones Sep 14, 2018

brentleyjones commented Sep 14, 2018

toshi0383 commented Sep 15, 2018 •

edited

brentleyjones commented Sep 17, 2018

brentleyjones commented Sep 17, 2018

yonaskolb commented Sep 19, 2018

brentleyjones commented Sep 19, 2018

brentleyjones commented Sep 28, 2018

yonaskolb Sep 29, 2018

yonaskolb Sep 29, 2018

brentleyjones Oct 15, 2018

yonaskolb Nov 2, 2018

brentleyjones Jun 10, 2019

yonaskolb commented Nov 2, 2018 •

edited

brentleyjones commented Nov 2, 2018

brentleyjones commented Nov 5, 2018

yonaskolb left a comment

Allow specifying run script and copy files phase ordering #402

Allow specifying run script and copy files phase ordering #402

Conversation

brentleyjones commented Sep 14, 2018 • edited

brentleyjones Sep 14, 2018

Choose a reason for hiding this comment

brentleyjones commented Sep 14, 2018

toshi0383 commented Sep 15, 2018 • edited

brentleyjones commented Sep 17, 2018

brentleyjones commented Sep 17, 2018

yonaskolb commented Sep 19, 2018

brentleyjones commented Sep 19, 2018

brentleyjones commented Sep 28, 2018

yonaskolb Sep 29, 2018

Choose a reason for hiding this comment

yonaskolb Sep 29, 2018

Choose a reason for hiding this comment

brentleyjones Oct 15, 2018

Choose a reason for hiding this comment

yonaskolb Nov 2, 2018

Choose a reason for hiding this comment

brentleyjones Jun 10, 2019

Choose a reason for hiding this comment

yonaskolb commented Nov 2, 2018 • edited

brentleyjones commented Nov 2, 2018

brentleyjones commented Nov 5, 2018

yonaskolb left a comment

Choose a reason for hiding this comment

brentleyjones commented Sep 14, 2018 •

edited

toshi0383 commented Sep 15, 2018 •

edited

yonaskolb commented Nov 2, 2018 •

edited