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

[rust] Bump clap to version 4.5.2 #13699

Merged
merged 4 commits into from
Mar 20, 2024
Merged

[rust] Bump clap to version 4.5.2 #13699

merged 4 commits into from
Mar 20, 2024

Conversation

bonigarcia
Copy link
Member

@bonigarcia bonigarcia commented Mar 15, 2024

User description

Description

This PR simply bumps the version of clap (a dependency in Selenium Manager) to 4.5.2.

Motivation and Context

This change is, in theory, harmless. But I'm doing it as a PR since it breaks the build. I don't know the cause, but it happens when Bazel is trying to compile Selenium Manager with clap 4.5.2. In particular, clap_lex 0.7.0, which is a transitive dependency of clap 4.5.2, cannot be compiled with Bazel:

ERROR: /home/runner/.bazel/external/crates__clap_lex-0.7.0/BUILD.bazel:20:13: Compiling Rust rlib clap_lex v0.7.0 (2 files) failed: (Exit 1): process_wrapper failed: error executing command (from target @crates__clap_lex-0.7.0//:clap_lex) 

...

error[E0599]: no method named `as_encoded_bytes` found for reference `&OsStr` in the current scope
   --> external/crates__clap_lex-0.7.0/src/ext.rs:186:26

I am not sure how Bazel compiles Rust. But using Cargo 1.7.6, this problem does not happen.

Any idea?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Type

enhancement


Description

  • Updated the clap dependency in rust/Cargo.toml from version 4.4.18 to 4.5.2 to include new features and improvements.
  • This update is part of enhancing Selenium Manager's CLI capabilities.
  • The previous attempt to update caused build issues with Bazel due to a transitive dependency (clap_lex 0.7.0). This PR aims to address or highlight this issue for further investigation.

Changes walkthrough

Relevant files
Dependencies
Cargo.toml
Update `clap` dependency to version 4.5.2                               

rust/Cargo.toml

  • Bumped clap version from 4.4.18 to 4.5.2.
+1/-1     

PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

Copy link
Contributor

PR Description updated to latest commit (bd487b5)

Copy link
Contributor

PR Review

⏱️ Estimated effort to review [1-5]

1, because the PR consists solely of a version bump for a single dependency. The complexity and size of the change are minimal, and the review focuses on potential compatibility issues rather than code quality or logic changes.

🧪 Relevant tests

No

🔍 Possible issues

Compilation Error: The user has already identified that this version bump causes a build failure due to an issue with compiling clap_lex 0.7.0, a transitive dependency of clap 4.5.2, with Bazel. This needs to be addressed either by finding a workaround for the Bazel compilation issue or by waiting for a fix in the dependency itself.

🔒 Security concerns

No


✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...
Utilizing extra instructions

The review tool can be configured with extra instructions, which can be used to guide the model to a feedback tailored to the needs of your project.

Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify the relevant sub-tool, and the relevant aspects of the PR that you want to emphasize.

Examples for extra instructions:

[pr_reviewer] # /review #
extra_instructions="""
In the 'possible issues' section, emphasize the following:
- Does the code logic cover relevant edge cases?
- Is the code logic clear and easy to understand?
- Is the code logic efficient?
...
"""

Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

How to enable\disable automation
  • When you first install PR-Agent app, the default mode for the review tool is:
pr_commands = ["/review", ...]

meaning the review tool will run automatically on every PR, with the default configuration.
Edit this field to enable/disable the tool, or to change the used configurations

Auto-labels

The review tool can auto-generate two specific types of labels for a PR:

  • a possible security issue label, that detects possible security issues (enable_review_labels_security flag)
  • a Review effort [1-5]: x label, where x is the estimated effort to review the PR (enable_review_labels_effort flag)
Extra sub-tools

The review tool provides a collection of possible feedbacks about a PR.
It is recommended to review the possible options, and choose the ones relevant for your use case.
Some of the feature that are disabled by default are quite useful, and should be considered for enabling. For example:
require_score_review, require_soc2_ticket, and more.

Auto-approve PRs

By invoking:

/review auto_approve

The tool will automatically approve the PR, and add a comment with the approval.

To ensure safety, the auto-approval feature is disabled by default. To enable auto-approval, you need to actively set in a pre-defined configuration file the following:

[pr_reviewer]
enable_auto_approval = true

(this specific flag cannot be set with a command line argument, only in the configuration file, committed to the repository)

You can also enable auto-approval only if the PR meets certain requirements, such as that the estimated_review_effort is equal or below a certain threshold, by adjusting the flag:

[pr_reviewer]
maximal_review_effort = 5
More PR-Agent commands

To invoke the PR-Agent, add a comment using one of the following commands:

  • /review: Request a review of your Pull Request.
  • /describe: Update the PR title and description based on the contents of the PR.
  • /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
  • /ask <QUESTION>: Ask a question about the PR.
  • /update_changelog: Update the changelog based on the PR's contents.
  • /add_docs 💎: Generate docstring for new components introduced in the PR.
  • /generate_labels 💎: Generate labels for the PR based on the PR's contents.
  • /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component.

See the tools guide for more details.
To list the possible configuration parameters, add a /config comment.

See the review usage page for a comprehensive guide on using this tool.

Copy link
Contributor

codiumai-pr-agent-pro bot commented Mar 15, 2024

PR Code Suggestions

No code suggestions found for PR.

@bonigarcia bonigarcia requested a review from p0deje March 15, 2024 13:16
Copy link
Contributor

codiumai-pr-agent-pro bot commented Mar 15, 2024

CI Failure Feedback

(Checks updated until commit 7a04007)

Action: Format / Check format script run

Failed stage: Run Bazel [❌]

Failure summary:

The action failed due to multiple issues:

  • A 404 Not Found error occurred when attempting to fetch the aspnetcore-targeting-pack-7.0 package
    from the specified mirror. This indicates that the package could not be located at the given URL.
  • The process completed with exit code 1, which generally indicates a failure in the execution of a
    script or command. The specific cause of this exit code is not detailed in the provided logs, but it
    could be related to the earlier package fetch failure or another issue in the build or test scripts.

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    295:  After this operation, 1616 MB disk space will be freed.
    296:  Get:1 file:/etc/apt/apt-mirrors.txt Mirrorlist [142 B]
    297:  Get:2 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe amd64 aspnetcore-targeting-pack-6.0 amd64 6.0.127-0ubuntu1~22.04.1 [1458 kB]
    298:  Ign:3 http://azure.archive.ubuntu.com/ubuntu jammy-updates/universe amd64 aspnetcore-targeting-pack-7.0 amd64 7.0.116-0ubuntu1~22.04.1
    299:  Ign:3 http://archive.ubuntu.com/ubuntu jammy-updates/universe amd64 aspnetcore-targeting-pack-7.0 amd64 7.0.116-0ubuntu1~22.04.1
    300:  Ign:3 http://security.ubuntu.com/ubuntu jammy-updates/universe amd64 aspnetcore-targeting-pack-7.0 amd64 7.0.116-0ubuntu1~22.04.1
    301:  Err:3 mirror+file:/etc/apt/apt-mirrors.txt jammy-updates/universe amd64 aspnetcore-targeting-pack-7.0 amd64 7.0.116-0ubuntu1~22.04.1
    302:  404  Not Found [IP: 52.252.75.106 80]
    303:  E: Failed to fetch mirror+file:/etc/apt/apt-mirrors.txt/pool/universe/d/dotnet7/aspnetcore-targeting-pack-7.0_7.0.116-0ubuntu1%7e22.04.1_amd64.deb  404  Not Found [IP: 52.252.75.106 80]
    ...
    
    826:  Package 'php-symfony-debug-bundle' is not installed, so not removed
    827:  Package 'php-symfony-dependency-injection' is not installed, so not removed
    828:  Package 'php-symfony-deprecation-contracts' is not installed, so not removed
    829:  Package 'php-symfony-discord-notifier' is not installed, so not removed
    830:  Package 'php-symfony-doctrine-bridge' is not installed, so not removed
    831:  Package 'php-symfony-doctrine-messenger' is not installed, so not removed
    832:  Package 'php-symfony-dom-crawler' is not installed, so not removed
    833:  Package 'php-symfony-dotenv' is not installed, so not removed
    834:  Package 'php-symfony-error-handler' is not installed, so not removed
    ...
    
    1662:  * `Zip::OutputStream`
    1663:  Please ensure that your Gemfiles and .gemspecs are suitably restrictive
    1664:  to avoid an unexpected breakage when 3.0 is released (e.g. ~> 2.3.0).
    1665:  See https://github.com/rubyzip/rubyzip for details. The Changelog also
    1666:  lists other enhancements and bugfixes that have been implemented since
    1667:  version 2.3.0.
    1668:  Bundler will use `/tmp/bundler20240320-3-cjvri23' as your home directory temporarily.
    1669:  �[32m[909 / 1,647]�[0m Running bundle install (@bundle//:bundle); 16s linux-sandbox ... (4 actions, 3 running)
    1670:  �[32m[954 / 1,647]�[0m Checking 2 JS files in @//javascript/atoms:errors; 2s worker ... (4 actions, 3 running)
    ...
    
    1785:  @@ -234,7 +234,7 @@ rules_rust_dependencies()
    1786:  rust_register_toolchains(
    1787:  edition = "2021",
    1788:  versions = [
    1789:  -        "1.76.0"
    1790:  +        "1.76.0",
    1791:  ],
    1792:  )
    1793:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @bonigarcia
    Copy link
    Member Author

    I don't want to merge this yet since, as explained in the description, it breaks the build. But I don't know the underlying reason. It seems this new crate version cannot be compiled using Bazel.

    @p0deje
    Copy link
    Member

    p0deje commented Mar 18, 2024

    I don't know what is wrong with the build, maybe there is a difference in toolchains?

    @bonigarcia
    Copy link
    Member Author

    I don't know what is wrong with the build, maybe there is a difference in toolchains?

    I'm not sure. I suppose it might be related to the Rust compiler version used by Bazel. For example, in my local installation, when using cargo as package manager, the compiler (rustc) version is the following:

    > rustc --version
    rustc 1.76.0 (07dca489a 2024-02-04)
    

    Is there any way to find out which version is using Bazel to compile Rust?

    @p0deje
    Copy link
    Member

    p0deje commented Mar 19, 2024

    My local is 1.70:

    $ bazel-selenium/external/rust_darwin_aarch64__aarch64-apple-darwin__stable_tools/bin/rustc --version
    rustc 1.70.0 (90c541806 2023-05-31)
    

    Can you try changing the version in WORKSPACE per https://bazelbuild.github.io/rules_rust/#specifying-rust-version and see if it helps?

    @bonigarcia
    Copy link
    Member Author

    @p0deje Indeed, that change made the trick, thanks a lot!

    rust_register_toolchains(
        edition = "2021",
        versions = [
            "1.76.0",
        ],
    )
    

    @bonigarcia bonigarcia merged commit e195d79 into trunk Mar 20, 2024
    20 checks passed
    @bonigarcia bonigarcia deleted the sm_bump_clap branch March 20, 2024 09:50
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    3 participants