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

fixed bug for the target definition of podfile #709

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

talka123456
Copy link

@talka123456 talka123456 commented Oct 12, 2021

When the class Pod::Podfile::TargetDefinition merges use_modular_headers_hash and inhibit_warnings_hash, if the inherited class overrides the settings of the parent class, it will cause the final stored data to exist in both types collections, so the data of the parent class needs to be deleted.

For example the following Podfile file

target 'CocoapodsDemo' do
  pod 'YYCategories', :inhibit_warnings => true

  target 'CocoapodsDemo_dp1' do
    pod 'YYCategories', :inhibit_warnings => false
  end
end

Set a breakpoint in the function inhibit_warnings_hash, and you will find that the dependency of the final merged result exists in the two sets: for_pods and: not_for_pods

Although cocoapods warning does not support two different settings at the same time, this may be a hidden danger in the future
Interestingly, it can be seen from the comments that the purpose is to de-duplicate, but this is not done.

Thanks for your review

@dnkoutso
Copy link
Contributor

hi @talka123456 can you please add a test, a changelog entry and also rebase to latest master? thanks!

@dnkoutso dnkoutso added this to the 1.12.0 milestone Dec 24, 2021
@Randall10
Copy link

When the class Pod::Podfile::TargetDefinition merges use_modular_headers_hash and inhibit_warnings_hash, if the inherited class overrides the settings of the parent class, it will cause the final stored data to exist in both types collections, so the data of the parent class needs to be deleted.

For example the following Podfile file

target 'CocoapodsDemo' do
  pod 'YYCategories', :inhibit_warnings => true

  target 'CocoapodsDemo_dp1' do
    pod 'YYCategories', :inhibit_warnings => false
  end
end

Set a breakpoint in the function inhibit_warnings_hash, and you will find that the dependency of the final merged result exists in the two sets: for_pods and: not_for_pods

Although cocoapods warning does not support two different settings at the same time, this may be a hidden danger in the future Interestingly, it can be seen from the comments that the purpose is to de-duplicate, but this is not done.

Thanks for your review

@dnkoutso dnkoutso removed this from the 1.12.0 milestone Feb 27, 2023
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