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

fix: greedy match may cause unwanted replacement #58960

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
3 participants
@centurycoder
Copy link

commented Jul 11, 2019

SUMMARY

say we have below 2 existing blocks in file, and we specify "marker_begin_text_1" in playbook to do an update.
But because of greedy match, the content of block 2 will also be overwritten.
So this pull request is to resolve this issue.

marker_begin_text_1
content1
end
marker_begin_text_2
content2
end

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

files/blockinfile.py

ADDITIONAL INFORMATION

fix: greedy match may cause unwanted replacement
say we have below 2 existing blocks in file, and we specify "marker_begin_text_1" in playbook to do an update.
But because of greedy match, the content of block 2 will also be overwritten.
So this pull request is to resolve this issue.


marker_begin_text_1
    content1
end
marker_begin_text_2
    content2
end
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/files/blockinfile.py:285:16: singleton-comparison Comparison to None should be 'expr is not None'

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/files/blockinfile.py:285:20: E711 comparison to None should be 'if cond is not None:'
lib/ansible/modules/files/blockinfile.py:286:15: E111 indentation is not a multiple of four

click here for bot help

@@ -282,6 +282,8 @@ def main():
n0 = i
if line == marker1:
n1 = i
if (n0 != None):
break

This comment has been minimized.

Copy link
@bcoca

bcoca Jul 11, 2019

Member

I would not change current behaviour, add an option that defaults to the current greedy=yes|no

This comment has been minimized.

Copy link
@centurycoder

centurycoder Jul 12, 2019

Author

I would not change current behaviour, add an option that defaults to the current greedy=yes|no

@bcoca
That would be wonderful to have such an option. Thank you for your adoption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.