Error in parsing patch with tabulations: `gitdiff: line 15: invalid line operation: '\t'`

[frenchbeast]

When a line starts with a tabulation the patch parsing fails with: gitdiff: line 15: invalid line operation: '\t'

I updated the switch statement to support it. Also added a test to verify it works.

I created a PR to eventually fix the problem: https://github.com/bluekeyes/go-gitdiff/pull/50

[bluekeyes]

Thanks for reporting this. Do you have an example patch you can share that demonstrates this problem?

[frenchbeast]

This is one example of a patch we have which will fail when parsed. On this version : sigs.k8s.io/controller-runtime v0.18.4

diff --git a/vendor/sigs.k8s.io/controller-runtime/pkg/manager/internal.go b/vendor/sigs.k8s.io/controller-runtime/pkg/manager/internal.go
index 2ce02b105c..a33de28eaf 100644
--- a/vendor/sigs.k8s.io/controller-runtime/pkg/manager/internal.go
+++ b/vendor/sigs.k8s.io/controller-runtime/pkg/manager/internal.go
@@ -187,7 +187,7 @@ func (cm *controllerManager) AddMetricsServerExtraHandler(path string, handler h
        return fmt.Errorf("unable to add new metrics handler because metrics endpoint has already been created")
    }
    if cm.metricsServer == nil {
-       cm.GetLogger().Info("warn: metrics server is currently disabled, registering extra handler %q will be ignored", path)
+       cm.GetLogger().Info("warn: metrics server is currently disabled, registering extra handler %q will be ignored", "path", path)
        return nil
    }
    if err := cm.metricsServer.AddExtraHandler(path, handler); err != nil {

[bluekeyes]

How was that patch generated? When I try to apply it with git (version 2.46.0), I also get an error about an invalid patch:

$ git apply tabs.patch
error: corrupt patch at line 6

In that sense, I think the current behavior of this library matches Git. My understanding was that all of the context lines in a patch had to start with a single space character, but maybe there's a flag or tool that does it differently?

[frenchbeast]

Thank you for checking. Unfortunately i inherited a bunch of patches. I'm replacing the old code with gitdiff.

The previous method is using patch 2.0-12u11-Apple instead of git patch. I just tried manually with no complain from the tool.

> patch -d vendor -p2 < vendor-patches/k8s-io-test-infra.pre.patch
patching file 'k8s.io/test-infra/prow/flagutil/kubernetes_cluster_clients.go'

And the same patch with my code update passes with gitdiff Hope that helps.

[frenchbeast]

Just talked with the author. The tool to create the patch was git diff

[bluekeyes]

I experimented with this a bit more, and I'm fairly confident that your patches are corrupt, at least by the standards of Git and this library. In contrast, the patch utility seems to be (intentionally) very lenient in what it will accept, so I think this hid the corruption.

Are your patches themselves stored in a Git repository? If they are, here's my best guess at what might have happened. When the original author generated the patches using git diff, the output was valid and included leading space characters on the context lines. However, by default, Git considers a line that starts with a space followed by a tab to contain a "whitespace error". From the git-config page for core.whitespace:

space-before-tab treats a space character that appears immediately before a tab character in the initial indent part of the line as an error (enabled by default).

This situation exactly describes an indented line of Go code that appears as context in a unified diff. You can see Git highlighting the "error" when I try to add my recreation of the patch to Git:

My guess is that something (an automated tool, a well-intentioned human) removed these leading spaces to "fix" the error, corrupting the patch in the process.

---

All this is to say that I think the current gitdiff behavior is correct, in that it matches Git by rejecting an invalid patch. If you'd like to use the gitdiff package in your application, I recommend fixing your patches by re-adding the leading space character on all of the context lines.

[frenchbeast]

Thank you for checking. I did check and experiment as well during the weekend. I ended up adding a validation pipeline so that i can move forward with gitdiff. I will update these patches manually for now and create steps for future ones.