tmspzz / Rome

Carthage cache for S3, Minio, Ceph, Google Storage, Artifactory and many others
MIT License
819 stars 57 forks source link

Issues with Rome for xcframeworks #249

Open deberle opened 2 years ago

deberle commented 2 years ago

Bug Report

Rome is not working for me when using the --use-xcframeworks option. My xcframeworks are not uploaded/downloaded as expected.

This is my setup:

$ carthage version
0.38.0
$ rome --version
0.24.0.65 - Romam uno die non fuisse conditam.
$ cat Cartfile
github "Alamofire/Alamofire" ~> 5.4

I can reproduce the issue with the following steps:

$ carthage update --use-xcframeworks
*** Fetching Alamofire
*** Checking out Alamofire at "5.4.4"
*** xcodebuild output can be found in /var/folders/bh/sn7pnkn54wl_9ksqxmvj7lrr0000gp/T/carthage-xcodebuild.uGeCAE.log
*** Building scheme "Alamofire tvOS" in Alamofire.xcworkspace
*** Building scheme "Alamofire macOS" in Alamofire.xcworkspace
*** Building scheme "Alamofire iOS" in Alamofire.xcworkspace
*** Building scheme "Alamofire watchOS" in Alamofire.xcworkspace

$ ls -1 -A Carthage/Build
.Alamofire.version
Alamofire.xcframework

$ rome upload --use-xcframeworks --cache-prefix swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3
Copied Alamofire.framework to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.xcframework-5.4.4.zip
Uploaded Alamofire to: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.xcframework-5.4.4.zip
Copied Alamofire.framework to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/Mac/Alamofire.xcframework-5.4.4.zip
Uploaded Alamofire to: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/Mac/Alamofire.xcframework-5.4.4.zip
Copied Alamofire.framework to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/watchOS/Alamofire.xcframework-5.4.4.zip
Uploaded Alamofire to: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/watchOS/Alamofire.xcframework-5.4.4.zip
Copied Alamofire.framework to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/tvOS/Alamofire.xcframework-5.4.4.zip
Uploaded Alamofire to: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/tvOS/Alamofire.xcframework-5.4.4.zip
Copied .Alamofire.version to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/.Alamofire.version-5.4.4
Uploaded .Alamofire.version to: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/.Alamofire.version-5.4.4

A few notes:

  1. [minor] The log output Copied Alamofire.framework is not correct, should be Copied Alamofire.xcframework
  2. the xcframework is copied 4 times into folders named after the platform. Should be only once.

Regardless, when continuing the download path I get the following:

$ rm -rf Carthage/Build
$ rm -rf /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/

$ bundle exec rome download --use-xcframeworks --cache-prefix swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3
Error: could not find Alamofire in local cache at : /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.xcframework-5.4.4.zip
Downloaded Alamofire from: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.xcframework-5.4.4.zip
Copied Alamofire to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.xcframework-5.4.4.zip
Error: Cannot retrieve symbolmaps ids for Alamofire
Error: could not find Alamofire.dSYM in local cache at : /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/iOS/Alamofire.framework.dSYM-5.4.4.zip
Error: could not download Alamofire.dSYM : The specified key does not exist.
Error: could not find Alamofire in local cache at : /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/Mac/Alamofire.xcframework-5.4.4.zip
Downloaded Alamofire from: swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/Mac/Alamofire.xcframework-5.4.4.zip
Copied Alamofire to: /Users/de/Library/Caches/Rome/swift_5_5_1-swiftlang-1300_0_31_4-clang-1300_0_29_3/Alamofire/Mac/Alamofire.xcframework-5.4.4.zip
rome: Carthage/Build/Alamofire.xcframework/macos-arm64_x86_64/Alamofire.framework/Alamofire: createSymbolicLink: already exists (File exists)

Notes:

  1. for unknown reasons only 2 of the previous 4 platform folders are restored to the rome cache. This may not be significant for the end result as imo we only need one xcframework anyway.
  2. [minor] errors about missing dSYM files should be removed for xcframeworks
  3. what is the meaning to the createSymbolicLink warning?

Now to the actual problem. In the Carthage/Build folder I now see:

$ ls -1 -A Carthage/Build
Alamofire.xcframework

The file .Alamofire.version is now missing.
Worse: I had cases where instead Alamofire.xcframework was missing.
It seems like only the first object was restored, sometimes the dot file, sometimes the xcframework. This is hard to reproduce.

Why would the enhancement be useful to most users

This causes random errors locally and on the CI which should affect everyone using xcframeworks.

Rome version:

0.24.0.65

OS and version:

macOS 11.5
tmspzz commented 2 years ago

@deberle Thanks for reporting the issue. I didn't develop this feature myself. Probably @vikrem @evandcoleman or @mpdifran can pitch in here as they are they main authors.

Feel free create a PR tho to fix the issues tho. I would start small.

