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

CocoaPods can't find local podspecs #5175

Open
malice00 opened this issue Mar 21, 2022 · 8 comments
Open

CocoaPods can't find local podspecs #5175

malice00 opened this issue Mar 21, 2022 · 8 comments
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements help wanted An issue where third-party help is wanted on

Comments

@malice00
Copy link

Analyzing CocoaPods uses pod spec which to find the location of a podspec file. Unfortunately CP only resolved the path if the pod is known in (one of) its repositories.
If a pod is included locally, the path is actually defined in the Podfile.lock, so imho there is no need to use pod spec which.

Eg in our Podfile we have:

require_relative '../node_modules/react-native/scripts/react_native_pods'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

And node_modules/react-native/scripts/react_native_pods.rb then contains (snippet):

def use_react_native! (options={})
  # The prefix to react-native
  prefix = options[:path] ||= "../node_modules/react-native"

  # The Pods which should be included in all projects
  pod 'FBLazyVector', :path => "#{prefix}/Libraries/FBLazyVector"
  pod 'FBReactNativeSpec', :path => "#{prefix}/React/FBReactNativeSpec"
  pod 'RCTRequired', :path => "#{prefix}/Libraries/RCTRequired"
  pod 'RCTTypeSafety', :path => "#{prefix}/Libraries/TypeSafety"
  pod 'React', :path => "#{prefix}/"

This then gives us a Podfile.lock like this (snippet):

DEPENDENCIES:
  - DoubleConversion (from `../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec`)
  - FBLazyVector (from `../node_modules/react-native/Libraries/FBLazyVector`)
  - FBReactNativeSpec (from `../node_modules/react-native/React/FBReactNativeSpec`)
  - glog (from `../node_modules/react-native/third-party-podspecs/glog.podspec`)
  - Permission-Camera (from `../node_modules/react-native-permissions/ios/Camera.podspec`)
  - RCT-Folly (from `../node_modules/react-native/third-party-podspecs/RCT-Folly.podspec`)
  - RCTRequired (from `../node_modules/react-native/Libraries/RCTRequired`)
  - RCTTypeSafety (from `../node_modules/react-native/Libraries/TypeSafety`)
  - React (from `../node_modules/react-native/`)
  - React-callinvoker (from `../node_modules/react-native/ReactCommon/callinvoker`)
  - React-Core (from `../node_modules/react-native/`)
  - React-Core/DevSupport (from `../node_modules/react-native/`)
--- SNIP ---
SPEC REPOS:
  https://XXX/CocoaPods/Specs.git:
    - boost-for-react-native
    - GCDWebServer

EXTERNAL SOURCES:
  DoubleConversion:
    :podspec: "../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec"
  FBLazyVector:
    :path: "../node_modules/react-native/Libraries/FBLazyVector"
  FBReactNativeSpec:
    :path: "../node_modules/react-native/React/FBReactNativeSpec"
  glog:
    :podspec: "../node_modules/react-native/third-party-podspecs/glog.podspec"
  Permission-Camera:
    :path: "../node_modules/react-native-permissions/ios/Camera.podspec"

Wouldn't it be easier (and faster) to try to retrieve the podspecs from this file and use pod spec which as a fallback? Running that would still be required for eg boost-for-react-native and GCDWebServer from the above snippet.

⚠️ The podspec files here are not written in JSON as those in a GIT-repo normally are ⚠️

@sschuberth
Copy link
Member

I was trying to recall our decision for not parsing Podfile.lock and found this comment from @tsteenbe:

Most SCA tools seem to parse the Podfile.lock file to get the list of dependencies but that not what we usually do in ORT as a project might not have committed a lock file.

But yes, we could still look for a Podfile.lock first and only fall back to another solution if it doesn't exist, where the fallback could either be implemented via pod spec which, or a way to create a Podfile.lock and then reuse the code from the first code path.

The latter is actually the way we should probably do it for all package managers that support lockfiles (which can be parsed conveniently incl. the dependency hierarchy, an which offer an efficient way to create lockfiles without actually installing all dependencies, for performance reasons).

@sschuberth sschuberth added enhancement Issues that are considered to be enhancements help wanted An issue where third-party help is wanted on analyzer About the analyzer tool labels Mar 22, 2022
@tsteenbe
Copy link
Member

Another reasons for not parsing package manager lock files:

  1. Package manager may select different packages than stated in the lock file - may happen if package manager has a new dependency version resolution, problems retrieving a specific version etc.
  2. Lock files are sometime not updated at the same time as product definition file - have seen many cases where package.json has different packages than the package-lock.json / yarn.lock. Whenever I followed up on these people simply forgot to include or had excluded it in their Git config.

Maybe ORT should offer its users package analysis options such as:

  1. 'Fast' - prefer speed of analysis over accuracy so use lock files if available
  2. 'Accurate' - prefer accuracy over speed use build tools for dependencies info instead of using lock files
  3. 'Balance' - Mix of speed and accuracy. Regenerate lock files for build tools that support it otherwise use build tools for dependencies info

Thoughts?

@sschuberth
Copy link
Member

Lock files are sometime not updated at the same time as product definition file

As a side question, wouldn't it be fair if ORT just did what the package manager alone would do in this case? After all we want to capture what goes into the build.

Like, what does NPM do in this case, does it use the package-lock.json or package.json? Or does it use neither and refuse to run?

@malice00
Copy link
Author

malice00 commented Mar 24, 2022

I believe what NPM does depends on the version you are using in your build... Not sure if a generic solution on your side would therefore be the best.

I have my team commit all lock files and try to run commands like npm ci and pod install --deployment during CI to make sure everything is as it should be (reproducible on mostly all systems -- got to love OS-specific dependencies!). Since I was planning on running ORT from CI as well, my preference would be the fast option that Thomas is describing -- since our builds would break when the lock-files aren't up-to-date anyway (although different versions of NPM again my decide to ignore breaking and just generate new lock files -- which would then be the 'correct' files to be used by ORT anyway).

I'm not sure if there is 1 single correct solution, especially in case of packaging tools in docker, because then you would be telling us which version of a specific tool is going to be used...

So again, my preference would be the fast version and to let me handle to correctness of my lock-files. But I can see validity in the other variants as well...

Edit:
If there was an option to 'only' read lock-files, that would solve #5174 as well...

Maybe you can make it a prerequisite for ORT to have run all package-managers before running ORT?

@sschuberth
Copy link
Member

This could probably be similarly solved to 4bbd26a.

@timo-HERE

This comment was marked as off-topic.

@sschuberth

This comment was marked as outdated.

@timo-HERE

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements help wanted An issue where third-party help is wanted on
Projects
None yet
Development

No branches or pull requests

4 participants