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

fix(types): IOSScriptPhase accepts "always_out_of_date" #2182

Merged

Conversation

mikehardy
Copy link
Contributor

@mikehardy mikehardy commented Nov 27, 2023

Summary:

This is useful to quiet the pod install warning:

"Run script build phase '<script phase name>' will be run during every build because..."

It already exists and works (parses correctly, is carried correctly through native_modules.rb, pod install consumes it and behaves correctly) as an attribute in the type - this just exposes it

ref https://www.rubydoc.info/gems/cocoapods-core/Pod/Podfile/DSL#script_phase-instance_method

I took the opportunity to move the dependency_file attr below execution_phase, so now the order of attributes matches upstream reference documentation order

Test Plan:

react-native-firebase has two script phases that depend on inputs but can't specify output because it would duplicate the output. The scripts themselves are idempotent so it is okay if they re-run but not okay if they never run, so just run them every time is the answer, but it would be nice not to have the xcodebuild warning about it running every time. This parameter of the script phase is how to do it

You may inspect the results of running with the currently-undocumented-but-functional attribute here: https://github.com/invertase/react-native-firebase/pull/7472/files#diff-7db2a7cda2dc850e1750fbcc3a31937233711ab4d154f3594e47673de404a6cc

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

This is useful to quiet the `pod install` warning:

"Run script build phase 'Create Symlinks to Header Folders' will be run during every build because..."
@mikehardy
Copy link
Contributor Author

If this were acceptable, it would be cool to use it upstream in react-native as well, the symlinks script in a few of the modules always squawks about being run all the time, example:

⚠️ Run script build phase 'Create Symlinks to Header Folders' will be run during every build because it does not specify any outputs. To address this warning, either add output dependencies to the script phase, or configure it to run in every build by unchecking "Based on dependency analysis" in the script phase. (in target 'ReactCommon' from project 'Pods')

What do you think?

@thymikee thymikee merged commit 189b975 into react-native-community:main Nov 28, 2023
@thymikee
Copy link
Member

Thanks @mikehardy!

@mikehardy
Copy link
Contributor Author

Just a quick note to make sure this is well-known: cocoapods doesn't support this until their v1.13.0 https://github.com/CocoaPods/CocoaPods/blob/master/CHANGELOG.md#1130-2023-09-22

It was only released a couple months ago

It's a very nice feature and since CLI is a "plumbing" type package, it's important to plumb through new features so they are even available, but if react-native (for instance...) tries to use this right now it would be a breaking change requiring everyone to update their cocoapods to 1.13+ which is maybe not desired.

Ask me how I know 😆 invertase/react-native-firebase#7482

In the future though hopefully yes :-)

Until then (extend the matcher to quiet whatever you need to...), works with current cocoapods:

# Fix Xcode 14 warnings like:
# warning: Run script build phase '[CP] Copy XCFrameworks' will be run during every build because it does not specify any outputs. To address this warning, either add output dependencies to the script phase, or configure it to run in every build by unchecking "Based on dependency analysis" in the script phase. (in target 'ATargetNameHere' from project 'YourProjectName')
# Ref.: https://github.com/CocoaPods/CocoaPods/issues/11444
post_integrate do |installer|
  main_project = installer.aggregate_targets[0].user_project
  pods_project = installer.pods_project
  targets = main_project.targets + pods_project.targets
  targets.each do |target|
    run_script_build_phases = target.build_phases.filter { |phase| phase.is_a?(Xcodeproj::Project::Object::PBXShellScriptBuildPhase) }
    cocoapods_run_script_build_phases = run_script_build_phases.filter { |phase| (phase.name&.start_with?("Create Symlinks to Header Folders") || phase.name&.start_with?("Bundle React Native") || phase.name&.start_with?("Copy Detox Framework")) }
    cocoapods_run_script_build_phases.each do |run_script|
      next unless (run_script.input_paths || []).empty? && (run_script.output_paths || []).empty?
      run_script.always_out_of_date = "1"
    end
  end
  main_project.save
  pods_project.save
end

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

Successfully merging this pull request may close these issues.

3 participants