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

Audit: make sure service commands exist #11129

Merged

Conversation

SMillerDev
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

@BrewTestBot
Copy link
Member

Review period will end on 2021-04-14 at 09:26:43 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Apr 13, 2021
@SMillerDev SMillerDev force-pushed the feature/audit/service_command branch from 281dacd to a8839da Compare April 13, 2021 09:28
@SMillerDev
Copy link
Member Author

And here we have 1 of the 2 reasons that I wanted to make the service block.

@SMillerDev
Copy link
Member Author

Hmm, I need to somehow make this audit require an installed formula. Any suggestions?

@MikeMcQuaid
Copy link
Member

Hmm, I need to somehow make this audit require an installed formula. Any suggestions?

def audit_installed
@new_formula ||= false
problem_if_output(check_manpages)
problem_if_output(check_infopages)
problem_if_output(check_jars)
problem_if_output(check_non_libraries) if @new_formula
problem_if_output(check_non_executables(formula.bin))
problem_if_output(check_generic_executables(formula.bin))
problem_if_output(check_non_executables(formula.sbin))
problem_if_output(check_generic_executables(formula.sbin))
problem_if_output(check_easy_install_pth(formula.lib))
problem_if_output(check_elisp_dirname(formula.share, formula.name))
problem_if_output(check_elisp_root(formula.share, formula.name))
problem_if_output(check_python_packages(formula.lib, formula.deps))
problem_if_output(check_shim_references(formula.prefix))
problem_if_output(check_plist(formula.prefix, formula.plist))
problem_if_output(check_python_symlinks(formula.name, formula.keg_only?))
end
alias generic_audit_installed audit_installed

@SMillerDev
Copy link
Member Author

It seems kinda weird to add it to the Cellar checks since it checks the formula, not the cellar. But I guess that can work until someone has a better idea.

@Bo98
Copy link
Member

Bo98 commented Apr 13, 2021

It seems kinda weird to add it to the Cellar checks since it checks the formula, not the cellar. But I guess that can work until someone has a better idea.

You can think of it as checking the Cellar that the file referenced by the service block exists.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Apr 14, 2021
@BrewTestBot
Copy link
Member

Review period ended.

BrewTestBot
BrewTestBot previously approved these changes Apr 14, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 14, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 14, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 20, 2021
@SMillerDev SMillerDev dismissed stale reviews from ghost and BrewTestBot via 9e826bb April 26, 2021 06:45
@SMillerDev SMillerDev force-pushed the feature/audit/service_command branch from f9ea99e to 9e826bb Compare April 26, 2021 06:45
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
Rylan12
Rylan12 previously approved these changes Apr 26, 2021
Copy link
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. One question if you don't mind: why/when/how would you specify multiple run commands?

@SMillerDev
Copy link
Member Author

Looks good to me. One question if you don't mind: why/when/how would you specify multiple run commands?

It's multiple parts, e.g. [bin/'httpd', '-D', 'foreground']

@Rylan12
Copy link
Member

Rylan12 commented Apr 26, 2021

Ah gotcha, thanks!

@SMillerDev SMillerDev dismissed stale reviews from Rylan12 and BrewTestBot via 88afa58 April 26, 2021 15:15
BrewTestBot
BrewTestBot previously approved these changes Apr 26, 2021
@SMillerDev SMillerDev merged commit afa99b4 into Homebrew:master Apr 27, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label May 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021
@SMillerDev SMillerDev deleted the feature/audit/service_command branch September 21, 2022 13:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants