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

Add help message when diff fails #1466

Merged
merged 2 commits into from
Feb 18, 2021
Merged

Conversation

runewake2
Copy link
Contributor

Related to #1378

This introduces an additional error message when a potential error is detected during a pkg diff (error codes non-0 or 1 - diff returning error code 1 indicates a difference was detected).

Kpt uses an external diff tool to compare packages using the provided diff strategy. Some of these tools may not support 3 way diffs, others may not support diffing over directories. In order to attempt to improve the experience of working with kpt pkg diff these new errors are added to attempt to help provide guidance around resolving the error.

For example this changes the behavior of a 3way diff on clients where diff does not support 3 directories from

➜  kpt pkg diff package@main --diff-type=3way
/usr/bin/diff: extra operand `/tmp/kpt-234176370/target-main'
/usr/bin/diff: Try `/usr/bin/diff --help' for more information.

error: exit status 2
exit status 1

to

➜  kpt pkg diff package@main --diff-type=3way
/usr/bin/diff: extra operand `/tmp/kpt-234176370/target-main'
/usr/bin/diff: Try `/usr/bin/diff --help' for more information.

The selected diff tool (/usr/bin/diff) exited with an error. It may not support the chosen diff type (3way). To use a different diff tool please provide the tool using the --diff-tool flag. 

For more information about using kpt's diff command please see the commands --help.
error: exit status 2
exit status 1

@runewake2 runewake2 merged commit 39c45a0 into kptdev:master Feb 18, 2021
@runewake2 runewake2 deleted the diff-warn branch February 18, 2021 17:43
runewake2 added a commit to runewake2/kpt that referenced this pull request May 6, 2021
* Add help message when diff fails
runewake2 added a commit that referenced this pull request May 13, 2021
* Update Diffing Display (#1419)

* Update diff info
- update upstream directory names

* Distinguish local and remote in dir names

* Introduce a descriptive reference name in dir

* Rename diffed directories
- reduces the length of diffed directories
- uses short commit sha or up to 7 branch characters

* Update local diff path.

* Create shared staging directory
- refactor shortSha
- use one staging directory instead of three

* Add error handling for staging directory

* Place staging dir naming under test
- add accounting for some extra edge cases

* Update tests

* Remove extra comment

* Add help message when diff fails (#1466)

* Add help message when diff fails

* Remove unused code

* Address Pr comments

* Refactor tests

* Update test to validate arguments

* Fix linting error
frankfarzan pushed a commit to frankfarzan/kpt that referenced this pull request Jun 3, 2021
* Update Diffing Display (kptdev#1419)

* Update diff info
- update upstream directory names

* Distinguish local and remote in dir names

* Introduce a descriptive reference name in dir

* Rename diffed directories
- reduces the length of diffed directories
- uses short commit sha or up to 7 branch characters

* Update local diff path.

* Create shared staging directory
- refactor shortSha
- use one staging directory instead of three

* Add error handling for staging directory

* Place staging dir naming under test
- add accounting for some extra edge cases

* Update tests

* Remove extra comment

* Add help message when diff fails (kptdev#1466)

* Add help message when diff fails

* Remove unused code

* Address Pr comments

* Refactor tests

* Update test to validate arguments

* Fix linting error
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.

None yet

2 participants