RB5: Ensure that revisions are text before converting to unicode

Review Request #12799 — Created Jan. 23, 2023 and updated

puremourning
Review Board
release-5.0.x
393919d...
reviewboard

In commit b972f20c55, the diff parser was refactored to remove
DiffParser.get_orig_commit_id(). As aprt of that, a regression was
introduced into create_filediff() that meant whenever the parent revision
was an instance of Revision (rather than a str or bytes), we had a crash
in convert_to_unicode.

The parent revision can be a Revision object specifically whan it is
PRE_CREATION, such as when adding a new file to a Perforce repository.

This change re-introduces the coersion to string of the
parent_source_revision which was lost in the refactoring.

Fixes https://hellosplat.com/s/beanbag/tickets/4988/.

NOTE: This differs slightly from
https://reviews.reviewboard.org/r/12798/ only in that we use `force_str`
in place of `force_text` from django.

See also https://reviews.reviewboard.org/r/12798/

Re-do the steps that trigger the issue in https://hellosplat.com/s/beanbag/tickets/4988/. PASS

I tried to run the pytest tests. This is inprogress. As with the rb4 branch, the Perforce tests just hang and i'm not sure how to fix this. I'd like to add a test which repros the issue with the perforce tool, but with the test setup not clear, that's not so easy.

puremourning
Review request changed

Description:

   

In commit b972f20c55, the diff parser was refactored to remove

    DiffParser.get_orig_commit_id(). As aprt of that, a regression was
    introduced into create_filediff() that meant whenever the parent revision
    was an instance of Revision (rather than a str or bytes), we had a crash
    in convert_to_unicode.

   
   

The parent revision can be a Revision object specifically whan it is

    PRE_CREATION, such as when adding a new file to a Perforce repository.

   
   

This change re-introduces the coersion to string of the

    parent_source_revision which was lost in the refactoring.

   
   

Fixes https://hellosplat.com/s/beanbag/tickets/4988/.

   
   

NOTE: This differs slightly from

~   https://reviews.reviewboard.org/r/12798/ only in that we use force_str
~   in place of force_text from django.

  ~ https://reviews.reviewboard.org/r/12798/ only in that we use `force_str`
  ~ in place of `force_text` from django.

  +
  +

  +
  +

See also https://reviews.reviewboard.org/r/12798/

Loading...