Bug 271954 - [NEW PORT] x11-wm/emwm Enhanced Motif Window Manager
Summary: [NEW PORT] x11-wm/emwm Enhanced Motif Window Manager
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Robert Clausecker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-11 17:11 UTC by Stephan Lichtenauer
Modified: 2023-07-14 14:25 UTC (History)
2 users (show)

See Also:


Attachments
emwm port (2.82 KB, patch)
2023-06-11 17:11 UTC, Stephan Lichtenauer
stephan: maintainer-approval+
Details | Diff
Updated patch including fixes for all comments. (3.10 KB, patch)
2023-06-30 16:12 UTC, Stephan Lichtenauer
stephan: maintainer-approval+
Details | Diff
New emwm port (4.05 KB, patch)
2023-07-11 12:47 UTC, Stephan Lichtenauer
stephan: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Lichtenauer 2023-06-11 17:11:45 UTC
Created attachment 242737 [details]
emwm port

New port for the Enhanced Motif Window Manager (EMWM) at https://fastestcode.org/emwm.html

This is my first port, so happy about any feedback.

Separate port will be submitted for the emwm-utils.
Comment 1 Stephan Lichtenauer 2023-06-14 15:29:00 UTC
Set maintainer-approval on attachment to + as I understand from https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=262318 that this is the right thing to do if I submit a new port and I am the maintainer?
Comment 2 Robert Clausecker freebsd_committer freebsd_triage 2023-06-25 17:04:49 UTC
Thank you for your submission.  Setting maintainer-approval is correct.

Here are some comments:

 - instead of EXTRACT_SUFFIX, add USES=tar:xz
 - please hard-break pkg-descr to 72 or 80 columns.
 - please add a blank line before the LICENSE block.  You can use portfmt
   to auto-format the port.
 - do not hard code /usr/bin/install.  Instead, use the INSTALL_... macros described in § 5.17.1 Porter's Handbook.
 - please set the author field in your patch to your true email address and
   name.  Also check if you can set name and surname in bugzilla to make it
   easier for us to enter correct metadata into version control.
 - make sure to hook the new port into x11-wm/Makefile.

Please check and resubmit.
Will then proceed with a build test.
Comment 3 Stephan Lichtenauer 2023-06-30 16:12:42 UTC
Created attachment 243082 [details]
Updated patch including fixes for all comments.

Thank you very much for your feedback, please find updated patch attached.

> instead of EXTRACT_SUFFIX, add USES=tar:xz
Done.

> please hard-break pkg-descr to 72 or 80 columns.
Done.

> please add a blank line before the LICENSE block.  You can use portfmt
  to auto-format the port.
Reformatted with portfmt.

> do not hard code /usr/bin/install.  Instead, use the INSTALL_... macros
  described in § 5.17.1 Porter's Handbook.
Changed and also fixed pkg-plist to include the directories.

> please set the author field in your patch to your true email address and
  name.  Also check if you can set name and surname in bugzilla to make it
  easier for us to enter correct metadata into version control.
Done.

> make sure to hook the new port into x11-wm/Makefile.
Done.
Comment 4 Robert Clausecker freebsd_committer freebsd_triage 2023-06-30 16:27:55 UTC
Thank you for the update.  This is a lot better.  Portlint still shows a few warnings though, perhaps you can check them?  It seems like it is confused because you swapped the MAINTAINER and LICENSE blocks.

A test build reveals that you use the PORTDOCS variable despite no DOCS variable being defined.  Either put the README file into pkg-plist or add a DOCS option (probably overkill).

A build test succeeds but there is this warning:

WmError.c:121:24: warning: incompatible function pointer types passing 'void (char *)' to parameter of type 'void (*)(S
tring) __attribute__((noreturn))' (aka 'void (*)(char *) __attribute__((noreturn))') [-Wincompatible-function-pointer-t
ypes]                                                                                                                  
    XtSetErrorHandler (WmXtErrorHandler);                  
                       ^~~~~~~~~~~~~~~~                                                                                
/usr/local/include/X11/Intrinsic.h:1776:1: note: passing argument to parameter here                                    
);                                                                                                                     
^                                                                                                                      
1 warning generated.

This warning is known to have become an error in LLVM16, which will be the default on FreeBSD 14.  Check if you can add a patch for it.  To test with LLVM16, install devel/llvm16 and build with

    make CC=clang16 CXX=clang++16

I noticed that the MAINTAINER email address in the port is different from the one you use on bugzilla.  This may lead to problems as people will not immediately see that you are the maintainer.  Also, we have hooks to auto-notify the maintainer when a bug is files against one of his/her ports.  Are you okay with me changing the MAINTAINER address in the patch to the same you use on bugzilla?
Comment 5 Stephan Lichtenauer 2023-07-11 12:47:27 UTC
Created attachment 243339 [details]
New emwm port

Thanks a lot for your very quick feedback, Robert.

> Thank you for the update.  This is a lot better.  Portlint still shows a few warnings though, perhaps you can check them?  It seems like it is confused because you swapped the MAINTAINER and LICENSE blocks.

Ran it through portlint, should be fine now.

> A test build reveals that you use the PORTDOCS variable despite no DOCS variable being defined.  Either put the README file into pkg-plist or add a DOCS option (probably overkill).

I removed the README (PORTDOCS) as it does not contain any useful information beyond build dependencies anyway.

> A build test succeeds but there is this warning:

> WmError.c:121:24: warning: incompatible function pointer types passing 'void (char *)' to parameter of type 'void (*)(String) __attribute__((noreturn))' (aka 'void (*)(char *) __attribute__((noreturn))') [-Wincompatible-function-pointer-types]                                                                                                                  
> [...]
> This warning is known to have become an error in LLVM16, which will be the default on FreeBSD 14.  

Added a patch for this error.

> I noticed that the MAINTAINER email address in the port is different from the one you use on bugzilla.  This may lead to problems as people will not immediately see that you are the maintainer.  Also, we have hooks to auto-notify the maintainer when a bug is files against one of his/her ports.  Are you okay with me changing the MAINTAINER address in the patch to the same you use on bugzilla?

Fixed this as well.
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-07-11 22:09:57 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=574f9547cf9a2f7105e05d706a5a149bd8fdc492

commit 574f9547cf9a2f7105e05d706a5a149bd8fdc492
Author:     Stephan Lichtenauer <stephan@lichtenauer.co.za>
AuthorDate: 2023-07-11 12:40:35 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2023-07-11 22:07:31 +0000

    x11-wm/emwm: Enhanced Motif Window Manager

    EMWM is a fork of the Motif Window Manager with fixes and enhancements. It
    provides compatibility with current xorg extensions and applications, without
    changing the way the window manager looks and behaves. This includes support for
    multi-monitor setups trough Xinerama/Xrandr, UFT-8 support with Xft fonts, and
    overall better compatibility with software that requires Extended Window Manager
    Hints.

    WWW: https://fastestcode.org/emwm.html

    PR:             271954

 x11-wm/Makefile                             |  1 +
 x11-wm/emwm/Makefile (new)                  | 26 ++++++++++++++++++++++++++
 x11-wm/emwm/distinfo (new)                  |  3 +++
 x11-wm/emwm/files/patch-src_WmError.h (new) | 11 +++++++++++
 x11-wm/emwm/pkg-descr (new)                 |  6 ++++++
 x11-wm/emwm/pkg-plist (new)                 |  7 +++++++
 6 files changed, 54 insertions(+)
Comment 7 Stephan Lichtenauer 2023-07-14 14:25:57 UTC
Thank you very much for your help, Robert.