Bug 227799 - editors/emacs editors/emacs-devel: Add 'ac_cv_lib_lockfile_maillock=no' to CONFIGURE_ENV
Summary: editors/emacs editors/emacs-devel: Add 'ac_cv_lib_lockfile_maillock=no' to CO...
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: Joseph Mingrone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-27 00:24 UTC by Yasuhiro Kimura
Modified: 2018-06-12 16:08 UTC (History)
2 users (show)

See Also:
yasu: maintainer-feedback? (emacs)


Attachments
patch file (1.92 KB, patch)
2018-04-27 00:24 UTC, Yasuhiro Kimura
no flags Details | Diff
updated patch file (1.21 KB, patch)
2018-04-29 19:18 UTC, Yasuhiro Kimura
no flags Details | Diff
updated patch file (1.74 KB, patch)
2018-06-02 17:45 UTC, Yasuhiro Kimura
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yasuhiro Kimura 2018-04-27 00:24:23 UTC
Created attachment 192833 [details]
patch file

At bug #225902 I reported build of editor/emacs and editors/emacs fails if devel/liblockfile is installed and 'nox' flavor is specified. I also proposed patch to disable configure script from detecting liblockfile and it was committed as ports r462408.

But yesterday ports r468320 was committed. I updated emacs-nox-25.3_3,3 on my 11.1-RELEASE-p9 box with portmaster and was surprised following message was displayed at stage-qa phase.

Error: /usr/local/libexec/emacs/25.3/amd64-portbld-freebsd11.1/movemail is linked to /usr/local/lib/liblockfile.so.1 from devel/liblockfile but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=liblockfile.so:devel/liblockfile

And actually movemail was liked to liblockfile.

yasu@maybe[2085]% ldd /usr/local/libexec/emacs/25.3/amd64-portbld-freebsd11.1/movemail
/usr/local/libexec/emacs/25.3/amd64-portbld-freebsd11.1/movemail:
        libutil.so.9 => /lib/libutil.so.9 (0x800829000)
        liblockfile.so.1 => /usr/local/lib/liblockfile.so.1 (0x800a3d000)
        libc.so.7 => /lib/libc.so.7 (0x800c40000)
yasu@maybe[2086]%

I means problem reported bug #225902 is disappeared. I have no idea why but anyway I propose attached patch. It includes following changes.

* Add devel/liblockfile to LIB_DEPENDS.
* Remove 'ac_cv_prog_liblockfile=no' from CONFIGURE_ENV.
* Bump PORTREVISION because of dependency change.

I tested all flavors (emacs@full, emacs@nox, emacs@canna, emacs-devel@full and emacs-devel@nox) with poudriere and 11.1-RELEASE-p9 jail and confirmed all of them can be build successfully.
Comment 1 Joseph Mingrone freebsd_committer 2018-04-27 16:49:14 UTC
- This does not cause a build error in poudriere, but if LIB_DEPENDS=liblockfile.so:devel/liblockfile is temporarily added, movemail does indeed link to liblockfile.so.1, regardless what the MAILUTILS option is set to.

- Also adding ac_cv_lib_lockfile_maillock=no to CONFIGURE_ENV prevents movemail from linking to liblockfile.so.1.

The important questions: Is liblockfile required for us?

configure.ac contains these two blocks, so I would guess we do not, but am far from clear.

dnl Debian, at least:
AC_CHECK_LIB(lockfile, maillock, have_lockfile=yes, have_lockfile=no)
if test $have_lockfile = yes; then
   LIBS_MAIL=-llockfile
   AC_DEFINE(HAVE_LIBLOCKFILE, 1, [Define to 1 if you have the 'lockfile' library (-llockfile).])
else
# If we have the shared liblockfile, assume we must use it for mail
# locking (e.g. Debian).  If we couldn't link against liblockfile
# (no liblockfile.a installed), ensure that we don't need to.
  dnl This works for files generally, not just executables.
  dnl Should we look elsewhere for it?  Maybe examine /etc/ld.so.conf?
  AC_CHECK_PROG(liblockfile, liblockfile.so, yes, no,
                /usr/lib:/lib:/usr/local/lib:$LD_LIBRARY_PATH)
  if test $ac_cv_prog_liblockfile = yes; then
    AC_MSG_ERROR([Shared liblockfile found but can't link against it.
This probably means that movemail could lose mail.
There may be a 'development' package to install containing liblockfile.])
  fi
fi
AC_CHECK_HEADERS_ONCE(maillock.h)
AC_SUBST(LIBS_MAIL)

------------

mail_lock=no
case "$opsys" in
  aix4-2) mail_lock="lockf" ;;

  gnu|freebsd|dragonfly|netbsd|openbsd|darwin|irix6-5) mail_lock="flock" ;;

  ## On GNU/Linux systems, both methods are used by various mail programs.
  ## I assume most people are using newer mailers that have heard of flock.
  ## Change this if you need to.
  ## Debian contains a patch which says: "On Debian/GNU/Linux systems,
  ## configure gets the right answers, and that means *NOT* using flock.
  ## Using flock is guaranteed to be the wrong thing. See Debian Policy
  ## for details." and then uses '#ifdef DEBIAN'.  Unfortunately the
  ## Debian maintainer hasn't provided a clean fix for Emacs.
  ## movemail.c will use 'maillock' when MAILDIR, HAVE_LIBMAIL and
  ## HAVE_MAILLOCK_H are defined, so the following appears to be the
  ## correct logic.  -- fx
  ## We must check for HAVE_LIBLOCKFILE too, as movemail does.
  ## liblockfile is a Free Software replacement for libmail, used on
  ## Debian systems and elsewhere. -rfr.
  gnu-*)
    mail_lock="flock"
    if test $have_mail = yes || test $have_lockfile = yes; then
      test $ac_cv_header_maillock_h = yes && mail_lock=no
    fi
    ;;
Comment 2 Yasuhiro Kimura 2018-04-29 19:18:20 UTC
Created attachment 192918 [details]
updated patch file

(In reply to Joseph Mingrone from comment #1)

I couldn't find 'ac_cv_lib_lockfile_maillock=no'.
Then I update patch so it only adds 'ac_cv_lib_lockfile_maillock=no' to CONFIGURE_ENV.
So please commit attached patch instead of original one.
Comment 3 Joseph Mingrone freebsd_committer 2018-05-14 18:31:53 UTC
Emacs 26.1 should be released any day now, so we will ensure this is correct in 26.1.

Thank you.
Comment 4 Joseph Mingrone freebsd_committer 2018-05-25 19:27:58 UTC
Testing indicates that this is no longer an issue with 26.1, at least with rc1.  If you would like to test and confirm this, there is a Phabricator Differential.

https://reviews.freebsd.org/D15044
Comment 5 Yasuhiro Kimura 2018-06-02 17:45:44 UTC
Created attachment 193944 [details]
updated patch file

I investigated this issue with ports r471374.
It still happens, but only when MAILUTILS option is off.
And it applies to all emacs flavors (full, canna, nox, devel_full and devel_nox).
They are confirmed with poudriere and 11.1-RELEASE amd64 jail.
I updated patch.
So please commit it instead of previous ones.

BTW, MAILUTILS options is on by default with editors/emacs.
But it is off with editors/emacs-devel.
Should it be consistent between them?
Comment 6 Joseph Mingrone freebsd_committer 2018-06-04 15:15:11 UTC
Thanks.  I will take a closer look within a few days.  I agree that we should try to minimize the diff between editors/emacs and editors/emacs-devel.  I have started some work on this and will submit it for review in a few days as well.
Comment 7 Joseph Mingrone freebsd_committer 2018-06-10 01:59:16 UTC
A fix is coming.  https://reviews.freebsd.org/D15728
Comment 8 commit-hook freebsd_committer 2018-06-12 16:04:01 UTC
A commit references this bug:

Author: jrm
Date: Tue Jun 12 16:03:57 UTC 2018
New revision: 472261
URL: https://svnweb.freebsd.org/changeset/ports/472261

Log:
  Emacs ports: Improve consistency between the two Emacs ports and...

  - [1] Do not link liblockfile when MAILUTILS option is off.  Users who want
    mail functionality should turn on the MAILUTILS option.  See PR 227799.

  - [2] Create a link under exec_directory pointing to
    ${PREFIX}/bin/movemail.  See upstream bug https://bugs.gnu.org/31737 and
    PR 228833.

  - Based on user feedback, change the package name for the nox flavor of
    editors/emacs-devel from emacs-devel_nox to emacs-devel-nox.  The
    original motivation for naming the package emacs-devel_nox was so that
    PKGNAMESUFFIX would match the flavor names for USES=emacs ports and
    flavor names cannot contain '-'.

  - Remove patches that are no longer necessary.  The patch
    emacs-devel/files/patch-configure.ac is no longer necessary because 10.3,
    which included an old version of texinfo in base, is EOL.  The lldb-gud
    patch is no longer necessary because the LLDB option has been removed.

  - Update editors/emacs-devel to a newer commit on the upstream master
    branch.

  - Customize COMMENT for nox flavor of editors/emacs-devel.

  PR:		227799 [1], 228833 [2]
  Submitted by:	yasu@utahime.org [1], bengta@sics.se [2]
  Approved by:	ashish
  Differential Revision:	https://reviews.freebsd.org/D15728

Changes:
  head/UPDATING
  head/editors/emacs/Makefile
  head/editors/emacs/pkg-plist
  head/editors/emacs-devel/Makefile
  head/editors/emacs-devel/distinfo
  head/editors/emacs-devel/files/extrapatch-lldb-gud.el
  head/editors/emacs-devel/files/patch-configure.ac
  head/editors/emacs-devel/pkg-plist
Comment 9 Joseph Mingrone freebsd_committer 2018-06-12 16:08:34 UTC
Committed. Thanks.  Please re-open if it's not working as it should.