Bug 230894 - [patch] x11-wm/compiz: add modesetting; use xfwm4; remove reference to former option GNOME, mark option METACITY as broken; update Makefile
Summary: [patch] x11-wm/compiz: add modesetting; use xfwm4; remove reference to former...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Mateusz Piotrowski
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2018-08-25 15:26 UTC by Samy Mahmoudi
Modified: 2019-05-02 19:00 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (freebsd-ports)


Attachments
Patch file generated with svn diff (4.52 KB, patch)
2018-08-25 15:26 UTC, Samy Mahmoudi
no flags Details | Diff
Patch file generated with svn diff (4.94 KB, patch)
2018-10-16 16:22 UTC, Samy Mahmoudi
no flags Details | Diff
Patch file generated with svn diff (5.22 KB, patch)
2019-02-24 05:29 UTC, Samy Mahmoudi
no flags Details | Diff
Patch file generated with svn diff (488 bytes, patch)
2019-05-02 17:55 UTC, Samy Mahmoudi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samy Mahmoudi 2018-08-25 15:26:17 UTC
Created attachment 196535 [details]
Patch file generated with svn diff

Hi,

• Remove reference to former option GNOME
• Mark option METACITY as broken *
• Simplify option DBUS
• Add missing dependencies
• Add LICENSE
• Add option NLS
• Add libinotify to pkg-plist
• Regenerate patch files
• Bump PORTREVISION

