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

🐛 Use product name instead of module name when patching XCFrameworks #266

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

PRESIDENT810
Copy link

@PRESIDENT810 PRESIDENT810 commented Nov 10, 2023

Description

When patching support files, we should use just product.name to replace XCFrameworks paths. This is because module name is used for importing modules in swift, which doesn't allow special characters like '-', so for modulemap search path we should use module name; however for header search path and library search path, special characters are allowed. Therefore when we have targets whose name has characters not supported in Swift, such as "foo-bar", using product.moduleName makes the regex pattern for patching XCFrameworks be "${PODS_XCFRAMEWORKS_BUILD_DIR}/foo_bar", which causes search path "${PODS_XCFRAMEWORKS_BUILD_DIR}/foo-bar" cannot be substituted because regex pattern doesn't match.

For example:
image
image
For patching modulemap path, we should use product.moduleName which doesn't include special characters like '-', but for patching XCFramework, we should use product.name

Checklist (I have ...)

  • 🧐 Followed the code style of the rest of the project
  • 📖 Updated the documentation if necessary
  • 👨🏻‍🔧 Added at least one test that validates that my change is working, if appropriate
  • 👮🏻‍♂️ Run make lint and fixed all warnings
  • ✅ Run make test and fixed all tests

❤️ Thanks for contributing to the 🏈 Rugby!

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e2bfb4b) 26.59% compared to head (743bfad) 26.61%.
Report is 2 commits behind head on main.

❗ Current head 743bfad differs from pull request most recent head ecb7766. Consider uploading reports for the commit ecb7766 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #266      +/-   ##
==========================================
+ Coverage   26.59%   26.61%   +0.01%     
==========================================
  Files         164      164              
  Lines        4756     4757       +1     
==========================================
+ Hits         1265     1266       +1     
  Misses       3491     3491              
Files Coverage Δ
...RugbyFoundation/Core/Use/SupportFilesPatcher.swift 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@swiftyfinch swiftyfinch added the 🐛 Bug Something isn't working label Nov 11, 2023
@swiftyfinch swiftyfinch changed the title Fix: should use product name instead of module name when patching XCFrameworks 🐛 Fix: should use product name instead of module name when patching XCFrameworks Nov 11, 2023
@swiftyfinch swiftyfinch changed the title 🐛 Fix: should use product name instead of module name when patching XCFrameworks 🐛 Use product name instead of module name when patching XCFrameworks Nov 12, 2023
@swiftyfinch swiftyfinch force-pushed the fix_patch_path branch 2 times, most recently from cc87179 to 743bfad Compare November 13, 2023 17:06
…rameworks

When patching support files, we should use just product.name to replace XCFrameworks paths. This is because module name is used for importing modules in swift, which doesn't allow special characters like '-'; however for header search path and library search path, special characters are allowed. Therefore when we have targets whose name has characters not supported in Swift, such as "foo-bar", using product.moduleName makes the regex pattern "${PODS_XCFRAMEWORKS_BUILD_DIR}/foo_bar", which causes search path "${PODS_XCFRAMEWORKS_BUILD_DIR}/foo-bar" cannot be substituted because regex pattern doesn't match.
@PRESIDENT810
Copy link
Author

@swiftyfinch Ah sorry, wasn't checking GitHub notifications on weekends... I fixed them and now it should be OK.

@swiftyfinch swiftyfinch enabled auto-merge (rebase) November 13, 2023 18:54
@swiftyfinch swiftyfinch merged commit 7c401ed into swiftyfinch:main Nov 13, 2023
7 checks passed
@swiftyfinch
Copy link
Owner

@PRESIDENT810 Thank you for your contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants