diff --git a/README.md b/README.md index 30bbf73..56e0502 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ In development, most functionality is currently missing, incomplete, or broken. ## Known Issues and Differences From Git 1. Certain types of invalid input that I believe are accepted by `git apply` - generate errors in this library. These include: + generate errors. These include: - Numbers immediately followed by non-numeric characters - Trailing characters on a line after valid or expected content @@ -46,6 +46,10 @@ In development, most functionality is currently missing, incomplete, or broken. Unicode file names are handled; these are bugs, so please report any issues of this type. -3. When reading headers, this library does not validate that OIDs present on an - `index` line are shorter than or equal to the maximum hash length, as this - requires knowing if the repository used SHA1 or SHA256 hashes. +3. When reading headers, there is no validation that OIDs present on an `index` + line are shorter than or equal to the maximum hash length, as this requires + knowing if the repository used SHA1 or SHA256 hashes. + +4. When reading "traditional" patches (those not produced by `git`), prefixes + are not stripped from file names (`git apply` attempts to remove prefixes + that match the current repository directory/prefix.) diff --git a/gitdiff/file_header.go b/gitdiff/file_header.go index b5b253d..f11a48e 100644 --- a/gitdiff/file_header.go +++ b/gitdiff/file_header.go @@ -5,6 +5,7 @@ import ( "os" "strconv" "strings" + "time" ) // parseGitHeaderName extracts a default file name from the Git file header @@ -280,3 +281,32 @@ func cleanName(name string, drop int) string { } return b.String() } + +// hasEpochTimestamp returns true if the string ends with a POSIX-formatted +// timestamp for the UNIX epoch after a tab character. According to git, this +// is used by GNU diff to mark creations and deletions. +func hasEpochTimestamp(s string) bool { + const posixTimeLayout = "2006-01-02 15:04:05.9 -0700" + + start := strings.IndexRune(s, '\t') + if start < 0 { + return false + } + + ts := strings.TrimSuffix(s[start+1:], "\n") + + // a valid timestamp can have optional ':' in zone specifier + // remove that if it exists so we have a single format + if ts[len(ts)-3] == ':' { + ts = ts[:len(ts)-3] + ts[len(ts)-2:] + } + + t, err := time.Parse(posixTimeLayout, ts) + if err != nil { + return false + } + if !t.Equal(time.Unix(0, 0)) { + return false + } + return true +} diff --git a/gitdiff/file_header_test.go b/gitdiff/file_header_test.go index 3c4a5d5..e866ed4 100644 --- a/gitdiff/file_header_test.go +++ b/gitdiff/file_header_test.go @@ -391,3 +391,52 @@ func TestParseGitHeaderName(t *testing.T) { }) } } + +func TestHasEpochTimestamp(t *testing.T) { + tests := map[string]struct { + Input string + Output bool + }{ + "utcTimestamp": { + Input: "+++ file.txt\t1970-01-01 00:00:00 +0000\n", + Output: true, + }, + "utcZoneWithColon": { + Input: "+++ file.txt\t1970-01-01 00:00:00 +00:00\n", + Output: true, + }, + "utcZoneWithMilliseconds": { + Input: "+++ file.txt\t1970-01-01 00:00:00.000000 +00:00\n", + Output: true, + }, + "westTimestamp": { + Input: "+++ file.txt\t1969-12-31 16:00:00 -0800\n", + Output: true, + }, + "eastTimestamp": { + Input: "+++ file.txt\t1970-01-01 04:00:00 +0400\n", + Output: true, + }, + "noTab": { + Input: "+++ file.txt 1970-01-01 00:00:00 +0000\n", + Output: false, + }, + "invalidFormat": { + Input: "+++ file.txt\t1970-01-01T00:00:00Z\n", + Output: false, + }, + "notEpoch": { + Input: "+++ file.txt\t2019-03-21 12:34:56.789 -0700\n", + Output: false, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + output := hasEpochTimestamp(test.Input) + if output != test.Output { + t.Errorf("incorrect output: expected %t, actual %t", test.Output, output) + } + }) + } +} diff --git a/gitdiff/parser.go b/gitdiff/parser.go index ae69704..28d0f55 100644 --- a/gitdiff/parser.go +++ b/gitdiff/parser.go @@ -36,6 +36,10 @@ func Parse(r io.Reader) (files []*File, err error) { return files, nil } +// TODO(bkeyes): consider exporting the parser type with configuration +// this would enable OID validation, p-value guessing, and prefix stripping +// by allowing users to set or override defaults + type parser struct { r *bufio.Reader lineno int64 @@ -168,8 +172,36 @@ func (p *parser) ParseGitFileHeader(f *File, header string) error { return nil } -func (p *parser) ParseTraditionalFileHeader(f *File, oldFile, newFile string) error { - panic("TODO(bkeyes): unimplemented") +func (p *parser) ParseTraditionalFileHeader(f *File, oldLine, newLine string) error { + oldName, _, err := parseName(strings.TrimPrefix(oldLine, oldFilePrefix), '\t', 0) + if err != nil { + return p.Errorf("file header: %v", err) + } + + newName, _, err := parseName(strings.TrimPrefix(newLine, newFilePrefix), '\t', 0) + if err != nil { + return p.Errorf("file header: %v", err) + } + + switch { + case oldName == devNull || hasEpochTimestamp(oldLine): + f.IsNew = true + f.NewName = newName + case newName == devNull || hasEpochTimestamp(newLine): + f.IsDelete = true + f.OldName = oldName + default: + // if old name is a prefix of new name, use that instead + // this avoids picking variants like "file.bak" or "file~" + if strings.HasPrefix(newName, oldName) { + f.OldName = oldName + f.NewName = oldName + } else { + f.OldName = newName + f.NewName = newName + } + } + return nil } // Line reads and returns the next line. The first call to Line after a call to @@ -197,6 +229,7 @@ func (p *parser) PeekLine() (line string, err error) { } // Errorf generates an error and appends the current line information. +// TODO(bkeyes): add linedelta to allow changing lineno per-error func (p *parser) Errorf(msg string, args ...interface{}) error { return fmt.Errorf("gitdiff: line %d: %s", p.lineno, fmt.Sprintf(msg, args...)) } diff --git a/gitdiff/parser_test.go b/gitdiff/parser_test.go index 02c9304..72d210c 100644 --- a/gitdiff/parser_test.go +++ b/gitdiff/parser_test.go @@ -286,3 +286,83 @@ index deadbeef }) } } + +func TestParseTraditionalFileHeader(t *testing.T) { + tests := map[string]struct { + OldLine string + NewLine string + Output *File + Err bool + }{ + "fileContentChange": { + OldLine: "--- dir/file_old.txt\t2019-03-21 23:00:00.0 -0700\n", + NewLine: "+++ dir/file_new.txt\t2019-03-21 23:30:00.0 -0700\n", + Output: &File{ + OldName: "dir/file_new.txt", + NewName: "dir/file_new.txt", + }, + }, + "newFile": { + OldLine: "--- /dev/null\t1969-12-31 17:00:00.0 -0700\n", + NewLine: "+++ dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", + Output: &File{ + NewName: "dir/file.txt", + IsNew: true, + }, + }, + "newFileTimestamp": { + OldLine: "--- dir/file.txt\t1969-12-31 17:00:00.0 -0700\n", + NewLine: "+++ dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", + Output: &File{ + NewName: "dir/file.txt", + IsNew: true, + }, + }, + "deleteFile": { + OldLine: "--- dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", + NewLine: "+++ /dev/null\t1969-12-31 17:00:00.0 -0700\n", + Output: &File{ + OldName: "dir/file.txt", + IsDelete: true, + }, + }, + "deleteFileTimestamp": { + OldLine: "--- dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", + NewLine: "+++ dir/file.txt\t1969-12-31 17:00:00.0 -0700\n", + Output: &File{ + OldName: "dir/file.txt", + IsDelete: true, + }, + }, + "useShortestPrefixName": { + OldLine: "--- dir/file.txt\t2019-03-21 23:00:00.0 -0700\n", + NewLine: "+++ dir/file.txt~\t2019-03-21 23:30:00.0 -0700\n", + Output: &File{ + OldName: "dir/file.txt", + NewName: "dir/file.txt", + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + p := &parser{r: bufio.NewReader(strings.NewReader(""))} + + var f File + err := p.ParseTraditionalFileHeader(&f, test.OldLine, test.NewLine) + if test.Err { + if err == nil { + t.Fatalf("expected error parsing traditional file header, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error parsing traditional file header: %v", err) + } + + if test.Output != nil && !reflect.DeepEqual(f, *test.Output) { + t.Errorf("incorrect file\nexpected: %+v\n actual: %+v", *test.Output, f) + } + }) + } +}