* Metacity migrated to Gtk+ 3 while Compiz still uses Gtk+ 2. This incompatibilty was left unseen at make time because of a reference to former option GNOME in an assertion which resulted in METACITY being always disabled.
Comment 1 Mateusz Piotrowski freebsd_committer freebsd_triage 2018-10-16 10:30:37 UTC
I'll take it.
Comment 2 Samy Mahmoudi 2018-10-16 10:43:12 UTC
Thank you Mateusz.
Comment 3 Mateusz Piotrowski freebsd_committer freebsd_triage 2018-10-16 11:58:55 UTC
(In reply to Samy Mahmoudi from comment #2)

In the future you may want to run `portlint -AC` to test for common errors :)
Comment 4 Samy Mahmoudi 2018-10-16 15:10:13 UTC
I am now correcting this !
Comment 5 Mateusz Piotrowski freebsd_committer freebsd_triage 2018-10-16 15:15:19 UTC
(In reply to Samy Mahmoudi from comment #4)

Also, I'm getting errors when testing your submission with poudriere:

====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: lib/compiz/libinotify.a
Error: Missing: lib/compiz/libinotify.so

It looks like those two files are not installed. Is there a reason why you added them to pkg-plist?
Comment 6 Samy Mahmoudi 2018-10-16 15:50:37 UTC
I may have done a "make check-orphans" which indicated me to include these two files.
Comment 7 Samy Mahmoudi 2018-10-16 15:56:40 UTC
[root@t520]/usr/ports/x11-wm/compiz#make check-orphans                                                                                          17:55:19 [0] 
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: lib/compiz/libinotify.a
Error: Orphaned: lib/compiz/libinotify.so
===> Checking for items in pkg-plist which are not in STAGEDIR
===> Error: Plist issues found.
*** Error code 1

Stop.
make: stopped in /usr/ports/x11-wm/compiz
Comment 8 Samy Mahmoudi 2018-10-16 15:59:29 UTC
I have also checked that the two libinotify files are indeed present in my work/stage directory.
Comment 9 Samy Mahmoudi 2018-10-16 16:22:23 UTC
Created attachment 198226 [details]
Patch file generated with svn diff
Comment 10 Samy Mahmoudi 2018-10-16 16:23:24 UTC
(In reply to Samy Mahmoudi from comment #9)

I have followed the guidelines generated by "portlint -AC". There is still one fatal error:

FATAL: Makefile: license specified is GPLv2+ LGPL21+ MIT, but LICENSE_FILE specified is for GPLv2+.

but I think portlint just misunderstands the spaces as literal spaces in "GPLv2+ LGPL21+ MIT".

Besides, I have added USES=gl and USES=gnome as suggested with DEVELOPER=YES, and I reordered the variables to follow the FreeBSD porter's handbook, as much as possible.

So here the new log:

• Remove reference to former option GNOME
• Mark option METACITY as broken *
• Simplify option DBUS
• Add missing dependencies
• Add LICENSE
• Add option NLS
• Reorder the variables
• Add libinotify to pkg-plist
• Regenerate patch files
• Bump PORTREVISION

The libinotify problem persists.
Comment 11 Samy Mahmoudi 2018-10-16 16:24:57 UTC
(In reply to Samy Mahmoudi from comment #10)

So here IS the new log...
Comment 12 Samy Mahmoudi 2019-02-24 05:19:53 UTC
Comment on attachment 198226 [details]
Patch file generated with svn diff

As you noticed, libinotify is not needed when building in a clean environment (I now use poudriere systematically).

The reason I got libinotify installed while staging is not clear yet but I suspect a port dependent on compiz (if not compiz itself) to trigger libinotify installation when reconfiguring compiz with that port already installed. Anyway, in most scenarios compiz gets built first because of the dependency tree so we do not have to change pkg-plist.
Comment 13 Samy Mahmoudi 2019-02-24 05:28:40 UTC
- Add modesetting to the drivers whitelist
- Use upstream name xfwm4 to properly fall back
- Remove reference to former option GNOME
- Mark option METACITY as broken *
- Simplify option DBUS
- Add missing dependencies
- Add LICENSE
- Reorder the variables to pet portlint
- Regenerate patch files to pet portlint

* Metacity migrated to Gtk+ 3 while Compiz still uses Gtk+ 2. This incompatibilty was left unseen at make time because of a reference to former option GNOME in an assertion which resulted in METACITY being always disabled.
Comment 14 Samy Mahmoudi 2019-02-24 05:29:29 UTC
Created attachment 202312 [details]
Patch file generated with svn diff
Comment 15 Samy Mahmoudi 2019-02-24 17:04:06 UTC
- Add modesetting to the drivers whitelist
- Use upstream name xfwm4 instead of xfwm to properly fall back
- Remove reference to former option GNOME
- Mark option METACITY as broken *
- Simplify option DBUS
- Add missing dependencies
- Add LICENSE
- Add USES=gnome, USES=gl and USE_GL=gl
- Reorder the variables to pet portlint
- Regenerate patch files to pet portlint

* Metacity migrated to Gtk+ 3 while Compiz still uses Gtk+ 2. This incompatibilty was left unseen at make time because of a reference to former option GNOME in an assertion which resulted in METACITY being always disabled.
Comment 16 Mateusz Piotrowski freebsd_committer freebsd_triage 2019-03-05 18:34:07 UTC
The patch is in review: https://reviews.freebsd.org/D19467
Comment 17 Samy Mahmoudi 2019-03-05 20:53:03 UTC
(In reply to Mateusz Piotrowski from comment #16)

Thank you Mateusz!
Comment 18 Mateusz Piotrowski freebsd_committer freebsd_triage 2019-03-05 21:08:13 UTC
(In reply to Samy Mahmoudi from comment #17)

Sorry for the delay. Please, don't hesitate to ping in the future. ;)
Comment 19 Samy Mahmoudi 2019-03-18 11:06:22 UTC
(In reply to Mateusz Piotrowski from comment #16)

I have commented the review some time ago (answering kwm) , but it is still "unsubmitted". Do I have the permission to comment inline?
Comment 20 Mateusz Piotrowski freebsd_committer freebsd_triage 2019-03-18 11:21:54 UTC
(In reply to Samy Mahmoudi from comment #19)

Of course! Please do comment, review and participate. :)
Comment 21 commit-hook freebsd_committer freebsd_triage 2019-05-02 15:38:54 UTC
A commit references this bug:

Author: 0mp
Date: Thu May  2 15:37:52 UTC 2019
New revision: 500666
URL: https://svnweb.freebsd.org/changeset/ports/500666

Log:
  x11-wm/compiz: Clean up port's makefile

  - Add modesetting to the drivers whitelist
  - Use upstream name xfwm4 instead of xfwm to properly fall back
  - Remove reference to former option GNOME
  - Simplify option DBUS
  - Add missing dependencies
  - Add LICENSE
  - Add USES=gnome, USES=gl and USE_GL=gl
  - Reorder the variables to pet portlint
  - Regenerate patch files to pet portlint

  Metacity migrated to GTK+ 3 while Compiz still uses GTK+ 2. This
  incompatibility was left unseen at make time because of a reference to
  former option GNOME in an assertion which resulted in METACITY being always
  disabled.

  Committer's changes:
  - Remove broken option METACITY
  - Further lint the makefile

  PR:		230894
  Submitted by:	Samy Mahmoudi <samy.mahmoudi@gmail.com>
  Reviewed by:	mat
  Approved by:	portmgr (maintainer timeout: > 14 days)
  Differential Revision:	https://reviews.freebsd.org/D19467

Changes:
  head/x11-wm/compiz/Makefile
  head/x11-wm/compiz/files/compiz-manager.in
  head/x11-wm/compiz/files/patch-plugins_fuse.c
  head/x11-wm/compiz/files/patch-src-Makefile.in
Comment 22 Mateusz Piotrowski freebsd_committer freebsd_triage 2019-05-02 15:40:56 UTC
(In reply to Samy Mahmoudi from comment #19)

Ok, we got it committed. Let me know if there's anything else you'd like to improve here.

Thanks a lot for you contribution! :)
Comment 23 Samy Mahmoudi 2019-05-02 17:51:02 UTC
Thank you for your review and your commit.

> Let me know if there's anything else you'd like to improve here.
While reviewing, a user commented and suggested we might also add amdgpu to WHITELIST. This was perfectly relevant but I could not manage to update the diff myself because of a problem with the URI scheme editor://. So I answered in a subsequent comment which, after a while, got ignored.

As soon as possible, I will attach a patch against the new revision.
Comment 24 Samy Mahmoudi 2019-05-02 17:55:38 UTC
Created attachment 204167 [details]
Patch file generated with svn diff

Done.
Comment 25 commit-hook freebsd_committer freebsd_triage 2019-05-02 18:52:26 UTC
A commit references this bug:

Author: 0mp
Date: Thu May  2 18:52:05 UTC 2019
New revision: 500677
URL: https://svnweb.freebsd.org/changeset/ports/500677

Log:
  x11-wm/compiz: Add the amdgpu to driver whitelist

  The elements of the list are prefixes of display drivers located in
  /usr/local/lib/xorg/modules/drivers, so these refer to Xorg display
  drivers, not kernel modules.

  The "intel" driver covers i915 if and only if intel_drv.so is loaded. If
  modesetting_drv.so is loaded then "intel" does not cover i915 anymore.

  As xf86-video-amdgpu installs amdgpu_drv.so, "amdgpu" should also be added
  to the list. "ati" and "radeon" cover display drivers installed by both
  xf86-video-ati and xf86-video-ati-legacy.

  PR:		230894
  Submitted by:	Samy Mahmoudi <samy.mahmoudi@gmail.com>
  Approved by:	portmgr (maintainer timeout, > 14 days)

Changes:
  head/x11-wm/compiz/Makefile
  head/x11-wm/compiz/files/compiz-manager.in
Comment 26 Mateusz Piotrowski freebsd_committer freebsd_triage 2019-05-02 19:00:42 UTC
(In reply to Samy Mahmoudi from comment #24)

I've already patched that and committed. Thank you for going through the effort of submitting yet another patch.