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

[fastlane_core] prevent negative argument error for message formatting #20857

Merged
merged 1 commit into from Nov 16, 2022
Merged

[fastlane_core] prevent negative argument error for message formatting #20857

merged 1 commit into from Nov 16, 2022

Conversation

ghost
Copy link

@ghost ghost commented Nov 15, 2022

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

When trying to run Fastlane on a Windows CI instance, it fails with negative argument

C:/tools/ruby31/lib/ruby/gems/3.1.0/gems/fastlane-2.211.0/fastlane_core/lib/fastlane_core/ui/implementations/shell.rb:95:in `*': \e[31m[!] negative argument\e[0m (ArgumentError)

This problem was identified in this issue:
#20123

Description

Seems TTY::Screen.width is not reliable and can result in the TTY::Screen.width - format.length calculation to be negative, which will break when trying to do success("-" * i) where i is negative. This change will limit the value i to be greater or equal to zero.

Testing Steps

Tested running with my changes in our CI (using Git bash for Windows), works as expected:

        bundle install && bundle update fastlane
        bundle show fastlane
        LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 FASTLANE_OPT_OUT_USAGE=1 bundle exec fastlane android validate_key

Output:

[2022-11-15T15:22:15.821Z] Using fastlane 2.211.0 from source at `C:/Users/Administrator/fastlane` and installing its executables
[2022-11-15T15:22:15.922Z] Using xcov 1.4.3
[2022-11-15T15:22:15.922Z] Bundler attempted to update fastlane but its version stayed the same
[2022-11-15T15:22:15.922Z] Bundle updated!
[2022-11-15T15:22:16.630Z] C:/Users/Administrator/fastlane
[2022-11-15T15:22:20.782Z] [15:22:20]: ---------------------------
[2022-11-15T15:22:20.782Z] [15:22:20]: --- Step: opt_out_usage ---
[2022-11-15T15:22:20.782Z] [15:22:20]: ---------------------------
[2022-11-15T15:22:20.782Z] [15:22:20]: Disabled upload of used actions
[2022-11-15T15:22:20.782Z] [15:22:20]: ------------------------------
[2022-11-15T15:22:20.782Z] [15:22:20]: --- Step: default_platform ---
[2022-11-15T15:22:20.782Z] [15:22:20]: ------------------------------
[2022-11-15T15:22:20.782Z] [15:22:20]: Driving the lane 'android validate_key' 🚀
[2022-11-15T15:22:20.782Z] [15:22:20]: ------------------------------------------
[2022-11-15T15:22:20.782Z] [15:22:20]: --- Step: validate_play_store_json_key ---
[2022-11-15T15:22:20.782Z] [15:22:20]: ------------------------------------------
[2022-11-15T15:22:20.782Z] 
[2022-11-15T15:22:20.782Z] +----------+------------------------------------+
[2022-11-15T15:22:20.782Z] |   Summary for validate_play_store_json_key    |
[2022-11-15T15:22:20.782Z] +----------+------------------------------------+
[2022-11-15T15:22:20.782Z] | json_key | ../ci/android/google-play-key.json |
[2022-11-15T15:22:20.782Z] | timeout  | 300                                |
[2022-11-15T15:22:20.782Z] +----------+------------------------------------+
[2022-11-15T15:22:20.782Z]
[2022-11-15T15:22:20.883Z] [15:22:20]: Successfully established connection to Google Play Store.
[2022-11-15T15:22:20.883Z] [15:22:20]: Successfully executed lane: validate_key
[2022-11-15T15:22:20.984Z] 
[2022-11-15T15:22:20.984Z] +------+------------------------------+-------------+
[2022-11-15T15:22:20.984Z] |                 fastlane summary                  |
[2022-11-15T15:22:20.984Z] +------+------------------------------+-------------+
[2022-11-15T15:22:20.984Z] | Step | Action                       | Time (in s) |
[2022-11-15T15:22:20.984Z] +------+------------------------------+-------------+
[2022-11-15T15:22:20.984Z] | 1    | opt_out_usage                | 0           |
[2022-11-15T15:22:20.984Z] | 2    | default_platform             | 0           |
[2022-11-15T15:22:20.984Z] | 3    | validate_play_store_json_key | 0           |
[2022-11-15T15:22:20.984Z] +------+------------------------------+-------------+
[2022-11-15T15:22:20.984Z] 
[2022-11-15T15:22:20.984Z] [15:22:20]: fastlane.tools finished successfully 🎉

@google-cla
Copy link

google-cla bot commented Nov 15, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@joshdholtz joshdholtz changed the title prevent negative argument error for message formatting [fastlane_core] prevent negative argument error for message formatting Nov 16, 2022
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Oh, this feels much safer! I love this 😊 Thank you for making this!

@joshdholtz joshdholtz merged commit 7cb9367 into fastlane:master Nov 16, 2022
@ghost ghost deleted the fix-negative-argument-on-windows branch November 16, 2022 08:03
@fastlane-bot
Copy link

Hey @akselilukkarila 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

@ghost
Copy link
Author

ghost commented Nov 29, 2022

Hey @akselilukkarila 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 The code change now lives in the master branch, however it wasn't released to RubyGems yet. We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍 We'll notify you once we shipped a new release with your changes 🚀

Are you planning to do a release soon? It has been two weeks since this was merged and we would like to start using the fixed version in our CI, since the current released version is broken...

@xucian
Copy link

xucian commented Dec 24, 2022

hello
any plans to create a release for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants