Differential D21556
x11/xlockmore: various fixes Authored by jrm on Sep 6 2019, 8:05 PM. Tags None Referenced Files
Subscribers None
Details
*Note*, even when supplying --disable-setuid to configure, xlock and portlint and poudriere testport are happy
Diff Detail
Event TimelineComment Actions I can take a look at this more later this week/weekend, but I did look at the previous revision, and MB was not enabled. I thought you had mentioned not making XINERAMA default. Do you still feel this way? I also sent a message to upstream regarding setuid note. > make -V CONFIGURE_ARGS
--without-motif --without-ttf --without-editres --without-dtsaver --without-rplay --without-nas --without-gtk --without-esound --disable-setuid --without-mesa --without-opengl --without-ftgl --without-gtk2 --disable-bomb --without-magick --with-xinerama --x-libraries=/usr/local/lib --x-includes=/usr/local/include --prefix=/usr/local ${_LATE_CONFIGURE_ARGS}
> make -V PKGNAME
xlockmore-5.58Comment Actions I made those comments before I was completely clear on what was going on. MB was never really disable before the switch to option helpers. The fonts look like terminal fonts without it, so I think most users would prefer it on. That's why my fonts changed after this latest update even though I had never explicitly turned on the MB knob. It's a similar situation with the Xinerama knob. It wasn't being disabled before and that's why I thought that the login screen was suddenly not being displayed on the primary display after the latest update. Later I realized what was really going on. The login screen was expanded over all displays without Xinerama on. Comment Actions Re-add GTK2 and KERBEROS5 options based on https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240526 Comment Actions
Comment Actions I will look this over, but I urge you to please not shift changes that aren't required for this change. It makes it difficult to review changes that are necessary with those that aren't for this particular fix. Why support options that aren't working? (ie. BUILD FAILS in description) Humbly, Comment Actions I can imagine users who prefer one kerberos implementation would be frustrated to have to install a different implementation just for x11/xlockmore. I thought adding the other kerboros options, even though the build currently fails with them, would indicate that we are not intentionally forcing them to use one specific kerberos implementation. I don't have a strong opinion either way, so I returned it back to they way you prefer, one option for kerberos5 support. Does this look okay? The other changes are objective in that they either fix something that was clearly broken or follow the Porter's Handbook.
There are subjective changes that I have left out, like maybe changing the DISABLE_ALLOW_ROOT option to just an ALLOW_ROOT OPTION and sorting options and plist. poudriere testport now passes with all options off and on and the default options for i386 and amd64. Comment Actions I can look at this more later this week, but I wasn't suggesting to not do them, but I was suggesting to do them after fixing the bugs. Cleaning up the Makefile is other commit, imho, that I would rather commit once bugs are fixed. I would prefer minimal change and not introduce any other variables at this time. I really appreciate your time on this! Comment Actions For some reason I am unable to update the diff and have it not create a brand new revision in the tool. Attached is the diff that I re-sorted so I could properly review the changes more effectively for me. Index: x11/xlockmore/Makefile =================================================================== --- x11/xlockmore/Makefile (revision 512085) +++ x11/xlockmore/Makefile (working copy) @@ -3,8 +3,8 @@ PORTNAME= xlockmore PORTVERSION= 5.58 -PORTREVISION= 1 -CATEGORIES?= x11 +PORTREVISION= 2 +CATEGORIES= x11 MASTER_SITES= http://www.sillycycle.com/xlock/ \ SF/bsdsrc/${.CURDIR:T} @@ -19,17 +19,18 @@ PORTDOCS= README ${PORTNAME}.README CONFIGURE_ARGS= --without-motif --without-ttf --without-editres \ --without-dtsaver --without-rplay --without-nas \ - --without-gtk --without-esound --disable-setuid + --without-gtk --without-esound --disable-setuid \ + --disable-kerberos4 -USE_XORG= x11 xaw ice sm xpm xext xinerama +USE_XORG= ice sm x11 xaw xext xpm -LIB_DEPENDS+= libfreetype.so:print/freetype2 \ - libpng.so:graphics/png +LIB_DEPENDS= libfreetype.so:print/freetype2 OPTIONS_DEFINE= MESAGL MB SYSLOG DISABLE_ALLOW_ROOT NICE_ONLY BLANK_ONLY \ KERBEROS5 BAD_PAM GTK2 TIME_BOMB CUSTOMIZATION MAGICK JP \ XINERAMA DOCS -OPTIONS_DEFAULT= XINERAMA +OPTIONS_DEFAULT= MB XINERAMA + OPTIONS_SUB= yes MESAGL_DESC= Mesa 3D (for GL modes) @@ -48,40 +49,46 @@ OPTIONS_RADIO= ONE OPTIONS_RADIO_ONE= PAM XLOCK_GROUP -NO_OPTIONS_SORT=yes +NO_OPTIONS_SORT= yes -JP_IMPLIES= MB - MAKE_ARGS+= bindir=${PREFIX}/bin \ mandir=${PREFIX}/man/man1 \ datarootdir=${PREFIX}/share \ xapploaddir=${PREFIX}/lib/X11/app-defaults -MESAGL_CONFIGURE_WITH= mesa -MESAGL_VARS= CFLAGS+="-I${LOCALBASE}/include/FTGL -I${LOCALBASE}/include/freetype2" USE_GL=yes +MESAGL_CONFIGURE_WITH= mesa opengl ftgl MESAGL_LIB_DEPENDS= libftgl.so:graphics/ftgl -MESAGL_CONFIGURE_ENV= XLOCKLIBS="-lpthread" -MESAGL_CONFIGURE_OFF= --without-opengl --without-ftgl +MESAGL_CONFIGURE_ENV= XLOCKLIBS+="-lpthread" +MESAGL_CFLAGS= -I${LOCALBASE}/include/FTGL +MESAGL_USES= gl +MESAGL_USE= gl=gl,glu +KERBEROS5_CFLAGS= ${GSSAPICPPFLAGS} KERBEROS5_CONFIGURE_ENABLE= kerberos5 -KERBEROS5_VARS= CFLAGS+=-I${KRB5_HOME}/include KRB5LIB=-L${KRB5_HOME}/lib -KERBEROS5_CONFIGURE_ENV= XLOCKLIBS="${KRB5LIB}" +KERBEROS5_CONFIGURE_ENV= XLOCKLIBS="${GSSAPILDFLAGS} ${GSSAPILIBS} -lcom_err" +KERBEROS5_USES= gssapi:mit BAD_PAM_CONFIGURE_ENABLE= bad-pam PAM_CONFIGURE_ENABLE= pam NICE_ONLY_CONFIGURE_ENABLE= nice-only BLANK_ONLY_CONFIGURE_ENABLE= blank-only -MB_CONFIGURE_ON= --enable-use-mb -MB_CONFIGURE_OFF= --disable-mb +MB_CONFIGURE_ENABLE= mb SYSLOG_CONFIGURE_ENABLE= syslog XINERAMA_CONFIGURE_WITH= xinerama +XINERAMA_USE= xorg=xinerama DISABLE_ALLOW_ROOT_CONFIGURE_ON= --disable-allow-root XLOCK_GROUP_CONFIGURE_ENABLE= xlock-group TIME_BOMB_CONFIGURE_ENABLE= bomb CUSTOMIZATION_CONFIGURE_ENABLE= customization -JP_VARS= XLOCKMORE_LANG=ja CATEGORIES+=japanese -MAGICK_CONFIGURE_OFF= --without-magick -MAGICK_LIB_DEPENDS= libMagickCore-6.so:graphics/ImageMagick6 +JP_IMPLIES= MB +JP_CATEGORIES= japanese +JP_VARS= XLOCKMORE_LANG=ja +MAGICK_CONFIGURE_WITH= magick +MAGICK_LIB_DEPENDS= libMagickCore-6.so:graphics/ImageMagick6 \ + libpng16.so:graphics/png GTK2_CONFIGURE_WITH= gtk2 -GTK2_VARS+= USE_GNOME+=gtk20 +GTK2_LIB_DEPENDS= libfontconfig.so:x11-fonts/fontconfig \ + libpng16.so:graphics/png +GTK2_USE= gnome=cairo,gdkpixbuf2,gtk20 +GTK2_USES= gnome pkgconfig .include <bsd.port.options.mk> @@ -89,10 +96,6 @@ CONFIGURE_ARGS+= --with-lang=${XLOCKMORE_LANG} .endif -.if ${PORT_OPTIONS:MKERBEROS5} && empty(KRB5_HOME) -BROKEN= KRB5_HOME is undefined! -.endif - .if ${PORT_OPTIONS:MPAM} && empty(PORT_OPTIONS:MBAD_PAM) pre-extract: @${ECHO} Index: x11/xlockmore/files/patch-xglock_xglock.c =================================================================== --- x11/xlockmore/files/patch-xglock_xglock.c (revision 512085) +++ x11/xlockmore/files/patch-xglock_xglock.c (nonexistent) @@ -1,30 +0,0 @@ ---- xglock/xglock.c.orig 2017-01-06 05:18:57 UTC -+++ xglock/xglock.c -@@ -1,3 +1,27 @@ -+Assertion failed: (SE.DT.dominates(Ret, &*BIP)), function ReuseOrCreateCast, file /usr/src/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp, line 83. -+cc: error: unable to execute command: Abort trap (core dumped) -+cc: error: clang frontend command failed due to signal (use -v to see invocation) -+ -+--- xglock/xglock.c.orig 2018-07-08 11:29:53 UTC -++++ xglock/xglock.c -+@@ -1489,7 +1489,7 @@ static void -+ create_boolOptions_buttons(GtkScrolledWindow *parent) -+ { -+ gint i; -+- gpointer iptr = 0; -++ guint iptr = 0; -+ GtkWidget *box0; -+ GtkWidget *box1; -+ GtkWidget *box2; -+@@ -1514,7 +1514,7 @@ create_boolOptions_buttons(GtkScrolledWindow *parent) -+ gtk_box_pack_start(GTK_BOX(i%2 ? box1 : box2), button, TRUE, TRUE, 0); -+ gtk_signal_connect(GTK_OBJECT(button), "clicked", -+ (GtkSignalFunc) bool_option_clicked_cb, -+- iptr++); -++ GUINT_TO_POINTER(iptr++)); -+ gtk_widget_show(button); -+ } -+ } - /*- - * xglock.c - main file for xglock, the GTK+ gui interface to xlock. - * I did find one issue, and not sure if you saw this, but when running with the default configuration I received this when building locally: ====> Running Q/A tests (stage-qa) Error: /usr/local/bin/xlock is linked to /usr/local/lib/libpng16.so.16 from graphics/png but it is not declared as a dependency Warning: you need LIB_DEPENDS+=libpng16.so:graphics/png I know these bits were touched, and wanted to give you this feedback. I really appreciate you putting a lot of work into this and fixing a number of style issues, as well, however mixing in style, formatting and bug fixes in one commit isn't how I prefer to approach this update. I would like to fix the bugs, and then it would be much easier for me to sort other issues out that aren't user impacting. Thanks, again! Comment Actions I didn't get any errors with poudriere testport building in 11/12 i386/amd64 with the default options. Here are the logs: All the outputs of ldd /usr/local/bin/xlock are empty, so xlock is not linking to libpng with the default options. root@11i386-default:~ # ldd /usr/local/bin/xlock | grep png root@11amd64-default:~ # ldd /usr/local/bin/xlock | grep png root@12i386-default:~ # ldd /usr/local/bin/xlock | grep png root@12amd64-default:~ # ldd /usr/local/bin/xlock | grep png Comment Actions I will test this poudriere, however I built this locally and have different results. This is on the newer version, that was just released yesterday, however I was getting these results on the previous software version, as well. > sudo ldd /usr/local/bin/xlock |grep -i png libpng16.so.16 => /usr/local/lib/libpng16.so.16 (0x8003be000) Thanks for the feedback! Comment Actions After building, I am getting the same error. Any ideas? In taking a brief look at log, I then verified, that it is coming from dependency on print/freetype2. This port has a dependency on graphics/png (PNG is a default option in print/freetype2). http://home.lerctr.org:8888/data/p112amd64-jgh-ports/2019-10-04_13h30m20s/logs/xlockmore-5.59.log Comment Actions Those are not identical builds to the patch that I submitted, are they? I see the version is 5.59, so I can't really say. Comment Actions I had it hard-coded before as a LIB_DEPENDS on print/freetype2. print/freetype2 has a hard default dependency on png. I believe this is why this is happening. This is not really dependent on the patch, or the software, I feel. It has always depended on it. I can rebuild with the older software, if that helps figure out the issue, however I believe with the dependency on print/freetype2, I am not sure how it would yield different results. Comment Actions It's also not the version change. Here are the build logs (default options) with 5.59: http://pkg.awarnach.mathstat.dal.ca/data/11i386-default/2019-10-04_15h51m57s/logs/xlockmore-5.59.log Comment Actions print/freetype2 does not have a hard dependency on graphics/png unless you turn on the non-default PNG knob. Comment Actions Confirmed. My bad. Rebuilding with your changes, as the patch is more clear to me with the changes being made. Comment Actions I'm using a nearly identical patch to the one you submitted... and same issue. http://home.lerctr.org:8888/data/p120-S-amd64-jgh-ports/2019-10-04_14h01m28s/logs/xlockmore-5.59.log The only bit I removed was the post-patch target, as that was moved into this release. -jgh Comment Actions I see what is happening! I am using a shared build system. The option of PNG is being added to the options for building of print/freetype. Thanks for all of your help. I'll send the patch through, as is, and bump to 5.59. Comment Actions It looks like your print/freetype2 is configured with the PNG knob on, so PNG is being pulled in and x11/xlockmore is linking to PNG when it is discovered. Fortunately xlockmore has configure options for both freetype2 and png (see below), so we could make this deterministic. I suspect many users including myself just use xlockmore to lock their screens (I just use the blank screensaver and immediately turn off the monitor), so things like freetype2 and png might be more appropriate as OFF by default. We could also turn those options on when something like MESAGL and/or MAGICK are turned. Or, we could create an BELLS_AND_WHISTLES_FANCY_SCREESAVERS knob (obvious not that name) and pull all these things in together. What do you think? Optional Features:
--disable-option-checking ignore unrecognized --enable/--with options
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
--disable-def-play=program
set def-play to program that plays sounds
--disable-gtktest do not try to compile and run a test GTK program
--enable-bitmapdir=DIR set directory for bitmaps, default is ./bitmaps
--enable-bitmaptype=name
set name for bitmap type
--enable-pixmapdir=DIR set directory for pixmaps, default is ./pixmaps
--enable-pixmaptype=name
set name for pixmap type
--enable-mapdir=DIR set directory for bitmaps and pixmaps
--enable-maptype=name set name for bitmap and pixmap type
--disable-vroot disables xlock from being able to run in root window
(some window managers have problems)
--disable-allow-root allows users to turn off allowroot (default is to
always allow root to be able to unlock xlock)
--enable-vtlock allows to turn on VT switch lock (default is to be
able to switch to another VT)
--enable-syslog enable syslog logging
--enable-multiple-user enable multiple users
--enable-multiple-root enable multiple root users
--enable-xlock-group enable all in xlock group to unlock
--enable-window-visibility
allow a window to be displayed over xlock
--enable-old-event-loop some machines may still need this
(fd_set errors may be a sign)
--enable-xlockrc for unknown shadow passwd algorithm
--enable-auto-logout=time
enable auto-logout code for lab use (time in minutes)
--enable-default-auto-logout=time
set default-auto-logout (time in minutes)
--enable-button-logout=time
enable button logout for lab use (time in minutes)
--enable-default-button-logout=time
set default-button-logout (time in minutes)
--disable-bomb disable automatic logout mode
--disable-interactive disables interactive modes (useful for prod env)
--enable-unstable enables unstable (alpha) modes
--disable-negative-logout
allows users to deny logout button and autologout
--enable-closedown-logout
define this one or next, with enable-auto-logout
--enable-session-logout enable-button-logout, and/or enable-bomb, for xdm
--enable-staff-file=filename
set file of staff who are exempt from auto-logout
--enable-staff-netgroup=netgroup
set netgroup of staff who are exempt from auto-logout
--enable-kerberos4 enable Kerberos 4
--enable-kerberos5 enable Kerberos 5
--enable-write-krb5 write new krb5 credentials to credential cache
--enable-dce-passwd enable DCE passwording
--enable-pam enable PAM
--enable-bad-pam enable BAD_PAM
--enable-good-pam enable GOOD_PAM
--enable-afs enable AFS
--enable-sia enable SIA
--enable-sunos-adjunct-passwd
enable SUNOS Adjunct passwording
--disable-mb disable Xmb function series so will only display ASCII
--enable-customization enable customization of XResource
--enable-modules enable modules (better to use xmkmf for this)
--enable-blank-only enable blank mode only (boring)
--enable-nice-only enable only low cpu modes
--enable-check enable check (experimental)
--enable-orig-xpm-patch use the original pixmap patch for mail icon
(have colormap problems in 8 bits depth visuals)
--enable-appdefaultdir=DIR
set directory installation of appdefaults,
default is where X is installed
--disable-setuid disables setuid installation if automatically chosen
Optional Packages:
--with-PACKAGE[=ARG] use PACKAGE [ARG=yes]
--without-PACKAGE do not use PACKAGE (same as --with-PACKAGE=no)
--without-pthread do not use POSIX threads
--with-purify configure to postprocess with purify
--with-x use the X Window System
--with-includes=DIR search include DIR for optional packages below
--with-libraries=DIR search library DIR for optional packages below
--without-motif disable Motif (for xmlock)
--without-editres disable debugger (for x?lock)
--without-xpm disable color pixmap XPM mode(s)
--without-libpng disable color pixmap LIBPNG mode(s)
--without-magick disable ImageMagick
--with-magick-config use this configator for ImageMagick
--with-magick-prefix use this prefix for ImageMagick
--with-magick-includes use this dir for ImageMagick headers
--with-magick-libraries use this dir for ImageMagick libs
--without-gltt disable GL True Text library
--without-ttf disable True Text Font mode(s)
--without-ftgl disable GL font rendering library
--without-freetype disable Freetype2 mode(s)
--without-opengl disable 3D OpenGL (for GL modes)
--without-mesa disable Mesa 3D (for GL modes)
--without-dtsaver disable -dtsaver option
--without-ext disable EXT
--without-dpms disable DPMS
--without-xinerama disable Xinerama support
--without-rplay disable RPLAY sounds
--with-esound enable Enlightenment Sound Daemon sounds
--with-esound-config use this configator for Enlightenment Sound Daemon
--with-esound-prefix use this prefix for Enlightenment Sound Daemon
--with-esound-includes use this dir for Enlightenment Sound Daemon headers
--with-esound-libraries use this dir for Enlightenment Sound Daemon libs
--without-nas disable NAS sounds
--without-crypt disable CRYPT
--without-gtk2 disable GTK2 (no xglock build)
--without-gtk disable GTK (no xglock build)
--with-gtk-prefix=DIR Prefix where GTK is installed (optional)
--with-gtk-exec-prefix=DIR
Exec prefix where GTK is installed (optional)
--with-krb5-config=PATH Location of krb5-config script
--with-passwd-helper=/path Use external helper program for authentication
--with-lang=lang use a foreign language (nl/fr/de/ja)
Some influential environment variables:
CC C compiler command
CFLAGS C compiler flags
LDFLAGS linker flags, e.g. -L<lib dir> if you have libraries in a
nonstandard directory <lib dir>
LIBS libraries to pass to the linker, e.g. -l<library>
CPPFLAGS (Objective) C/C++ preprocessor flags, e.g. -I<include dir> if
you have headers in a nonstandard directory <include dir>
CPP C preprocessor
CXX C++ compiler command
CXXFLAGS C++ compiler flags
XMKMF Path to xmkmf, Makefile generator for X Window SystemComment Actions I am fine with the way it is now, however we can improve. Thanks very much for your time on this! |