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

samba: avoid conflicting with /usr/bin/{mdfind,profiles} #81592

Conversation

AkihiroSuda
Copy link
Contributor

@AkihiroSuda AkihiroSuda commented Jul 21, 2021

Rename samba version of {mdfind,profiles} to samba-dot-org-{mdfind,profiles} to avoid conflicting with macOS systemd binaries /usr/bin/{mdfind,profiles}

The "samba-dot-org-" prefix corresponds to samba-dot-org-smbd.

Also remove custom CFLAGS.

Follow-up to #80171

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@BrewTestBot BrewTestBot added the perl Perl use is a significant feature of the PR or issue label Jul 21, 2021
Formula/samba.rb Outdated Show resolved Hide resolved
Formula/samba.rb Outdated Show resolved Hide resolved
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Can we change that rather heavy-handed -include in CFLAGS too?

Also, you should probably bump the revision so that users who have this installed get the update to the formula with brew upgrade.

@AkihiroSuda AkihiroSuda force-pushed the fix-samba-mdfind-profiles-conflict branch 2 times, most recently from 62ac921 to 5212fc3 Compare July 21, 2021 09:54
@AkihiroSuda
Copy link
Contributor Author

Can we change that rather heavy-handed -include in CFLAGS too?

Applying this patch is enough?

https://attachments.samba.org/attachment.cgi?id=16579 (from https://bugzilla.samba.org/show_bug.cgi?id=14680 )

Also, you should probably bump the revision so that users who have this installed get the update to the formula with brew upgrade.

Thanks, updated

@AkihiroSuda AkihiroSuda force-pushed the fix-samba-mdfind-profiles-conflict branch from 5212fc3 to 52a5105 Compare July 21, 2021 10:18
@carlocab
Copy link
Member

The patch is fine. It'll be easier to notice that it needs removing anyway. Please indicate in a comment when the patch can be removed.

@AkihiroSuda AkihiroSuda marked this pull request as draft July 21, 2021 10:49
Formula/samba.rb Outdated Show resolved Hide resolved
@AkihiroSuda AkihiroSuda force-pushed the fix-samba-mdfind-profiles-conflict branch from 52a5105 to c9b75cd Compare July 22, 2021 13:37
@AkihiroSuda AkihiroSuda marked this pull request as ready for review July 22, 2021 13:37
Rename samba version of {mdfind,profiles} to samba-dot-org-{mdfind,profiles}
to avoid conflicting with macOS systemd binaries /usr/bin/{mdfind,profiles}

The "samba-dot-org-" prefix corresponds to samba-dot-org-smbd.

Also remove custom CFLAGS.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda force-pushed the fix-samba-mdfind-profiles-conflict branch from c9b75cd to c6588bd Compare July 22, 2021 13:38
@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@github-actions github-actions bot added the outdated PR was locked due to age label Aug 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2021
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 perl Perl use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants