Re: Review Board Ticket #4988: Fail to post diff with perforce repo: 'Value to convert is unexpected type %s',

[Ben Jackson]

--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4988/
--

New update by benjackson
For Beanbag, Inc. > Review Board > Ticket #4988


Reply:

https://reviews.reviewboard.org/r/12798/
https://reviews.reviewboard.org/r/12799/

-- 
You received this message because you are subscribed to the Google Groups 
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/reviewboard-issues/20230123101747.23038.44754%40ip-10-1-54-209.ec2.internal.

Re: Review Board Ticket #4988: Fail to post diff with perforce repo: 'Value to convert is unexpected type %s',

[Ben Jackson]

--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4988/
--

New update by benjackson
For Beanbag, Inc. > Review Board > Ticket #4988


Reply:

I just tried with release-5.0.x and same behaviour.

-- 
You received this message because you are subscribed to the Google Groups 
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/reviewboard-issues/20230120223821.4660.34457%40ip-10-1-54-209.ec2.internal.

Review Board Ticket #4988: Fail to post diff with perforce repo: 'Value to convert is unexpected type %s',

[Ben Jackson]

--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4988/
--

New ticket #4988 by benjackson
For Beanbag, Inc. > Review Board

Status: New
Tags: Priority:Medium, Type:Defect

File attachments:

 * PRE-CREATION.png
   

 * PRE-CREATION-assignment-call-stack.png
   



--
Fail to post diff with perforce repo: 'Value to convert is unexpected type %s', 

==

# What version are you running?

4.0.11

# What's the URL of the page containing the problem?

`│>>> Making HTTP POST request to http://ukfd-ltp4l:49320/api/validation/diffs/`


# What steps will reproduce the problem?

It doesn't seem to happen for all diffs where a new file is added, but it's 
certainly related to adding new files, with perforce as the repo type. 
Specifically, it needs to go though these lines in `perforce.py` where the old 
revision is set to `PRE_CREATION`.

```
# Older versions of Perforce had this lovely idiosyncracy that diffs
# show revision #1 both for pre-creation and when there's an actual
# revision. In this case, we need to check if the file already exists
# in the repository.
#
# Newer versions use #0, so it's quicker to check.
if (revision == b'0' or
(revision == b'1' and
 not self.repository.get_file_exists(filename.decode('utf-8'),
 revision.decode('utf-8':
revision = PRE_CREATION


```

1. Using git-p4 client repo, add a new file and commit that
2. rbt post

As i said, i debugged it a bit but wasn't 100% sure why this didn't happen for 
every new file add. See below for the exact exception and code which is 
triggering it. Attached screenshots of debugger showing the call stacks where 
it's assigned etc.

I can easily repro this with a specific commit right now, and have a debug 
environment set up, so please shout if there is more info I can provide.


# What is the expected output? What do you see instead?

Diff created.
Actual: Exceptoin TypeError: ('Value to convert is unexpected type %s', )

```
$>rbt post --repository perforce:1666 --server http://ukfd-ltp4l:49320 
675417754ac980cc60d9b72eddaf9f9cb80817dd

Unexpected error when validating the diff: ('Value to convert is unexpected 
type %s', ) (API Error 224: Diff 
Parsing Failed)
ukfd-ltp4l(benj:TESTKIT_main.git) TESTKIT/main.git>

```

# What operating system are you using? What browser?

RHEL7

Also reproduced in a minimal ubuntu 20.02 container (based on 
contrib/docker/Dockerfile)

# Please provide any additional information below.

The issue appears to be with this code in filediff_creator.py:

```
# Store the information on the parent's filename and revision.
# It's important we force these to text, since they may be
# byte strings and the revision may be a Revision instance.
#
# Starting in Review Board 4.0.5, we store this any time there's
# a parent diff, whether or not the file existed in the parent
# diff.
extra_data.update({
FileDiff._IS_PARENT_EMPTY_KEY: parent_is_empty,
'parent_source_filename':
convert_to_unicode(parent_source_filename,
   encoding_list)[1],
'parent_source_revision':
convert_to_unicode(parent_source_revision,
   encoding_list)[1],
})



```

