Page MenuHomeFreeBSD

krb5: Install profile.h again
Needs ReviewPublic

Authored by cy on Thu, Jul 16, 5:18 PM.
Tags
None
Referenced Files
F163472686: D58286.id182085.diff
Thu, Jul 23, 12:30 PM
F163472378: D58286.id182085.diff
Thu, Jul 23, 12:24 PM
F163454463: D58286.id182095.diff
Thu, Jul 23, 8:20 AM
F163437040: D58286.id182157.diff
Thu, Jul 23, 4:50 AM
F163432141: D58286.id.diff
Thu, Jul 23, 4:00 AM
F163422049: D58286.id182157.diff
Thu, Jul 23, 2:04 AM
F163421872: D58286.id182157.diff
Thu, Jul 23, 2:01 AM
F163415849: D58286.diff
Thu, Jul 23, 12:44 AM
Subscribers

Details

Reviewers
fluffy
emaste
markj
ivy
des
Group Reviewers
krb5
Summary

Commit 1876de606eb8 exposed missing symbols that the port security/krb5
installed that the base system did not install. Part of the solution
was to make libprofile.so private (not libprofile.a) just as the port
does. The actual fix for this was to put symbols and their corresponding
functions into the correct librarires, i.e. libkrb5.so and othes, so
that the libraries produced by buildworld were the same as those
produced by the port.

Unfortunately INTERNALLIB disables the include of bsd.incs.mk. This is
still needed to install profile.h into /usr/include (just as the port
installs it into ${LOCALBASE}/include. This commit fixes this by
installing profile.h itself as the port and the manual krb5 install
outside ports do.

Reported by: fluffy
Tested by: fluffy
Fixes: 1876de606eb8
MFC after: 3 days

Test Plan

Tested locally with buildworld and installworld. Patch sent to fluffy@. (fluffy@ sent a private email notifying me of the problem.)

Also tested by fluffy@.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 75021
Build 71904: arc lint + arc unit

Event Timeline

cy requested review of this revision.Thu, Jul 16, 5:18 PM

Remove OptionalObsoleteFiles designation.

Properly address the problem during installworld.

i really don't like the filename /usr/include/profile.h, it's very generic and seems certain to conflict with something else in the future.

is there any way we could move the krb5 headers into a subdirectory, like /usr/include/krb5? this shouldn't break things which use pkgconf to discover paths, which i believe applies to most ports, but i accept it may break some older software that expects the headers to be in /usr/include.

To be honest, /usr/include/profile.h is a conventional place for this kerberos header and consumers expecting to find it here; for example take a look at net/freerdp3 port

However, moving headers onto dedicated subdir will require not only pkgconf patching but autoconf/aclocal too as many consumers checks for includes presence while run ./configure script

/usr/include is the standard location for profile.h. See below on a Red Hat server.

[cschuber@rhkam ~]$ cat /etc/redhat-release
Red Hat Enterprise Linux release 10.2 (Coughlan)
[cschuber@rhkam~]$
[cschuber@rhkam ~]$ rpm -qf /usr/include/profile.h
krb5-devel-1.21.3-10.el10_2.x86_64
[cschuber@rhkam ~]$

Prior to 1876de606eb8 profile.h was in /usr/include but when 1876de606eb8 converted libprofile.so to a private library (libprofile.a) the include file was no longer installed, though it should still have been. profile.h must be in /usr/include.

I'm pretty sure this header needs to be in /usr/include due, well, its seniority status among headers (ie, what software expects.)

I think it's fine being here.

cy edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Fri, Jul 17, 9:04 PM
des requested changes to this revision.Sun, Jul 19, 8:46 AM

A simple .include <bsd.incs.mk> at the bottom should suffice.

This revision now requires changes to proceed.Sun, Jul 19, 8:46 AM
In D58286#1337791, @des wrote:

A simple .include <bsd.incs.mk> at the bottom should suffice.

It doesn't work, as tested by fluffy@ and myself with a previous patch with bsd.incs.mk. I'll try to understand why a this won't work with INTERNALLIB tomorrow.

This update installs profile.h from krb5/include along with other
header files. This also avoids the issue of bsd.incs.mk not
installing the header file when INTERNALLIB is defined.

bsd.incs.mk absolutely requires INCLUDEDIR.

In D58286#1339612, @cy wrote:

bsd.incs.mk absolutely requires INCLUDEDIR.

fluffy@ tells me via private email this does not work.

May be

INCSDIR= ${INCLUDEDIR}

should be used here as it defined in other makefiles?

Tested by fluffy@ to work with poudriere.

Just re-checked.

This doesn't work on yesterday's -current

  • a/krb5/util/profile/Makefile

+++ b/krb5/util/profile/Makefile
@@ -67,6 +67,8 @@ prof_file.c: profile.h

.include <bsd.lib.mk>

-.SUFFIXES: .et
+.SUFFIXES: .et .h

.PATH: ${KRB5_DIR}/util/profile

+

+.include <bsd.incs.mk>

Simplified version (withot SUFFIXES modification) doesn't work too.

Latest patch from review does the trick as intended, jail/packages/real system was populated with profile.h as needed.
It is ready to land now.