Podfile.lock not honored for local podspecs #4211

Closed
dnkoutso opened this Issue Sep 18, 2015 · 27 comments

Comments

Projects
None yet
8 participants
@dnkoutso
Contributor

dnkoutso commented Sep 18, 2015

We have a dependency that the .lock file has it locked in version 1.2.3.

Going back to this commit and doing pod install we see the .lock file being updated with version 1.2.4.

The podspec that depends on this dependency is a local podspec using :path, could that be a clue? How and why does CocoaPods thinks there is a new version despite the fact the lock file mentions otherwise?

Can you provide more info on how the lock file is honored and when is it considered "dirty"?

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 18, 2015

Contributor

Version 0.39.0.beta4

Contributor

dnkoutso commented Sep 18, 2015

Version 0.39.0.beta4

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 18, 2015

Contributor

This appears to be that the local podspecs are always treated as specs to be updated. Is there a reason for this? Perhaps due to the fact that they are considered local pods?

Why dont local podspecs fall through the same workflow of using pod update?

Contributor

dnkoutso commented Sep 18, 2015

This appears to be that the local podspecs are always treated as specs to be updated. Is there a reason for this? Perhaps due to the fact that they are considered local pods?

Why dont local podspecs fall through the same workflow of using pod update?

@dnkoutso dnkoutso changed the title from Podfile.lock not honored? to Podfile.lock not honored for local podspecs Sep 18, 2015

@justinseanmartin

This comment has been minimized.

Show comment
Hide comment
@justinseanmartin

justinseanmartin Sep 18, 2015

Contributor

It is tied back to this change from July: #3764

Contributor

justinseanmartin commented Sep 18, 2015

It is tied back to this change from July: #3764

@justinseanmartin

This comment has been minimized.

Show comment
Hide comment
@justinseanmartin

justinseanmartin Sep 18, 2015

Contributor

Should there be something like a --frozen equivalent as bundler?

https://github.com/bundler/bundler/blob/406e069b7a6c870fe7db4dc6d891adde85b96f47/lib/bundler/definition.rb#L326

Alternatively, we could remove the change from the previous commit that has the 'Podfile.lock' to be respected, and implement some other behavior as the default for pod install that also works for local podspecs.

Contributor

justinseanmartin commented Sep 18, 2015

Should there be something like a --frozen equivalent as bundler?

https://github.com/bundler/bundler/blob/406e069b7a6c870fe7db4dc6d891adde85b96f47/lib/bundler/definition.rb#L326

Alternatively, we could remove the change from the previous commit that has the 'Podfile.lock' to be respected, and implement some other behavior as the default for pod install that also works for local podspecs.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Sep 19, 2015

Member

Pretty sure this is the intended behavior?

Member

segiddins commented Sep 19, 2015

Pretty sure this is the intended behavior?

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 19, 2015

Contributor

There should be at least a flag perhaps similar to -frozen that will not update the dependencies...We use some local podspecs that we commit into the repos without publishing them so we can slice our project structure.

Contributor

dnkoutso commented Sep 19, 2015

There should be at least a flag perhaps similar to -frozen that will not update the dependencies...We use some local podspecs that we commit into the repos without publishing them so we can slice our project structure.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Sep 19, 2015

Member

Unfortunately, that's just not how development pods work :(

Member

segiddins commented Sep 19, 2015

Unfortunately, that's just not how development pods work :(

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 19, 2015

Contributor

Would you consider a --frozen flag in pod install command that doesnt update any pod?

Contributor

dnkoutso commented Sep 19, 2015

Would you consider a --frozen flag in pod install command that doesnt update any pod?

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Sep 19, 2015

Member

If anything, it would belong in the Podfile... Maybe we could have something like an alias for path that doesn't implicitly unlock? Thoughts, @orta?

Member

segiddins commented Sep 19, 2015

If anything, it would belong in the Podfile... Maybe we could have something like an alias for path that doesn't implicitly unlock? Thoughts, @orta?

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 19, 2015

Contributor

I feel like a :frozen => true would be sufficient within the Podfile. It looks like it might be rather easy to add support as well.

