Summary:
- When you try to "arc patch" a change which adds or removes a trailing newline or alters a file with a trailing newline near the newline, the patch fails because we fail to reconstuct the "\ No newline at end of file" line.
- We don't currently record enough information about these diffs to reconstruct them with a sensible amount of effort. Store the "\ No newline at end of file" line instead of just a flag.
- There are some special rules for these lines; implement them.
NOTE: This causes some display glitching in Differential, but nothing major; I'll fix that in a followup patch.
Test Plan:
- Created a diff which added a trailing newline, removed a trailing newline, and altered a file without a trailing newline near the end of the file.
- Verified that the git version of this patch and the 'arc export --git' version of this patch are (essentially) identical.
- Verified that "arc diff" + "arc patch" can apply these changes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T533
Differential Revision: https://secure.phabricator.com/D1810
Summary: This is probably just the first step down a long road of not handling
exotic filenames correctly, but if you diff a file named "∆.jpg" it currently
chokes while parsing the diff.
Test Plan: Added minimal failing unit test, fixed parser, test passed.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1592
Summary:
Mark all applicable Arcanist classes as "final", except PhutilLintEngine, which
needs a little finesse.
@jungejason / @nh, does this break any Facebook stuff?
Test Plan: Linter no longer raises warnings. Ran "testEverythingImplemented" in
Phabricator.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1519
removed
Summary: Mercurial output diverges from git output when binary files are
removed. Parse the Mercurial flavor.
Test Plan:
- Unit test fails before this change and passes afterward.
- Removed a binary file in my hg local and successfully "arc diff --only"'d
the change.
NOTE: Git can't apply these patches, and reports:
$ git apply < ~/Desktop/patch.txt
error: removal patch leaves file contents
error: level.png: patch does not apply
Reviewers: Makinde, btrahan, jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason, Makinde
Differential Revision: 1126
Test Plan:
Tried viewing a diff detected as binary in diffusion with and without
setting my desired encoding. Worked as expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1008
mercurial changesets
Summary: Makes the parsing slightly more liberal so a test case from @Makinde
passes.
Test Plan: Ran unit tests.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1054
Summary:
See T584. Git renders these types of changes somewhat unusually. Ensure they are
rendered as changes.
The fact that a symlink was replaced is not explicitly rendered (e.g., "This
path was changed from a symlink to a regular file."), but can be inferred from
the 'unix:filemode' property change that will accompany the changeset. We could
also make it explicit but this type of change is rare enough that it's probably
good enough as-is.
Test Plan:
- Ran unit test, which previously failed and now passes.
- Created a diff which replaces a file with a symlink, verified it rendered a
little more sensibly.
Reviewers: aravindn, dschleimer, jungejason, nh, tuomaspelkonen
Reviewed By: nh
CC: aran, goldshlager, nh
Differential Revision: 1030
changes which add or alter binary files
Summary: See test cases, derived from "hg diff --git" and "git diff --binary".
Test Plan: Test cases work now.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1047
Test Plan:
Go to /differential/diff/create and try to upload a patch that was created with
`git diff` when diff.mnemonicprefix was set to true.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1022
Summary: A minor bug in the parser prevented it from handling git diffs where an
empty file appears at the end of the diff. Since git appends an extra newline,
we failed to jump into the '$line === null' block.
Test Plan: Ran unit tests. Generated a revision which only deleted an empty
file.
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, nh, epriestley
Differential Revision: 998
Summary:
A better definition of "binary" is "not utf-8", instead of "has some characters
not in this arbitrary regexp". Principally, this makes files with windows
newlines not autodetect as binary.
This might fix some of the issues in T365.
Test Plan: @egillth applied this patch and verified that Diffusion now shows
file content instead of detecting everything as binary in his repo full of
Windows newlines.
Reviewed By: jungejason
Reviewers: egillth, tuomaspelkonen, jungejason, aran
CC: aran, jungejason
Differential Revision: 799
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
Summary:
See task. This is a bizarre edge case which we incorrectly reject but should do
our best with.
Test Plan:
Created an added copy of a file and diffed it successfully.
Reviewed By: alex
Reviewers: alex, jungejason, tuomaspelkonen, aran
CC: aran, alex
Differential Revision: 547
Summary:
I'm probably missing some edge cases but it took me almost 3 hours to get this
far and I think it only makes things work that didn't work before. Some stuff
like SVN binary patches still won't work, although they should be far easier to
implement.
Most of the magic here just comes from reading the git source code. It appears
to work correctly; I sprinkled printf() around git liberally and recompiled it
during development. Took me about 45 minutes to figure out that "Index" vs
"index" causes git to silently fail in a confusing way. :/
Git has a diff mode for binary changes but I don't think we lose much by always
using the full binaries. We can enhance it later if we want.
Test Plan:
Exported and patched binary changes (a picture of a duck) into a working copy.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: simpkins, aran, epriestley
Differential Revision: 327
Summary:
When an arcbundle includes a symlink, we fail to apply it correctly
when applying to a subversion working copy.
Test Plan:
created a diff which added a symlink, removed it locally, bundled
and applied the patch, got a good symlink out
Reviewed By: aran
Reviewers: aran
CC: epriestley, aran
Differential Revision: 63
Summary: We have some diffs which predate property tracking and are shipping
down sketch data, work around it in the client for now.
Test Plan:
Reviewers:
CC:
svn.
Summary: when you "svn cp" with a revision, we end up with some path collision
problems. This isn't a complete fix but it gets the change to Differential,
at least.
Test Plan:
Reviewers:
CC:
