Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with Rome for xcframeworks #249

Open
deberle opened this issue Nov 29, 2021 · 24 comments
Open

Issues with Rome for xcframeworks #249

deberle opened this issue Nov 29, 2021 · 24 comments
Assignees

Comments

@deberle
Copy link

deberle commented Nov 29, 2021

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
Copy link
Owner

tmspzz commented Nov 29, 2021

@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
Copy link
Contributor

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

@deberle
Copy link
Author

deberle commented Nov 30, 2021

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

@tmspzz tmspzz assigned mpdifran and unassigned mpdifran Dec 2, 2021
@tmspzz tmspzz pinned this issue Dec 2, 2021
@wangjiejacques
Copy link

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

@asenchenkov
Copy link

Hi!
Is there any progress with this error?

@tmspzz
Copy link
Owner

tmspzz commented Feb 4, 2022

@mpdifran @vikrem ;)

@mpdifran
Copy link
Contributor

mpdifran commented Feb 4, 2022

We'll take a look next week!

@mpdifran
Copy link
Contributor

mpdifran commented Feb 8, 2022

@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
Copy link
Owner

tmspzz commented Feb 10, 2022

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
Copy link
Contributor

vikrem commented Feb 10, 2022

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
Copy link
Owner

tmspzz commented Feb 10, 2022

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
Copy link

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
Copy link
Owner

tmspzz commented Feb 16, 2022

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
Copy link

jonathanlu813 commented Feb 17, 2022

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
Copy link
Contributor

vikrem commented Feb 22, 2022

createSymbolicLink: already exists (File exists)

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

@jonathanlu813
Copy link

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
Copy link
Contributor

vikrem commented Mar 2, 2022

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
Copy link

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
Copy link

jostster commented Mar 22, 2022

Any update on this fix?

@jonathanlu813
Copy link

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
Copy link

Any chance of a release of this feature?

@osxd
Copy link

osxd commented Sep 26, 2022

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
Copy link

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
Copy link

osxd commented Nov 28, 2022

@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... ¯_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests