Add support for multiple platforms

[tmspzz]

Enhancement Suggestion

Add support for more platforms (watchOS, tvOS, macOS)

Current and suggested behavior

Currently only iOS is supported, this should be expanded to other platform too.

Why would the enhancement be useful to most users

This enhancement should be useful for frameworks that target more than 1 platform.

Rome version: v0.7.1.13
OS and version: Mac OS 10.11.6 - El Capitan

[r-peck]

How do you envision this working with respect to the list command?

[tmspzz]

@r-peck how about a switch just like carthage? like list [--platform iOS]

in the Romefile one could specify a default-platform key

[r-peck]

So for the --missing/--present options, should a platform or list of platforms be required? If not, should having a cached version for any platform be counted?

Without the presence option it could probably just list broken out by platform.

[r-peck]

It feels a little odd since you may have frameworks that don't support a given platform and that you don't care to use there (maybe a framework that's only used in your main application but not in your watch app).

[tmspzz]

I think it's appropriate to consider how carthage handles this.

By default carthage is platform agnostic and will perform the given command for all platforms in sequence unless a --platform [list, of, platforms] option is given to the command.

That said let's imagine how this would work for Rome.

• rome upload awesome-cat-names will upload awesome-cat-names for all platforms
• rome upload --platform iOS watchOS awesome-cat-names will upload awesome-cat-names for both iOS and watch.
• rome download awesome-cat-names will download awesome-cat-names for all platforms
• rome download --platform iOS watchOS awesome-cat-names will download awesome-cat-names for both iOS and watch
• rome list [--present | missing] lists frameworks per platform (Note: section dividers and overall output must be easy to parse since other processes might rely on being able to understand the output)
• rome list [--present | missing] --platform iOS watchOS lists frameworks restricted to the iOS and watchOS platform (Note: section dividers and overall output must be easy to parse since other processes might rely on being able to understand the output)
• rome list [--present | missing] --platform tvOS lists frameworks restricted to the tvOS platform only (Note: Should there be a section header here? If yes overall output must be easy to parse since other processes might rely on being able to understand the output)

Now, let's consider a few examples:

Example 1:

• You have an iOS app with watchOS companion app
• You have awesome-cat-names that builds AwesomeCatNames-iOS.framework and AwesomeCatNames-watchOS.framework
• In your Romefile RepositoryMap section you have awesome-cat-names = AwesomeCatNames-iOS, AwesomeCatNames-watchOS
• rome download will try to download both AND macOS, tvOS (this fails for the last two platforms. No big deal)
• rome upload will try to upload both AND macOS, tvOS (this fails for the last two platforms. No big deal)
• rome list --missing --platform iOS watchOS (no frameworks are reported missing)

Example 2:

• You have a macOS app
• You have awesome-cat-names that builds AwesomeCatNames-macOS.framework
• In your Romefile RepositoryMap section you have awesome-cat-names = AwesomeCatNames-macOS
• rome download will try to download all platforms (this fails for the 3 platforms. No big deal)
• rome upload will try to upload all platforms (this fails for 3 platforms. No big deal)
• rome list --missing --platform macOS (no frameworks are reported missing)

Seems to me that on the contrary of what I initially suggested there is no need for a default platform.

Regarding the cache structure I think it should mirror the way carthage does it with Build/<platform>/Name.framework

@r-peck what do you think?

I will soon merge #41 that should also work transparently with the multi platform feature

[r-peck]

That mirrors my thinking for the most part. My biggest concern was with changing the listing format:

• Let's say you've got an app with A.framework and B.framework that also embeds a watch app that uses A.framework. B.framework is not available for watchOS.
• rome list --missing might have something like iOS:;watchOS:B (not actual suggested format, but just an example of something parseable containing the needed info).
• It would be on the user to run with the appropriate platform for the information they are needing (if the expected availability is different per-platform, scripting that uses list needs to be done per-platform).
• Existing users might have scripts that incorrectly indicate something is missing because they depend on the old output format. This could break things in a quiet way.
• If the command requires the platform to be specified (at least for --missing and --present where the current output is just a list), the output format could change while breaking existing scripts in a more obvious way.

[tmspzz]

I agree that it is upon the user to run the command for the appropriate platform. With the current Romefile format there is no way to specify frameworks per platform. This is doable but it would mean going in a completely different direction.

Wether a platform argument should be specified or not for the list command it will still be a breaking change. I'm afraid that there is not easy way around it. The breaking change should also be indicated with a major change in version number (0.8 to 0.9).

If rome refuses to list without a platform argument it will still output an error (form the CLI command parser) and the error will still be parsed by whatever is next.

Example with required platform (thus bad command):
rome list | build_missing will still continue to build_missing even if rome exits with non zero status (which currently is not the case, upon bad command rome still exits with zero). Garbage will be fed to build_missing and the build will eventually fail.

Example with no required platform:
rome list | build_missing will still continue to build_missing feeding something that the script will not parse and the build will eventually fail.

So builds will fail anyways and once the user realises that missing dependencies have not been downloaded it should give them an hint of where things have failed.

For the sake or consistency, I would prefer to have all commands behave the same where possible. There is no "semantic" reason (i.e. the command does not make sense) why list would require a platform.

@r-peck do you see a better way? Am I missing something?

[r-peck]

Makes sense to me.

[tmspzz]

Great! Thanks for helping defining this :)