Per the comment, the argument `parent_source_revision` may be a `Revision` 
object (in this case, it is literally `PRE_CREATION` which is an instance of 
`Revision`, however `convert_to_unicode` requires arguments to be some type of 
string, and throws an exception when they aren'

Git blame points the finger at this commit on the branch: b972f20c55 (it's the 
same on master)



---


Here's the full stack trace from the log:

```
2023-01-20 20:59:32,986 - ERROR -  - reviewboard.webapi.resources.validate_diff 
- Unexpected error whe
n validating diff.
Traceback (most recent call last):
  File "/build_root/reviewboard/reviewboard/webapi/resources/validate_diff.py", 
line 161, in create
DiffSet.objects.create_from_upload(
  File "/build_root/reviewboard/reviewboard/diffviewer/managers.py", line 767, 
in create_from_upload
return self.create_from_data(
  File "/build_root/reviewboard/reviewboard/diffviewer/managers.py", line 1060, 
in create_from_data
create_filediffs(
  File "/build_root/reviewboard/reviewboard/diffviewer/filediff_creator.py", 
line 210, in create_filed
iffs

Re: RBTools Ticket #4825: rbt diff fails for git-p4 repo with Python 3.7.2 on Windows.

[Ben Jackson]

--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4825/
--

New update by alebastr
For Beanbag, Inc. > RBTools > Ticket #4825


Reply:

i posted it really in the hope that it would be useful. apologies, but i 
don't have the bandwidth right now to do a proper 9 yards on it, not least 
because it was many months ago that i made the change and i no longer have the 
code paged in :)

-- 
You received this message because you are subscribed to the Google Groups 
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/reviewboard-issues/20200117182147.23864.85934%40ip-10-1-54-209.ec2.internal.

Re: RBTools Ticket #4825: rbt diff fails for git-p4 repo with Python 3.7.2 on Windows.

[Ben Jackson]

--
To reply, visit https://hellosplat.com/s/beanbag/tickets/4825/
--

New update by alebastr
For Beanbag, Inc. > RBTools > Ticket #4825


Reply:

How about this patch ?

```
commit ad51dadc52685520293a0ed226b95649b4104298
Author: Ben Jackson 
Date:   Wed Apr 24 10:16:37 2019 +0100

Fix unicode errors

diff --git a/rbtools/clients/git.py b/rbtools/clients/git.py
index 142e547..cd4d459 100644
--- a/rbtools/clients/git.py
+++ b/rbtools/clients/git.py
@@ -925,7 +925,9 @@ class GitClient(SCMClient):
 p4rev = b''

 # Find which depot changelist we're based on
-log = self._execute([self.git, 'log', merge_base], 
ignore_errors=True)
+log = self._execute([self.git, 'log', merge_base],
+ignore_errors=True,
+results_unicode=False)

 for line in log:
 m = re.search(br'[rd]epo.-paths = "(.+)": change = (\d+).*]',
@@ -952,23 +954,24 @@ class GitClient(SCMClient):
 elif (line.startswith(b'--- ') and i + 1 < len(diff_lines) and
   diff_lines[i + 1].startswith(b'+++ ')):
 data = self._execute(
-['p4', 'files', base_path + filename + '@' + p4rev],
-ignore_errors=True, results_unicode=False)
+[b'p4', b'files', base_path + filename + b'@' + p4rev],
+ignore_errors=True,
+results_unicode=False)
 m = re.search(br'^%s%s#(\d+).*$' % (re.escape(base_path),
 re.escape(filename)),
   data, re.M)
 if m:
 file_version = m.group(1).strip()
 else:
-file_version = 1
+file_version = b'1'

-diff_data += b'--- %s%s\t%s%s#%s\n' % (base_path, filename,
+diff_data += ( b'--- %s%s\t%s%s#%s\n' % (base_path, 
filename,
base_path, filename,
-   file_version)
+   file_version) )
 elif line.startswith(b'+++ '):
 # TODO: add a real timestamp
-diff_data += b'+++ %s%s\t%s\n' % (base_path, filename,
-  b'TIMESTAMP')
+diff_data += ( b'+++ %s%s\t%s\n' % (base_path, filename,
+  b'TIMESTAMP') )
 else:
 diff_data += line

```

works for me...

-- 
You received this message because you are subscribed to the Google Groups 
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/reviewboard-issues/20191203171142.30925.62351%40ip-10-1-54-209.ec2.internal.