Add a hack to patch cocoapods #78
Conversation
This hack unblocks us from releasing libraries from CI. Hopefully CocoaPods can fix the issue properly and we can remove this hack soon.
bin/publish_pod
Outdated
| @@ -10,6 +10,9 @@ if [ -n "$BUILDKITE_TAG" ] && [ "$BUILDKITE_TAG" != "$POD_VERSION" ]; then | |||
| exit 1 | |||
| fi | |||
|
|
|||
| echo "⚠️ Remove the patch-cocoapods step once this issue is fixed: https://github.com/CocoaPods/CocoaPods/issues/12033" | |||
| patch-cocoapods | |||
There was a problem hiding this comment.
I wonder if we shouldn't instead let the caller projects call patch-cocoapods before they call validate_pod and publish_pod in their own .buildkite/commands/*.sh scripts.
The reason I suggest separating those is because the patch depends on the version of CocoaPods being installed. If/when CocoaPods releases a new version with this issue fixed, some projects will adopt the new version quickly while others might take more time.
At least by letting the caller script decide, it will be easier once the new CP version arrives to just update CP in the project's Gemfile.lock then stop calling patch-cocoapods on the buildkite scripts in the same PR updating CP, rather than having to release a new version of a8c-ci-toolkit that will remove that patch's call… but then only work with projects that updated CocoaPods.
If we strongly couple the version of a8c-ci-toolkit with the version of CocoaPods used, if some projects want to update CocoaPods but are not ready to update a8c-ci-toolkit (eg say there was a major version of the toolkit since for unrelated changes, and the project isn't ready to adopt those yet) they'd be stuck.
There was a problem hiding this comment.
The main reason for putting this script here is: install_gems overwrites the local gems, so patch-cocoapods needs to run straight after install_gems. Also, it'd be nice to avoid copying scripts among libraries repos.
But I agree with you it's not ideally that this script may break pipelines.
Initially I tried to add an optional plugin configuration to the plugin, patch-cocoapods: true, but I can't get the environment variable BUILDKITE_PLUGIN_A8C_CI_TOOLKIT_PATCH_COCOAPODS to work: it doesn't exist even though the new option is set in pipeline.
What do you think adding a new option to the existing shell script, like validate_podspec --patch_cocoapods?
There was a problem hiding this comment.
The main reason for putting this script here is: install_gems overwrites the local gems, so patch-cocoapods needs to run straight after install_gems. Also, it'd be nice to avoid copying scripts among libraries repos.
Ah. Damn.
Initially I tried to add an optional plugin configuration to the plugin,
patch-cocoapods: true, but I can't get the environment variableBUILDKITE_PLUGIN_A8C_CI_TOOLKIT_PATCH_COCOAPODSto work: it doesn't exist even though the new option is set in pipeline.
Just checking: did you declare the additional plugin config parameter in plugin.yml here (then pointed to the commit that added it in your WPKiOS pipeline) when you attempted that?
iinm, the plugin has additionalProperties: false in its plugin.yml config, so all additional properties have to be declared explicitly in plugin.yml for them to be exposed as env var and then accessible within the plugin.
That being said…
What do you think adding a new option to the existing shell script, like validate_podspec --patch_cocoapods?
I actually think that's probably a better approach 👍 so that the option is tied to the commands that need it instead of to the whole plugin (especially wouldn't make sense to expose that option to app / non-pod repos, which use the a8c-ci-toolkit plugin… but never call validate_pod nor publish_pod in their context)
There was a problem hiding this comment.
did you declare the additional plugin config parameter in plugin.yml
Yes, I did. I tried naming it as patch-cocoapods, patch_cocoapods. I tried declaring it as string, boolean. None worked...
I have implemented the new --patch_cocoapods argument in the new commits. And this test job passed.
bin/patch-cocoapods
Outdated
| echo "$patch_content" | patch --reverse --force --directory "$cocoapods_gem_path" --strip 0 | ||
| echo "$patch_content" | patch --forward --force --directory "$cocoapods_gem_path" --strip 0; |
There was a problem hiding this comment.
Funny how one line has the ; but not the other 😄
There was a problem hiding this comment.
🙈 It's an copy-paste error. Removed in 6e15474
|
I will publish a 3.0.1 release after this PR is merged. |
This will unblock us from testing and release pod from CI. See [this PR][PR] for details. [PR]: Automattic/a8c-ci-toolkit-buildkite-plugin#78
This will unblock us from testing and release pod from CI. See [this PR][PR] for details. [PR]: Automattic/a8c-ci-toolkit-buildkite-plugin#78
This will unblock us from testing and release pod from CI. See [this PR][PR] for details. [PR]: Automattic/a8c-ci-toolkit-buildkite-plugin#78
This will unblock us from testing and release pod from CI. See: - wordpress-mobile/WordPressAuthenticator-iOS@6bb3c53 - Automattic/a8c-ci-toolkit-buildkite-plugin#78
This hack unblocks us from releasing libraries from CI. Hopefully CocoaPods can fix the issue properly and we can remove this hack soon.
This change is tested in this build.
CHANGELOG.mdif necessary.