[tmspzz]

Heads up, #41 was merged and #38 closed. This restructures some stuff.

[erichoracek]

Thanks for moving this proposal forwards guys! It's something we'll be needing soon. A few questions/comments:

With respect to argument style, to match the style of Carthage, I believe that this would be --platform iOS,watchOS. See Carthage/Carthage#726.

With respect to the missing/present behavior, I have a question that I believe will affect the implementation of this. Specifically, can you elaborate on how you will know whether a dependency does not produce a framework for a specific architecture vs. the dependency not being present in the cache?

For example, say that frameworkA builds for watchOS and iOS, while frameworkB builds for just iOS. How will rome list --missing know that frameworkA is missing for watchOS while frameworkB is not missing for watchOS since it does not produce a framework for it?

As far as I can tell, this would require either:

• Parsing Xcode projects from the carthage checkouts (this assumes that the user has a local carthage checkout)
• Adding all possible target paths to the Romefile RepositoryMap

Let me know if I've missed something. Thanks!

[tmspzz]

I think --platform in carthage works with or without the comma, or if it doesn't I never noticed :D

The behaviour with list [--missing | present] is problematic only in the case of multiple platforms being used in the same project at the same time. This is the case for and iOS & watchOS app or for a frameworks that builds for all platforms.

Assumptions

Let's assume the situation that @erichoracek described:

• frameworkA builds for watchOS and iOS, while frameworkB builds for just iOS
• list output is in the format: <framework-name>: ((+|-)<platform>)?

Solutions

Solution 1 - Ignore the problem

The easiest way to solve this is to ignore the problem just like carthage does.

This means that the --platform switch an it's argument are really just a tip for where to look in the cache for a certain artefact.

Example:

$ rome list
frameworkB: +iOS, -watchOS
frameworkA: +iOS, +watchOS

$ rome list --platform iOS, watchOS
frameworkB: +iOS, -watchOS
frameworkA: +iOS, +watchOS

$ rome list --missing
frameworkB: -watchOS

$rome list --missing --platform watchOS
frameworkB: -watchOS #this is not really missing see assumptions

In a few cases frameworkB is a false positive missing which means that if the output is further used to trigger builds for the missing dependencies in the cache it will trigger a build for frameworkB for platform watchOS. However In the assumptions frameworkB does not have a watchOS target in the first place so triggering this build should do nothing. This is the case if you would try to trigger a carthage build frameworkB --platform watchOS

The real issue here is if frameworkB actually does have a watchOS target that you're just not using. This will trigger a unnecessary build.

Solution 2 - [IgnoreMap] per platform

This solutions entails creating an [IgnoreMap] section per platform.
The INI file format does not support nested sections so in a Romefile one would have:

• optional global [IgnoreMap]
• optional per platform [IgnoreMap-<platform>]

Example:

#Romefile - other parts omitted
[IgnoreMap-watchOS]
frameworkB = frameworkB

$rome list --missing --platform watchOS

Solution 3 - [IgnoreMap] and [RepositoyMap] per platform

This solutions entails creating an [IgnoreMap] and a [RepositoyMap] section per platform.
The INI file format does not support nested sections so in a Romefile one would have:

• optional global [RepositoyMap]
• optional per platform [RepositoyMap-<platform>]
• optional global [IgnoreMap]
• optional per platform [IgnoreMap-<platform>]

#Romefile - other parts omitted
[RepositoyMap]
frameworkA = frameworkA # this is a global dependency

[RepositoryMap-iOS]
frameworkB = frameworkB

or

#Romefile - other parts omitted
[RepositoyMap-iOS]
frameworkB = frameworkB
frameworkA = frameworkA #this is really a global dependency but the user made a mistake

