Page MenuHomeFreeBSD
Differential D29508

graphics/imv: Update to 4.2.0
ClosedPublic
Actions

Authored by eduardo on Mar 31 2021, 9:14 AM.
Tags
None
Referenced Files
F105781074: D29508.id86945.diff
Fri, Dec 20, 2:46 PM
F105780599: D29508.id88291.diff
Fri, Dec 20, 2:35 PM
F105779877: D29508.id86951.diff
Fri, Dec 20, 2:21 PM
F105779341: D29508.id88094.diff
Fri, Dec 20, 2:09 PM
F105779095: D29508.id88247.diff
Fri, Dec 20, 2:04 PM
F105764761: D29508.diff
Fri, Dec 20, 9:01 AM
Unknown Object (File)
Sat, Dec 14, 9:16 PM
Unknown Object (File)
Sat, Nov 30, 4:31 PM
View All Files
Subscribers
mat

Details

Reviewers
dbaio
garga
Commits
R11:82d01b1cc3d6: graphics/imv: Update to 4.2.0
Summary
graphics/imv: Update to 4.2.0

ChangeLog: https://github.com/eXeC64/imv/blob/v4.2.0/CHANGELOG

Reviewed by:	mat
Approved by:	dbaio, garga (mentors)
Differential Revision:	https://reviews.freebsd.org/D29508
Test Plan
$ portlint -C
WARN: Makefile: Consider adding support for a NLS knob to conditionally disable gettext support.

poudriere testport logs:

140amd64
140i386
130amd64
130i386
122amd64
122i386
114amd64
114i386

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

eduardo created this revision.Mar 31 2021, 9:14 AM
Herald added a subscriber: mat. · View Herald TranscriptMar 31 2021, 9:14 AM
eduardo requested review of this revision.Mar 31 2021, 9:14 AM
eduardo edited the summary of this revision. (Show Details)
garga added a comment.Mar 31 2021, 12:25 PM

Changes look good to me, but ports repository is being converted to git right now. Lets wait the new repository to be open for commits and then you can update the patch using git.

eduardo updated this revision to Diff 86945.Apr 7 2021, 6:21 AM

git diff:
git diff -U999999 . > imv.diff

mat added a comment.Apr 7 2021, 10:17 AM

Please, convert all the *_MESON_ON/OFF into *_MESON_ENABLED. https://docs.freebsd.org/en/books/porters-handbook/makefiles/#options-meson_enabled

graphics/imv/Makefile
46–47 ↗(On Diff #86945)
50–51 ↗(On Diff #86945)
54–55 ↗(On Diff #86945)
eduardo updated this revision to Diff 86951.Apr 7 2021, 10:48 AM

Use _MESON_ENABLED instead of _MESON_ON/OFF

eduardo marked 3 inline comments as done.Apr 8 2021, 6:07 AM

Can't use _MESON_ENABLED for windows option:
"meson.build:1:0: ERROR: Value "x11=disabled" (of type "string") for combo option "window system to use" is not one of the choices. Possible choices are (as string): "all", "wayland", "x11"."

eduardo changed the repository for this revision from rP FreeBSD ports repository to R11 FreeBSD ports repository.Apr 15 2021, 6:25 AM
dbaio added inline comments.Apr 18 2021, 8:06 PM
graphics/imv/Makefile
72–85 ↗(On Diff #86951)

I think you can't have two -Dwindows= args:
https://github.com/eXeC64/imv/blob/master/meson.build#L25-L38

Maybe the better approach is using OPTIONS SINGLE here.

eduardo updated this revision to Diff 88094.Apr 25 2021, 1:03 AM

move "Windowing system" port option from multi to single option.

dbaio added a comment.Apr 25 2021, 1:06 PM
In D29508#672367, @eduardo wrote:

move "Windowing system" port option from multi to single option.

Nice.

I did some tests as well:

====> Windowing system: you have to select exactly one of them
     WAYLAND=on: Wayland (graphics) support
     X11=off: X11 (graphics) support
     ALL=off
ok
====> Windowing system: you have to select exactly one of them
     WAYLAND=off: Wayland (graphics) support
     X11=on: X11 (graphics) support
     ALL=off

ok
====> Windowing system: you have to select exactly one of them
     WAYLAND=off: Wayland (graphics) support
     X11=off: X11 (graphics) support
     ALL=on

====> Running Q/A tests (stage-qa)
Error: /usr/local/bin/imv-wayland is linked to /usr/local/lib/libwayland-client.so.0 from graphics/wayland but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libwayland-client.so:graphics/wayland
Error: /usr/local/bin/imv-wayland is linked to /usr/local/lib/libwayland-egl.so.1 from graphics/wayland but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libwayland-egl.so:graphics/wayland

Final adjustments, add WAYLAND_LIB_DEPENDS to the ALL option as well.

eduardo updated this revision to Diff 88247.EditedApr 27 2021, 8:45 AM
eduardo marked an inline comment as done.
eduardo edited the test plan for this revision. (Show Details)
  • add missing wayland dependency to ALL "Windowing system" option
  • refreshed poudriere testport logs
dbaio accepted this revision.Apr 27 2021, 11:17 AM

Good job.
Don't forget to give credits to mat@ for the review in the commit message.

This revision is now accepted and ready to land.Apr 27 2021, 11:17 AM
eduardo edited the summary of this revision. (Show Details)Apr 28 2021, 6:42 AM

Thanks mat and my mentors dbaio and garga!

graphics/imv/Makefile
72–85 ↗(On Diff #86951)

According to https://github.com/eXeC64/imv/blob/master/meson_options.txt,
we can have both wayland and x11 selected that is equal to -Dwindows=all

graphics/imv/files/patch-meson.build
7–8 ↗(On Diff #86951)

"find_library" is deprecated:

https://mesonbuild.com/Reference-manual.html#find_library

Closed by commit R11:82d01b1cc3d6: graphics/imv: Update to 4.2.0 (authored by eduardo). · Explain WhyApr 28 2021, 6:49 AM
This revision was automatically updated to reflect the committed changes.
eduardo added a commit: R11:82d01b1cc3d6: graphics/imv: Update to 4.2.0.

Revision Contents
Changeset List

PathSize
66 lines
6 lines
files/
11 lines

Diff 86602

View Options

Makefile

Loading...
View Options

distinfo

Loading...
View Options

files/patch-meson.build

Loading...