Bug 226396 - emulators/wine-devel: build error if krb5 is installed
Summary: emulators/wine-devel: build error if krb5 is installed
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Gerald Pfeifer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-06 16:33 UTC by John Hein
Modified: 2018-09-01 13:19 UTC (History)
0 users

See Also:


Attachments
[patch] fix spelling in configure for disabling kerberos (1021 bytes, patch)
2018-03-06 18:06 UTC, John Hein
no flags Details | Diff
[patch] add KERBEROS option (plus a couple other changes) (3.40 KB, patch)
2018-03-19 18:26 UTC, John Hein
no flags Details | Diff
[patch] add GSSAPI option (plus a couple other changes) (3.00 KB, patch)
2018-03-19 20:21 UTC, John Hein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2018-03-06 16:33:52 UTC
If you try to build wine-devel-3.3,1 and you happen to have krb5 installed, the build fails:

gmake[3]: Entering directory '/usr/ports/emulators/wine-devel/work/wine-3.3/dlls/kerberos'
cc -m64 -c -o krb5_ap.o krb5_ap.c -I. -I../../include -I/usr/local/include -D__WINESRC__ -D_REENTRANT -fPIC \
  -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \
  -Wstrict-prototypes -Wtype-limits -Wvla -Wwrite-strings -Wpointer-arith -I/usr/local/include \
  -D__builtin_ms_va_list=__builtin_va_list -D__builtin_ms_va_start=__builtin_va_start \
  -D__builtin_ms_va_end=__builtin_va_end -D__builtin_ms_va_copy=__builtin_va_copy -O2 -pipe  -fstack-protector -fno-strict-aliasing
krb5_ap.c:871:5: error: use of undeclared identifier 'krb5_context'
    krb5_context ctx;
    ^
krb5_ap.c:872:5: error: use of undeclared identifier 'krb5_principal'
    krb5_principal principal = NULL;
    ^
krb5_ap.c:873:5: error: use of undeclared identifier 'krb5_get_init_creds_opt'
    krb5_get_init_creds_opt *options = NULL;
    ^
krb5_ap.c:873:30: error: use of undeclared identifier 'options'; did you mean 'optind'?
    krb5_get_init_creds_opt *options = NULL;
                             ^~~~~~~
                             optind
Comment 1 John Hein 2018-03-06 18:06:03 UTC
Created attachment 191254 [details]
[patch] fix spelling in configure for disabling kerberos

The attached patch fixes the problem by changing the spelling from enable_kerberos to with_krb5 in one place (the only place where enable_kerberos was used - never set).

To change this for upstream, we'd have to fix configure.ac, but it's not as simple since the WINE_CONFIG_DLL macro only knows "enable_*".  We'd either have to have the with_krb5 parsing bits know to also set enable_kerberos appropriately or teach WINE_CONFIG_DLL to take with_krb5 as an alternate spelling of enable_kerberos.

Maybe the fix for upstream is to change aclocal.m4 to use with_* instead of enable_*.  I don't see where any of the enable_* variables are set.

I suppose you could use --without-krb5 and --disable-kerberos, but I can't believe they intended to require that.  I did not test that yet.

In any case, the attached patch will work for the freebsd port for now until upstream figures it out.  Note: I did not yet contact upstream about this.

