Attachment #103214 for bug #48075

View Details Raw Unified Return to Bug 48075

| Differences between



2011-08-06  Jochen Eisinger  <jochen@chromium.org>

        webkit-patch doesn't get along with renamed files
        https://bugs.webkit.org/show_bug.cgi?id=48075

        Possibly a bit heavy handed - I removed all instances of -C and
        changed every instance of -M with '--no-renames' in git.py. This
        forces git to not try to tell us about renames at all, which is
        ultimately the behaviour we want. The old file is shown deleted,
        then the new file is shown added, followed by any changes that
        occurred. Also gets rid of the problem where deleting one file
        and adding another file which has similar content would
        unexpectedly show up as a rename, and fall out of a diff.

        Based on a patch by Wyatt Carss.

        Reviewed by NOBODY (OOPS!).

        * Scripts/webkitpy/common/checkout/scm/git.py:

2011-08-05  Dimitri Glazkov  <dglazkov@chromium.org>

        Fix unit test breakage by plumbing convert_404_to_None in MockWeb.



        return self._filesystem.exists(self.absolute_path(self._filesystem.join('.git', 'rebase-apply')))

    def working_directory_is_clean(self):
        return self.run(['git', 'diff', 'HEAD', '--no-renames', '--name-only'], cwd=self.checkout_root) == ""

    def clean_working_directory(self):
        # FIXME: These should probably use cwd=self.checkout_root.

    def status_command(self):
        # git status returns non-zero when there are changes, so we use git diff name --name-status HEAD instead.
        # No file contents printed, thus utf-8 autodecoding in self.run is fine.
        return ["git", "diff", "--name-status", "--no-renames", "HEAD"]

    def _status_regexp(self, expected_types):
        return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types


    def changed_files(self, git_commit=None):
        # FIXME: --diff-filter could be used to avoid the "extract_filenames" step.
        status_command = ['git', 'diff', '-r', '--name-status', "--no-renames", "--no-ext-diff", "--full-index", self.merge_base(git_commit)]
        # FIXME: I'm not sure we're returning the same set of files that SVN.changed_files is.
        # Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)
        return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM"))

    def conflicted_files(self):
        # We do not need to pass decode_output for this diff command
        # as we're passing --name-status which does not output any data.
        status_command = ['git', 'diff', '--name-status', '--no-renames', '--diff-filter=U']
        return self.run_status_and_extract_filenames(status_command, self._status_regexp("U"))

    def added_files(self):

        """Returns a byte array (str()) representing the patch file.
        Patch files are effectively binary since they may contain
        files of multiple different encodings."""
        command = ['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "--no-renames", self.merge_base(git_commit), "--"]
        if changed_files:
            command += changed_files
        return self.prepend_svn_revision(self.run(command, decode_output=False, cwd=self.checkout_root))

        return self.create_patch(git_commit)

    def diff_for_file(self, path, log=None):
        return self.run(['git', 'diff', 'HEAD', '--no-renames', '--', path], cwd=self.checkout_root)

    def show_head(self, path):
        return self.run(['git', 'show', 'HEAD:' + self.to_object_name(path)], decode_output=False)

        return CommitMessage(commit_lines[first_line_after_headers:])

    def files_changed_summary_for_commit(self, commit_id):
        return self.run(['git', 'diff-tree', '--shortstat', '--no-renames', '--no-commit-id', commit_id])



        # self._shared_test_head_svn_revision().
        self.assertEqual(scm.head_svn_revision(), '')

    def test_rename_files(self):
        os.chdir(self.tracking_git_checkout_path)
        scm = self.tracking_scm

        run_command(['git', 'mv', 'foo_file', 'bar_file'])
        scm.commit_locally_with_message('message')

        patch = scm.create_patch()
        self.assertFalse(re.search(r'rename from ', patch))
        self.assertFalse(re.search(r'rename to ', patch))


class GitSVNTest(SCMTest):

    def _setup_git_checkout(self):

Return to Bug 48075

Lines 962-967 class GitTest(SCMTest): a/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py_sec1
- a/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py +12 lines
962	# self._shared_test_head_svn_revision().	962	# self._shared_test_head_svn_revision().
963	self.assertEqual(scm.head_svn_revision(), '')	963	self.assertEqual(scm.head_svn_revision(), '')
964		964
		965	def test_rename_files(self):
		966	os.chdir(self.tracking_git_checkout_path)
		967	scm = self.tracking_scm
		968
		969	run_command(['git', 'mv', 'foo_file', 'bar_file'])
		970	scm.commit_locally_with_message('message')
		971
		972	patch = scm.create_patch()
		973	self.assertFalse(re.search(r'rename from ', patch))
		974	self.assertFalse(re.search(r'rename to ', patch))
		975
		976
965	class GitSVNTest(SCMTest):	977	class GitSVNTest(SCMTest):
966		978
967	def _setup_git_checkout(self):	979	def _setup_git_checkout(self):

Lines 1-3 a/Tools/ChangeLog_sec1
- a/Tools/ChangeLog +20 lines
		1	2011-08-06 Jochen Eisinger <jochen@chromium.org>
		2
		3	webkit-patch doesn't get along with renamed files
		4	https://bugs.webkit.org/show_bug.cgi?id=48075
		5
		6	Possibly a bit heavy handed - I removed all instances of -C and
		7	changed every instance of -M with '--no-renames' in git.py. This
		8	forces git to not try to tell us about renames at all, which is
		9	ultimately the behaviour we want. The old file is shown deleted,
		10	then the new file is shown added, followed by any changes that
		11	occurred. Also gets rid of the problem where deleting one file
		12	and adding another file which has similar content would
		13	unexpectedly show up as a rename, and fall out of a diff.
		14
		15	Based on a patch by Wyatt Carss.
		16
		17	Reviewed by NOBODY (OOPS!).
		18
		19	* Scripts/webkitpy/common/checkout/scm/git.py:
		20
1	2011-08-05 Dimitri Glazkov <dglazkov@chromium.org>	21	2011-08-05 Dimitri Glazkov <dglazkov@chromium.org>
2		22
3	Fix unit test breakage by plumbing convert_404_to_None in MockWeb.	23	Fix unit test breakage by plumbing convert_404_to_None in MockWeb.

Lines 144-150 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec1
- a/Tools/Scripts/webkitpy/common/checkout/scm/git.py -7 / +7 lines
144	return self._filesystem.exists(self.absolute_path(self._filesystem.join('.git', 'rebase-apply')))	144	return self._filesystem.exists(self.absolute_path(self._filesystem.join('.git', 'rebase-apply')))
145		145
146	def working_directory_is_clean(self):	146	def working_directory_is_clean(self):
147	return self.run(['git', 'diff', 'HEAD', '--name-only'], cwd=self.checkout_root) == ""	147	return self.run(['git', 'diff', 'HEAD', '--no-renames', '--name-only'], cwd=self.checkout_root) == ""
148		148
149	def clean_working_directory(self):	149	def clean_working_directory(self):
150	# FIXME: These should probably use cwd=self.checkout_root.	150	# FIXME: These should probably use cwd=self.checkout_root.
Lines 157-163 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec2
157	def status_command(self):	157	def status_command(self):
158	# git status returns non-zero when there are changes, so we use git diff name --name-status HEAD instead.	158	# git status returns non-zero when there are changes, so we use git diff name --name-status HEAD instead.
159	# No file contents printed, thus utf-8 autodecoding in self.run is fine.	159	# No file contents printed, thus utf-8 autodecoding in self.run is fine.
160	return ["git", "diff", "--name-status", "HEAD"]	160	return ["git", "diff", "--name-status", "--no-renames", "HEAD"]
161		161
162	def _status_regexp(self, expected_types):	162	def _status_regexp(self, expected_types):
163	return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types	163	return '^(?P<status>[%s])\t(?P<filename>.+)$' % expected_types
Lines 186-192 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec3
186		186
187	def changed_files(self, git_commit=None):	187	def changed_files(self, git_commit=None):
188	# FIXME: --diff-filter could be used to avoid the "extract_filenames" step.	188	# FIXME: --diff-filter could be used to avoid the "extract_filenames" step.
189	status_command = ['git', 'diff', '-r', '--name-status', '-C', '-M', "--no-ext-diff", "--full-index", self.merge_base(git_commit)]	189	status_command = ['git', 'diff', '-r', '--name-status', "--no-renames", "--no-ext-diff", "--full-index", self.merge_base(git_commit)]
190	# FIXME: I'm not sure we're returning the same set of files that SVN.changed_files is.	190	# FIXME: I'm not sure we're returning the same set of files that SVN.changed_files is.
191	# Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)	191	# Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)
192	return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM"))	192	return self.run_status_and_extract_filenames(status_command, self._status_regexp("ADM"))
Lines 209-215 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec4
209	def conflicted_files(self):	209	def conflicted_files(self):
210	# We do not need to pass decode_output for this diff command	210	# We do not need to pass decode_output for this diff command
211	# as we're passing --name-status which does not output any data.	211	# as we're passing --name-status which does not output any data.
212	status_command = ['git', 'diff', '--name-status', '-C', '-M', '--diff-filter=U']	212	status_command = ['git', 'diff', '--name-status', '--no-renames', '--diff-filter=U']
213	return self.run_status_and_extract_filenames(status_command, self._status_regexp("U"))	213	return self.run_status_and_extract_filenames(status_command, self._status_regexp("U"))
214		214
215	def added_files(self):	215	def added_files(self):
Lines 244-250 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec5
244	"""Returns a byte array (str()) representing the patch file.	244	"""Returns a byte array (str()) representing the patch file.
245	Patch files are effectively binary since they may contain	245	Patch files are effectively binary since they may contain
246	files of multiple different encodings."""	246	files of multiple different encodings."""
247	command = ['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"]	247	command = ['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "--no-renames", self.merge_base(git_commit), "--"]
248	if changed_files:	248	if changed_files:
249	command += changed_files	249	command += changed_files
250	return self.prepend_svn_revision(self.run(command, decode_output=False, cwd=self.checkout_root))	250	return self.prepend_svn_revision(self.run(command, decode_output=False, cwd=self.checkout_root))
Lines 282-288 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec6
282	return self.create_patch(git_commit)	282	return self.create_patch(git_commit)
283		283
284	def diff_for_file(self, path, log=None):	284	def diff_for_file(self, path, log=None):
285	return self.run(['git', 'diff', 'HEAD', '--', path], cwd=self.checkout_root)	285	return self.run(['git', 'diff', 'HEAD', '--no-renames', '--', path], cwd=self.checkout_root)
286		286
287	def show_head(self, path):	287	def show_head(self, path):
288	return self.run(['git', 'show', 'HEAD:' + self.to_object_name(path)], decode_output=False)	288	return self.run(['git', 'show', 'HEAD:' + self.to_object_name(path)], decode_output=False)
Lines 462-465 class Git(SCM, SVNRepository): a/Tools/Scripts/webkitpy/common/checkout/scm/git.py_sec7
462	return CommitMessage(commit_lines[first_line_after_headers:])	462	return CommitMessage(commit_lines[first_line_after_headers:])
463		463
464	def files_changed_summary_for_commit(self, commit_id):	464	def files_changed_summary_for_commit(self, commit_id):
465	return self.run(['git', 'diff-tree', '--shortstat', '--no-commit-id', commit_id])	465	return self.run(['git', 'diff-tree', '--shortstat', '--no-renames', '--no-commit-id', commit_id])