grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2.03k stars 420 forks source link

'let' property 'value' may not be initialized directly in SwiftNIOConcurrencyHelpers #1063

Closed huy-lv closed 3 years ago

huy-lv commented 3 years ago

Describe the bug

Hello, i'm building my own framework ios, I included GRPC in my podfile:

# Uncomment the next line to define a global platform for your project
platform :ios, '10.0'

target 'SwiftFootprint' do
  # Comment the next line if you don't want to use dynamic frameworks
  use_frameworks!

  # Pods for SwiftFootprint
  pod 'gRPC-Swift', '~> 1.0.0-alpha.21'

end

And then build the project, but some errors happened:

/Users/huylv/Documents/project/ccc/xxx/Pods/SwiftNIOConcurrencyHelpers/Sources/NIOConcurrencyHelpers/atomics.swift:61:14: 
'let' property 'value' may not be initialized directly; use "self.init(...)" or "self = ..." instead

image

Additional information

Im using MacOS 11.0.1 Big Sur, Xcode 12.1, Swift 5.3. I think this problem because of swift version. Thank for reading <3

Lukasa commented 3 years ago

This error is incorrect: initialising the let property there is entirely fine. Are you compiling with any unusual Swift settings, e.g. library evolution mode enabled?

huy-lv commented 3 years ago

Here are the steps to reproduce this error:

  1. Create new project -> framework -> finish
  2. pod init and pod install
  3. Add pod 'gRPC-Swift', '~> 1.0.0-alpha.21' to pod file and install
  4. select current project in project navigator, create new target -> Aggregate -> give a name
  5. select the target you have created -> build phases -> add Run script:
    
    UNIVERSAL_OUTPUTFOLDER=${BUILD_DIR}/${CONFIGURATION}-universal
    WORKSPACE="demofr.xcworkspace"
    NAME_OUTPUT_FOLDER="Outputs"
    OUTPUT_FOLDER="${PROJECT_DIR}/${NAME_OUTPUT_FOLDER}/"

SDK_TARGET_NAME="demofr" ZIP_NAME="${SDK_TARGET_NAME}.zip"

mkdir -p "${UNIVERSAL_OUTPUTFOLDER}" mkdir -p "${OUTPUT_FOLDER}"

Build Device and Simulator versions

xcodebuild -workspace "${WORKSPACE}" -scheme "${SDK_TARGET_NAME}" -configuration ${CONFIGURATION} -sdk iphoneos BUILD_DIR="${BUILD_DIR}" ONLY_ACTIVE_ARCH=NO BUILD_ROOT="${BUILD_ROOT}" OBJROOT="${OBJROOT}/DependentBuilds" BUILD_LIBRARY_FOR_DISTRIBUTION=YES


6. Run with the selected target.
Lukasa commented 3 years ago

grpc and SwiftNIO do not support BUILD_LIBRARY_FOR_DISTRIBUTION. That is not a supported configuration, and should be unnecessary for your use-case.

huy-lv commented 3 years ago

can you make it support BUILD_LIBRARY_FOR_DISTRIBUTION because you are distributing it? :D

huy-lv commented 3 years ago

i have tried to remove BUILD_LIBRARY_FOR_DISTRIBUTION from build script but still get same error :( dont know why

Lukasa commented 3 years ago

Nope, BUILD_LIBRARIES_FOR_DISTRIBUTION is used only when we are distributing binaries. We only distribute source, so it’s unnecessary.

Continuing to get the error implies that the issue is cached. Try throwing away DerivedData.

huy-lv commented 3 years ago

Thanks, i create new project and everything seems work fine.

huy-lv commented 3 years ago

Nope, BUILD_LIBRARIES_FOR_DISTRIBUTION is used only when we are distributing binaries. We only distribute source, so it’s unnecessary.

Continuing to get the error implies that the issue is cached. Try throwing away DerivedData.

But when I want to distribute a .framework that depends on grpc-swift (not distributing source), It is needed to set Build Libraries for Distribution to YES.

How can I do this without publicizing my framework source code?

Lukasa commented 3 years ago

You can do that if you only depend on grpc-swift via @_implementationOnly import. This will allow you to "hide" grpc-swift from your ABI, which means you no longer need to compile grpc-swift with BUILD_LIBRARY_FOR_DISTRIBUTION.

6epreu commented 3 years ago

Could somebody provide an example how to use this import? I have the same situation. Im developing the xcframework which force me to compile with BUILD_LIBRARY_FOR_DISTRIBUTION=yes

My dependencies added over cocoapods but I did not call any import of GRPC lib and I have the same error on compilation time 2021-02-20_14-56-38

6epreu commented 3 years ago

For those who fighting with this I have added this to Podfile of my Framework and also @_implementationOnly import should be used where necessary

post_install do |installer|
  installer.pods_project.targets.each do |target|
    puts "#{target.name}"
    if target.name == "gRPC-Swift" 
        || target.name == "SwiftNIO" 
        || target.name == "SwiftNIOConcurrencyHelpers"
        || target.name == "SwiftNIOExtras"
        || target.name == "SwiftNIOFoundationCompat"
        || target.name == "SwiftNIOHPACK"
        || target.name == "SwiftNIOHTTP1"
        || target.name == "SwiftNIOHTTP2"
        || target.name == "SwiftNIOSSL"
        || target.name == "SwiftNIOTLS"
        || target.name == "SwiftNIOTransportServices"
      target.build_configurations.each do |config|
          config.build_settings['BUILD_LIBRARY_FOR_DISTRIBUTION'] = 'NO'
      end
    end
  end
end

Sorry, dont know why tabulation not working (

fortmarek commented 3 years ago

Hi @Lukasa!

I can see why BUILD_LIBRARY_FOR_DISTRIBUTION is not needed to distribute this library but it can cause issues for users of this library down the road.

I wonder how difficult it would be to support this setting because:

To sum up:

Sidenote: many thanks for this library, I really appreciate all the effort put into it, it has made our lives so much easier 🙇

Lukasa commented 3 years ago

@_implementationOnly is not very well-known and while it's a good solution for a lot of use cases, it should probably at least be mentioned in the documentation (or do you feel it's fine that users will find this issue with a solution?)

I agree that we should document this requirement.

a) if you want to distribute xcframework for your library while publishing GRPC interface, bad luck.

I’m curious as to what use-case requires publishing a GRPC interface. It seems to me that it would be useful to abstract away GRPC, as this avoids exposing users to the details of how you manage your I/O. I’d be very interested to hear why that’s not a good idea in your use-case.

how difficult would adding support for BUILD_LIBRARY_FOR_DISTRIBUTION be in your opinion?

Moderately. None of grpc-swift’s dependencies today provide stable ABIs, including protobuf. This would require that grpc-swift either abstract those dependencies away entirely (possible for everything except swift-nio and swift-protobuf) or work with those dependencies to provide a stable ABI.

Providing a stable ABI does provide some substantial limitations, particularly around performance. I’ll go into that more in the next section.

are there any specific reasons not to do this?

Yes, several.

Firstly, and most importantly, enabling BUILD_LIBRARY_FOR_DISTRIBUTION as a fully supported configuration requires that we define and implement a stable ABI on any library that does so. This is burdensome because it interacts very poorly with performance concerns.

Two particular concerns are with @inlinable and @frozen. grpc and its dependencies use @inlinable annotations very heavily to improve the performance of generic code. These annotations have cumulatively granted swift-nio well in excess of 2x the performance it would have without them. The concern with @inlinable is that, when combined with the stable ABI implied by BUILD_LIBRARY_FOR_DISTRIBUTION, the body of any @inlinable method is essentially frozen in stone. It becomes very dangerous for us to change it in any way. This is because it becomes implicitly necessary for you to be able to use a version of your code where you inlined an old version of that method with a new version of our code where we may have wanted to change the implementation of that method.

A way to think about the concern there is that, with BUILD_LIBRARY_FOR_DISTRIBUTION turned on, anything that is @inlinable or touched by an @inlinable method sort of becomes a part of our public API. This is a lot of extra API surface that we are now required to leave in place.

The @frozen annotation is another concern. Right now when you build grpc-swift any structure has a known layout at build time: you know how wide it is, you can cheaply stack allocate it, you can dereference stored properties by simple offset calculation and so-on. If we enable BUILD_LIBRARY_FOR_DISTRIBUTION this stops being true: the structure becomes resilient, and its layout is hidden from you. This slows down the performance of code that touches those structures. We can get that performance back using @frozen, but if we ever mark anything @frozen we are forbidden from changing its layout ever again. This is much like @inlinable: essentially, the number of fields and their order in the structure has become part of our API. Today it isn’t!

The second concern is less important than the first. We’d be fine saying that none of our types are @frozen and forcing users that want the stable ABI to get the slower version, as anyone compiling from source (e.g. via SwiftPM) would still get the fast version. @inlinable is harder, because removing it from things makes code slower for everyone. This will get better over time as cross-module-optimization improves, but it’s not the default for anyone right now and it has a tendency to miss important use-cases. grpc-swift will just fundamentally be drastically slower for all users if we support the use-case.

Finally, BUILD_LIBRARY_FOR_DISTRIBUTION is really only supported by Xcode at the current moment. Swift does not have a stable ABI on Linux, so it is meaningless there, and SwiftPM can’t use the functionality either. That substantially limits the utility of enabling it, as we incur costs on all platforms but only get the benefits in a specific use-case.

if so, are we OK that it is possible that more people will hit this issue as xcframeworks will become more widespread?

I think for now the answer is “yes”, but I should stress that the answer is “yes, for now”. If cross-module-optimization improves and becomes the default for source-available compilation, and Linux gets a stable Swift ABI, we are highly likely to re-evaluate this decision. However, for now it is possible to work around the issue, and so that’s going to be the best course of action.

fortmarek commented 3 years ago

Thanks @Lukasa for the exhaustive explanation!

I agree that we should document this requirement.

LMK if you want me to create a PR for that or if that's something you feel like it'd better be done by someone more familiar with the codebase - otherwise, I'd happily provide a helping hand.

It seems to me that it would be useful to abstract away GRPC, as this avoids exposing users to the details of how you manage your I/O.

Fair point, option a) was not something I need for my project but I felt like it could be a problem for someone else (but I agree they should rather abstract the GRPC API away)

I think for now the answer is “yes”, but I should stress that the answer is “yes, for now”.

Given the drawbacks of being able to compile GRPC with BUILD_LIBRARY_FOR_DISTRIBUTION, I agree it's not something that should be done for a quite specific use case we have (and which we should probably be able to get around, although, it will require some non-trivial changes to the structure of our codebase). Let's see how the situation around BUILD_LIBRARY_FOR_DISTRIBUTION evolves 👍

Lukasa commented 3 years ago

I think it’d be fab if you wanted to put together a docs PR.

skippetr commented 2 years ago

Hi!

I don't know why issue with BUILD_LIBRARY_FOR_DISTRIBUTION is closed.

Have faced with the same problem: I need to get my xcframework. that depends on grpc-swift, hence I use BUILD_LIBRARY_FOR_DISTRIBUTION = YES. All of steps above doesn't help me.

Do I understand it right, that this library is useless, when we need to build our own framework?

Lukasa commented 2 years ago

No, the only thing you cannot do is expose grpc-swift types. So long as your interface is independent of grpc-swift then you should be able to use @_implementationOnly to safely build an xcframework.

skippetr commented 2 years ago

Thanks for answer, @Lukasa.

Yes, I don't use anything from that library as part of my module's public interface. So, I tried to use @_implementationOnly import GRPC in my source. Then the errors, such like

Cannot find 'GRPCChannelPool' in scope Cannot find 'PlatformSupport' in scope

and others appeared...

I guess I am doing something wrong, but what then.

Lukasa commented 2 years ago

Where are those warnings coming from?

skippetr commented 2 years ago

Ok, a little recap for someone, who struggled with this.

Guess my competence is too low, that don't know difference, but in my Podlife I used use_modular_headers!, but should used use_frameworks!. Also, add

post_install do |installer|
  installer.pods_project.targets.each do |target|
    puts "#{target.name}"
    if target.name == "gRPC-Swift" || target.name == "SwiftNIO" || target.name == "SwiftNIOConcurrencyHelpers" || target.name == "SwiftNIOExtras" || target.name == "SwiftNIOFoundationCompat" || target.name == "SwiftNIOHPACK" || target.name == "SwiftNIOHTTP1" || target.name == "SwiftNIOHTTP2" || target.name == "SwiftNIOSSL" || target.name == "SwiftNIOTLS" || target.name == "SwiftNIOTransportServices" || target.name == "SwiftProtobuf" || target.name == "_NIODataStructures" || target.name == "SwiftNIOCore" || target.name == "NIOCore" || target.name == "SwiftNIOPosix"
      target.build_configurations.each do |config|
          config.build_settings['BUILD_LIBRARY_FOR_DISTRIBUTION'] = 'NO'
      end
    end
  end
end

Import gRPC like this @_implementationOnly import GRPC.

soharang commented 2 years ago

@skippetr Thanks for your wrap-up. What if I don't use Podfile but only Package Manager?

I really hope grpc-swift support BUILD_LIBRARY_FOR_DISTRIBUTION option.

skippetr commented 2 years ago

@soharang unfortunately don't know how to deal with Package Manager