Page MenuHomeFreeBSD

[NEW PORT] x11/xfce4-goodies: Meta-port for software and artwork from the XFCE4 Goodies Project
ClosedPublic

Authored by woodsb02 on Jun 27 2016, 9:21 AM.

Details

Summary

Add x11/xfce4-goodies: Meta-port for ports from the XFCE4 Goodies Project

This meta-port gathers together each of the ports from the XFCE4 Goodies
Project and allows them to be easily installed to create a functional
desktop environment.

Default options include those which do not bring in too many additional
dependencies, and are generally recommended for use on a modern XFCE
desktop.

It also includes ports which are not officially part of the XFCE4
Goodies Project, but are generally recommended for use with XFCE4
on FreeBSD.

Test Plan

poudriere testport x11/xfce4-goodies

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

woodsb02 updated this revision to Diff 17918.Jun 27 2016, 9:21 AM
woodsb02 retitled this revision from to [NEW PORT] x11/xfce4-goodies: Meta-port for software and artwork from the XFCE4 Goodies Project.
woodsb02 updated this object.
woodsb02 edited the test plan for this revision. (Show Details)
woodsb02 added reviewers: mat, adamw, koobs.
woodsb02 added a project: xfce.
mat edited edge metadata.Jun 27 2016, 10:00 AM

For all the depends, maybe you should replace the dependency on a file by a dependency on the package, say, replace:

XKB_RUN_DEPENDS= ${LOCALBASE}/share/xfce4/panel-plugins/xkb-plugin.desktop:deskutils/xfce4-xkb-plugin

with

XKB_RUN_DEPENDS= xfce4-xkb-plugin>=0:deskutils/xfce4-xkb-plugin

It's more semantically correct as you want the package, you don't care about the file, and it doesn't depend on a file that could move, or disappear.

In D6981#146305, @mat wrote:

For all the depends, maybe you should replace the dependency on a file by a dependency on the package, say, replace:

XKB_RUN_DEPENDS= ${LOCALBASE}/share/xfce4/panel-plugins/xkb-plugin.desktop:deskutils/xfce4-xkb-plugin

with

XKB_RUN_DEPENDS= xfce4-xkb-plugin>=0:deskutils/xfce4-xkb-plugin

It's more semantically correct as you want the package, you don't care about the file, and it doesn't depend on a file that could move, or disappear.

Thanks for the tip. It's interesting - while I have seen this before, I don't think that is mentioned in the porter's handbook in the section discussing RUN_DEPENDS. In that section it states that you can either use a full path to a file (tested with test -e), or a binary name that will be found in PATH (tested with which -s). Seems that it needs an update to mention it can also be a package name.

mat added a comment.Jun 27 2016, 4:40 PM

It is documented in bsd.port.mk:

  1. The first field also supports a
  2. package name with a version range, in the form package>=1.2
  3. if a particular version is desired.

But it is true that it should be in the porter's handbook, I'll try to make sure it ends up there too.

woodsb02 updated this revision to Diff 17951.Jun 27 2016, 9:33 PM
woodsb02 edited edge metadata.

As per comment from mat, replace the dependency on a file by a dependency on the package.

