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

script_phases don't preserve order after pod install #7065

Closed
1 task done
djbe opened this issue Sep 25, 2017 · 8 comments · Fixed by #7101
Closed
1 task done

script_phases don't preserve order after pod install #7065

djbe opened this issue Sep 25, 2017 · 8 comments · Fixed by #7101
Labels
d1:easy An easy ticket that is a good start for first-time contributors s2:confirmed Issues that have been confirmed by a CocoaPods contributor t1:enhancement Enhancements that have not been picked up yet. Please comment if you plan to work on it

Comments

@djbe
Copy link

djbe commented Sep 25, 2017

Refs CocoaPods/Core#389 (comment).

Report

What did you do?

  • Added some script_phases to my Podfile
  • Ran pod install
  • Moved the custom script phases around (manually drag them)
  • Ran pod install again

What did you expect to happen?

I expected my custom script phases to maintain their order. For example, If I move a script phase to before the "Compile sources" build step, I expect it to stay there on the next pod install.

What happened instead?

All my custom script phases were moved back to the end again (after all other phases). This makes the script_phase DSL only useful for scripts that should run after everything is built.

CocoaPods Environment

Stack

   CocoaPods : 1.4.0.beta.1
        Ruby : ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-darwin16]
    RubyGems : 2.6.8
        Host : Mac OS X 10.12.6 (16G29)
       Xcode : 9.0 (9A235)
         Git : git version 2.13.5 (Apple Git-94)
Ruby lib dir : /Users/davidjennes/.rbenv/versions/2.4.0/lib
Repositories : bitbucket-appwise-podspecs - [email protected]:appwise/appwise-podspecs.git @ 4021fff1fa9a0961b55dc42f374a2cfafff4a920
               master - https://github.com/CocoaPods/Specs.git @ c56e432971d6d938d2bb30b33837459782a4b46a

Installation Source

Executable Path: /Users/davidjennes/.rbenv/versions/2.4.0/bin/pod

Plugins

claide-plugins        : 0.9.2
cocoapods-deintegrate : 1.0.1
cocoapods-plugins     : 1.0.0
cocoapods-search      : 1.0.0
cocoapods-stats       : 1.0.0
cocoapods-trunk       : 1.2.0
cocoapods-try         : 1.1.0

Podfile

platform :macos, '10.12'

inhibit_all_warnings!
use_frameworks!
source 'https://github.com/CocoaPods/Specs.git'

target 'Test' do
	# 3rd party SDKs
	pod 'Crashlytics'
	pod 'Fabric'

	script_phase :name => 'Update Version Number',
		:script => 'ruby "${PROJECT_DIR}/Scripts/add_revision_to_version_number.rb"'

	script_phase :name => 'Fabric',
		:script => '"$PROJECT_DIR/Pods/Fabric/run" 123456789101112131415 123456789101112131415'
end

Project that demonstrates the issue

Test.zip

Comment

Not all custom script phases are meant to run after everything is built. If I look at most of my projects, most of them actually run before the "compile sources" build step (for example: SwiftGen, SwiftLint, Mogenerator, etc...)

@dnkoutso dnkoutso added d1:easy An easy ticket that is a good start for first-time contributors s2:confirmed Issues that have been confirmed by a CocoaPods contributor t1:enhancement Enhancements that have not been picked up yet. Please comment if you plan to work on it labels Sep 25, 2017
@dnkoutso
Copy link
Contributor

thank you! I am marking this as an "enhancement" since that was as initially planned.

We need to basically preserve the offset relative to all other custom user script phases instead of the whole list.

This is the part of the code that does this:

https://github.com/CocoaPods/CocoaPods/blob/master/lib/cocoapods/installer/user_project_integrator/target_integrator.rb#L268-L301

If interested to improve it and make a PR would be great. Tests should be easy to write too.

@dnkoutso
Copy link
Contributor

Considering actually removing entirely the reshuffling and re-ordering of script phases... I don't think we can ever get it 100% right for all use cases.

@djbe
Copy link
Author

djbe commented Sep 25, 2017

That would be a workable solution actually, as the xcodeproj will always be commited by the developer, so other teammates won't have to do the ordering themselves. 👍

@dnkoutso
Copy link
Contributor

@djbe I have a bit of an update and bigger aspirations for this. It will fix your case but also provide some additional restrictions. I will close my PR and re-open it when it's complete.

@dnkoutso
Copy link
Contributor

dnkoutso commented Oct 5, 2017

@djbe this would do it #7101

@djbe
Copy link
Author

djbe commented Oct 5, 2017

Sorry, didn't have the time yet to check the new PR.

If it understand the code correctly, this would be the originally intended behaviour, right? Maintain the order for old phases, append at the end (using the order from the Podfile) otherwise.

@dnkoutso
Copy link
Contributor

dnkoutso commented Oct 5, 2017

It will no longer re-adjust script phases based on the order in the Podfile. It will only do it if you specify that you want it :before_compile or :after_compile. You should be able to run pod install once and then drag the script to the position you want and a second pod install won't affect the order.

@djbe
Copy link
Author

djbe commented Oct 5, 2017

Oh I didn't notice the related CocoaPods/Core#414 PR. Cool! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d1:easy An easy ticket that is a good start for first-time contributors s2:confirmed Issues that have been confirmed by a CocoaPods contributor t1:enhancement Enhancements that have not been picked up yet. Please comment if you plan to work on it
Projects
None yet
2 participants