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

Fix issue 41 #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pb-new-username
Copy link

@pb-new-username pb-new-username commented Oct 21, 2021

Fix for issue 41.

Link here #41

Werkov added a commit to Werkov/git-deps that referenced this pull request Jul 4, 2024
New files in patch are shown as
	diff --git a/git_deps/blame.py b/git_deps/blame.py
	new file mode 100644
	index 0000000..41d79fe
	--- /dev/null
	+++ b/git_deps/blame.py
	@@ -0,0 +1,61 @@

This is even visible in pygit2.Diff.patch.
However, when you look at the same diff's
pygit2.Patch.delta.old_file.path it is not '/dev/null' but
'git_deps/blame.py'. This could have been caught up by tree_lookup() in
blame_diff_hunk if '/dev/null' was checked. Because old_file.path is a
valid filename though, tree_lookup() may succeed. When?
Consider case of moving a file and replacing it with a symlink:

	diff --git a/some-script b/some-script
	deleted file mode 100755
	index 21c9f09..0000000
	--- a/some-script
	+++ /dev/null
	@@ -1,3 +0,0 @@  <-- git-deps examines this hunk
	-#!/usr/bin/perl
	-...
	-

	diff --git a/some-script b/some-script
	new file mode 120000
	index 0000000..0098051
	--- /dev/null
	+++ b/some-script
	@@ -0,0 +1 @@  <-- this can be skipped, new file
	+newdir/some-script

As a fix look at file mode of each Patch.delta's file and detect new
files from old_file.mode, new files don't bring about any deps so they
can be skipped in hunk examination.

Fixes aspiers#41
Ref aspiers#108
Werkov added a commit to Werkov/git-deps that referenced this pull request Jul 4, 2024
New files in patch are shown as
	diff --git a/git_deps/blame.py b/git_deps/blame.py
	new file mode 100644
	index 0000000..41d79fe
	--- /dev/null
	+++ b/git_deps/blame.py
	@@ -0,0 +1,61 @@

This is even visible in pygit2.Diff.patch.
However, when you look at the same diff's
pygit2.Patch.delta.old_file.path it is not '/dev/null' but
'git_deps/blame.py'. This could have been caught up by tree_lookup() in
blame_diff_hunk if '/dev/null' was checked. Because old_file.path is a
valid filename though, tree_lookup() may succeed. When?
Consider case of moving a file and replacing it with a symlink:

	diff --git a/some-script b/some-script
	deleted file mode 100755
	index 21c9f09..0000000
	--- a/some-script
	+++ /dev/null
	@@ -1,3 +0,0 @@  <-- git-deps examines this hunk
	-#!/usr/bin/perl
	-...
	-

	diff --git a/some-script b/some-script
	new file mode 120000
	index 0000000..0098051
	--- /dev/null
	+++ b/some-script
	@@ -0,0 +1 @@  <-- this can be skipped, new file
	+newdir/some-script

As a fix look at file mode of each Patch.delta's file and detect new
files from old_file.mode, new files don't bring about any deps so they
can be skipped in hunk examination.

Without the fix crash like below happes:
	fatal: -L invalid line number: 0
	subprocess.CalledProcessError: Command '['git', 'blame', '--porcelain', '-L', '0,+0', 'abcde', '--', 'some-script']' returned non-zero exit status 128.

Fixes aspiers#41
Ref aspiers#108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant