Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Bitbucket patch diff handling #1118

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pr_agent/algo/git_patch_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


def extend_patch(original_file_str, patch_str, patch_extra_lines_before=0, patch_extra_lines_after=0) -> str:
if not patch_str or (patch_extra_lines_before == 0 and patch_extra_lines_after == 0):
if not patch_str or (patch_extra_lines_before == 0 and patch_extra_lines_after == 0) or not original_file_str:
return patch_str

if type(original_file_str) == bytes:
Expand Down Expand Up @@ -52,9 +52,9 @@ def extend_patch(original_file_str, patch_str, patch_extra_lines_before=0, patch
extended_size1 = size1 + (start1 - extended_start1) + patch_extra_lines_after
extended_start2 = max(1, start2 - patch_extra_lines_before)
extended_size2 = size2 + (start2 - extended_start2) + patch_extra_lines_after
if extended_start1 - 1 + extended_size1 > len(original_lines):
if extended_start1 - 1 + extended_size1 > len_original_lines:
# we cannot extend beyond the original file
delta_cap = extended_start1 - 1 + extended_size1 - len(original_lines)
delta_cap = extended_start1 - 1 + extended_size1 - len_original_lines
extended_size1 = max(extended_size1 - delta_cap, size1)
extended_size2 = max(extended_size2 - delta_cap, size2)
delta_lines = original_lines[extended_start1 - 1:start1 - 1]
Expand Down
56 changes: 46 additions & 10 deletions pr_agent/git_providers/bitbucket_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,12 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool:
get_logger().error(f"Failed to publish code suggestion, error: {e}")
return False

def publish_file_comments(self, file_comments: list) -> bool:
pass

def is_supported(self, capability: str) -> bool:
if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown']:
if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown',
'publish_file_comments']:
return False
return True

Expand Down Expand Up @@ -137,9 +141,28 @@ def get_diff_files(self) -> list[FilePatchInfo]:
except Exception as e:
pass

diff_split = [
"diff --git%s" % x for x in self.pr.diff().split("diff --git") if x.strip()
]
# get the pr patches
pr_patch = self.pr.diff()
diff_split = ["diff --git" + x for x in pr_patch.split("diff --git") if x.strip()]
if len(diff_split) != len(diffs):
get_logger().error(f"Error - failed to split the diff into {len(diffs)} parts")
return []
# bitbucket diff has a header for each file, we need to remove it:
# "diff --git filename
# index caa56f0..61528d7 100644
# --- a/pr_agent/cli_pip.py
# +++ b/pr_agent/cli_pip.py
# @@ -... @@"
for i, _ in enumerate(diff_split):
diff_split_lines = diff_split[i].splitlines()
if (len(diff_split_lines) > 5 and
diff_split_lines[2].startswith("---") and
diff_split_lines[3].startswith("+++") and
diff_split_lines[4].startswith("@@")):
diff_split[i] = "\n".join(diff_split_lines[4:])
else:
get_logger().error(f"Error - failed to remove the bitbucket header from diff {i}")
break

invalid_files_names = []
diff_files = []
Expand All @@ -148,10 +171,14 @@ def get_diff_files(self) -> list[FilePatchInfo]:
invalid_files_names.append(diff.new.path)
continue

original_file_content_str = self._get_pr_file_content(
diff.old.get_data("links")
)
new_file_content_str = self._get_pr_file_content(diff.new.get_data("links"))
try:
original_file_content_str = self._get_pr_file_content(diff.old.get_data("links")['self']['href'])
new_file_content_str = self._get_pr_file_content(diff.new.get_data("links")['self']['href'])
except Exception as e:
get_logger().exception(f"Error - bitbucket failed to get file content, error: {e}")
original_file_content_str = ""
new_file_content_str = ""

file_patch_canonic_structure = FilePatchInfo(
original_file_content_str,
new_file_content_str,
Expand All @@ -172,7 +199,6 @@ def get_diff_files(self) -> list[FilePatchInfo]:
if invalid_files_names:
get_logger().info(f"Disregarding files with invalid extensions:\n{invalid_files_names}")


self.diff_files = diff_files
return diff_files

Expand Down Expand Up @@ -314,6 +340,9 @@ def get_languages(self):
def get_pr_branch(self):
return self.pr.source_branch

def get_pr_owner_id(self) -> str | None:
return self.workspace_slug

def get_pr_description_full(self):
return self.pr.description

Expand Down Expand Up @@ -400,7 +429,14 @@ def create_or_update_pr_file(self, file_path: str, branch: str, contents="", mes
get_logger().exception(f"Failed to create empty file {file_path} in branch {branch}")

def _get_pr_file_content(self, remote_link: str):
return ""
try:
response = requests.request("GET", remote_link, headers=self.headers)
if response.status_code == 404: # not found
return ""
contents = response.text
return contents
except Exception:
return ""

def get_commit_messages(self):
return "" # not implemented yet
Expand Down
Loading