homebrew-core icon indicating copy to clipboard operation
homebrew-core copied to clipboard

Published 20 hours ago • verified publisher icon Homebrew

krb5: add dependencies

Open DE0CH opened this issue 1 year ago • 10 comments

  • [x] Have you followed the guidelines for contributing?
  • [x] Have you ensured that your commits follow the commit style guide?
  • [x] Have you checked that there aren't other open pull requests for the same formula update/change?
  • [x] Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • [x] Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • [x] 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>?

Building krb5 requires libedit for the header editline/readline.h.krb5 builds fine on debian for some reason but it didn't build on other linux distros that don't have the header file somewhere. In any case, including libedit should be safer.

DE0CH avatar Sep 21 '22 11:09 DE0CH

Sorry about the commit style issue. I have fixed the problems. Please review.

DE0CH avatar Sep 21 '22 13:09 DE0CH

Building krb5 requires libedit for the header editline/readline.h.

This should only happen if pkg-config is detecting libedit is installed:

https://github.com/krb5/krb5/blob/krb5-1.20-final/src/configure.ac#L1340-L1376 https://github.com/krb5/krb5/blob/krb5-1.20-final/src/util/ss/listen.c#L17-L18

It's maybe a good idea to depend on libedit or readline anyway, though it is only used in kadmin so it's not something dependents will be using much (nor has anyone asked for it). In any case, I don't think it's build-only.

Bo98 avatar Sep 21 '22 13:09 Bo98

That's interesting. On my system (which is running Scientific Linux 7.9 with a ton of really old packages), it failed with the make step (not the configure setup).

In any case, I don't think it's build-only.

I see, I'll remove the build only option

DE0CH avatar Sep 21 '22 14:09 DE0CH

Alternative would be to just pass --without-libedit and --without-readline on Linux if the benefits are limited. I don't particularly mind though given libedit is quite light.

Bo98 avatar Sep 21 '22 14:09 Bo98

I would prefer having libedit because it is quite lite and it can prevent having problems with other packages that depend on it.

DE0CH avatar Sep 21 '22 14:09 DE0CH

Not sure why CI too so long to finish that it got canceled.

DE0CH avatar Sep 21 '22 21:09 DE0CH

It takes a long time because this formula has many dependents.

carlocab avatar Sep 22 '22 09:09 carlocab

I'll review this once we've had the chance to give this a full CI run.

carlocab avatar Sep 22 '22 09:09 carlocab

Can you re-trigger CI? I don't think I can trigger it (besides pushing).

DE0CH avatar Sep 22 '22 15:09 DE0CH

CI failed because of the following error

No CI-syntax-only label found. Running tests job.


No CI-no-fail-fast label found. Stopping tests on first failing matrix build.

No CI-skip-dependents label found. Running brew test-bot --only-formulae-dependents.

No CI-long-timeout label found. Setting short GitHub Actions timeout.

Error: PR requires the CI-long-timeout label but it is not set!
Error: If the longer timeout is not required, remove the "long build" label.
Error: Otherwise, add the "CI-long-timeout" label.
Error: No more than 2 PRs at a time may use "CI-long-timeout".
No CI-test-bot-fail-fast label found. Not passing --fail-fast to brew test-bot.
No CI-build-dependents-from-source label found. Not passing --build-dependents-from-source to brew test-bot.
No CI-skip-recursive-dependents label found. Not passing --skip-recursive-dependents to brew test-bot.
No CI-skip-livecheck label found. Not passing --skip-livecheck to brew test-bot.

DE0CH avatar Sep 23 '22 00:09 DE0CH

Yes, CI needs a special slot to schedule a CI run for this and such a slot is not available right now.

SMillerDev avatar Sep 23 '22 10:09 SMillerDev

The common way of packaging krb5 on Linux would be to use libss.so from e2fsprogs (as done by Alpine, Arch, Debian, etc.). I think the krb5 internal copy is where the libedit dependency comes from.

However, in Homebrew, it may be simpler to just use libedit for now or disable it.

If we were to properly package e2fsprogs (current formula is a bit off compared to common distribution), then Linux would require depending on util-linux as that formula has main libuuid/blkid and that would pull in a couple other dependencies: libxcrypt, ncurses, zlib.

Since Homebrew doesn't supporting split/sub packages which can reduce dependency trees (in this case, we shouldn't need libuuid/etc but cannot avoid those), pulling in e2fsprogs may not be ideal as krb5 is a heavily used dependency on Linux.

As side note, if we were to pick one, libedit is probably better than readline in this case to avoid propagating GPL licensing restrictions. Would mainly impact the corresponding library (libss.so), but makes it more likely to hit license conflicts.

Though, in other cases, readline may be better as it is the GNU variant which is more commonly used on Linux.

cho-m avatar Oct 18 '22 00:10 cho-m

I don't really know much about the package dependencies in linux, so I am quite lost with your comment.

Are you saying using libedit in the current form is ok? Or is there anything that need to be changed in the formula?

DE0CH avatar Oct 18 '22 12:10 DE0CH