Skip to content

Maintenance: rework SASL detection #1694

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.

Sign up for GitHub

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

Jump to bottom
Closed
kinkie wants to merge 6 commits into from
Closed

Maintenance: rework SASL detection #1694

kinkie wants to merge 6 commits into from

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