Add Support for XCFrameworks

[vikrem]

This builds on the work in #244, and addresses the review comment left in there:

• Only one of -xc-frameworks or --platform can be provided on the command line.
• Providing both causes an error.
• Providing neither goes ahead with the defaults of all platforms without xc framework support:

Usage: rome upload [FRAMEWORKS...] [--use-xcframeworks | --platform PLATFORMS] 
                   [--cache-prefix PREFIX] [--skip-local-cache] [--no-ignore] 
                   [--no-skip-current] [--concurrently]
  Uploads frameworks and dSYMs contained in the local Carthage/Build/<platform>
  to S3, according to the local Cartfile.resolved

Available options:
  -h,--help                Show this help text
  FRAMEWORKS...            Zero or more framework names. If zero, all frameworks
                           and dSYMs are uploaded.
  --use-xcframeworks       Search for .xcframeworks when performing the
                           operation.
  --platform PLATFORMS     Applicable platforms for the command. One of iOS,
                           MacOS, tvOS, watchOS, or a comma-separated list of
                           any of these values.

Combining them:

> rome upload --use-xcframeworks --platform iOS
Invalid option `--platform'

[mpdifran]

@tmspzz let us know if this works! Hopefully everything requested in #244 is fixed and we can merge this!

[tmspzz]

Thank you very much for this effort, I will take a look asap and hopefully get it merged

[tmspzz]

LGTM 🥳

But now... the most painful part...

Can we add integration tests to confirm this works? (apart from the fact that the CI stopped working. I will have to fix that)

[vikrem]

what do you have in mind? there's no integration tests on anything at all, only property tests :(

[tmspzz]

@vikrem coughs coughs https://github.com/tmspzz/Rome/tree/master/integration-tests

[vikrem]

was looking in tests/, i'm blind

[mpdifran]

@tmspzz do you have an estimate on when you can get CI up and running again? Do we need CI running before we merge this? (i.e. can we just test locally?)

[tmspzz]

@tmspzz do you have an estimate on when you can get CI up and running again? Do we need CI running before we merge this? (i.e. can we just test locally?)

We don't need the CI to merge this. I am willing to accept a "it works on my machine".

I am trying to get it up again now but my experience with Github actions is limited, you can follow my attempts on master.

[ivanrein]

@tmspzz hi, do you intend to merge this soon?

[tmspzz]

@tmspzz hi, do you intend to merge this soon?

I will merge it when the integration tests are added

[mpdifran]

Working on the integration tests now!

[mpdifran]

Integration test PR here. Once approved, it'll go into this branch, and we can merge this.

[ivanrein]

I'm getting this errors when using rome download with --use-xcframeworks
Is it expected? I don't know what dSYM files and symbolmap ids are.
But I can find frameworkname.framework.dSYM in my carthage/build folder (inside each .xcframework) even after these error messages.

[mpdifran]

Hmm the XCFrameworks is supposed to contain the dSYMS and BCSymbolMaps within itself, so less things need to be stored on AWS. You can check my PR's integration tests to see what the folder structure is.

[mpdifran]

@vikrem regarding @ivanrein 's comment, we may need to skip downloading dSYMs and BCSymbolMaps when using the --use-xcframeworks flag, since they're already stored in the XCFrameworks.

[mpdifran]

@tmspzz when you have time feel free to double check my integration tests in this PR

[tmspzz]

Yep, skip bcsymbolmaps and dsyms. They are bundled in the xcframework

[vikrem]

Yep, skip bcsymbolmaps and dsyms. They are bundled in the xcframework

🤔 i had thought this should already be the case, but i suppose not. i'll be able to take a look at this soon

[Aranoledur]

Great work, guys. Maybe you can merge it as is for now and release "skip bcsymbolmaps and dsyms" later? Really waiting for this feature :/

[mpdifran]

What do you think @tmspzz? This is blocking us at Faire from using the new MacBooks.

[tmspzz]

@mpdifran alright but I will assign you the issue :)

[mpdifran]

@vikrem let's do it!

[mpdifran]

Actually it looks like you need to merge @tmspzz.

[tmspzz]

@vikrem @evandcoleman @mpdifran Released as v0.24.0.65 🥳