I'll wait for your feedback and then I can implement this.

Contributor

dnkoutso commented Sep 19, 2015

I feel like a :frozen => true would be sufficient within the Podfile. It looks like it might be rather easy to add support as well.

I'll wait for your feedback and then I can implement this.

@neonichu

This comment has been minimized.

Show comment
Hide comment
@neonichu

neonichu Sep 19, 2015

Member

How would this work, though? Local Pods use symlinks, so there's just the newest version of everything available, no matter which version it would pretend to be.

Member

neonichu commented Sep 19, 2015

How would this work, though? Local Pods use symlinks, so there's just the newest version of everything available, no matter which version it would pretend to be.

@dnkoutso

This comment has been minimized.

Show comment
Hide comment
@dnkoutso

dnkoutso Sep 19, 2015

Contributor

This is more about its transitive dependencies that have been published rather than the sources of the local pod itself.

Overall it caught us by surprise to see that going back to a release branch our pod install command actually updated the published dependencies of the local pod.

Contributor

dnkoutso commented Sep 19, 2015

This is more about its transitive dependencies that have been published rather than the sources of the local pod itself.

Overall it caught us by surprise to see that going back to a release branch our pod install command actually updated the published dependencies of the local pod.

@kyleve

This comment has been minimized.

Show comment
Hide comment
@kyleve

kyleve Sep 22, 2015

I think I agree with @dnkoutso (though I admittedly know little about Pods or Bundler). Seems like the lock file should be the source of truth.

kyleve commented Sep 22, 2015

I think I agree with @dnkoutso (though I admittedly know little about Pods or Bundler). Seems like the lock file should be the source of truth.

@danielribeiro

This comment has been minimized.

Show comment
Hide comment
@orta

This comment has been minimized.

Show comment
Hide comment
@orta

orta Sep 22, 2015

Member

Interesting, so this is about it breaking the dependencies outside of the one that is defined as a dev/path pod, I can see why you'd expect the Podfile.lock to be respected here. Just because the dependency is defined in the dev pod, shouldn't mean "always make the transitive reps be the highest in the constraints." I'd say this is something we should look at. Thanks for the demo, made it much easier to understand the problem.

Member

orta commented Sep 22, 2015

Interesting, so this is about it breaking the dependencies outside of the one that is defined as a dev/path pod, I can see why you'd expect the Podfile.lock to be respected here. Just because the dependency is defined in the dev pod, shouldn't mean "always make the transitive reps be the highest in the constraints." I'd say this is something we should look at. Thanks for the demo, made it much easier to understand the problem.

@danielribeiro

This comment has been minimized.

Show comment
Hide comment
@danielribeiro

danielribeiro Sep 22, 2015

Contributor

No problem @orta !

Contributor

danielribeiro commented Sep 22, 2015

No problem @orta !

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Oct 14, 2015

@segiddins, what's the status of this issue? Can we just make pod try ignore Podfile.lock for local dependencies?

@segiddins, what's the status of this issue? Can we just make pod try ignore Podfile.lock for local dependencies?

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 14, 2015

Member

@dtweston I don't think this issue has anything to do with pod try, and I'm of the belief that the current behavior is the least wrong.

Member

segiddins commented Oct 14, 2015

@dtweston I don't think this issue has anything to do with pod try, and I'm of the belief that the current behavior is the least wrong.

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Oct 14, 2015

As @dnkoutso said a while back this is more about the dependencies of the pod that is being referenced via path as opposed to the pod itself.

It makes sense that we don't enforce the version for the pod on the path, but it makes less sense (to me, at least) that we consider each of that local pod's dependencies as targets to be updated.

As @dnkoutso said a while back this is more about the dependencies of the pod that is being referenced via path as opposed to the pod itself.

It makes sense that we don't enforce the version for the pod on the path, but it makes less sense (to me, at least) that we consider each of that local pod's dependencies as targets to be updated.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 14, 2015

Member

If you care about those versions though, you should be explicitly locking them in your podfile imho.

Member

segiddins commented Oct 14, 2015

If you care about those versions though, you should be explicitly locking them in your podfile imho.

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Oct 14, 2015

