4573: Cannot show diff for changes in a file that was created in parent diff

Misery

What version are you running?

2.0.15

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

https://reviewboard/r/6319/diff/1#index_header

What steps will reproduce the problem?

  1. Create two changesets in mercurial on top of a changeset in the repository (base)
  2. First changeset: create a new file
  3. Second changeset: change something in the new file
  4. Use rbt to upload second changeset as a review request with given base changeset and first changeset as parent diff
  5. Open diff viewer: There was an error displaying this diff.

It tries to get the "changed file" of second changeset from repository (base) changeset. But since it is created by first changeset (parent diff) it cannot be found in repository.
The diff viewer should check the parent diff if it was created by the parent diff if it cannot be found in repository.

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

Show correct diff

What operating system are you using? What browser?

Linux, Firefox

Please provide any additional information below.

ERROR

None - user - /r/6338/diff/1/fragment/123631/ - root - ReviewsDiffFragmentView.get: Error when rendering diffset for filediff ID=123631, interfilediff ID=None, chunkindex=None: The file 'bla.java' (r58d64593b8f603257e5ae7e5d817f41bcec838a8) could not be found in the repository
Traceback (most recent call last):
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/views.py", line 299, in get
response = renderer.render_to_response(request)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/renderers.py", line 56, in render_to_response
return HttpResponse(self.render_to_string(request))
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/renderers.py", line 74, in render_to_string
large_data=True)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 298, in cache_memoize
compress_large_data))
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 252, in cache_memoize_iter
items = items_or_callable()
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 295, in <lambda>
lambda: [lookup_callable()],
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/renderers.py", line 73, in <lambda>
lambda: self.render_to_string_uncached(request),
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/renderers.py", line 87, in render_to_string_uncached
request=request)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 669, in populate_diff_chunks
chunks = list(generator.get_chunks())
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 786, in get_chunks
for chunk in super(DiffChunkGenerator, self).get_chunks(cache_key):
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 107, in get_chunks
large_data=True)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 298, in cache_memoize
compress_large_data))
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 252, in cache_memoize_iter
items = items_or_callable()
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 295, in <lambda>
lambda: [lookup_callable()],
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 106, in <lambda>
lambda: list(self.get_chunks_uncached()),
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 792, in get_chunks_uncached
self.encoding_list)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 199, in get_original_file
request=request)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 362, in get_file
large_data=True)[0]
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 298, in cache_memoize
compress_large_data))
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 252, in cache_memoize_iter
items = items_or_callable()
File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 295, in <lambda>
lambda: [lookup_callable()],
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 361, in <lambda>
request)],
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 555, in _get_file_uncached
base_commit_id=base_commit_id)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 50, in get_file
base_commit_id=base_commit_id)
File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 470, in cat_file
raise FileNotFoundError(path, rev)
FileNotFoundError: The file 'bla.java' (r58d64593b8f603257e5ae7e5d817f41bcec838a8) could not be found in the repository

Misery
#1 Misery

I mean reviewboard 2.5.15.
Of course: Same happens if we use "https://reviewboard/r/new/" instead of rbt.

Misery
#2 Misery

Ok, same problem with 3.0.2 and a moved file.

That is reproducible everytime.

  1. Create a new file
  2. Create a commit (commit 1)
  3. push it to repository
  4. "hg move filename newfilename"
  5. Create a commit (commit 2)
  6. do not push!
  7. Change something in "newfilename"
  8. Create a commit (commit 3)
  9. Create review request for "commit 3" and use "commit 2" as a parent diff
  10. Go to diffviewer
  11. Cannot show diff of "newfilename" as it throws an exception "FileNotFoundError" from above

Looks like the diffviewer needs to learn that it do not try to get the file from repository if it is created/renamed in parent diff.
"is_new" must be extended to parent diff here....

https://github.com/reviewboard/reviewboard/blob/release-3.0.x/reviewboard/diffviewer/diffutils.py#L221

parent diff:

diff --git a/filename b/newfilename
rename from filename
rename to newfilename

Misery
#3 Misery

