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

Fix Archive failures when using header_mappings_dir and Catalyst #10224

Merged

Conversation

tgoyne
Copy link
Contributor

@tgoyne tgoyne commented Nov 19, 2020

Catalyst uses the macOS-style versioned framework layout, so it needs the same workaround for header_mappings_dir as macOS frameworks do. We don't know at install time if a pod will be used with Catalyst, so I made it always add the build step for iOS targets and adjusted the script to do nothing if it's running on a non-versioned framework.

I updated the things added in #5352 to also test the equivalent thing for iOS, but there doesn't appear to be any existing tests which verify that the end result can actually be archived. I manually tested it on both the example project and on Realm, but is there anything further I should do here test-wise?

@joeldrotleff
Copy link

I'm just commenting to bump this up - as mentioned above this issue is causing gRPC builds to fail when archiving for Catalyst, which was a major surprise for me trying to port our Firebase app over from iOS to macOS (It was working great in development, then lo and behold the archive step failed).

So very grateful for figuring this out and looking forward to when it's merged!

@appfrilans
Copy link

I'm also super interested in this one! 🙌

@paulb777
Copy link
Member

@tgoyne It's great to see progress on this long-standing Catalyst issue. Since this is a bugfix, you may want to consider rebasing to the 1-10-stable branch. Also the CI needs to be made green.

@tgoyne
Copy link
Contributor Author

tgoyne commented Jan 21, 2021

Is there a guide to updating the integration specs? The CI failures are just that the generated project has changed.

@paulb777
Copy link
Member

bundle exec rake spec:rebuild_integration_fixtures will rebuild the integration tests, then you can make a PR to the integration test git submodule, and update the submodule reference to the integration test git hash in this repo.

@tgoyne
Copy link
Contributor Author

tgoyne commented Jan 21, 2021

I've retargeted this to stable-1-10 and updated the integration tests (and manually verified that the changes there look sensible to me). I had to bump up the maximum block length in the linter settings since the suite I added the new tests to was already the longest one, so I hope that's okay.

CHANGELOG.md Outdated Show resolved Hide resolved
@tgoyne tgoyne force-pushed the tg/catalyst-header-mappings-dir branch from a3bf2b0 to 0b389b9 Compare January 21, 2021 21:17
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tgoyne added a commit to realm/realm-swift that referenced this pull request Jan 25, 2021
These will continue failing until
CocoaPods/CocoaPods#10224 is fixed and it isn't very
useful to have it run on every PR until then.
@appfrilans
Copy link

So excited about this one! Great work! Any ETA on public release?

@dnkoutso dnkoutso added this to the 1.10.2 milestone Jan 28, 2021
@tgoyne
Copy link
Contributor Author

tgoyne commented Jan 29, 2021

The new CI failures appear to be due to the CI environment changing rather than anything related to the PR.

CHANGELOG.md Outdated
* None.
* Fix errors when archiving a Catalyst app which depends on a pod which uses header_mappings_dir.
[Thomas Goyne](https://github.com/tgoyne)
[#10016](https://github.com/CocoaPods/CocoaPods/pull/10016)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this link is pointing to the wrong PR. Can you update it to 10224?

@dnkoutso
Copy link
Contributor

@paulb777 havent checked on CI, is it a flake?

@paulb777
Copy link
Member

Doesn't seem related to this PR. Restarted CI

@paulb777
Copy link
Member

@dnkoutso Yep, they were flakes

@dnkoutso
Copy link
Contributor

@paulb777 I think CHANGELOG needs updating still and I can land it.

Catalyst uses the macOS-style versioned framework layout, so it needs the same
workaround for header_mappings_dir as macOS frameworks do. We don't know at
install time if a pod will be used with Catalyst, so I made it always add the
build step for iOS targets and adjusted the script to do nothing if it's
running on a non-versioned framework.
@appfrilans
Copy link

Any news on when 1.10.2 is about to be released?

@appfrilans
Copy link

Is Cocoapods not developed anymore? Have not seen any releases and no new progress on the issues for 1.10.2 in months.

@igor-makarov
Copy link
Contributor

@appfrilans CocoaPods is maintained by volunteers in their free time. Because of this, there's no fixed release schedule.

@appfrilans
Copy link

@appfrilans CocoaPods is maintained by volunteers in their free time. Because of this, there's no fixed release schedule.

I get that. But perhaps releases could be done more often? Finished (in this case major) fixes are now delayed several months to come out.

@dnkoutso dnkoutso merged commit 84d8154 into CocoaPods:1-10-stable Apr 26, 2021
@shmakovigor
Copy link

Any ideas on ETA for 1.10.2 with Catalyst issue fix?

@varun-pm-tact
Copy link

Any ETA for 1.10.2?

@appfrilans
Copy link

Wonders the same!

@lukemmtt
Copy link

Would really love to see 1.10.2 released with this fix, or at least as a beta I could install. Thanks very much to @tgoyne for putting together a fix!

@willm132
Copy link

Any beta I can install to fix this issue?

@lukemmtt
Copy link

Any beta I can install to fix this issue?

I haven’t tried this myself (since I found a workaround for my issue), but it looks like you could install the master branch of CocoaPods directly using this guide if needed:
https://guides.cocoapods.org/using/unreleased-features

@jmcnamara
Copy link

jmcnamara commented Jun 15, 2021

+1 to getting this merged to main and released. It caused the same issue with the Libxlsxwriter Cocoapod: jmcnamara/libxlsxwriter#337

For what it is worth I followed the instructions linked to by @lukemmtt to get a patched version of pod. Like this:

# This took a couple of goes:
$ bundle update --bundler


$ cd /tmp
$ git clone git@github.com:CocoaPods/CocoaPods.git
$ cd CocoaPods/
$ git checkout 1-10-stable
$ bundle install
$ echo $(pwd)"/bin/pod"

# So now you should have a patched version of pod. For me this was:
$ echo $(pwd)"/bin/pod"
/tmp/CocoaPods/bin/pod

# Then use this version of pod to install the required library/pod:
/tmp/CocoaPods/bin/pod install

@dnkoutso
Copy link
Contributor

dnkoutso commented Jul 2, 2021

Going to ship 1.10.2 in a few days.

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.

None yet