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

Building via Carthage with BUILD_LIBRARY_FOR_DISTRIBUTION = YES fails #4938

Closed
1 task done
djs-code opened this issue Sep 27, 2022 · 19 comments
Closed
1 task done

Building via Carthage with BUILD_LIBRARY_FOR_DISTRIBUTION = YES fails #4938

djs-code opened this issue Sep 27, 2022 · 19 comments
Assignees
Milestone

Comments

@djs-code
Copy link

djs-code commented Sep 27, 2022

What did you do?

Attempted to build Charts 4.1.0 via Carthage with BUILD_LIBRARY_FOR_DISTRIBUTION set to YES. The following script was used:

# carthage.sh
# Usage example: ./carthage.sh build --platform iOS

set -euo pipefail

xcconfig=$(mktemp /tmp/static.xcconfig.XXXXXX)
trap 'rm -f "$xcconfig"' INT TERM HUP EXIT

# For Xcode 12 make sure EXCLUDED_ARCHS is set to arm architectures otherwise
# the build will fail on lipo due to duplicate architectures.

CURRENT_XCODE_VERSION=$(xcodebuild -version | grep "Build version" | cut -d' ' -f3)
echo "EXCLUDED_ARCHS__EFFECTIVE_PLATFORM_SUFFIX_simulator__NATIVE_ARCH_64_BIT_x86_64__XCODE_1200__BUILD_$CURRENT_XCODE_VERSION = arm64 arm64e armv7 armv7s armv6 armv8" >> $xcconfig

echo 'EXCLUDED_ARCHS__EFFECTIVE_PLATFORM_SUFFIX_simulator__NATIVE_ARCH_64_BIT_x86_64__XCODE_1200 = $(EXCLUDED_ARCHS__EFFECTIVE_PLATFORM_SUFFIX_simulator__NATIVE_ARCH_64_BIT_x86_64__XCODE_1200__BUILD_$(XCODE_PRODUCT_BUILD_VERSION))' >> $xcconfig
echo 'EXCLUDED_ARCHS = $(inherited) $(EXCLUDED_ARCHS__EFFECTIVE_PLATFORM_SUFFIX_$(EFFECTIVE_PLATFORM_SUFFIX)__NATIVE_ARCH_64_BIT_$(NATIVE_ARCH_64_BIT)__XCODE_$(XCODE_VERSION_MAJOR))' >> $xcconfig

echo 'BUILD_LIBRARY_FOR_DISTRIBUTION = YES' >> $xcconfig

export XCODE_XCCONFIG_FILE="$xcconfig"
carthage "$@"

This script was invoked via: ./carthage.sh bootstrap --platform iOS --use-xcframeworks

What did you expect to happen?

Charts to build.

What happened instead?

The build failed, with errors stemming from the Swift Algorithms package.

Charts Environment

Charts version/Branch/Commit Number: 4.1.0
Xcode version: 14.0
Swift version: 5.7
Platform(s) running Charts: iOS 13 and above
macOS version running Xcode: macOS 12.5 and above

Demo Project

N/A (carthage.sh script above)

@matopeto
Copy link
Contributor

See my PR: #4912

@djs-code
Copy link
Author

@matopeto This PR does not fix the issue. After testing this out myself, I've determined that the problem originates from Algorithms and its lack of ABI stability.

I've opened up a discussion on this in the Swift forums: https://forums.swift.org/t/should-algorithms-public-structs-be-frozen/60668

@matopeto
Copy link
Contributor

@djs-code the PR fix the issue, but you have to build the package with plain carthage without injecting BUILD_LIBRARY_FOR_DISTRIBUTION to the carthage (because it inject also for algorithm)

but plain carthage for this PR is ok, result will be stable

@djs-code
Copy link
Author

As far as I can tell, the two potentials paths forward are:

  1. Work with the Algorithms folks to make the library ABI stable (e.g. annotating their generic structs with the @frozen decorator).
  2. Remove the Algorithms dependency from Charts.

