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

Adjust the position of require File.expand_path #141521

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

LinXunFeng
Copy link
Member

@LinXunFeng LinXunFeng commented Jan 14, 2024

On Podfile:

flutter_application_path = '../flutter_module'
load File.join(flutter_application_path, '.ios', 'Flutter', 'podhelper.rb')

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

  # Pods for OCProject
  # install_all_flutter_pods(flutter_application_path)
  # install_flutter_engine_pod(flutter_application_path)
  # install_flutter_application_pod(flutter_application_path)
  install_flutter_plugin_pods(flutter_application_path)

end

post_install do |installer|
  flutter_post_install(installer)
end

Encountering the following error after executing pod install:

pod install

[!] Invalid `Podfile` file: undefined method `flutter_relative_path_from_podfile' for #<Pod::Podfile:0x000000010e74c520 @defined_in_file=#<Pathname:/Users/lxf/gitHub/flutter_hybrid_bug/OCProject/Podfile>, @internal_hash={}, @root_target_definitions=[#<Pod::Podfile::TargetDefinition label=Pods>], @current_target_definition=#<Pod::Podfile::TargetDefinition label=Pods>>

  relative = flutter_relative_path_from_podfile(export_script_directory)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^.

 #  from /Users/lxf/gitHub/flutter_hybrid_bug/OCProject/Podfile:17
 #  -------------------------------------------
 #    # install_flutter_plugin_pods(flutter_application_path)
 >    install_flutter_application_pod(flutter_application_path)
 #
 #  -------------------------------------------

The flutter_relative_path_from_podfile method is in flutter_tools/bin/podhelper.rb, but now flutter_tools/bin/podhelper.rb is only required in install_all_flutter_pods in podhelper.rb.tmpl.

Sometimes we only need to use the install_flutter_plugin_pods method in podhelper.rb. For example, using Shorebird in an iOS hybird app scenario, we need to build Flutter.xcframework and App.xcframework and embed them into the iOS native project. In order to avoid unnecessary conflicts, use install_flutter_plugin_pods method to install Flutter plugin pods.

Shorebird - Code Push In Hybrid Apps

So I adjust the position of require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root).

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 14, 2024
@@ -112,6 +110,8 @@ def flutter_root
raise "FLUTTER_ROOT not found in #{generated_xcode_build_settings_path}. Try deleting Generated.xcconfig, then run flutter pub get"
end

require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why this shouldn't be at the top of the file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line of code must be placed after the flutter_root method, otherwise the following error will be reported.

❯ pod install

[!] Invalid `Podfile` file: undefined local variable or method `flutter_root' for main:Object

require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)
                                                                                     ^^^^^^^^^^^^.

It is also used like this in Podfile-ios-objc.

def flutter_root
generated_xcode_build_settings_path = File.expand_path(File.join('..', 'Flutter', 'Generated.xcconfig'), __FILE__)
unless File.exist?(generated_xcode_build_settings_path)
raise "#{generated_xcode_build_settings_path} must exist. If you're running pod install manually, make sure flutter pub get is executed first"
end
File.foreach(generated_xcode_build_settings_path) do |line|
matches = line.match(/FLUTTER_ROOT\=(.*)/)
return matches[1].strip if matches
end
raise "FLUTTER_ROOT not found in #{generated_xcode_build_settings_path}. Try deleting Generated.xcconfig, then run flutter pub get"
end
require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I see, makes sense

@christopherfujino
Copy link
Member

Sometimes we only need to use the install_flutter_plugin_pods method in podhelper.rb. For example, using Shorebird in an iOS hybird app scenario, we need to build Flutter.xcframework and App.xcframework and embed them into the iOS native project. In order to avoid unnecessary conflicts, use install_flutter_plugin_pods method to install Flutter plugin pods.

@jmagman this code change seems ok to me, but probably more important, is this a supported workflow?

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes we only need to use the install_flutter_plugin_pods method in podhelper.rb. For example, using Shorebird in an iOS hybird app scenario, we need to build Flutter.xcframework and App.xcframework and embed them into the iOS native project. In order to avoid unnecessary conflicts, use install_flutter_plugin_pods method to install Flutter plugin pods.

@LinXunFeng can you explain this a bit more, I'm curious. Is the scenario that your hybrid app uses CocoaPods and needs the same pods between the native app and the Flutter module? So you'd rather pod install run only once and deal with all the version solving so they don't conflict? Or is there something else going on?

is this a supported workflow?

It's not "supported" but I wouldn't go out of my way to break it. This change LGTM.


podfileContent = podfileContent.replaceFirst('''
install_all_flutter_pods flutter_application_path
''', '''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, thank you so much for the test!

@jmagman jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 20, 2024
Copy link
Member

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LinXunFeng
Copy link
Member Author

LinXunFeng commented Jan 20, 2024

can you explain this a bit more, I'm curious. Is the scenario that your hybrid app uses CocoaPods and needs the same pods between the native app and the Flutter module? So you'd rather pod install run only once and deal with all the version solving so they don't conflict? Or is there something else going on?

@jmagman According to the various ways to integrate the Flutter module introduced in the document, our app initially used Option A to integrate the Flutter module. When using Shorebird, it requires switching to something like Option B.

shorebird release ios-framework-alpha

When using the above command, Shorebird will use its own modified Flutter framework and engine to build Flutter.xcframework and App.xcframework.

In my Flutter module, I use some Flutter plugins based on native third packages, such as sqflite, which relies on FMDB, and FMDB is a dependency in our native project.

Pod::Spec.new do |s|
  s.name             = 'sqflite'
  ...
  s.dependency 'FMDB', '>= 2.7.5'
  ...
end

https://github.com/tekartik/sqflite/blob/70d127224b33594963f2df51157ad4481d41a5a4/sqflite/ios/sqflite.podspec#L18

When using Option B to build xcframework, there will be FMDB.xcframework. I dragged these xcframeworks into Build Settings > Build Phases > Link Binary With Libraries.

The following error will occur when compiling.

Showing Recent Messages
Multiple commands produce '/Users/lxf/.../xxx.app/Frameworks/FMDB.framework'

And the Option B will miss some vendored_frameworks such as realm

Pod::Spec.new do |s|
  s.name                      = 'realm'
  ...
  s.vendored_frameworks       = 'realm_dart.xcframework'
  ...
end

https://github.com/realm/realm-dart/blob/47e1ee72a6182bc3d5c13c0bac588066fdc6fffb/flutter/realm_flutter/ios/realm.podspec#L33

Related feedback: #125530

I also tried to embed these vendored_frameworks through scripts, but this way is troublesome.

I have tried putting the s.dependency that the Flutter plugin depends on one by one into the Podfile of the native project, and no longer embedding these xcframeworks. This solution is not friendly, because I need to judge whether the native project already has the same dependency. If there is, it needs to be discarded.

Even if the above problem was solved, our app would crash as soon as it started. After investigation, I found that it was because connectivity_plus relied on ReachabilitySwift. I put its dependency in the Podfile of our native project, but what it requires is Reachability.xcframework.

dyld[31764]: Symbol not found: _$s12ReachabilityAAC10ConnectionO4wifiyA2DmFWC
  Referenced from: <8142F86E-4C9C-3513-AD29-D3522FC6677F> /Users/lxf/.../connectivity_plus.framework/connectivity_plus
  Expected in:     <DA318000-9A97-35AD-87EA-7C5B635DE010> /Users/lxf/.../Frameworks/Reachability.framework/Reachability

In addition, we also need to determine whether each xcframework is a static framework or a dynamic framework, otherwise it will not be installed and run on the iPhone.

Related feedback: #122183

Based on so many problems mentioned above, I finally chose to create a podspec file to declare dependencies on the already built Flutter.xcframework and App.xcframework, and then combined with install_flutter_plugin_pods
method to integrate native third packages. 😂

@LinXunFeng LinXunFeng added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Jan 21, 2024
@auto-submit auto-submit bot merged commit f340d20 into flutter:master Jan 21, 2024
127 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 21, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 21, 2024
flutter/flutter@ddf60fb...5dea6b9

2024-01-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2b31ad2fb819 to a7b207d5a1fe (1 revision) (flutter/flutter#141945)
2024-01-21 christopherfujino@gmail.com [flutter_tools] update analyze_once_test.dart to be null-safe (flutter/flutter#141790)
2024-01-21 linxunfeng@yeah.net Adjust the position of require File.expand_path (flutter/flutter#141521)
2024-01-21 yjbanov@google.com Add RadioListItem use-case to a11y_assessments (flutter/flutter#140984)
2024-01-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 704ef3399012 to 2b31ad2fb819 (1 revision) (flutter/flutter#141937)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
…#5951)

flutter/flutter@ddf60fb...5dea6b9

2024-01-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2b31ad2fb819 to a7b207d5a1fe (1 revision) (flutter/flutter#141945)
2024-01-21 christopherfujino@gmail.com [flutter_tools] update analyze_once_test.dart to be null-safe (flutter/flutter#141790)
2024-01-21 linxunfeng@yeah.net Adjust the position of require File.expand_path (flutter/flutter#141521)
2024-01-21 yjbanov@google.com Add RadioListItem use-case to a11y_assessments (flutter/flutter#140984)
2024-01-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 704ef3399012 to 2b31ad2fb819 (1 revision) (flutter/flutter#141937)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants