Page MenuHomeFreeBSD
Differential D16978

multimedia/gxine: Remove dependency that no longer exists, other cleanup
ClosedPublic
Actions

Authored by ndowens_yahoo.com on Sep 1 2018, 12:19 AM.
Tags
None
Referenced Files
F105824561: D16978.diff
Sat, Dec 21, 6:43 AM
Unknown Object (File)
Fri, Dec 6, 10:26 PM
Unknown Object (File)
Mon, Dec 2, 8:26 AM
Unknown Object (File)
Sat, Nov 30, 7:49 PM
Unknown Object (File)
Fri, Nov 29, 10:18 AM
Unknown Object (File)
Thu, Nov 28, 2:26 PM
Unknown Object (File)
Nov 13 2024, 1:33 AM
Unknown Object (File)
Nov 11 2024, 11:47 AM
View All Files
Subscribers
mat
tobik
Contributor Reviewers (ports)

Details

Reviewers
tobik
Commits
rP479201: Cleanup multimedia/gxine
Summary

Remove dependency that no longer exists, and may not be needed
Changed a few OPTION switches
General clean up

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 19332
Build 18937: arc lint + arc unit

Event Timeline

ndowens_yahoo.com created this revision.Sep 1 2018, 12:19 AM
Herald added subscribers: Contributor Reviewers (ports), mat. · View Herald TranscriptSep 1 2018, 12:19 AM
Harbormaster completed remote builds in B19331: Diff 47543.Sep 1 2018, 12:19 AM
ndowens_yahoo.com updated this revision to Diff 47547.Sep 1 2018, 12:30 AM

Updating D16978: multimedia/gxine
Add NLS knob to remove gettext as a must dep

Harbormaster completed remote builds in B19332: Diff 47547.Sep 1 2018, 12:30 AM
linimon retitled this revision from multimedia/gxine to multimedia/gxine: Remove dependency that no longer exists, other cleanup.Sep 1 2018, 5:29 AM
mat added inline comments.Sep 1 2018, 8:21 AM
multimedia/gxine/Makefile
26

This is probably a bad idea.

27–28

Most of this would be USES=localbase

34–35

Part of this should be:

GNOME_CONFIGURE_WITH=  dbus
40

DESCs should happen earlier, See Chapter 15. Order of Variables in Port Makefiles.

57–58

You could probably fold those two commands into one.

tobik added inline comments.Sep 1 2018, 9:28 AM
multimedia/gxine/Makefile
14

LICENSE_FILE (no S at the end)

26

I feel like this is a problem that must be fixed in libxine and not worked around in every consumer.

I don't understand why compiling gxine with -fPIC fixes linking to libxine with lld.

28

I don't think -lm is needed anymore.

33–35

I am wondering if this does anything without GNOME screensaver running or if it is useless without it?

--without-dbus          disable dbus support (for GNOME screensaver)
56–58

logo.jpg is already being installed. No need to do it manually. gxine now uses logo.jpg since the libxine update, so logo.mpv is not needed anymore. Let's drop post-install completely.

ndowens_yahoo.com marked 8 inline comments as done.Sep 1 2018, 10:31 AM
ndowens_yahoo.com added inline comments.
multimedia/gxine/Makefile
26

Ed Maste replied to what I figured out that fPIC works around linker issue
@tobik @mat
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230994

LLDunsafe didn’t work nor adding the ldflags Ed said is medium

27–28

Was thinking about removing before posting here but didn’t know what -1m was for

33–35

I have no idea myself. I don’t use gnome and I don’t understand how a media player would even need screensaver dep. IIRC when I use to use gnome stuff I felt as gnome apps relied on dbus, but I could be wrong

ndowens_yahoo.com updated this revision to Diff 47560.Sep 1 2018, 10:43 AM
ndowens_yahoo.com marked an inline comment as done.

Updating D16978: multimedia/gxine: Remove dependency that no longer exists, other cleanup
Fix typo in license file
remove cpp and ldflags
remove post-install block

Harbormaster completed remote builds in B19344: Diff 47560.Sep 1 2018, 10:43 AM
tobik added inline comments.Sep 1 2018, 10:50 AM
multimedia/gxine/Makefile
26

LLDunsafe didn’t work

?

Of course LLD_UNSAFE works here or we wouldn't have been able to build gxine on 12.0/amd64 at all previously.

ndowens_yahoo.com added inline comments.Sep 1 2018, 10:53 AM
multimedia/gxine/Makefile
26

Retying but I do believe I was getting linker issues with it

tobik added inline comments.Sep 1 2018, 10:56 AM
multimedia/gxine/Makefile
33–35

I say remove it in that case. The option has been broken for 4 years (when gnome-screensaver was removed) and nobody has complained so I doubt anybody cares about it.

When you remove it make sure to append --disable-integration-wizard --without-dbus to CONFIGURE_ARGS in lieu of GNOME_CONFIGURE_OFF.

ndowens_yahoo.com marked 5 inline comments as done.Sep 1 2018, 11:34 AM
ndowens_yahoo.com updated this revision to Diff 47562.Sep 1 2018, 11:36 AM

Updating D16978: multimedia/gxine: Remove dependency that no longer exists, other cleanup
Re-add lld_unsafe
Remove gnome option

Harbormaster completed remote builds in B19345: Diff 47562.Sep 1 2018, 11:36 AM
ndowens_yahoo.com abandoned this revision.Sep 5 2018, 4:58 AM
tobik added a reviewer: tobik.Sep 5 2018, 5:08 AM

Why did you abandon this?

ndowens_yahoo.com reclaimed this revision.Sep 5 2018, 5:16 AM
In D16978#363146, @tobik wrote:

Why did you abandon this?

For some reason, thought it was commited along with the update with libxine :) But reclaimed

tobik accepted this revision.Sep 5 2018, 5:22 AM
This revision is now accepted and ready to land.Sep 5 2018, 5:22 AM
ndowens_yahoo.com added a comment.Sep 5 2018, 10:32 PM

@tobik As you may know, I can't commit :)

Closed by commit rP479201: Cleanup multimedia/gxine (authored by tobik). · Explain WhySep 7 2018, 5:16 PM
This revision was automatically updated to reflect the committed changes.
tobik added a comment.Sep 7 2018, 5:21 PM
In D16978#363397, @ndowens_yahoo.com wrote:

@tobik As you may know, I can't commit :)

Yeah, I know. Please do not ping me after only a couple of days though. The review is barely even a week old and hardly urgent.

In general it's also a good idea to open a corresponding bug on Bugzilla for every review, so that others can take care of it too. Phabricator is not a patch queue.

Revision Contents
Changeset List

PathSize
multimedia/
gxine/
40 lines
49 lines

Diff 47547

View Options

multimedia/gxine/Makefile

Loading...
View Options

multimedia/gxine/pkg-plist

Loading...