mat added inline comments.Jun 27 2016, 10:10 PM
x11/xfce4-goodies/Makefile
60 ↗(On Diff #17951)

Ah, that variable has a - in it, I'm not sure it's safe to use, as shell variables can't contain a -, and the variables will be passed to the shell when running make config

(I'm not saying it won't work, just that it's something to check, or avoid, in case the config framework gets rewritten and this breaks)

woodsb02 updated this revision to Diff 17962.Jun 28 2016, 8:19 AM

As per comments from mat, remove hyphens from all OPTIONS_GROUP names, and also all OPTIONS names just to be safe.

woodsb02 marked an inline comment as done.Jun 28 2016, 8:23 AM

Thanks for the heads up.

mat accepted this revision.Jun 28 2016, 11:47 AM
mat edited edge metadata.

Thanks for the heads up.

All the things I know that I wished I did not…

This revision is now accepted and ready to land.Jun 28 2016, 11:47 AM
sbruno added a subscriber: sbruno.Jun 28 2016, 4:33 PM

I'll give this a build test on -current tonight.

olivierd added inline comments.Jul 6 2016, 6:39 PM
x11/xfce4-goodies/Makefile
9 ↗(On Diff #17962)

XFCE4 is misspelled, the first character is only capitalized, others are lowercase.

13 ↗(On Diff #17962)

Personally I would avoid obsolete applications by default

27 ↗(On Diff #17962)

NOTIFICATIONDAEMON (deskutils/xfce4-notification-daemon) should be removed, it's not anymore supported by upstream.

NOTIFYD (deskutils/xfce4-notifyd) is already present (and set by default) in meta-port x11-wm/xfce4.
Moreover it's called NOTIFY (without final "D"). I think it's better to have homogeneous options.

131 ↗(On Diff #17962)

DEVTOOLS (devel/xfce4-dev-tools) is part of core component.

WHISKERMENU should be placed here, because it's the only application written in C++.

132 ↗(On Diff #17962)

VFS (x11-fm/thunar-vfs) is obsolete (not anymore maintained by upstream), no update since 4 years and most of methods are implemented in Thunar and Gvfs.

woodsb02 marked 3 inline comments as done.Jul 12 2016, 9:27 PM

Hi olivierd, thanks for you review. I have updated my patch and responded to your comments below.

x11/xfce4-goodies/Makefile
13 ↗(On Diff #17962)

I have removed any ports from OPTIONS_DEFAULT if they are marked as deprecated (due to dependency on x11-toolkits/libxfce4gui).

27 ↗(On Diff #17962)

Done. I will submit a separate review to mark xfce4-notification-daemon as deprecated, with an expiration date of 1 month.

131 ↗(On Diff #17962)

As far as I can tell, devel/xfce4-dev-tools is not a dependency of any other ports, and as such this seems like a good place to notify people of it's existence (albeit not required by default).

WHISKERMENU is part of the official goodies project, so I think it is better for it to remain in the PANELPLUGINS group, even if it uses C++.
http://goodies.xfce.org/projects/panel-plugins/xfce4-whiskermenu-plugin

132 ↗(On Diff #17962)

Ok, I have removed VFS from OPTIONS_DEFAULT. If we think it should not be in ports anymore, I can submit a separate change to mark it as deprecated with a 1 month expiry?

woodsb02 marked 2 inline comments as done.Jul 12 2016, 9:28 PM
woodsb02 updated this revision to Diff 18343.Jul 12 2016, 9:30 PM
woodsb02 edited edge metadata.

Update diff inline with comments from olivierd

This revision now requires review to proceed.Jul 12 2016, 9:30 PM
sbruno accepted this revision.Aug 30 2016, 2:43 PM
sbruno added a reviewer: sbruno.

You may fire when ready.

This revision is now accepted and ready to land.Aug 30 2016, 2:43 PM

UNOFFICIAL term is not good word (imho), because it contains unmaintained applications, so we must inform users these ports (in a future) can be removed.

x11/xfce4-goodies/Makefile
55 ↗(On Diff #18343)

GENERICSLIDER should go to UNOFFICIAL option group, because there is no official release (development seems stopped).

126 ↗(On Diff #18343)

I persist, but I think DEVTOOLS (devel/xfce4-deve-tools) should be removed from UNOFFICIAL option group. We can leave this port outside from meta-port.

UNOFFICIAL term is not good word (imho), because it contains unmaintained applications, so we must inform users these ports (in a future) can be removed.

By unofficial, I meant anything which can not be found at the website: http://goodies.xfce.org/

x11/xfce4-goodies/Makefile
55 ↗(On Diff #18343)

The generic-slider is on the website http://goodies.xfce.org/projects/panel-plugins/xfce4-generic-slider.

I have not enabled the option by default, because although it is available on the official xfce goodies website, it is old and outdated. This is the logic I was hoping we could follow with this port.

126 ↗(On Diff #18343)

Ok, I will remove it. This port does tend to serve desktop users rather than developers, so the logic makes sense.

This revision was automatically updated to reflect the committed changes.