p.s. I think 3.2 did not have this problem because krb5_ap.c because the #ifdefs in krb5_ap.c were such that the references that require definitions in the krb5.h include file were not compiled.  For both 3.2 and 3.3 libgssapi_krb5 is found and enabled (maybe that should be disabled too when using --without-krb5).  It's just that the code in krb5_ap.c differs in 3.2 vs. 3.3 a bit.
Comment 2 John Hein 2018-03-06 18:45:14 UTC
(In reply to John Hein from comment #1)
> I suppose you could use --without-krb5 and --disable-kerberos, but I can't believe they intended to require that.  I did not test that yet.

I tried with --disable-kerberos in addition to --without-krb5 (and removed patch-configure).  That worked.  I tried --disable-kerberos instead of --without-krb5 (again without patch-configure).  That worked, too.

Why are we disabling kerberos by the way?
Comment 3 John Hein 2018-03-06 21:29:51 UTC
(In reply to John Hein from comment #2)
> Why are we disabling kerberos by the way?

I think it was just to avoid this configure message? ...
configure: libkrb5 64-bit development files not found, Kerberos won't be supported.
Comment 4 Gerald Pfeifer freebsd_committer 2018-03-18 21:58:34 UTC
Thank you for the report, and especially the great debugging work, John!

And you're right on, I added --without-krb5 based on the configure warning,
but really not just to silence the warning, but to help make the builds
more reproducible. 

Wine simply picks up whatever it finds in terms of optional components, 
so when one builds in a live system as opposed to a clean build environment, 
that can be all sorts of effects.  Hence all those --without and --disable
options in this port.

Per your findings and similar testing on my end I plan on adding
--disable-kerberos to the configure options to avoid the problem
you reported.  (I have not received any input from anyone actually
requiring Kerberos.)
Comment 5 commit-hook freebsd_committer 2018-03-18 22:24:53 UTC
A commit references this bug:

Author: gerald
Date: Sun Mar 18 22:24:43 UTC 2018
New revision: 464959
URL: https://svnweb.freebsd.org/changeset/ports/464959

Log:
  Fully disable building Kerberos components.

  We already had --without-krb5, alas when someone had krb5 installed
  in their build environment, the build still failed.  This takes care
  using a bigger hammer.

  PR:		226396
  Analyzed by:	John Hein <z7dr6ut7gs@snkmail.com>

Changes:
  head/emulators/wine-devel/Makefile
  head/emulators/wine-devel/pkg-plist
Comment 6 Gerald Pfeifer freebsd_committer 2018-03-18 22:33:19 UTC
It would be great could you file your report and findings (incl.
prototype patch) upstream at winehq.org, John!

Thank you again!
Comment 7 John Hein 2018-03-19 18:13:12 UTC
(In reply to Gerald Pfeifer from comment #6)
I'll work on an upstream patch.

In the mean time, I have a KERBEROS option patch for the freebsd port.  I'll attach that next.  I could open a new bug, but it seems like more trouble than it's worth - I'm really just curious if you have a comment about it.
Comment 8 John Hein 2018-03-19 18:26:12 UTC
Created attachment 191637 [details]
[patch] add KERBEROS option (plus a couple other changes)

This patch does three things:

 (1) Add GSSAPI option

   Right now it only supports MIT krb5 or none.
   Heimdal does not have gssapi_ext.h
   Base also does not have gssapi_ext.h and also does not have a krb5/ directory in /usr/include (so the #include <krb5/krb5.h> would need to be worked around).

   I left BASE & HEIMDAL commented in the patch, but I think they should just be removed.  If they are removed, there's not much point to having GSSAPI_NONE & GSSAPI_MIT single selection - should probably just have a GSSAPI option (on or off where "on" uses ports MIT krb5).


The next two are unrelated to kerberos/gssapi...

 (2) Add USES=pkgconfig

   The configure script uses pkg-config a lot.

 (3) Remove CFLAGS & LDFLAGS in favor of USES=localbase

   Instead of globally adding 'localbase' to USES, it only adds it if needed.  It is needed if CPUS, LDAP, MPG123, OPENAL or V4L is on since they do not have pkg-config .pc files (or in openal's case, the wine configure does not currently try to use pkg-config to look for openal).
Comment 9 John Hein 2018-03-19 20:21:12 UTC
Created attachment 191640 [details]
[patch] add GSSAPI option (plus a couple other changes)

(In reply to John Hein from comment #8)
 >  I left BASE & HEIMDAL commented in the patch, but I think they should just be removed.  If they are removed, there's not much point to having GSSAPI_NONE & GSSAPI_MIT single selection - should probably just have a GSSAPI option (on or off where "on" uses ports MIT krb5).


I simplified the patch to remove the commented GSSAPI_HEIMDAL & GSSAPI_BASE and marked the patch from comment 8 obsolete.  It would be major surgery to support the heimdal flavor of kerberos in the current wine code - MIT kerberos should do the job just fine.

Except for the GSSAPI_HEIMDAL/BASE discussion in comment 8, the same description from comment 8 still holds for this simplified patch.

This patch has been tested in poudriere with GSSAPI on and off.  stage-qa and check-plist both pass as well.  I also tested all other options on and a few other option combinations.
Comment 10 Gerald Pfeifer freebsd_committer 2018-05-31 12:27:35 UTC
(In reply to John Hein from comment #8)
> The next two are unrelated to kerberos/gssapi...
>
> (3) Remove CFLAGS & LDFLAGS in favor of USES=localbase

Thank you, John, and sorry it's taken a bit which is partly due to
this being a combination of various things, the original bug being
resolved (and the report closed) and me traveling around that time.

I'll go ahead and apply a change wrt. USES=localbase in a minute,
though note I'll go for the simpler approach (fewer lines in the
Makefile, more stable wrt. future changes, and not really a drawback).
Comment 11 commit-hook freebsd_committer 2018-05-31 12:28:07 UTC
A commit references this bug:

Author: gerald
Date: Thu May 31 12:27:52 UTC 2018
New revision: 471203
URL: https://svnweb.freebsd.org/changeset/ports/471203

Log:
  Replace explicit settings of CFLAGS and LDFLAGS in favor of USES=localbase
  (the former predating the latter by at least a decade alas with variation).

  PR:		226396
  Submitted by:	John Hein <z7dr6ut7gs@snkmail.com>

Changes:
  head/emulators/wine-devel/Makefile
Comment 12 commit-hook freebsd_committer 2018-06-16 12:42:44 UTC
A commit references this bug:

Author: gerald
Date: Sat Jun 16 12:42:06 UTC 2018
New revision: 472543
URL: https://svnweb.freebsd.org/changeset/ports/472543

Log:
  Port revision 471203 from the wine-devel port to the main wine port:

  Replace explicit settings of CFLAGS and LDFLAGS in favor of USES=localbase
  (the former predating the latter by at least a decade alas with variation).

  PR:		226396
  Submitted by:	John Hein <z7dr6ut7gs@snkmail.com>

Changes:
  head/emulators/wine/Makefile
Comment 13 commit-hook freebsd_committer 2018-09-01 13:19:05 UTC
A commit references this bug:

Author: gerald
Date: Sat Sep  1 13:18:22 UTC 2018
New revision: 478690
URL: https://svnweb.freebsd.org/changeset/ports/478690

Log:
  Backport r464959 | gerald | 2018-03-18 22:24:42 from emulators/wine-devel:

  Fully disable building Kerberos components.

  We already had --without-krb5, alas when someone had krb5 installed
  in their build environment, the build still failed.  This takes care
  using a bigger hammer.

  PR:		226396
  Analyzed by:	John Hein <z7dr6ut7gs@snkmail.com>

Changes:
  head/emulators/wine/Makefile
  head/emulators/wine/pkg-plist