Here is a screenshot of filediff view.
Does ""parent_moved": true," mean that the parsed identified it correctly and the diff-viewer needs to check that?

Misery
#4 Misery

Same problem if the file is copied and modified. :-(

Example diff:

diff --git a/product/src/main/java/de/company/TrustForm.java b/product/src/main/java/de/company/CertForm.java
copy from product/src/main/java/de/company/TrustForm.java
copy to product/src/main/java/de/company/CertForm.java
--- a/product/src/main/java/de/company/TrustForm.java
+++ b/product/src/main/java/de/company/CertForm.java
@@ -4,6 +4,7 @@

import javax.validation.constraints.NotNull;

+import de.company.Certificate;
import org.springframework.web.multipart.MultipartFile;

import de.company.Truststore;

Misery
#5 Misery

Same problem if a review bot try to request it via web api (RB 3.0.6)

Even we applied our patch: https://reviews.reviewboard.org/r/9637/
Same review request is shown correctly in diffviewer with our patch.

Traceback (most recent call last):

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/core/handlers/base.py", line 112, in get_response
response = wrapped_callback(request, callback_args, *callback_kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
response = view_func(request, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/views/decorators/vary.py", line 19, in inner_func
response = func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/base.py", line 243, in call
request, method, view, api_format=api_format, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/base.py", line 338, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/mixins/api_tokens.py", line 66, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/mixins/oauth2_tokens.py", line 102, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/base.py", line 368, in call_method_view
return view(request, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/decorators.py", line 125, in _call
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/decorators.py", line 39, in _check
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/decorators.py", line 125, in _call
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/decorators.py", line 169, in _check
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/resources/filediff.py", line 389, in get
return self._get_diff_data(request, mimetype, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/resources/filediff.py", line 457, in _get_diff_data
populate_diff_chunks(files, highlighting, request=request)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 772, in populate_diff_chunks
chunks = list(generator.get_chunks())

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 786, in get_chunks
for chunk in super(DiffChunkGenerator, self).get_chunks(cache_key):

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 107, in get_chunks
large_data=True)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 299, in cache_memoize
compress_large_data))

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 253, in cache_memoize_iter
items = items_or_callable()

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 296, in <lambda>
lambda: [lookup_callable()],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 106, in <lambda>
lambda: list(self.get_chunks_uncached()),

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 792, in get_chunks_uncached
self.encoding_list)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 261, in get_original_file
request=request)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 404, in get_file
large_data=True)[0]

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 299, in cache_memoize
compress_large_data))

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 253, in cache_memoize_iter
items = items_or_callable()

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 296, in <lambda>
lambda: [lookup_callable()],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 403, in <lambda>
request)],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 597, in _get_file_uncached
base_commit_id=base_commit_id)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 48, in get_file
base_commit_id=base_commit_id)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 289, in cat_file
raise FileNotFoundError(path, rev)

FileNotFoundError: The file 'src/test/java/de/[...]/Test_Format.java' (rdd86cf31d3094dd59012d84cefd605df85e00e1d) could not be found in the repository

<WSGIRequest
path:/api/review-requests/12488/diffs/1/files/246289/,
GET:<QueryDict: {}>,
POST:<QueryDict: {}>,
COOKIES:{'rbsessionid': 'abc'},
META:{'CONTENT_LENGTH': '0',
'CONTENT_TYPE': '',
'DOCUMENT_ROOT': '/var/lib/nginx/html',
'HTTPS': 'on',
'HTTP_ACCEPT': 'application/vnd.reviewboard.org.diff.data+json',
'HTTP_ACCEPT_ENCODING': 'identity',
'HTTP_CONNECTION': 'close',
'HTTP_CONTENT_LENGTH': '0',
'HTTP_COOKIE': 'rbsessionid=abc',
'HTTP_HOST': 'rb.company.de',
'HTTP_USER_AGENT': 'RBTools/0.7.11',
u'LOCAL_SITE': None,
'PATH_INFO': u'/api/review-requests/12488/diffs/1/files/246289/',
'QUERY_STRING': '',
'REMOTE_ADDR': '10.210.249.102',
'REMOTE_PORT': '53000',
'REQUEST_METHOD': 'GET',
'REQUEST_SCHEME': 'https',
'REQUEST_URI': '/api/review-requests/12488/diffs/1/files/246289/',
u'SCRIPT_NAME': u'',
'SERVER_NAME': 'rb',
'SERVER_PORT': '443',
'SERVER_PROTOCOL': 'HTTP/1.1',
u'USERNAME': u'Bot',
u'USER_EMAIL': u'bot@rb.company.de',
'uwsgi.node': 'vpi-rb',
'uwsgi.version': '2.0.17',
'wsgi.errors': <open file 'wsgi_errors', mode 'w' at 0x7f6ccfe6b150>,
'wsgi.file_wrapper': <built-in function uwsgi_sendfile>,
'wsgi.input': <uwsgi._Input object at 0x7f6cca9f2120>,
'wsgi.multiprocess': True,
'wsgi.multithread': False,
'wsgi.run_once': False,
'wsgi.url_scheme': u'https',
'wsgi.version': (1, 0)}>

david
#6 david

Fixed in release-3.0.x (503b9bf14). This will ship in 3.0.9. Thanks!

  • -New
    +Fixed
Misery
#7 Misery

Still a problem on RB 3.0.9

Traceback (most recent call last):

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/core/handlers/base.py", line 112, in get_response
response = wrapped_callback(request, callback_args, *callback_kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/views/decorators/cache.py", line 52, in _wrapped_view_func
response = view_func(request, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/django/views/decorators/vary.py", line 19, in inner_func
response = func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/base.py", line 243, in call
request, method, view, api_format=api_format, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/base.py", line 338, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/mixins/api_tokens.py", line 66, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/mixins/oauth2_tokens.py", line 102, in call_method_view
request, method, view, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/resources/base.py", line 368, in call_method_view
return view(request, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/decorators.py", line 125, in _call
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/decorators.py", line 39, in _check
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/webapi/decorators.py", line 125, in _call
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/decorators.py", line 169, in _check
return view_func(args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/resources/filediff.py", line 389, in get
return self._get_diff_data(request, mimetype, args, *kwargs)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/webapi/resources/filediff.py", line 457, in _get_diff_data
populate_diff_chunks(files, highlighting, request=request)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 750, in populate_diff_chunks
chunks = list(generator.get_chunks())

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 786, in get_chunks
for chunk in super(DiffChunkGenerator, self).get_chunks(cache_key):

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 107, in get_chunks
large_data=True)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 299, in cache_memoize
compress_large_data))

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 253, in cache_memoize_iter
items = items_or_callable()

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 296, in <lambda>
lambda: [lookup_callable()],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 106, in <lambda>
lambda: list(self.get_chunks_uncached()),

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/chunk_generator.py", line 792, in get_chunks_uncached
self.encoding_list)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/diffviewer/diffutils.py", line 230, in get_original_file
request=request)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 404, in get_file
large_data=True)[0]

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 299, in cache_memoize
compress_large_data))

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 253, in cache_memoize_iter
items = items_or_callable()

File "/opt/reviewboard/dist/lib/python2.7/site-packages/djblets/cache/backend.py", line 296, in <lambda>
lambda: [lookup_callable()],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 403, in <lambda>
request)],

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/models.py", line 597, in _get_file_uncached
base_commit_id=base_commit_id)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 48, in get_file
base_commit_id=base_commit_id)

File "/opt/reviewboard/dist/lib/python2.7/site-packages/reviewboard/scmtools/hg.py", line 289, in cat_file
raise FileNotFoundError(path, rev)

FileNotFoundError: The file "dummy/UserHelper.java" (revision 1c076953fc2a19c47072f8dec6a6bb2288e62cf8) could not be found in the repository

Misery
#8 Misery

After applying our patch again it works: https://reviews.reviewboard.org/r/9637/

Misery
#9 Misery

still a problem with review board 3.0.16. :-( any help?