Fix action_env for ipa post processor #1
Conversation
apple/internal/processor.bzl
Outdated
| @@ -702,7 +711,8 @@ def _process( | |||
| process_and_sign_template, | |||
| provisioning_profile = None, | |||
| rule_descriptor, | |||
| rule_label): | |||
| rule_label, | |||
| env = None): | |||
There was a problem hiding this comment.
[question] Why default value to None? Can this be mandatory?
There was a problem hiding this comment.
I did it because there may be other potential users for _process method that do not call it with env parameter. If we do it mandatory, it may break something in other peace of code that does not require env for itself.
There was a problem hiding this comment.
Could you test that, make env mandatory and see what other places are using this?
Context:
- Not sure if there are any other places using this. Maybe you can make it mandatory and it just works
- In case of new code is written that needs this function, having this optional can lead to the same problem you are experiencing now
There was a problem hiding this comment.
I found a lot of places that use it - ios_rules (our case, fixed in this PR), watchos_rules, macos_rules, tvos_rules, xcframework_rules. I would rather save env = None for _process method so far and make separate PRs with tests for other platforms if it's necessary.
MIP9
force-pushed
the
bugfix/ipa_post_processor_env
branch
from
c4b6636 to
f3fe5b3
Compare
MIP9
force-pushed
the
bugfix/ipa_post_processor_env
branch
from
f3fe5b3 to
31c218f
Compare
|
Related PR upstream: bazelbuild#2232 |
No description provided.