[RepositoyMap-watchOS]
frameworkA = frameworkA #this is really a global dependency but the user made a mistake

or

#Romefile - other parts omitted
[RepositoyMap]
frameworkB = frameworkB # this is NOT a global dependency but the user made a mistake 
frameworkA = frameworkA # this is a global dependency

[IgnoreMap-watchOS]
frameworkB = frameworkB

$rome list --missing --platform watchOS

I believe that here there are other scenarios I can't think of right now.

Solution 4 - DependencyMap global and per platform

This solutions introduces a new optional section in which dependencies for a given platform are specified.

This replicates Solution 3 but adds another section leaving [RepositoryMap] and [IgnoreMap] as they are instead of abusing them to implicitly specify dependencies. They idea with [RepositoryMap] is just to provide a name resolution system for repository<->frameworkName(s).

#Romefile - other parts omitted

[RepositoyMap]
frameworkB = frameworkB
frameworkA = frameworkA

[IgnoreMap]
xcconfig =  xcconfig

[DependencyMap]
frameworkA = frameworkA #global dependency

[DependencyMap-iOS]
frameworkB = frameworkB

$rome list --missing --platform watchOS

Observations

• Solution 1 is the easiest but might trigger unnecessary builds
• Solution 2 is abuses the IgnoreMap to implicitly specify dependencies
• Solution 3 is exponentially complex and error prone imho
• Solution 4 is clear but you the [IgnoreMap] remains global. Maybe it's worth having one per platform ? See solution 2 concerns.

@erichoracek @r-peck what do you guys think?

[r-peck]

For the platforms delimiter, Carthage documents requiring comma. It will, however, handle spaces as long as the list is treated as a single argument by the shell (enclosed in quotes, for example).

On listing per-platform, Solutions 2-4 are additive to solution 1 since not specifying something in the Romefile would fall back to ignoring. I would lean toward get the functionality in place first (solution 1), then enhance with solution 4.

[r-peck]

@blender @erichoracek Any strong feelings about the updated list output format, especially regarding sectioning—how are sections delimited, should it nest frameworks per platform or platforms per framework, etc.

[tmspzz]

@r-peck I can get behind Solution 1 which solves the problem for the time being and is also retro-compatible

I suggest the following format that also gets rid of headers completely:

<framework-name>: ((+|-)<platform>)?

Example:

$ rome list
FrameworkA: +iOS, +watchOS
FrameworkB: -iOS, +watchOS

$rome list --present
FrameworkA: +iOS, +watchOS

$rome list --missing
FrameworkB:  -iOS

$rome list --present --platform watchOS
FrameworkA: +watchOS

[r-peck]

From an implementation standpoint, I like the simplicity of that, and I like that it is consistent in all modes (default, missing, present). Would it make sense to include an example in the readme of how to extract a list of frameworks to forward to Carthage for a given platform since this will be less straightforward than with the current list --missing behavior of just printing the list?

[tmspzz]

Sure or a output formatter flag could be provided like --format=carthage. I guess that for the sake of a simple first implementation it's easier to provide instructions in the Readme.

[tmspzz]

I have added a ready-to-implement label that I will add to this issue once we agree on the design of this feature. I would like to wait for @erichoracek comments on both the solution approch and the output format. Then give it a few days to sink in. Hopefully all of this can happen before the weekend.

I would encourage anyone to try to sketch the CLI command and the output in a textual format. I find that it really helps to simulate the usage & desired outcome.

If anyone is interested in implementing this please let me know here now or once the ready-to-implement label is applied.

[r-peck]

@blender I had actually taken a stab at the upload/download part of it, but stopped short of listing since that seemed to be the most contentious area. Also, if upload/download does change direction, I'm not worried about the lost effort.

[tmspzz]

That is awesome! I don't think upload / download behavior will change anyhow. It's pretty clear that the only real problem is list and what the output format should look like.

I have also realized that the current output of list also reports the version of the framework. So a more compete format definition would be <framework-name> <version-hash>: ((+|-)<platform>)?

[erichoracek]

Of all of the options, I agree that Solution 1 is the most reasonable in terms of complexity and as a first pass. We may have to update our script that does a carthage build for each Rome cache miss to run once for each platform (rather than building both at the same time) to prevent unnecessary builds. However, this doesn't seem like too much of an issue for me.

One small comment with respect to output. I feel that if a single platform is specified, platform should not be included after the framework name, as it is redundant with what the user just typed. E.g.:

$rome list --present
FrameworkA: +iOS, +watchOS -tvOS, -macOS
FrameworkB: +iOS, -watchOS -tvOS, -macOS

$rome list --present --platform watchOS
FrameworkA

$rome list --present --platform iOS
FrameworkA
FrameworkB

Something like --format=carthage feels a bit like overkill until it's explicitly requested. An example of how to strip trailing platforms from the output in the README seems like the way to go.

Thanks for the detailed responses!

[tmspzz]

It seems we have an agreement for Solution 1, a.k.a ignore the problem.

I let this sink in for a bit and these are my comments

Clarification about presence switches --missing|--present

@erichoracek I think we have a misunderstanding on$rome list --present from what I see in from your first command. The --present switch will only report cache hits per framework per platform. So the actual output should be.

$rome list --present
FrameworkA : +iOS, +watchOS
FrameworkB : +iOS

Since FrameworkA is not a cache hit for tvOS nor macOS and FrameworkB is not a cache hit for -watchOS, tvOS and macOS.

Thus the output for --missing should be

$rome list --missing
FrameworkA : -tvOS, -macOS
FrameworkB : -watchOS, -tvOS, -macOS

If you don't activate any presence switch (--missing|--present) then you would get your output

$rome list 
FrameworkA : +iOS, +watchOS -tvOS, -macOS
FrameworkB : +iOS, -watchOS -tvOS, -macOS

I think what if the presence switches (--missing|--present) don't filter per framework per platform but just per framework then eventually all frameworks will be reported as either missing or present for some platform, thus rendering the filters useless in practice.

Simplifying output when only one platform is present in the --platform filter parameters

@erichoracek Unfortunately with the introduction of the platforms I think repeating the platform name is necessary simply because if you don't activate a presence filter the output doesn't make sense.

Example:

$rome list --present --platform iOS #note the --present 
FrameworkA #makes sense, FrameworkA is present
FrameworkB  #makes sense, FrameworkB is present

$rome list --platform watchOS #note the --present or --missing are omitted 
FrameworkA #does NOT makes sense, is FrameworkA present or missing?
FrameworkB  #does NOT makes sense, is FrameworkB present or missing?

I think a uniform solution solves this elegantly

$rome list --present --platform iOS #note the --present 
FrameworkA : +iOS #makes sense, FrameworkA is present
FrameworkB : +iOS  #makes sense, FrameworkB is present

$rome list --platform watchOS #note the --present or --missing are omitted 
FrameworkA : +watchOS #makes sense, FrameworkA is present
FrameworkB : -watchOS  #makes sense, FrameworkB is missing

@r-peck suggested to provide examples of how to parse this in the README so this should ease some pain for the users.

On another note as I mentioned before when listing all the current output also echoes the versions back. I think there is value in this since it does not require whoever/whatever will read the output to
read again the Cartfile.resolved to figure out the version numbers.

#rome 0.8.0.17

$ rome list
Alamofire 3.5.1 ✔︎
GCDKit 1.3.0 ✔︎
HockeySDK-iOS 3.8.6 ✔︎
KeychainAccess v2.4.0 ✔︎
frameworkA d702e1e120f1f930abc568508386845b470596d2 ✔︎

What do you guys think @r-peck @erichoracek ? Should the version be reported? If yes I would put it after the name for all cases i.e. : <git-repo-name> <version> : ...
Note that the version is not added if you toggle any additional filter at the moment.

Going further I think that in the future it might be valuable to have formatters, for example the list command could output JSON

Proposed format so far

Note that in our previous discussions we have simply referred to the identifier of the framework as <framework-name> but that is not actually what that is. For sake of simplicity I will just call it <git-repo-name>.

Format definition and examples

• In plain english: <The name of the git repository> space <the version hash> space : space <ios-platform-cache-status>, <watchOS-platform-cache-status>, <tvOS-platform-cache-status>, <macOS-platform-cache-status>
• <git-repository-name> <version> : ((+|-)<platform-name>){1-4} . Note the max 4 repetitions

Examples:

$rome list 
FrameworkA 3.5.1 : +iOS, +watchOS, -tvOS, -macOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : +iOS, -watchOS -tvOS, -macOS

$rome list --missing
FrameworkA 3.5.1 : -tvOS, -macOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : -watchOS -tvOS, -macOS

$rome list --platform watchOS
FrameworkA 3.5.1 : +watchOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : -watchOS

$rome list --missing --platform watchOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : -watchOS

$rome list --missing --platform watchOS, iOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : -watchOS

$rome list --missing --platform watchOS, iOS, macOS #note I added a platform where BOTH are missing
FrameworkA 3.5.1 : -macOS
FrameworkB d702e1e120f1f930abc568508386845b470596d2 : -watchOS, -macOS

