Add podspec `script_phases` DSL #413

dnkoutso · 2017-09-28T21:29:25Z

No description provided.

dnkoutso · 2017-09-28T21:56:45Z

First run and green.

dnkoutso · 2017-09-28T21:57:00Z

lib/cocoapods-core/podfile/target_definition.rb

@@ -585,12 +585,6 @@ def store_podspec(options = nil)

      #--------------------------------------#

-      SCRIPT_PHASE_REQUIRED_KEYS = [:name, :script].freeze


Thosed moved to specification.rb, no logical changes here.

segiddins · 2017-09-28T23:22:23Z

lib/cocoapods-core/specification/consumer.rb

+      # @return [Array<Hash{String=>String}>] the normalized script phases array.
+      #
+      def _prepare_script_phases(value)
+        if value


return unless value

was cargo culting from the other methods :)

segiddins · 2017-09-28T23:24:36Z

lib/cocoapods-core/specification/dsl.rb

+
+      SCRIPT_PHASE_OPTIONAL_KEYS = [:shell_path, :input_files, :output_files, :show_env_vars_in_log].freeze
+
+      ALL_SCRIPT_PHASE_KEYS = (SCRIPT_PHASE_REQUIRED_KEYS + SCRIPT_PHASE_OPTIONAL_KEYS).freeze


spec linter should validate this

yeap working on that as we speak.

tests added

dnkoutso · 2017-09-29T05:59:30Z

This needs a bit more time which I will spend some tomorrow and the next few days to flesh out.

I basically want the container of the attribute to be an array of hashes as I want to preserve the order.

Will update with a comment once its ready again.

dnkoutso · 2017-09-29T20:02:16Z

lib/cocoapods-core/specification/consumer.rb

@@ -324,7 +329,11 @@ def merge_values(attr, existing_value, new_value)
      #
      def prepare_value(attr, value)
        if attr.container == Array
-          value = [*value].compact
+          value = if value.is_a?(Hash)


@segiddins this is the primary fix that helps me accomplish this. Basically I want to the container to be an "Array" for this attribute since I want to preserve the order in which the script phases are added.

However, without this change the Hash becomes an array and then I lose the ability to recreate the hashes from the arrays.

This is the first attribute that requires an array of hashes so this is probably why I hit this.

dnkoutso · 2017-09-29T20:24:28Z

Alright, this is now ready again. I decided to format the DSL the same way it is in the Podfile DSL.

In other words, users are required to specify :name key for their script instead of us trying to normalize it. Its a bit more symmetric and we can improve again in the future if needed.

dnkoutso · 2017-09-29T20:35:12Z

lib/cocoapods-core/specification.rb

@@ -376,11 +382,6 @@ def local?
    #
    # @overload   supported_on_platform?(symbolic_name, deployment_target)
    #
-    #   @param    [Symbol] symbolic_name


unused params, deleted the doc

dnkoutso · 2017-09-29T20:40:05Z

lib/cocoapods-core/specification.rb

    # Converts the keys of the given hash to a string.
    #
    # @param  [Object] value
    #         the value that needs to be stripped from the Symbols.
    #
    # @return [Hash] the hash with the strings instead of the keys.
    #
-    def convert_keys_to_string(value)
+    def self.convert_keys_to_string(value)


for symmetry, decided to make this a class method as well as it maintains no state.

dnkoutso · 2017-09-29T21:09:18Z

Alright this is set! I verified that passing wrong type to script phases fails at the linter level as well.

Also JSON looks like:

{
  "name": "CannonPodder",
  "version": "1.0.0",
  "summary": "Small library that can be used to test CocoaPod stuff (new features, CI, building etc.)",
  "homepage": "https://www.cannonpodder.com",
  "license": {
    "type": "Proprietary",
    "text": "© 2017 Something, Inc."
  },
  "authors": {
    "Someone": "someone@something.com"
  },
  "source": {
    "git": "somewhere/in/git.git",
    "tag": "1.0.0"
  },
  "platforms": {
    "ios": "9.0",
    "osx": "10.10"
  },
  "source_files": "Sources/**/*.{h,m}",
  "private_header_files": "Sources/Internal/**/*.h",
  "script_phases": [
    {
      "name": "Hello World",
      "script": "echo \"Hello World\""
    },
    {
      "name": "Hello Ruby World",
      "script": "puts \"Hello World\"",
      "shell_path": "/usr/bin/ruby"
    }
  ],
  "resource_bundles": {
    "CannonPodderResources": [
      "Resources/**/*"
    ]
  },
  "testspecs": [
    {
      "name": "Tests",
      "test_type": "unit",
      "source_files": "Tests/**/*.{h,m}",
      "resource_bundles": {
        "CannonPodderResourcesTests": [
          "Tests/Resources/**/*"
        ]
      }
    }
  ]
}

dnkoutso · 2017-10-03T21:41:34Z

@paulb777 I believe you can now review and approve stuff :) Want to take a stab at this?

paulb777

Awesome! Looking forward to using script_phases.

dnkoutso · 2017-10-04T03:13:50Z

