Add caching #412

yonaskolb · 2018-09-29T07:07:47Z

EDIT: Renamed feature to caching

This is a start at resolving #409, and solving the problem of generating projects when they don't need to be. This makes using xcodegen in a git-checkout hook much better

How it currently works is you have to pass a --lockfile argument to xcodegen specifying a file it should read and write to for determining if the project has changed. eg xcodegen --lockfile project.lock. This file would be added to .gitignore.

If this file exists it is read before generation. Only if any of the following has changed will it continue with generation:

the xcodegen version
the fully resolved spec (including includes)
any files the the spec references have been added or removed, via project filegroups, targets sources and xconfig files

I'm not sold on the idea of the name lockfile. Not sure if the filename should be configurable either or just be a default filename, or configurable via the spec itself?

Another possible enhancement would be handling only running pod install if no generation occurs.
This could happen via:

not doing anything and making the user parse the stdout manually to check
a --pod-install flag that automatically runs pod install
xcodegen erroring if the project isn't generated
?

Tasks:

yonaskolb · 2018-09-29T07:12:12Z

Sources/ProjectSpec/Project.swift

+            files.append(contentsOf: target.configFilePaths)
+            for source in target.sources {
+                let sourcePath = basePath + source.path
+                let sourceChildren = (try? sourcePath.recursiveChildren()) ?? []


I guess I could filter out directories?

yonaskolb · 2018-09-29T07:20:59Z

project.lock

@@ -0,0 +1,348 @@
+# XCODEGEN VERSION
+1.11.2


This is an example of a lockfile

keith · 2018-10-01T20:15:14Z

I get this error when trying to use this:

Couldn't serialize spec for lockfile

keith · 2018-10-01T20:16:01Z

I added the specific error to the log and got this:

Couldn't serialize spec for lockfile Failed to represent {
    Alpha = release;
    Beta = release;
    Debug = debug;
    Labs = release;
    Profile = release;
    QA = debug;
    Release = release;
}

keith · 2018-10-01T20:16:34Z

We can test this once this is working fine, but in our experience using the json project spec instead of the yaml one was significantly faster, that might make sense to do for the lockfile too, especially since the format of it shouldn't matter for consumers

keith · 2018-10-01T20:37:39Z

Ok with this change (we'd have to solve the 10.13 issue some other way):

diff --git i/Sources/XcodeGen/main.swift w/Sources/XcodeGen/main.swift
index 132c48b..dcf7caf 100644
--- i/Sources/XcodeGen/main.swift
+++ w/Sources/XcodeGen/main.swift
@@ -44,7 +44,7 @@ func fatalError(_ message: String) -> Never {
     // Lock file
     var lockFileContent: String = ""
     let lockFilePath = lockfile.isEmpty ? nil : Path(lockfile)
-    if let lockFilePath = lockFilePath {
+    if let lockFilePath = lockFilePath, #available(macOS 10.13, *) {
 
         let files = Array(Set(project.allFiles))
             .map { $0.byRemovingBase(path: project.basePath).string }
@@ -53,8 +53,9 @@ if let lockFilePath = lockFilePath {
 
         let spec: String
         do {
-            let node = try Node(projectDictionary)
-            spec = try Yams.serialize(node: node)
+            let data = try JSONSerialization.data(withJSONObject: projectDictionary,
+                                                  options: [.sortedKeys,.prettyPrinted])
+            spec = String(data: data, encoding: .utf8)!
         } catch {
             fatalError("Couldn't serialize spec for lockfile")
         }

I was able to use this successfully. Here are our timings:

% time ~/dev/XcodeGen/.build/release/xcodegen --quiet --spec project_generated.json --lockfile foo.lock
~/dev/XcodeGen/.build/release/xcodegen --quiet --spec project_generated.json   15.74s user 1.08s system 99% cpu 16.863 total
% time ~/dev/XcodeGen/.build/release/xcodegen --quiet --spec project_generated.json --lockfile foo.lock
~/dev/XcodeGen/.build/release/xcodegen --quiet --spec project_generated.json   2.39s user 0.30s system 99% cpu 2.697 total

We're currently on 1.11.1 and our gen time on my machine is ~13 seconds. So we're seeing a pretty serious hit here. I think the no-op time is likely a little higher than we'd want. Maybe that's different because of my change though. Happy to help profile more if you'd like.

toshi0383 · 2018-10-01T23:30:55Z

--lockfile

Is there any reason why it's opt-in, instead of defaulting to somewhere like ~/.xcodegen/{unique-hash-from-project-path} ?
I thought it's better to "use cache by default".

--pod-install

I don't think we should know about how to run other third party commands. Especially cocoapods does a lot of complicated stuff, so I would leave it alone, not adding --pod-install.
BTW I use cocoa pods via bundler by bundle exec pod install to make sure to stick with specific version of cocoapods.

ryohey · 2018-10-14T04:16:24Z

I tried it for now. It works fine with a project with 3000 source files. awesome!
I compared the generation time with lockfile with and without lockfile, there is a variation in measurement, but there seemed to be no problem.

$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
        7.52 real         6.82 user         0.44 sys
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
        1.59 real         0.78 user         0.09 sys
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
        0.61 real         0.51 user         0.07 sys
$ time ./XcodeGen --spec project.yml --quiet
        6.87 real         6.23 user         0.39 sys
$ time ./XcodeGen --spec project.yml --quiet
       10.82 real         8.00 user         0.49 sys
$ rm project.lock
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
       12.68 real         9.10 user         0.49 sys
$ rm project.lock
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
       12.24 real         8.90 user         0.62 sys
$ rm project.lock
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
        6.00 real         5.58 user         0.35 sys
$ rm project.lock
$ time ./XcodeGen --spec project.yml --lockfile project.lock --quiet
        6.34 real         5.83 user         0.39 sys

yonaskolb · 2018-10-23T10:38:43Z

@keith yeah json will be better 👍

@toshi0383 it's opt in for now as it's just a test, but will be opt out if it runs performantly and without issues. I like your idea of writing the cache file to a special directory. That way people don't have to bother adding it to their .gitignore.
I agree to not calling pod install explicitly. We still need a way to run a command after project generation (or rather not run a command if it doesn't generate).

Any ideas for another name for this functionality? Maybe cache file, diff file, project hash?

toshi0383 · 2018-10-24T01:51:00Z

We still need a way to run a command after project generation

Why don't we just do xcodegen && rm -rf Pods/; pod install ?

yonaskolb · 2018-10-24T02:33:40Z

@toshi0383 because you wouldn't want to run an unnecessary and lengthy pod install if the project hasn't changed.
One option is to add an argument to xcodegen that makes it fail if it doesn't generate anything due to no changes found. You could make it fail be default but that might be weird and unexpected?

toshi0383 · 2018-10-24T05:37:39Z

Or we can add option like --exec ?
xcodegen --exec pod install
xcodegen --exec echo $(rm -rf Pods/; pod install)

It gets a little complex when you want to execute multiple commands.

ref: https://github.com/sharkdp/fd#parallel-command-execution

keith · 2018-10-24T05:39:02Z

If possible I think XcodeGen should avoid tying itself to CocoaPods or any other tool that isn't required for using XcodeGen itself

toshi0383 · 2018-10-24T12:59:09Z

--exec is too complex. Please forget.

One option is to add an argument to xcodegen that makes it fail if it doesn't generate anything due to no changes found. You could make it fail be default but that might be weird and unexpected?

I'm 👌 to this option. Maybe with optional exit code from user.
e.g. --gen-or-fail[=n]

yonaskolb · 2018-11-03T12:09:44Z

I've updated and renamed to --use-cache which saves the file to ~/.xcodegen/cache/{project_spec_path_hash}.
This could be turned on by default if all is well

yonaskolb · 2018-12-01T07:48:35Z

Also added a --cache-path for a custom path. This will allow more flexibility for places like a CI server

yonaskolb · 2018-12-01T07:49:17Z

Still not sure on the naming of this feature. Does anyone have any thoughts on it?

yonaskolb mentioned this pull request Sep 29, 2018

Project lock file #409

Closed

yonaskolb requested review from brentleyjones, keith, rahul-malik, ryohey and toshi0383 September 29, 2018 07:09

yonaskolb commented Sep 29, 2018

View reviewed changes

project.lock Outdated

@@ -0,0 +1,348 @@

# XCODEGEN VERSION

1.11.2

Copy link

Owner Author

yonaskolb Sep 29, 2018

Choose a reason for hiding this comment

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

This is an example of a lockfile

yonaskolb force-pushed the lockfile branch from 6d26aa8 to 08bb28f Compare September 29, 2018 07:22

yonaskolb force-pushed the lockfile branch 2 times, most recently from f513dd8 to b5d3fb5 Compare November 3, 2018 12:08

yonaskolb force-pushed the lockfile branch from b5d3fb5 to 1cefc78 Compare November 4, 2018 02:50

yonaskolb added 5 commits November 8, 2018 21:35

add lockfile

e30aeee

use json for serialization

ca5ad9b

rename to cache and refactor

f9be03e

update changelog

4560a7d

add cache fixture perf test

0644f18

yonaskolb added 3 commits November 8, 2018 21:37

check if project exists before reading from cache

8b5ac6f

don't fail if cache file can't be created

4b9edce

Merge master into lockfile

20eaafd

yonaskolb force-pushed the lockfile branch from 480562b to 20eaafd Compare November 12, 2018 12:07

add cache-path argument

c4b9b19

yonaskolb changed the title ~~Add lockfile~~ Add caching Dec 5, 2018

yonaskolb added 2 commits December 5, 2018 19:36

fix project not generating if private settings are checked in

ed80164

update documentation

cf32c87

yonaskolb force-pushed the lockfile branch from 36fb737 to cf32c87 Compare December 5, 2018 08:36

yonaskolb added 2 commits December 18, 2018 20:54

Merge master into lockfile

f061c03

update docs

c8db304

yonaskolb force-pushed the lockfile branch from b971cbe to c8db304 Compare December 18, 2018 11:17

yonaskolb merged commit 4cc4906 into master Dec 18, 2018

yonaskolb deleted the lockfile branch December 18, 2018 11:32

yonaskolb mentioned this pull request Jan 7, 2019

Add postGen hook #479

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add caching #412

Add caching #412

yonaskolb commented Sep 29, 2018 •

edited

Loading

yonaskolb Sep 29, 2018

yonaskolb Sep 29, 2018

keith commented Oct 1, 2018 •

edited

Loading

keith commented Oct 1, 2018

keith commented Oct 1, 2018

keith commented Oct 1, 2018

toshi0383 commented Oct 1, 2018

ryohey commented Oct 14, 2018

yonaskolb commented Oct 23, 2018

toshi0383 commented Oct 24, 2018

yonaskolb commented Oct 24, 2018

toshi0383 commented Oct 24, 2018

keith commented Oct 24, 2018

toshi0383 commented Oct 24, 2018

yonaskolb commented Nov 3, 2018

yonaskolb commented Dec 1, 2018

yonaskolb commented Dec 1, 2018

Add caching #412

Add caching #412

Conversation

yonaskolb commented Sep 29, 2018 • edited Loading

yonaskolb Sep 29, 2018

Choose a reason for hiding this comment

yonaskolb Sep 29, 2018

Choose a reason for hiding this comment

keith commented Oct 1, 2018 • edited Loading

keith commented Oct 1, 2018

keith commented Oct 1, 2018

keith commented Oct 1, 2018

toshi0383 commented Oct 1, 2018

ryohey commented Oct 14, 2018

yonaskolb commented Oct 23, 2018

toshi0383 commented Oct 24, 2018

yonaskolb commented Oct 24, 2018

toshi0383 commented Oct 24, 2018

keith commented Oct 24, 2018

toshi0383 commented Oct 24, 2018

yonaskolb commented Nov 3, 2018

yonaskolb commented Dec 1, 2018

yonaskolb commented Dec 1, 2018

yonaskolb commented Sep 29, 2018 •

edited

Loading

keith commented Oct 1, 2018 •

edited

Loading