mpdifran commented 2 years ago

Thanks for logging, @vikrem and I will look into the issue.

deberle commented 2 years ago

@mpdifran @vikrem - awesome πŸ‘ @tmspzz - I started looking into the code myself but Haskell goes over my head at this point.

wangjiejacques commented 2 years ago

rome list --missing --use-xcframeworks doesn't work as expected neither.

asenchenkov commented 2 years ago

Hi! Is there any progress with this error?

tmspzz commented 2 years ago

@mpdifran @vikrem ;)

mpdifran commented 2 years ago

We'll take a look next week!

mpdifran commented 2 years ago

@tmspzz Vikrem and I were able to fix the issues attempting to download dSYMs and BCSymbolMaps for XCFrameworks (which don't need to be explicitly cached since they live inside the XCFramework).

We had a bit more trouble handling the platform flags due to the architecture of the codebase. Because the platform flag is passed into a lot of functions, there's a lot of code to refactor for XCFrameworks, which don't need to be stored in platform specific folders (the platform folders are located within the XCFramework).

What's your recommendation for how we should proceed?

As an example, we want to go from

.../iOS/MyXCFramework.zip
.../watchOS/MyXCFramework.zip
.../macOS/MyXCFramework.zip
.../tvOS/MyXCFramework.zip

to

.../MyXCFramework.zip
tmspzz commented 2 years ago

if both:

--use-xcframeworks --platform

are specified on CLI, just stop right away with an error (that's already the case)

In the functions (I know the design is suboptimal), pass a flag for "isXCFramework" and in that case just skip all the platform specific stuff and/or shortcut the path to the correct one with something like

pathForPlatform :: Platform -> Bool -> String
pathForPlatform p isXCFrameworks = ....

Also, I'm open to other design solutions. Obviously the "cleanest" solution would be to abstract the logic but I am afraid that's a longer deal.

vikrem commented 2 years ago

we already pass an useXcFrameworks around, but it doesn't structurally prevent doing the wrong thing when both are set. Right now useXcFrameworks implies that all target platforms are enabled, which isn't quite right and that ends up doing extra copies of things.

another design idea would be to transform the type of the Platform to sum over either being that xcframeworks are in use, or a list of platforms, but not possible to be both. maybe that isn't so much work?

the longer term idea would be to do that, plus also lift all the functions into Reader monad to not need to thread so many arguments around the application, but it is a bit of a longer deal as you say :)

tmspzz commented 2 years ago

another design idea would be to transform the type of the Platform to sum over either being that xcframeworks are in use, or a list of platforms, but not possible to be both. maybe that isn't so much work?

Either<Platform, UseXCFramework> or Either<Platform, Bool> if we really really have to. But in general I see you get the idea. Thanks for the great suggestion and for the initiative.

the longer term idea would be to do that, plus also lift all the functions into Reader monad to not need to thread so many arguments around the application, but it is a bit of a longer deal as you say :)

Yeah... no. I hope by that time everyone will have moved on or younger more energetic minds than me will have come along.

Also, if you are having specific trouble point me to the line of code and I will (or I will get) help.

jonathanlu813 commented 2 years ago

I just got a createSymbolicLink: already exists (File exists) error when integrating a package with --use-xcframeworks. They have a prebuilt XCFramework in their repo. The interesting thing about it is that there's a ios-arm64_x86_64-maccatalyst slice, which for the most part just contains symlinks to the other slice. When Rome tries to download and unzip the cache, it exits with the error below.

Found Adyen3DS2 in local cache at: /Users/joluv/Library/Caches/Rome/swiftlang-1300.0.47.5-clang-1300.0.29.30-XCFrameworks/adyen-3ds2-ios/Mac/Adyen3DS2.xcframework-2.2.4.zip
rome: Carthage/Build/Adyen3DS2.xcframework/ios-arm64_x86_64-maccatalyst/Adyen3DS2.framework/Resources: createSymbolicLink: already exists (File exists)

@mpdifran @vikrem not sure if it's already been fixed by #248? If not, is there a way that we can ignore this symlink error and make it non-blocking?

tmspzz commented 2 years ago

I just got a createSymbolicLink: already exists (File exists) error when integrating a package with --use-xcframeworks. They have a prebuilt XCFramework in their repo. The interesting thing about it is that there's a ios-arm64_x86_64-maccatalyst slice, which for the most part just contains symlinks to the other slice. When Rome tries to download and unzip the cache, it exits with the error below.

Found Adyen3DS2 in local cache at: /Users/joluv/Library/Caches/Rome/swiftlang-1300.0.47.5-clang-1300.0.29.30-XCFrameworks/adyen-3ds2-ios/Mac/Adyen3DS2.xcframework-2.2.4.zip
rome: Carthage/Build/Adyen3DS2.xcframework/ios-arm64_x86_64-maccatalyst/Adyen3DS2.framework/Resources: createSymbolicLink: already exists (File exists)

@mpdifran @vikrem not sure if it's already been fixed by #248? If not, is there a way that we can ignore this symlink error and make it non-blocking?

This is an unrelated issue.

Off the top of my head I would say no, because this is how zip extraction works. It inflates the archive and when it encounters a symlink it recreates it.

If I misunderstood please correct me.

jonathanlu813 commented 2 years ago

Just took a closer look at the log, and I think it's related to the old platform based cache folder structure

Found Adyen3DS2 in local cache at: /Users/joluv/Library/Caches/Rome/swiftlang-1300.0.47.5-clang-1300.0.29.30-XCFrameworks/adyen-3ds2-ios/iOS/Adyen3DS2.xcframework-2.2.4.zip
Error: Cannot retrieve symbolmaps ids for Adyen3DS2
Error: could not find Adyen3DS2.dSYM in local cache at : /Users/joluv/Library/Caches/Rome/swiftlang-1300.0.47.5-clang-1300.0.29.30-XCFrameworks/adyen-3ds2-ios/iOS/Adyen3DS2.framework.dSYM-2.2.4.zip
Error: could not download Adyen3DS2.dSYM : The specified key does not exist.
......
Found Adyen3DS2 in local cache at: /Users/joluv/Library/Caches/Rome/swiftlang-1300.0.47.5-clang-1300.0.29.30-XCFrameworks/adyen-3ds2-ios/Mac/Adyen3DS2.xcframework-2.2.4.zip
rome: Carthage/Build/Adyen3DS2.xcframework/ios-arm64_x86_64-maccatalyst/Adyen3DS2.framework/Resources: createSymbolicLink: already exists (File exists)

See that the cache was actually unzipped and copied to Carthage/Build correctly from the adyen-3ds2-ios/iOS/Adyen3DS2.xcframework-2.2.4.zip, but then Rome tried to unzip the one under adyen-3ds2-ios/Mac/Adyen3DS2.xcframework-2.2.4.zip. Normally this won't be a problem and the files would just be overridden when there are no symlinks in the archive, but when there is, zip failed because the symlinks at the same path already exist.

Although I think a better support for --use-xcframework and --platform would help, there's still gonna be a problem when the XCFramework already exists in the Carthage/Build folder from previous carthage build. We might need to manually rm the Carthage/Build before using Rome to bypass that. With more providers publishing XCFrameworks with ios-arm64_x86_64-maccatalyst in it, I guess people will run into this more often. That said, I'm not sure how this could be elegantly fixed.

Let me know if it makes sense and love to hear your thoughts on how to fix it.

vikrem commented 2 years ago

createSymbolicLink: already exists (File exists)

Could this be solved by just forcing the zip deflating to overwrite symlinks?

jonathanlu813 commented 2 years ago

createSymbolicLink: already exists (File exists)

Could this be solved by just forcing the zip deflating to overwrite symlinks?

Yeah, that'll be enough for fixing our current problem, but I don't find zip providing the option to overwrite symlinks. Any reference on that?

vikrem commented 2 years ago

createSymbolicLink: already exists (File exists)

Could this be solved by just forcing the zip deflating to overwrite symlinks?

Yeah, that'll be enough for fixing our current problem, but I don't find zip providing the option to overwrite symlinks. Any reference on that?

It isn't provided by the original library, and is a bug in any sense. I've upstreamed a patch to the zip-archive library and opened a PR to upgrade the version in Rome (#251)

jonathanlu813 commented 2 years ago

I just tested locally with the master branch build and confirmed the createSymbolicLink: already exists (File exists) issue is fixed. Do we have a plan for a new release? πŸ™

jostster commented 2 years ago

Any update on this fix?

jonathanlu813 commented 2 years ago

I just tested locally with the master branch build and confirmed the createSymbolicLink: already exists (File exists) issue is fixed. Do we have a plan for a new release? πŸ™

Circling back here. Can we get a new release that includes the zip fix? @tmspzz

lukegardiner commented 2 years ago

Any chance of a release of this feature?

osxd commented 1 year ago

Hi! my 5Β’

although, it's a problem, overwriting isn't the best solution, it will work.

maybe just live now with the error messages? (so, if I got it right, this is the only problem?)

[ yeah, I'm also moving finally to xcframeworks πŸ˜…, only 100 to go! ]

UPD: it's getting even worse if you have external binary dependency with maccatalyst πŸ’₯

rhys-rant commented 1 year ago

Is there a workaround for this at the moment? I'm having issues updating deps for an app which uses Carthage + Rome, as I'm hitting this problem.

osxd commented 1 year ago

@rhys-rantmedia try using Romefile with platform specifier, according to doc, it works differently as one you provide via command line.

repositoryMap:
- Swinject:
  - name: Swinject
    platforms: [iOS]

we all waiting those fixes... Β―_(ツ)_/Β―