@djs-code
Copy link
Author

@djs-code the PR fix the issue, but you have to build the package with plain carthage without injecting BUILD_LIBRARY_FOR_DISTRIBUTION to the carthage (because it inject also for algorithm)

but plain carthage for this PR is ok, result will be stable

@matopeto Charts is not the only dependency we're pulling in, and as far as I can tell, Carthage doesn't support the granularity to choose where to enable and disable the flag.

@matopeto
Copy link
Contributor

In my PR i turn the BUILD_LIBRARY_FOR_DISTRIBUTION for the charts and used @_implementationOnly for import Algorithms

then I build the package with xcode 13 with plain carthage

I am using this stable library in xcode 13 and 14 now without recompiling without problems

@djs-code
Copy link
Author

@matopeto What about when the Cartfile also includes SnapKit, MarkdownKit, and/or SwiftOTP? We pull in these libraries via Carthage as well, and we need BUILD_LIBRARY_FOR_DISTRIBUTION enabled in each one. Even with your PR, we cannot do that without using the script above.

@matopeto
Copy link
Contributor

matopeto commented Oct 24, 2022

Yes this may be a problem. I have more dependencies too. I compile other dependencies with the script and this one without. Works for me.

But I am caching resulting xcframeworks and recompile only when i need to update some dependency and I do not update very often, so this is not a problem for me.

The script is workaround for dependencies without BUILD_LIBRARY_FOR_DISTRIBUTION, and I am not injecting it where is not needed

@djs-code
Copy link
Author

@matopeto That workflow is not going to be practical for most folks, and I would generally regard it as bad developer experience. Unforunately, I'm sad to say that #4912 is not a hollistic fix for this problem.

As stated above, the root of the problem here is in the interplay with Algorithms. We can either update Algorithms directly, or remove it from Charts altogether. After perusing the Charts codebase a bit, the latter might be the more practical option, since Algorithms is only used in a small handful of spots.

@djs-code
Copy link
Author

@pmairoldi Based on the PR history, it looks like you've been the most active maintainer here in recent times. Do you have any thoughts on this?

@pmairoldi
Copy link
Collaborator

Off the top of my head no. Do you absolutely need this setting? Does it work with it off?

@djs-code
Copy link
Author

@pmairoldi We distribute a pre-built binary framework that depends on Charts (among other libraries), which we ask our users to pull in via Cocoapods or Carthage. Unfortunately, while our clients can choose between either package manager for Charts and the other dependencies, the build process for our framework is staunchly reliant on Carthage.

The only other alternative here is for us to disable BUILD_LIBRARY_FOR_DISTRIBUTION in our own framework, which we very much want to avoid.

@ansonlv
Copy link

ansonlv commented Oct 27, 2022

we are also using Charts in a Swift framework via Cocoapods. we have to turn on BUILD_LIBRARY_FOR_DISTRIBUTION. Chart is blocking us to use v4.* because of the SwiftAlgorithms dependency. without v4.1.0, we can't use Charts in Xcode 14. Please provide a solution.

@pmairoldi
Copy link
Collaborator

Closed by #4912

@djs-code
Copy link
Author

@pmairoldi #4912 Does not fix this issue, for the reasons described earlier in this thread.

@pmairoldi pmairoldi reopened this Mar 21, 2023
@pmairoldi
Copy link
Collaborator

I haven't done any actual swift development in the last couple years. Just mostly doing maintenance here. @djs-code Do you have solution or update you could provide us?

@liuxuan30
Copy link
Member

liuxuan30 commented Jun 8, 2023

@djs-code the 5.0.0 branch DGCharts.framework now builds with BUILD_LIBRARY_FOR_DISTRIBUTION = YES, this can be close now? and both Demos works now. Could you check? We are trying to address left issues in milestone 5.0
image

@liuxuan30
Copy link
Member

#4951
seems both similar, I was able to build now with branch release/5.0.0, please help check on your side

@pmairoldi
Copy link
Collaborator

Closed by #5069

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

5 participants