• 
      

    Fix up SVN patcher behavior when basedirs don't overlap.

    Review Request #14343 — Created Feb. 12, 2025 and submitted

    Information

    RBTools
    release-5.x

    Reviewers

    The new SVN patcher implementation tried to be very smart when the patch
    base directory was not the same as the client base directory, filtering
    out files that are not patchable. This would happen, for example, if a
    patch was created in /trunk/ but the local checkout only contained
    /trunk/project1/.

    Unfortunately, this completely broke the use case where a patch is
    created in a directory that has no overlap at all with the client (for
    example, the patch was created in /trunk/project1/ but the local
    checkout is /branches/release/project1/). In this case, we'd filter out
    everything, attempt to apply an empty patch, and error out.

    This change updates the patcher behavior to be smarter about how base
    directories are dealt with. If the client is a subdirectory of the patch
    base directory, we do exactly as before: filter out any files from the
    patch which are not present in the client's root, and try to patch. In
    all other cases, we now do two new things:

    1. The prefix level will be computed from the patch base directory
      instead of the client's.
    2. We just take the patch as-is (with the computed prefix level) and try
      to apply it. If there are any conflicts, it'll be up to the user to
      figure out why.
    • Ran unit tests, including two new tests for the subdirectory and
      no-overlap scenarios.
    • Checked out /trunk/ and /branches/1.14.x/ from the apache SVN
      repository. Posted one change from trunk, and then used rbt patch to
      apply it into the branch. Before this would error out with the empty
      patch, after it works correctly.
    Summary ID
    Fix up SVN patcher behavior when basedirs don't overlap.
    The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.
    bd989b6d99044fab476df564f0c8a07571b2b76c
    Description From Last Updated

    line too long (80 > 79 characters) Column: 80 Error code: E501

    reviewbotreviewbot

    Add , optional

    maubinmaubin

    The second line of the bullet point must be aligned with the content of the first line.

    chipx86chipx86

    Missing Version Added.

    chipx86chipx86

    Rather than invoking the whole command twice, let's just compute what base_dir value we want to use.

    chipx86chipx86

    Should probably be a Sequence.

    chipx86chipx86

    undefined name 'Sequence' Column: 18 Error code: F821

    reviewbotreviewbot
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    david
    maubin
    1. 
        
    2. rbtools/clients/svn.py (Diff revision 2)
       
       
      Show all issues

      Add , optional

    3. 
        
    david
    maubin
    1. Ship It!
    2. 
        
    chipx86
    1. 
        
    2. rbtools/clients/svn.py (Diff revision 3)
       
       
       
      Show all issues

      The second line of the bullet point must be aligned with the content of the first line.

    3. rbtools/clients/svn.py (Diff revision 3)
       
       
       
       
      Show all issues

      Missing Version Added.

    4. rbtools/clients/svn.py (Diff revision 3)
       
       
       
       
       
       
       
      Show all issues

      Rather than invoking the whole command twice, let's just compute what base_dir value we want to use.

    5. rbtools/clients/tests/test_svn.py (Diff revision 3)
       
       
      Show all issues

      Should probably be a Sequence.

    6. 
        
    david
    Review request changed
    Commits:
    Summary ID
    Fix up SVN patcher behavior when basedirs don't overlap.
    The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.
    6fb5c396bda71236511e4ba5a53130c27836cfaa
    Fix up SVN patcher behavior when basedirs don't overlap.
    The new SVN patcher implementation tried to be very smart when the patch base directory was not the same as the client base directory, filtering out files that are not patchable. This would happen, for example, if a patch was created in /trunk/ but the local checkout only contained /trunk/project1/. Unfortunately, this completely broke the use case where a patch is created in a directory that has no overlap at all with the client (for example, the patch was created in /trunk/project1/ but the local checkout is /branches/release/project1/). In this case, we'd filter out everything, attempt to apply an empty patch, and error out. This change updates the patcher behavior to be smarter about how base directories are dealt with. If the client is a subdirectory of the patch base directory, we do exactly as before: filter out any files from the patch which are not present in the client's root, and try to patch. In all other cases, we now do two new things: 1. The prefix level will be computed from the patch base directory instead of the client's. 2. We just take the patch as-is (with the computed prefix level) and try to apply it. If there are any conflicts, it'll be up to the user to figure out why. Testing Done: - Ran unit tests, including two new tests for the subdirectory and no-overlap scenarios. - Checked out /trunk/ and /branches/1.14.x/ from the apache SVN repository. Posted one change from trunk, and then used `rbt patch` to apply it into the branch. Before this would error out with the empty patch, after it works correctly.
    bd989b6d99044fab476df564f0c8a07571b2b76c

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    chipx86
    1. I'm happy once Review Bot is happy.

    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.x (e7e6b73)