Page MenuHomePhabricator
Differential D14755

Preserve the exit code of the diff command in binary_safe_diff.sh
ClosedPublic
Actions

Authored by dim on Dec 12 2015, 6:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 30, 5:50 PM
Unknown Object (File)
Mon, Apr 29, 4:19 PM
Unknown Object (File)
Mon, Apr 29, 8:47 AM
Unknown Object (File)
Wed, Apr 24, 11:59 PM
Unknown Object (File)
Sat, Apr 20, 9:02 PM
Unknown Object (File)
Thu, Apr 11, 8:16 AM
Unknown Object (File)
Wed, Apr 10, 2:16 AM
Unknown Object (File)
Tue, Apr 2, 6:36 PM
View All Files
Subscribers
eadler
Korvin
• stevenh

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Maniphest Tasks
T9970: new files are empty for arc diff --create and --update
Commits
rARC97056a3b85bb: Preserve the exit code of the diff command in binary_safe_diff.sh
Required Signatures
L28 Phacility Individual Contributor License Agreement
Summary

Some versions of Subversion (1.9 in any case, maybe others) will
duplicate diff headers, if the diff command run through --diff-cmd
returns 0.

This lead to T9970, where the addition of a new file with properties
only shows the properties themselves in the review, not the content of
the new file.

Test Plan

This is a trivial change, is a test needed at all?

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dim updated this revision to Diff 35681.Dec 12 2015, 6:58 PM
dim retitled this revision from to Preserve the exit code of the diff command in binary_safe_diff.sh.
dim updated this object.
dim edited the test plan for this revision. (Show Details)
dim added a reviewer: epriestley.
dim added subscribers: eadler, • stevenh.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptDec 12 2015, 6:58 PM
Herald added a required legal document: L28 Phacility Individual Contributor License Agreement. · View Herald Transcript
Herald added a subscriber: Korvin. · View Herald Transcript
dim mentioned this in T9970: new files are empty for arc diff --create and --update .Dec 12 2015, 6:59 PM
• stevenh added inline comments.Dec 13 2015, 10:18 AM
scripts/repository/binary_safe_diff.sh
8

As exit takes an int I don't think which should be quoted

dim added inline comments.Dec 13 2015, 10:19 AM
scripts/repository/binary_safe_diff.sh
8

Me neither, but I just imitated the existing coding style, which appears to quote everything.

epriestley accepted this revision.Dec 13 2015, 10:21 AM
epriestley edited edge metadata.
This revision is now accepted and ready to land.Dec 13 2015, 10:21 AM
Closed by commit rARC97056a3b85bb: Preserve the exit code of the diff command in binary_safe_diff.sh (authored by dim, committed by epriestley). · Explain WhyDec 13 2015, 10:21 AM
This revision was automatically updated to reflect the committed changes.
epriestley added a task: T9970: new files are empty for arc diff --create and --update .Dec 13 2015, 10:22 AM
dim added a comment.Dec 13 2015, 10:24 AM

Now that I think of it, the simplest possible way to get the desired effect (return 0 if diff succeeds, 1 if it does not succeed), is to use:

diff "$@" || exit 1
epriestley added a comment.Dec 13 2015, 10:25 AM

Happy to accept that if you want to double-check that it works and send me a diff for it.

epriestley added a comment.Dec 13 2015, 10:26 AM

(The quoting is me just being excessively-cautious/grossly-incompetent with bash, not an explicit coding style choice.)

dim added a comment.Dec 13 2015, 10:35 AM
In D14755#167774, @epriestley wrote:

Happy to accept that if you want to double-check that it works and send me a diff for it.

Please see D14759.

Revision Contents
Changeset List

PathSize
scripts/
repository/
4 lines

Diff 35690

View Options

scripts/repository/binary_safe_diff.sh

Loading...
Phabricator · Built by Phacility