Bug 264381 - multimedia/cx88: update to 1.5.4
Summary: multimedia/cx88: update to 1.5.4
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: Jason A. Harmening
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-01 00:05 UTC by Jason A. Harmening
Modified: 2022-06-10 14:13 UTC (History)
2 users (show)

See Also:


Attachments
proposed change (1.39 KB, text/plain)
2022-06-01 00:05 UTC, Jason A. Harmening
no flags Details
proposed change (2.17 KB, patch)
2022-06-02 22:02 UTC, Jason A. Harmening
fernape: maintainer-approval+
Details | Diff
proposed change (1.94 KB, patch)
2022-06-03 16:09 UTC, Jason A. Harmening
no flags Details | Diff
Patch for cx88 (alternative variant) (2.89 KB, patch)
2022-06-03 21:43 UTC, Daniel Engberg
no flags Details | Diff
proposed change (6.82 KB, patch)
2022-06-04 00:19 UTC, Jason A. Harmening
no flags Details | Diff
proposed change (7.30 KB, patch)
2022-06-04 21:36 UTC, Jason A. Harmening
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason A. Harmening freebsd_committer freebsd_triage 2022-06-01 00:05:29 UTC
Created attachment 234365 [details]
proposed change

--Fix build on -current after removal of iic_devclass global
--Fix typos in help strings
--Incorporate ports tree patches since 2019
Comment 1 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-01 00:06:54 UTC
Since I have a src commit bit, would you guys just prefer a phab review in the future?  I'm not sure what the etiquette is for adding reviewers on the ports side of things.
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2022-06-01 09:52:34 UTC
Can you provide a MASTER_SITE mirror that supports http/https since ftp is being (indirectly) deprecated? Is WITHOUT_PIE still valid? Please concert options to the "new" format, see "5.13. Makefile Options" in Porters Handbook

Best regards,
Daniel
Comment 3 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-01 17:04:25 UTC
--I've been meaning to switch everything over to https anyway, so I'll just do that now.

--I can confirm that WITHOUT_PIE is still required, linking fails if I remove it.  I would assume that someone will do an exp-run to remove WITHOUT_PIE once the underlying staticlib issue is fixed.

--I'm not sure what is meant by the "new" options format; the OPTIONS_DEFINE/<opt>_DESC syntax in the handbook looks identical to what I already have.  Do you mean that I should use <opt>_MAKE_ARGS/<opt>_BUILD_DEPENDS instead of testing PORT_OPTIONS?
Comment 4 Fernando Apesteguía freebsd_committer freebsd_triage 2022-06-02 16:27:14 UTC
^Triage: Reporter is committer, assign accordingly.

^Triage: If there is a changelog or release notes URL available for this version, please add it to the URL field.

^Triage: Please set the maintainer-approval attachment flag (to +) on patches for ports you maintain to signify approval.
--
Attachment -> Details -> maintainer-approval [+]

Committers may obtain review via a Differential in Phabricator, adding the "Contributor Reviewers ($Repository)" group as a Reviewer, reaching out to other committers; directly or via mailing lists, or setting the attachment flag to: maintainer-approval ? <person-youd-like-to-review>

Personally, approving directly in Bugzilla works fine :-)

Thanks!
Comment 5 Daniel Engberg freebsd_committer freebsd_triage 2022-06-02 20:04:33 UTC
(In reply to Jason A. Harmening from comment #3)
Yes, use OPT_* instead of using if statements

Thanks for looking into my other questions

Best regards,
Daniel
Comment 6 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-02 22:02:32 UTC
Created attachment 234399 [details]
proposed change
Comment 7 Fernando Apesteguía freebsd_committer freebsd_triage 2022-06-03 15:12:10 UTC
(In reply to Jason A. Harmening from comment #6)
Hi Jason,

The patch seems outdated. The $FreeBSD$ line the patch removes has already been removed in the repository. Would you mind adjusting the patch?

Cheers.
Comment 8 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-03 16:09:18 UTC
Created attachment 234424 [details]
proposed change
Comment 9 Daniel Engberg freebsd_committer freebsd_triage 2022-06-03 21:20:49 UTC
Looks good but doesn't compile as files/patch-client_Makefile doesn't apply
Comment 10 Daniel Engberg freebsd_committer freebsd_triage 2022-06-03 21:43:22 UTC
Created attachment 234428 [details]
Patch for cx88 (alternative variant)

Here's a variant I hacked together, feel free to adopt it if you want.

- PORTVERSION --> DISTVERSION , while this isn't mandatory it's what Porters Handbook now suggests. [1]
- Use http, while https works it's selfsigned which causes all sorts of warnings (you might want to have a look at something like letsencrypt)
- Rephrase COMMENT to make it more compact, this is also more common with how the series are referred to
- Rearrange Makefile to fit within 78 columns for readability, try to follow modern "coding" standards for ports tree
- Combine all MAKE_ARGS definitions into one
- Rename LINUX_COMPAT to LINUXAPI, improves overall style
- Break out VERBOSE output as separate option

[1] - https://docs.freebsd.org/en/books/porters-handbook/book/#porting-makefile and "Table 2. Package Naming Examples"

In general I'd say that we try to avoid using underscores in variables mainly because of readability and we use it as a separator for various functions which is the main reason why I changed it. Making it short also improved (made) stile more consistent within the Makefile. I would also recommend that we try unless necessary to avoid redefining already defined variable descriptions within Ports framework for consistency which I suggested to break out VERBOSE.

This is just a suggestion from my end, feel free to discard it if you don't like it.
Comment 11 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-04 00:19:34 UTC
Created attachment 234429 [details]
proposed change

Responding to last 2 comments:

--Oops, the reason those patches don't apply is that they shouldn't be present in the update at all; I've incorporated them in the core build.  I had to rewrite my patch generator to handle the ports tree switch from svn to git, and I missed a couple of things.

--Regarding the proposed changes to the Makefile, I'm mostly ok with them, but with a couple of tweaks:
1) I'd rather keep WITHOUT_PIE on its own line, as that should make it easier to delete once the underlying issue is fixed.
2) I slightly prefer LINUXCOMPAT (without the _) to LINUXAPI.
I've included those changes in this update.
Comment 12 Jason A. Harmening freebsd_committer freebsd_triage 2022-06-04 21:36:39 UTC
Created attachment 234450 [details]
proposed change

Decided to go ahead and adopt non-self-signed certificate; updating both MASTER_SITES and wiki link to use https.
Comment 13 Daniel Engberg freebsd_committer freebsd_triage 2022-06-05 06:01:50 UTC
LGTM, you can drop "+" when using OPT_MARK_ARGS+= OPT_BUILD_DEPENDS+= and so on

Compiles on 13.1-STABLE (amd64)

Please also consider moving libtuner to https

Best regards,
Daniel
Comment 14 Daniel Engberg freebsd_committer freebsd_triage 2022-06-05 06:04:39 UTC
..and consider keep the same order for menu options (alphabetical order) through the Makefile
Comment 15 commit-hook freebsd_committer freebsd_triage 2022-06-10 03:44:57 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=88aef5f532d02d50b3bffd191092e8887629361f

commit 88aef5f532d02d50b3bffd191092e8887629361f
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2022-06-10 03:26:32 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2022-06-10 03:42:55 +0000

    multimedia/cx88: update to 1.5.4

    --Fix build on -current after removal of iic_devclass
    --Fix typos in help strings
    --Incorporate ports tree patches since 2019
    --Modernize port makefile and switch MASTER_SITES to https

    PR:             264381
    Approved by:    diizzy

 multimedia/cx88/Makefile                           | 43 +++++++++++-----------
 multimedia/cx88/distinfo                           |  6 +--
 multimedia/cx88/files/cx88d.in (mode +x)           |  0
 multimedia/cx88/files/patch-client_Makefile (gone) | 20 ----------
 .../patch-client_dvb_cx88__dvb__buffer.cpp (gone)  | 11 ------
 .../patch-client_dvb_cx88__dvb__capture.h (gone)   | 11 ------
 .../patch-client_dvb_cx88__dvb__demux.cpp (gone)   | 11 ------
 ...ch-client_dvb_cx88__dvb__demux__feed.cpp (gone) | 11 ------
 ...patch-client_dvb_cx88__dvb__frontend.cpp (gone) | 11 ------
 .../patch-client_v4l_cx88__radio__capture.h (gone) | 11 ------
 multimedia/cx88/pkg-descr                          |  2 +-
 11 files changed, 25 insertions(+), 112 deletions(-)