I don't care about the versions per se, I just want the build to be repeatable.

I don't care about the versions per se, I just want the build to be repeatable.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Oct 14, 2015

Member

If you want repeatability, you probably shouldn't be using dev pods, since cocoapods can't lock them.

Member

segiddins commented Oct 14, 2015

If you want repeatability, you probably shouldn't be using dev pods, since cocoapods can't lock them.

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Oct 14, 2015

We have repeatability for the dev pod because it's referenced via a git submodule. Might be considered an odd setup, but it's been more or less working so far.

We have repeatability for the dev pod because it's referenced via a git submodule. Might be considered an odd setup, but it's been more or less working so far.

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Jan 16, 2016

bump Can I ask what the current feeling is on this issue?

We'd really like to move to the latest version of Cocoapods (especially now that you're getting to 1.0), but this is a blocker for us, so we're also looking at alternatives.

@dnkoutso, does your offer to implement this still stand? 😁

bump Can I ask what the current feeling is on this issue?

We'd really like to move to the latest version of Cocoapods (especially now that you're getting to 1.0), but this is a blocker for us, so we're also looking at alternatives.

@dnkoutso, does your offer to implement this still stand? 😁

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Jan 16, 2016

Member

We've asked before for a PR to stop implicitly unlocking all dependencies of Dev pods. We've yet to receive one. If you send a documented, well-tested PR, we'd be happy to merge it.

Member

segiddins commented Jan 16, 2016

We've asked before for a PR to stop implicitly unlocking all dependencies of Dev pods. We've yet to receive one. If you send a documented, well-tested PR, we'd be happy to merge it.

@dtweston

This comment has been minimized.

Show comment
Hide comment
@dtweston

dtweston Jan 16, 2016

I must have missed that request. Thanks for the clarification @segiddins!

I would love to contribute a PR, and I'll get started on it, but it may take me quite a while, so if someone else has the ability to step in and help out, that would be awesome.

I must have missed that request. Thanks for the clarification @segiddins!

I would love to contribute a PR, and I'll get started on it, but it may take me quite a while, so if someone else has the ability to step in and help out, that would be awesome.

@justinseanmartin

This comment has been minimized.

Show comment
Hide comment
@justinseanmartin

justinseanmartin Jan 18, 2016

Contributor

I think this is the same issue called out in #4580.

Our current workaround is to monkey patch Cocoapods with this:

class ::Pod::Installer::Analyzer
  # Github source: https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/installer/analyzer.rb
  def generate_version_locking_dependencies
    if update_mode == :all || !lockfile
      LockingDependencyAnalyzer.unlocked_dependency_graph
    else
      pods_to_update = result.podfile_state.changed + result.podfile_state.deleted
      pods_to_update += update[:pods] if update_mode == :selected
      # FIXME: go back to master once this is fixed: https://github.com/CocoaPods/CocoaPods/issues/4211
      # pods_to_update += podfile.dependencies.select(&:local?).map(&:name)
      LockingDependencyAnalyzer.generate_version_locking_dependencies(lockfile, pods_to_update)
    end
  end
end

I think this will break pods try though, as it explicitly reverses the behavior from #3764.

Contributor

justinseanmartin commented Jan 18, 2016

I think this is the same issue called out in #4580.

Our current workaround is to monkey patch Cocoapods with this:

class ::Pod::Installer::Analyzer
  # Github source: https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/installer/analyzer.rb
  def generate_version_locking_dependencies
    if update_mode == :all || !lockfile
      LockingDependencyAnalyzer.unlocked_dependency_graph
    else
      pods_to_update = result.podfile_state.changed + result.podfile_state.deleted
      pods_to_update += update[:pods] if update_mode == :selected
      # FIXME: go back to master once this is fixed: https://github.com/CocoaPods/CocoaPods/issues/4211
      # pods_to_update += podfile.dependencies.select(&:local?).map(&:name)
      LockingDependencyAnalyzer.generate_version_locking_dependencies(lockfile, pods_to_update)
    end
  end
end

I think this will break pods try though, as it explicitly reverses the behavior from #3764.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment