• 
      

    Fix rbt patch sending prefix levels as strings.

    Review Request #14254 — Created Dec. 3, 2024 and submitted

    Information

    RBTools
    release-5.x

    Reviewers

    The work done on the new patcher was meant to avoid the case where
    strings were used instead of integers for path prefix levels. However,
    the work here was incomplete.

    SCMClient.apply_patch() does normalize strings to integers, which is
    correct when applying patches using that method. For a brief period of
    time, Subversion (which was susceptible to the string prefix level bug)
    benefited from this, but when it was converted to Patcher, this
    workaround was lost. It's worth noting that apply_patch(), for
    historical reasons, does expect a string, but not all SCMs handled
    this right.

    A core issue here was that rbt patch --px was configured to save the
    value as a string, not an integer. Still, though, given
    apply_patch()'s expectations, this wasn't technically incorrect until
    now. With the new Patcher API, we do now set this to store as an
    integer explicitly, and assert this.

    Patch() has been updated to decode string-wrapped integers, with a
    deprecation warning. This ensures that any other calls that result in a
    Patch object will normalize to the right type.

    Unit tests pass.

    Manually tested rbt patch --px with integers, invalid values, and with
    the argument omitted.

    Summary ID
    Fix rbt patch sending prefix levels as strings.
    The work done on the new patcher was meant to avoid the case where strings were used instead of integers for path prefix levels. However, the work here was incomplete. `SCMClient.apply_patch()` does normalize strings to integers, which is correct when applying patches using that method. For a brief period of time, Subversion (which was susceptible to the string prefix level bug) benefited from this, but when it was converted to `Patcher`, this workaround was lost. It's worth noting that `apply_patch()`, for historical reasons, *does* expect a string, but not all SCMs handled this right. A core issue here was that `rbt patch --px` was configured to save the value as a string, not an integer. Still, though, given `apply_patch()`'s expectations, this wasn't technically incorrect until now. With the new `Patcher` API, we do now set this to store as an integer explicitly, and assert this. `Patch()` has been updated to decode string-wrapped integers, with a deprecation warning. This ensures that any other calls that result in a `Patch` object will normalize to the right type.
    461855d27ee7aba005281c21227e79b98435e701
    maubin
    1. Ship It!
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.x (8b1afd8)