Fix incompatible encoding regexp match for linting non-ascii pod name
#9776
Conversation
Gemfile.lock
Outdated
| @@ -134,8 +134,8 @@ GEM | |||
| minitest (~> 5.1) | |||
| thread_safe (~> 0.3, >= 0.3.4) | |||
| tzinfo (~> 1.1) | |||
| addressable (2.5.1) | |||
There was a problem hiding this comment.
why does this file need to change at all here?
There was a problem hiding this comment.
it has some strange points but bundle update webmock emit this error. updating webmock & danger I can install webmock 3.5.
Bundler could not find compatible versions for gem "addressable":
In Gemfile:
cocoapods-core was resolved to 1.9.1, which depends on
addressable (~> 2.6)
danger was resolved to 5.3.0, which depends on
octokit (~> 4.2) was resolved to 4.7.0, which depends on
sawyer (>= 0.5.3, ~> 0.8.0) was resolved to 0.8.1, which depends on
addressable (>= 2.3.5, < 2.6)
webmock was resolved to 3.8.3, which depends on
addressable (>= 2.3.6)
There was a problem hiding this comment.
Perhaps bundle install instead of bundle update?
There was a problem hiding this comment.
possibly it's unnecessary change. Gemfile refs to cocoapods-core at master and core needs addressable (~> 2.6) on current master but core 1.9.1 does not require it. I think this may cause bundle update error.
bundle update for webmock is required to run the added test case on ruby 2.6. ruby 2.6 is system ruby on Catalina and used in the latest machines in github actions and bitrise. anyway, travis here uses ruby 2.0..? It might be better to remove the test case for ruby 2.6...
There was a problem hiding this comment.
I found disabling webmock in the testcase can avoid unknown keyword: write_timeout error without modifying Gemfile.lock. I have deleted the Gemfile.lock commit and force pushed. I think it should work.
How can this WebMock handling in the test case be better? How do you think of it?
There was a problem hiding this comment.
I think this resolved in #9776 (comment)
|
nice! Can you add a changelog entry and point to the GH issue in it? |
banjun
force-pushed
the
fix-ascii-regex-match
branch
from
4559eb1
to
97195ee
Compare
| @@ -59,5 +60,14 @@ module Pod | |||
| UI.output.should.include 'could not be loaded' | |||
| end | |||
| end | |||
|
|
|||
| it 'lints a spec with non-ascii pod name' do | |||
There was a problem hiding this comment.
maybe dont add the test here but instead add it to the validator_spec.rb and just ensure there is no crash by asking to validate this spec.
There was a problem hiding this comment.
dc14392
move the test into validator_spec.rb and that resolves WebMock and the Gemfile modification issues.
banjun
force-pushed
the
fix-ascii-regex-match
branch
from
20efae6
to
dc14392
Compare
CHANGELOG.md
Outdated
| @@ -19,6 +19,11 @@ To install release candidates run `[sudo] gem install cocoapods --pre` | |||
|
|
|||
| ##### Bug Fixes | |||
|
|
|||
| * Fix `incompatible encoding regexp match` for linting non-ascii pod name | |||
There was a problem hiding this comment.
last nit: please add two spaces at the end of this sentence for markdown formatting
|
@dnkoutso is xcodebuild unavailable on travis? if so, we have to stub validator.stubs(:_xcodebuild).returns("note: Execution policy exception registration failed and was skipped: Error Domain=NSPOSIXErrorDomain Code=1 \"Operation not permitted\" (in target '※ikemen' from project 'Pods')".force_encoding('ASCII-8BIT')) |
|
@dnkoutso thanks for approve! I've fixed the travis build. I'd like you to re-review with the latest commit. |
Fixes #9765
Added a unit test for the issue and fix with minimal changes.
When the validator parses an
xcodebuildoutput, String encoding isASCII-8BITon some environments (see the issue). As I can't tell how to fix the root cause in a safe way, I choose to apply a minimum fix in this PR.The testcase potentially requires xcodebuild to be executed because the key is reading from its outputs from the system. I've done stub a part of xcodebuild output value for travis.