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

Reorder switches in dbpath_fix find command #572

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pavloos
Copy link

@pavloos pavloos commented Dec 30, 2019

Pull Request (PR) description

Fix ordering of switches in dbpath_fix's find command

This Pull Request (PR) fixes the following issues

Fixes #571

@bastelfreak
Copy link
Member

Hi. I gave this a quick look and I'm not sure if this change is correct. The find output differs, depending on the command order. I am not sure if the new output is still correct.

@bastelfreak bastelfreak added bug Something isn't working needs-feedback Further information is requested labels Dec 30, 2019
@pavloos
Copy link
Author

pavloos commented Dec 30, 2019

My understanding is that this block of code is supposed to run if the output return code of only_if command is 0. This will happen only if grep matches any lines and for that find needs to produce any output which does not happen with -print in its current position. In fact -print could be skipped altogether because it is default if no other expression is specified.

@bastelfreak
Copy link
Member

@pjfbashton can you provide an acceptance test that valides this PR?

This was still wrong as it would always return results regardless if
ownership on files. The correct solution is to drop `-print` altogether.
@pavloos
Copy link
Author

pavloos commented Mar 18, 2020

Apologies for delayed response.

You are actually correct. Proposed solution was not working as intended as it would always return results thus that exec would always run.
Existing find command is not correct though because it will never return results and it seems that the correct solution is to drop -print altogether and I have update the PR to that effect.

Please see the following

test 1 - mixed ownership

# find m/ -ls
  2886300      4 drwxr-xr-x   5  root     root         4096 Mar 18 11:47 m
  2886303      4 drwxr-xr-x   2  root     root         4096 Mar 18 11:47 m/3
  2886301      4 drwxr-xr-x   2  mongod   mongod       4096 Mar 18 11:47 m/1
  2886302      4 drwxr-xr-x   2  mongod   mongod       4096 Mar 18 11:47 m/2
  • current solution is wrong - it should return 2 results so that grep matches and exec is run
# find m -not -user mongod -o -not -group mongod -print -quit  | wc -l
0
  • working solution is to actually drop -print altogether
# find m/ -not -user mongod -o -not -group mongod -quit | wc -l
2

test 2 - ownership on all files is wrong

# find m/ -ls
  2886300      4 drwxr-xr-x   5  root     root         4096 Mar 18 11:47 m
  2886303      4 drwxr-xr-x   2  root     root         4096 Mar 18 11:47 m/3
  2886301      4 drwxr-xr-x   2  root     root         4096 Mar 18 11:47 m/1
  2886302      4 drwxr-xr-x   2  root     root         4096 Mar 18 11:47 m/2
  • existing solution returns no results and it should return 4 lines
# find m/ -not -user mongod -o -not -group mongod -print -quit  | wc -l
0
  • no -print solution - correct result
# find m/ -not -user mongod -o -not -group mongod -quit  | wc -l
4

test 3 - ownership on all files is correct

# find m -ls
  2886300      4 drwxr-xr-x   5  mongod   mongod       4096 Mar 18 11:47 m
  2886303      4 drwxr-xr-x   2  mongod   mongod       4096 Mar 18 11:47 m/3
  2886301      4 drwxr-xr-x   2  mongod   mongod       4096 Mar 18 11:47 m/1
  2886302      4 drwxr-xr-x   2  mongod   mongod       4096 Mar 18 11:47 m/2
  • existing solution
# find m/ -not -user mongod -o -not -group mongod -print -quit  | wc -l
0
  • proposed solution
# find m/ -not -user mongod -o -not -group mongod -quit  | wc -l
0

As you can see from above 3 cases only find command without -print works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-feedback Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dbpath_fix find command issue
2 participants