@erichoracek @r-peck Let's give this a final round and sign it off. I'm very excited to see it happen :)

[erichoracek]

@blender ah yes—I think I left something out in my response. My intent was to say that if a single --platform argument and either --present or --missing were both specified at the same time, (+/-)<platform-name> could be omitted from the output as it would be redundant. This would be similar to the existing behavior of --present or --missing with iOS as the only available platform (e.g. you don't see checkmarks next to every framework name when you specify --present).

I don't feel super strongly about this point. Either way seems fine for me, as it's simple to just take the first word of each line when parsing the output. Since it is now a rare case (you need to specify both a single platform as well as a presence flag), filtering out part of the output in that scenario does feel like it may be unexpected for users, especially if they're passing a dynamic number of platform flags to rome, as they could potentially get differing output depending on the number of flags passed.

[r-peck]

Yeah, I would be in favor of consistency. having different output formats based on different combinations of otherwise-unrelated options seems like unnecessary complexity. As mentioned by @blender, a formatted could always be added in the future to do this more explicitly.

As far as including version number, no strong feelings from my side.

[tmspzz]

I prefer consistency at this point. I would also include the version number for the reasons I stated above.

We can provide scripts to parse the output for the special case we're talking about.

For me this is ready for implementation. I will add the tag. Nice discussion everyone!

[tmspzz]

Update about --platform option

The CLI parsing library does not allow options with multiple arguments so it's either --platform which works in both like the following fashion

• $ rome list --platform iOS --platform watchOS
• $ rome list --platform iOS,watchOS # no spaces or whatever comes next will be considered a new CLI option

or

• $ rome list iOS, watchOS # note no --platform

[erichoracek]

In the past it seems that the decision has always been to closest match the style of Carthage. In this case, to match documented behavior, it seems like it should be comma-separated, e.g. iOS,watchOS.

From carthage help build:

[--platform (platform)]
	the platforms to build for (one of 'all', 'macOS', 'iOS', 'watchOS', 'tvOS', or comma-separated values of the formers except for 'all')

[tmspzz]

So it is then :)

[tmspzz]

And thanks to @r-peck 's incredible work, we have a pre-release https://github.com/blender/Rome/releases/tag/v0.9.0.18 !

Please test if you can and report any bugs. I will promote it to a proper release if no issues are found in a week or so.

[erichoracek]

Thanks guys, this is awesome! Can this new directory structure coexist in the same bucket that previous versions of rome have (or will) use? E.g. can we try this out without potentially breaking our bucket's backwards compatibility with previous versions?

[tmspzz]

@erichoracek you can try this out without breaking compatibility with previous versions.

The bucket/local cache structure was:
<git-repo-name>/<framework-name-and-version>.[framework|dSYM]
it is now:
<git-repo-name>/<platform>/<framework-name-and-version>.[framework|dSYM]

This means that versions of rome >= 0.9.0.18 won't find frameworks uploaded by older versions of rome and the other way around. However old version and new version won't interfere with each other.

I will add this to the release notes.

[r-peck]

@blender @erichoracek I think part of the cache layout was overlooked in my implementation. This doesn't affect whether or not cache layout is interfering with previous versions - that is still fine, but the current build is using

<framework-name>/<platform>/<framework-name-and-version>.[framework|dSYM]

I'm putting together a fix to use git repo name at the top level since it is reasonable to expect different repos may have targets with the same framework names. This shouldn't affect single project caches since the conflict would pose a problem for the consuming project, but if you have different projects with different dependencies using the same bucket, you could see conflicts.

[tmspzz]

New pre-release with fix: https://github.com/blender/Rome/releases/tag/v0.9.0.19

[tmspzz]

Hello,

I just want to reiterate that the cache paths have changed from:

<framework-name>/<framework-name-and-version>.[framework|dSYM]

to

<git-repo-name>/<platform>/<framework-name-and-version>.[framework|dSYM]

Example:

Suppose you have the following

Cartfile.resolved:

...
git "ssh://[email protected]:7999/iossdk/savingtheworld-ios-core.git" "v0.8.9"
...

and Romefile:

...
[RepositoryMap]
savingtheworld-ios-core  = Kernel

The cache path for rome <= 0.8.x.x was

Kernel/Kernel-v0.8.9.framework

it is now

savingtheworld-ios-core/<platform>/Kernel-v0.8.9.framework

same for the dSYMs

[tmspzz]

I have not heard any complains about this version thus I will close the issue. Thank you all for the discussion and thanks @r-peck for you great contribution.