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

Add extra check when resolving INFOPLIST_FILE and fail with a meaningful error message #24

Merged
merged 2 commits into from
Jan 17, 2019

Conversation

iby
Copy link
Contributor

@iby iby commented Apr 24, 2018

This takes care when INFOPLIST_FILE setting fails to resolve, which can happen here and here without an actual error raised.

@iby
Copy link
Contributor Author

iby commented Apr 24, 2018

This is partly a continuation of my quest with #13. Looks like it's failing because of a very old fastlane. Can we upgrade it to something higher?

@SiarheiFedartsou
Copy link
Owner

SiarheiFedartsou commented Apr 24, 2018

@ianbytchek Thanks for excellent improvement!
It is a one of the first plugins for fastlane, so fastlane version is really old :) And, yes, of course, we can update it. Could you do it?

@iby
Copy link
Contributor Author

iby commented Apr 25, 2018

This isn't outdated Fastlane… Before I dig further can you help me out a little?

Here you have:

path = plist.gsub('$(SRCROOT)', project.path.parent.to_path)
path = path.gsub('${SRCROOT}', project.path.parent.to_path)

This is what failing current tests – when variables are resolved it simply returns SRCROOT, not $(SRCROOT). I'd expect it to be an actual expanded path, but instead Xcodeproj returns variable name.

Do you know anything about this? Why? Doesn't Xcodeproj know that path?

P. S. I can simply gsub for SRCROOT, not $(SRCROOT), but it feels hacky in this case. So, would like to get to the bottom of this.

@iby
Copy link
Contributor Author

iby commented Apr 25, 2018

🚦 @SiarheiFedartsou green light! I went ahead with gsubing approach since it's a hack either way and chances are people having SRCROOT in their INFOPLIST_FILE are zero to none. 🤞

…ng INFOPLIST_FILE resolution

`common_resolved_build_setting` and `common_resolved_build_setting` don't explicitly check against available xcconfig files, though they use `resolved_build_setting`, which allows to request for that, so we should use that to ensure things don't get missed from the final resolved value. Also requires fresher and updates dependencies! 🎉
@iby
Copy link
Contributor Author

iby commented Jan 17, 2019

@SiarheiFedartsou 👋 Can we merge this? Rebased everything onto the latest version. Looks good from my end.

@SiarheiFedartsou
Copy link
Owner

@ianbytchek Yes, of course, we can. And maybe you want to be a maintainer of this repo? :) Unfortunately, for now I don't have enough time to support this plugin...

@SiarheiFedartsou SiarheiFedartsou merged commit ffe335f into SiarheiFedartsou:master Jan 17, 2019
@iby
Copy link
Contributor Author

iby commented Jan 17, 2019

Feel free to add me to the maintainers list. I know my way a around the project a little, will contribute with what I can! 👍

@SiarheiFedartsou
Copy link
Owner

@ianbytchek Thanks for accepting this proposition :) Added you as collaborator for now. As I understood it is the only thing that I can do without moving this repo to another account/organization. But it will be enough to support it.

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

2 participants