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

netatalk 4.0.8 (new formula) #202713

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rdmark
Copy link

@rdmark rdmark commented Dec 29, 2024

  • 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 HOMEBREW_NO_INSTALL_FROM_API=1 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 HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>? (see caveat below)

Please note that brew audit --new netatalk throws non-compliance as per the below. However, the native macOS berkeley-db and openldap libraries are both unusable: The former lacks required symbols, and the latter (represented by LDAP.Framework) isn't fork safe while netatalk is a forking application.

brew audit --new netatalk                                                  
netatalk
  * Dependency 'berkeley-db' is provided by macOS; please replace 'depends_on' with 'uses_from_macos'.
  * Dependency 'openldap' is provided by macOS; please replace 'depends_on' with 'uses_from_macos'.

@github-actions github-actions bot added new formula PR adds a new formula to Homebrew/homebrew-core perl Perl use is a significant feature of the PR or issue labels Dec 29, 2024
Copy link
Contributor

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@rdmark rdmark force-pushed the netatalk-4-0-8 branch 3 times, most recently from 4c5e0b4 to 8bb667f Compare December 29, 2024 15:02
@chenrui333 chenrui333 added build failure CI fails while building the software CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Dec 29, 2024
@rdmark
Copy link
Author

rdmark commented Dec 29, 2024

The remaining CI failures are triggered by the brew audit checks, AFAICT.

As mentioned in the summary, complying with them and using the macOS supplied libraries will break the software. Can these CI failures be ignored?

@carlocab carlocab added CI-skip-new-formulae Pass --skip-new to brew test-bot. CI-skip-new-formulae-strict Pass --skip-new-strictw to brew test-bot. and removed build failure CI fails while building the software labels Dec 29, 2024
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.

Thanks for the PR; a few comments.

Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Show resolved Hide resolved
@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 29, 2024
@rdmark rdmark force-pushed the netatalk-4-0-8 branch 2 times, most recently from 5e94138 to 3565ce2 Compare December 29, 2024 22:18
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 29, 2024
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 30, 2024
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 30, 2024
@rdmark rdmark force-pushed the netatalk-4-0-8 branch 3 times, most recently from 72a454a to 5e76486 Compare December 30, 2024 16:47
@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 31, 2024
Formula/n/netatalk.rb Outdated Show resolved Hide resolved
auto-merge was automatically disabled January 2, 2025 17:53

Head branch was pushed to by a user without write access

@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Jan 2, 2025
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Jan 2, 2025
@carlocab carlocab enabled auto-merge January 2, 2025 17:57
auto-merge was automatically disabled January 2, 2025 19:13

Head branch was pushed to by a user without write access

@rdmark
Copy link
Author

rdmark commented Jan 2, 2025

@carlocab Thanks for all your help! I'm pleased that we got it working in the end.

FYI, I had to force-push the branch again because one of the macOS CI runners got flaky and posted a failure. Seems to have been an intermittent connection issue with the runner machine. Hopefully we get a green slate this time...

Edit: All jobs passed this time!

@rdmark rdmark requested a review from carlocab January 3, 2025 05:50
@rdmark
Copy link
Author

rdmark commented Jan 3, 2025

I figured out how to register the launchd / systemd services for netatalk. It basically works, if you invoke brew services with sudo. However I'm on the fence about whether it's worth adding or not, because brew services is clearly designed to be run as non-root, so I'm getting various warnings and errors.

diff --git a/Formula/n/netatalk.rb b/Formula/n/netatalk.rb
index 7258a9604f7..8fff22103e7 100644
--- a/Formula/n/netatalk.rb
+++ b/Formula/n/netatalk.rb
@@ -45,7 +45,8 @@ class Netatalk < Formula
       "-Dwith-appletalk=#{OS.linux?}", # macOS doesn't have an AppleTalk stack
       "-Dwith-bdb-path=#{Formula["berkeley-db@5"].opt_prefix}",
       "-Dwith-docbook-path=#{Formula["docbook-xsl"].opt_prefix}/docbook-xsl",
-      "-Dwith-init-style=none",
+      "-Dwith-init-dir=#{prefix}",
+      "-Dwith-init-hooks=false",
       "-Dwith-install-hooks=false",
       "-Dwith-pam-config-path=#{etc}/pam.d",
       "-Dwith-rpath=false",
@@ -57,6 +58,10 @@ class Netatalk < Formula
     system "meson", "install", "-C", "build"
   end
 