Thank you! I got one more PR after this that extends script phase DSL to add execution_placement so that script phases can be executed before compilation etc.

dnkoutso force-pushed the script_phase_podspec_dsl branch from 278a9a3 to 45fa351 Compare September 28, 2017 21:29

dnkoutso mentioned this pull request Sep 28, 2017

Additional environmental variables for prepare_command CocoaPods/CocoaPods#2115

Closed

dnkoutso commented Sep 28, 2017

View reviewed changes

segiddins reviewed Sep 28, 2017

View reviewed changes

dnkoutso force-pushed the script_phase_podspec_dsl branch 4 times, most recently from 6906c4b to 4287d59 Compare September 29, 2017 00:40

dnkoutso force-pushed the script_phase_podspec_dsl branch from 4287d59 to 90c9055 Compare September 29, 2017 20:00

dnkoutso commented Sep 29, 2017

View reviewed changes

dnkoutso force-pushed the script_phase_podspec_dsl branch 3 times, most recently from 60abf96 to 88b1346 Compare September 29, 2017 20:23

dnkoutso force-pushed the script_phase_podspec_dsl branch from 88b1346 to ec23ebf Compare September 29, 2017 20:34

dnkoutso commented Sep 29, 2017

View reviewed changes

dnkoutso force-pushed the script_phase_podspec_dsl branch 3 times, most recently from 30e9ac4 to 3b55e1e Compare September 29, 2017 20:37

dnkoutso commented Sep 29, 2017

View reviewed changes

dnkoutso force-pushed the script_phase_podspec_dsl branch 3 times, most recently from 86edeaa to b3d7c0b Compare September 29, 2017 21:04

dnkoutso mentioned this pull request Sep 29, 2017

Add support to integrate script phases from podspecs CocoaPods/CocoaPods#7092

Merged

dnkoutso requested a review from paulb777 October 3, 2017 21:41

Add podspec script_phases DSL

ca6f465

dnkoutso force-pushed the script_phase_podspec_dsl branch from b3d7c0b to ca6f465 Compare October 3, 2017 21:56

dnkoutso added this to the 1.4 milestone Oct 3, 2017

paulb777 approved these changes Oct 4, 2017

View reviewed changes

dnkoutso merged commit 3f98bce into CocoaPods:master Oct 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add podspec `script_phases` DSL #413

Add podspec `script_phases` DSL #413

dnkoutso commented Sep 28, 2017

dnkoutso commented Sep 28, 2017

dnkoutso Sep 28, 2017

segiddins Sep 28, 2017

dnkoutso Sep 28, 2017 •

edited

Loading

segiddins Sep 28, 2017

dnkoutso Sep 28, 2017

dnkoutso Oct 3, 2017

dnkoutso commented Sep 29, 2017

dnkoutso Sep 29, 2017

dnkoutso commented Sep 29, 2017

dnkoutso Sep 29, 2017

dnkoutso Sep 29, 2017 •

edited

Loading

dnkoutso commented Sep 29, 2017 •

edited

Loading

dnkoutso commented Oct 3, 2017

paulb777 left a comment

dnkoutso commented Oct 4, 2017

		@@ -585,12 +585,6 @@ def store_podspec(options = nil)

		#--------------------------------------#

		SCRIPT_PHASE_REQUIRED_KEYS = [:name, :script].freeze


		SCRIPT_PHASE_OPTIONAL_KEYS = [:shell_path, :input_files, :output_files, :show_env_vars_in_log].freeze

		ALL_SCRIPT_PHASE_KEYS = (SCRIPT_PHASE_REQUIRED_KEYS + SCRIPT_PHASE_OPTIONAL_KEYS).freeze

Add podspec script_phases DSL #413

Add podspec script_phases DSL #413

Conversation

dnkoutso commented Sep 28, 2017

dnkoutso commented Sep 28, 2017

dnkoutso Sep 28, 2017

Choose a reason for hiding this comment

segiddins Sep 28, 2017

Choose a reason for hiding this comment

dnkoutso Sep 28, 2017 • edited Loading

Choose a reason for hiding this comment

segiddins Sep 28, 2017

Choose a reason for hiding this comment

dnkoutso Sep 28, 2017

Choose a reason for hiding this comment

dnkoutso Oct 3, 2017

Choose a reason for hiding this comment

dnkoutso commented Sep 29, 2017

dnkoutso Sep 29, 2017

Choose a reason for hiding this comment

dnkoutso commented Sep 29, 2017

dnkoutso Sep 29, 2017

Choose a reason for hiding this comment

dnkoutso Sep 29, 2017 • edited Loading

Choose a reason for hiding this comment

dnkoutso commented Sep 29, 2017 • edited Loading

dnkoutso commented Oct 3, 2017

paulb777 left a comment

Choose a reason for hiding this comment

dnkoutso commented Oct 4, 2017

Add podspec `script_phases` DSL #413

Add podspec `script_phases` DSL #413

dnkoutso Sep 28, 2017 •

edited

Loading

dnkoutso Sep 29, 2017 •

edited

Loading

dnkoutso commented Sep 29, 2017 •

edited

Loading