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

Return error if patch file passed to state file.patch is malformed #59806

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

msciciel
Copy link
Contributor

@msciciel msciciel commented Mar 15, 2021

What does this PR do?

If patch file provided for file.patch state is malformed then state
returns Patch was already applied but patch is not applied.
It is better to return error in such case.

What issues does this PR fix or reference?

Fixes: #59805

Previous Behavior

          ID: patch_example
    Function: file.patch
        Name: /tmp/example
      Result: True
     Comment: Patch was already applied
     Started: 12:20:50.953163
    Duration: 61.558 ms
     Changes:

New Behavior

          ID: patch_example
    Function: file.patch
        Name: /tmp/example
      Result: False
     Comment: /usr/bin/patch: **** malformed patch at line 7:
     Started: 12:33:44.915605
    Duration: 59.202 ms
     Changes:

Test needs to be fixed also because it return OK(false positive) with original code but log file contains and patch is not applied:

Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -ur a/foo/bar/math.txt b/foo/bar/math.txt
|--- a/foo/bar/math.txt	2018-04-09 18:43:52.883205365 -0500
|+++ b/foo/bar/math.txt	2018-04-09 18:44:58.525061654 -0500
--------------------------
File to patch: 
Skip this patch? [y] 
Skipping patch.
1 out of 1 hunk ignored
can't find file to patch at input line 13
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -ur a/foo/numbers.txt b/foo/numbers.txt
|--- a/foo/numbers.txt	2018-04-09 18:43:58.014272504 -0500
|+++ b/foo/numbers.txt	2018-04-09 18:44:46.487905044 -0500
--------------------------
File to patch: 
Skip this patch? [y] 
Skipping patch.
1 out of 1 hunk ignored

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

@sagetherage sagetherage requested a review from dwoz April 16, 2021 03:05
salt/states/file.py Show resolved Hide resolved
@msciciel msciciel force-pushed the file-patch-state-malformed-patch branch 5 times, most recently from d43679b to 6098a52 Compare April 28, 2021 07:39
@sagetherage
Copy link
Contributor

@msciciel if you can get this out of a draft status it will run the tests and I can get it reviewed! Thank you!

@msciciel msciciel marked this pull request as ready for review May 20, 2021 13:45
@msciciel msciciel requested a review from a team as a code owner May 20, 2021 13:45
@msciciel
Copy link
Contributor Author

@msciciel if you can get this out of a draft status it will run the tests and I can get it reviewed! Thank you!

Done.

@msciciel msciciel removed the request for review from a team May 26, 2021 15:21
@msciciel
Copy link
Contributor Author

Any chance for final review and possible merge?

@msciciel msciciel force-pushed the file-patch-state-malformed-patch branch 3 times, most recently from daae1d4 to 17cb00a Compare April 14, 2023 19:05
@msciciel msciciel changed the title Return error if patch file passed to state file.patch is malformed WIP: Return error if patch file passed to state file.patch is malformed Apr 14, 2023
@msciciel msciciel changed the title WIP: Return error if patch file passed to state file.patch is malformed Return error if patch file passed to state file.patch is malformed Apr 15, 2023
@msciciel
Copy link
Contributor Author

msciciel commented Apr 17, 2023

@danielrobbins @dwoz @sagetherage is there a change for a review of this fix?

@msciciel msciciel requested a review from danielrobbins April 17, 2023 16:20
If patch file provided for file.patch state is malformed then state
returns `Patch was already applied` but patch is not applied.

          ID: patch_example
    Function: file.patch
        Name: /tmp/example
      Result: True
     Comment: Patch was already applied
     Started: 12:20:50.953163
    Duration: 61.558 ms
     Changes:

It is better to return error in such case.

          ID: patch_example
    Function: file.patch
        Name: /tmp/example
      Result: False
     Comment: /usr/bin/patch: **** malformed patch at line 7:
     Started: 12:33:44.915605
    Duration: 59.202 ms
     Changes:
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.

[BUG] state file.patch pretend to apply malformed patch
4 participants