RB5: Ensure that revisions are text before converting to unicode
Review Request #12799 — Created Jan. 23, 2023 and submitted
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.
- 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/