Skip to content

Maintenance: rework SASL detection#1694

Closed
kinkie wants to merge 6 commits intosquid-cache:masterfrom
kinkie:compat-macos-2
Closed

Maintenance: rework SASL detection#1694
kinkie wants to merge 6 commits intosquid-cache:masterfrom
kinkie:compat-macos-2

Conversation

@kinkie
Copy link
Contributor

@kinkie kinkie commented Feb 21, 2024 •
edited by yadij
Loading

MacOS Homebrew offers Cyrus SASL.

@rousskov rousskov changed the title MacOS compat: add -Wno-deprecated-declarations CXXFLAGS MacOS compat: Build with -Wno-deprecated-declarations Feb 21, 2024
rousskov
rousskov reviewed Feb 21, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

I adjusted PR title a little because SQUID_CC_ADD_CXXFLAG_WARNING_IF_SUPPORTED() updates SQUID_CXXFLAGS rather than CXXFLAGS and because that is a low-level technical detail we can, IMO, omit to emphasize the gravity of these changes. I do not insist on that adjustment.

BTW, I think all these "MacOS compat: X" PR titles should be refactored to become "X on MacOS" (or some such) to avoid unnecessary abbreviations and prefix pollution, but there are too many of them for me to fight :-(.

I also pushed a minor adjustment. Please check.

@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Feb 21, 2024
@kinkie kinkie changed the title MacOS compat: Build with -Wno-deprecated-declarations Enable use of MacOS frameworks for LDAP and SASL Feb 23, 2024
@kinkie kinkie requested a review from rousskov February 23, 2024 11:48
@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box S-waiting-for-more-reviewers needs a reviewer and/or a second opinion and removed S-waiting-for-author author action is expected (and usually required) labels Feb 23, 2024
rousskov
rousskov requested changes Feb 23, 2024
View reviewed changes
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Feb 23, 2024
@kinkie kinkie requested a review from rousskov February 23, 2024 15:53
@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Feb 23, 2024
rousskov
rousskov previously requested changes Feb 23, 2024
View reviewed changes
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Feb 23, 2024
yadij
yadij requested changes Feb 23, 2024
View reviewed changes
@yadij yadij removed the S-waiting-for-more-reviewers needs a reviewer and/or a second opinion label Feb 23, 2024
@kinkie kinkie mentioned this pull request Feb 24, 2024
Closed
@kinkie

This comment was marked as outdated.

Sign in to view
@kinkie kinkie added the S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) label Mar 4, 2024
@kinkie kinkie added the M-ignored-by-merge-bots https://github.com/measurement-factory/anubis/blob/master/README.md#pull-request-labels label Mar 13, 2024
@rousskov rousskov marked this pull request as draft March 13, 2024 21:45
@yadij
Copy link
Contributor

yadij commented Mar 18, 2024

@kinkie , ping. PR #1736 (was PR #1710) has merged. I have merged and resolved the resulting conflicts. Over to you for the remaining work on this.

@yadij yadij removed the S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) label Mar 18, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Mar 18, 2024

@kinkie , ping. PR #1736 (was PR #1710) has merged. I have merged and resolved the resulting conflicts. Over to you for the remaining work on this.

yes, thanks!

@kinkie kinkie changed the title Enable use of MacOS frameworks for LDAP and SASL Maintenance: rework SASL detection Mar 18, 2024
@kinkie kinkie requested a review from yadij March 18, 2024 14:40
yadij

This comment was marked as resolved.

Sign in to view

@yadij yadij added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Mar 19, 2024
kinkie and others added 2 commits March 20, 2024 05:49
@kinkie @yadij
Apply suggestions from code review
04bcccd 
Co-authored-by: Amos Jeffries <yadij@users.noreply.github.com>
@kinkie
Add release notes for --without-sasl
78ab1d0
@kinkie
Copy link
Contributor Author

kinkie commented Mar 20, 2024

Thanks @yadij

Excerpt from ./configure output on MacOS Sonoma:

checking for LIBSASL... yes
checking for sasl/sasl.h... yes
checking for sasl.h... no
configure: Library 'sasl' support: yes (auto)  -L/opt/homebrew/Cellar/cyrus-sasl/2.1.28_2/lib -lsasl2
[...]
configure: auth/basic helpers to be built:  DB LDAP NCSA PAM POP3 RADIUS SASL SMB fake getpwnam

@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Mar 20, 2024
@kinkie kinkie requested a review from yadij March 20, 2024 06:25
yadij
yadij requested changes Mar 20, 2024
View reviewed changes
Copy link
Contributor

@yadij yadij 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. One last issue with HAVE_SASL_DARWIN, sorry for not noticing earlier.

@kinkie kinkie requested a review from yadij March 20, 2024 12:42
@yadij yadij added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels S-could-use-an-approval An approval may speed this PR merger (but is not required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Mar 20, 2024
squid-anubis pushed a commit that referenced this pull request Mar 20, 2024
@kinkie @squid-anubis
Maintenance: rework SASL detection (#1694)
388d024 
MacOS Homebrew offers Cyrus SASL.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Mar 20, 2024
@kinkie kinkie removed the S-could-use-an-approval An approval may speed this PR merger (but is not required) label Mar 20, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Mar 20, 2024
kinkie added a commit to kinkie/squid that referenced this pull request Mar 21, 2024
@kinkie
Maintenance: rework SASL detection (squid-cache#1694)
53270a7 
MacOS Homebrew offers Cyrus SASL.
kinkie added a commit to kinkie/squid that referenced this pull request Apr 9, 2024
@kinkie
Maintenance: rework SASL detection (squid-cache#1694)
bf6d60d 
MacOS Homebrew offers Cyrus SASL.
kinkie added a commit to kinkie/squid that referenced this pull request Apr 9, 2024
@kinkie
Maintenance: rework SASL detection (squid-cache#1694)
bd6ebd7 
MacOS Homebrew offers Cyrus SASL.
kinkie added a commit to kinkie/squid that referenced this pull request Apr 14, 2024
@kinkie
Maintenance: rework SASL detection (squid-cache#1694)
5f27228 
MacOS Homebrew offers Cyrus SASL.
@kinkie kinkie deleted the compat-macos-2 branch November 9, 2024 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rousskov rousskov rousskov left review comments

@yadij yadij yadij approved these changes

Assignees

No one assigned

Labels

M-merged https://github.com/measurement-factory/anubis#pull-request-labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kinkie @yadij @rousskov @squid-anubis

Comments