+  service do
+    name macos: "io.netatalk.daemon", linux: "netatalk"
+  end
+
   test do
     system sbin/"netatalk", "-V"
     system sbin/"afpd", "-V"

@SMillerDev
Copy link
Member

There is a requires_root for the service block for exactly that.

@rdmark
Copy link
Author

rdmark commented Jan 4, 2025

@SMillerDev Right on, that did the trick. Thanks!

@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Jan 6, 2025
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Jan 6, 2025
@rdmark rdmark requested a review from EricFromCanada January 6, 2025 22:13
Formula/n/netatalk.rb Show resolved Hide resolved
end

service do
name macos: "io.netatalk.daemon", linux: "netatalk"
Copy link
Member

Choose a reason for hiding this comment

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

What do these service files look like? In particular, where do they send logs, and what working directory do they use?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

We probably want to fix up some of those path references, since they'll likely be Cellar paths that aren't suitable for long-lived services

Copy link
Author

Choose a reason for hiding this comment

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

@carlocab So you're arguing that the two /tmp paths for the macOS logs should be rewritten to start with #{prefix}, for instance /opt/homebrew/Cellar/netatalk/4.0.8/var/log ?

Or am I getting your comment backwards?

Copy link
Author

Choose a reason for hiding this comment

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

On Linux, we log to journald by default, and not to a particular file.

The working directory is not controlled by the service files, but rather the build system. On aarch64 macOS, for instance, the working directory is /opt/homebrew/Cellar/netatalk/4.0.8/var/netatalk.

Copy link
Author

@rdmark rdmark Jan 7, 2025

Choose a reason for hiding this comment

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

All of the paths in plist, and the systemd unit file, are agnostic to the absolute path. The netatalkd file is just a helper script used by the plist (or can be used directly by users: sudo netatalkd start) that can be installed and reinstalled freely by brew.

However, thanks to this discussion I realized that it's critical to set a custom path for -Dwith-statedir-path which translates to /opt/homebrew/opt/netatalk/var or perhaps /opt/homebrew/var. This is where netatalk stores the CNID database which contains a structured representation of all the files in the shared volumes. This must be carried over when upgrading netatalk.

Copy link
Author

Choose a reason for hiding this comment

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

Hm, never mind, I see now that there's a symlink in place that makes /opt/homebrew/opt/netatalk/var a stable path between installed versions.

Copy link
Author

Choose a reason for hiding this comment

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

Actually strike that, I ran some empirical tests. With the current configuration, updating from one version to another will wipe the contents of /opt/homebrew/opt/netatalk/var, because it symlinks to f.e. /opt/homebrew/Cellar/netatalk/4.0.8/var which gets replaced in an upgrade.

What gets me the expected behavior is -Dwith-statedir-path=#{var} which stores the CNID database under /opt/homebrew/var/netatalk/CNID

Copy link
Member

Choose a reason for hiding this comment

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

I'd still suggest using the stable opt_bin path instead of the versioned path in the service files. See the inreplace directive in the batt formula for an example. Something like: inreplace "distrib/initscripts/macos.netatalk.plist.in", "@bindir@", opt_bin

Copy link
Author

Choose a reason for hiding this comment

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

I do like these paths better. Now I've pushed this, plus the same substitution for the sbin dirs in all init scripts.

@EricFromCanada
Copy link
Member

I've filed a PR to fix the launchd item not being able to stop the netatalk process: Netatalk/netatalk#1859

For now, I'd suggest adding a patch stanza to integrate the above fix, unless the next netatalk release is imminent.

Side note: what's it take to get its user authentication working in modern macOS? With the server running on Sonoma on M3 and logging enabled in afp.conf, clicking "Connect As…" on another Mac and entering my credentials just logs (info:ad): DHX2: PAM_Error: authentication error.

@rdmark
Copy link
Author

rdmark commented Jan 8, 2025

@EricFromCanada Thanks for filing the PR. Let me follow up over there. The next stable netatalk release is about a week out.

Authentication should work out of the box on any host, as long as the Mac client is running Mac OS 8.6, 9.x, or any version of OSX / macOS. What OS is the client Mac running?

My first guess would be some kind of PAM library failure on Sonoma. Alternatively, you're using a password that the DHX2 UAM doesn't like for some reason. If you like, you can open up a Discussion thread over at the netatalk GitHub project and we can dig into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-skip-new-formulae Pass --skip-new to brew test-bot. CI-skip-new-formulae-strict Pass --skip-new-strictw to brew test-bot. new formula PR adds a new formula to Homebrew/homebrew-core 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.

7 participants