Bug 273810

Summary: Update multimedia/motion to 4.5.1
Product: Ports & Packages Reporter: ports
Component: Individual Port(s)Assignee: Joel Bodenmann <jbo>
Status: Closed FIXED    
Severity: Affects Only Me CC: jbo, sirl33tname
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Update to 4.5.1
none
without mbox rubbish.
none
Update the port version too
none
Workaround webp 1.31 pkg-config issue (in a way that is safe with 1.30)
none
All previous work rolled into a single patch for ease of use.
none
Switched to DISTVERSION rather than PORTVERSION
none
multimedia/motion: update 4.3.2 -> 4.5.1
none
Re-upload
none
Rebased.
none
Updated as per suggestion.
none
Removed unnecessary flags, add pkg-plist file
none
Made portclippy and portlint happy none

Description ports 2023-09-15 04:37:46 UTC
Created attachment 244865 [details]
Update to 4.5.1

multimedia/motion has been unloved lately (I stopped using it), but this updates it to the latest release version.
Comment 1 ports 2023-09-15 04:39:37 UTC
Created attachment 244867 [details]
without mbox rubbish.
Comment 2 ports 2023-09-15 04:51:41 UTC
Created attachment 244868 [details]
Update the port version too
Comment 3 ports 2023-09-15 07:04:31 UTC
Created attachment 244870 [details]
Workaround webp 1.31 pkg-config issue (in a way that is safe with 1.30)
Comment 4 ports 2023-09-15 07:10:47 UTC
Created attachment 244871 [details]
All previous work rolled into a single patch for ease of use.

Rolled up changes into a single patch.
Comment 5 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-15 16:55:48 UTC
Hi,
Thank you for your patch.

DISTVERSION is preferred over PORTVERSION when possible: https://docs.freebsd.org/en/books/porters-handbook/makefiles/#makefile-naming
Comment 6 ports 2023-09-15 23:00:04 UTC
Created attachment 244906 [details]
Switched to DISTVERSION rather than PORTVERSION
Comment 7 Sir l33tname 2023-09-16 10:32:08 UTC
Created attachment 244929 [details]
multimedia/motion: update 4.3.2 -> 4.5.1

I came up with almost the same patch but i think LICENSE_FILE can be removed now and PORTREVISION is reset (removed) for a new version.
Comment 8 ports 2023-09-17 00:10:58 UTC
(In reply to Sir l33tname from comment #7)
I don't use motion any more, so it's great to see someone else contribute patches! Thanks and keep it up!
Comment 9 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-17 00:29:13 UTC
ports@blievers.net, could you please update the patch? It's ill-formatted (line 12).
Comment 10 ports 2023-09-17 05:11:18 UTC
Created attachment 244950 [details]
Re-upload

Sorry about that, not sure how that happened.
Comment 11 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-18 19:01:48 UTC
I think that the patch is still malformed. It also doesn't render correctly in the diff view here in bugzilla - check the 2nd hunk header (https://bugs.freebsd.org/bugzilla/attachment.cgi?id=244950&action=diff).
Comment 12 Sir l33tname 2023-09-18 19:09:24 UTC
you know that you could just commit my patch?
Comment 13 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-18 19:37:45 UTC
(In reply to Sir l33tname from comment #12)

I'd argue that if the maintainer provides a patch that is the preferable patch to commit. Furthermore, your patch is lacking maintainer-approval+.
Comment 14 Sir l33tname 2023-09-18 19:49:53 UTC
fair but my patch is not broken, it also removes LICENSE_FILE which isn't used anymore and PORTREVISION is also still wrong in the patch and otherwise it is exactly the same patch
Comment 15 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-18 20:27:46 UTC
I'd say we'll wait for the maintainer. Either they'll fix/update their patch or give approval to commit yours.
Comment 16 ports 2023-09-18 20:33:46 UTC
I don't see it? It renders fine here, and it's just the output of git diff. I'm also completely happy to go ahead with Sir l33tname's patch.
Comment 17 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-18 21:54:16 UTC
The patch is not applying cleanly because there was a commit affecting this port in the meantime (42ce6b36ff709ca04247b85eec6b4e8f370ec2e3).
Can you please submit a rebased patch?
Comment 18 ports 2023-09-19 01:11:14 UTC
Created attachment 245012 [details]
Rebased.

Ah! sorry about that. Much cleaner patch now.
Comment 19 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-21 14:29:53 UTC
A few things that need checking here:
  - Does the new upstream version really require the legacy ffmpeg4 version?
  - In post-patch (line 91), is the REINPLACE on configure.ac still needed?
  - autotools based projects usually come with a working make install target. Is the custom install recipe (still) needed?
Comment 20 ports 2023-09-22 05:02:32 UTC
Created attachment 245106 [details]
Updated as per suggestion.

This patch makes a few changes:
- Switch to current ffmpeg 
- Removed post-patch hack that didn't do anything
- removed custom install stuff
- Attempted to make the build honour NLS as an option

I followed this example https://docs.freebsd.org/en/books/porters-handbook/book/#using-gettext-optional and it seems to work.

One big change is that the example files get installed directly into  /usr/local/etc/motion/ rather than into an example directory. This is the way the motion package installer handles it, but is a change from how the package used to work. Not sure if this is acceptable (and also not sure how to change it if it is not).

Any suggestions on this one?
Comment 21 ports 2023-09-22 05:50:33 UTC
Example *config* files I should say. I don't think it's too bad.
Comment 22 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-22 15:24:22 UTC
This is starting to look much better - nice!

- According to the handbook, PLIST_FILES should only be used when there are "a handful" of files. I'd argue that we've passed the generally accepted "handfullness" here. Lets create a pkg-plist file instead.
- Is manually messing with CLFAGS, LDFLAGS, CONFIGURE_ENV and MAKE_ENV still required when ffmpeg can be listed as a LIB or RUN dependency?
- Is autoreconf really needed?
Comment 23 ports 2023-09-22 22:45:27 UTC
Created attachment 245130 [details]
Removed unnecessary flags, add pkg-plist file

I figured we might be pushing the file list size... 

Okay, added pkg-plist, and you are correct the various flag overrides are not necessary, so I removed them. autoreconf is, however, required. tarball doesn't include configure, only configure.ac, so it's a required and documented build step.

Definitely looks better, thanks for the help!
Comment 24 Joel Bodenmann freebsd_committer freebsd_triage 2023-09-25 11:40:01 UTC
Awesome - good work!
Could you address the QA complaints reported by portlint and portclippy?
Comment 25 ports 2023-09-25 20:42:30 UTC
Created attachment 245235 [details]
Made portclippy and portlint happy

Portlint and portclippy now happy
Comment 26 commit-hook freebsd_committer freebsd_triage 2023-10-03 11:10:25 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=9c67d516b56bd08cd3d95f0c2095cbf7803407f7

commit 9c67d516b56bd08cd3d95f0c2095cbf7803407f7
Author:     ports@blievers.net <ports@blievers.net>
AuthorDate: 2023-09-25 20:45:20 +0000
Commit:     Joel Bodenmann <jbo@FreeBSD.org>
CommitDate: 2023-10-03 11:06:54 +0000

    multimedia/motion: Update to 4.5.1

    Changelogs:
      4.5.1: https://github.com/Motion-Project/motion/releases/tag/release-4.5.1
      4.5.0: https://github.com/Motion-Project/motion/releases/tag/release-4.5.0
      4.4.0: https://github.com/Motion-Project/motion/releases/tag/release-4.4.0

    PR:                     273810
    Approved by:            zirias (mentor)
    Differential Revision:  https://reviews.freebsd.org/D41915

 multimedia/motion/Makefile        | 67 ++++++++-------------------------------
 multimedia/motion/distinfo        |  8 ++---
 multimedia/motion/pkg-plist (new) | 31 ++++++++++++++++++
 3 files changed, 48 insertions(+), 58 deletions(-)
Comment 27 Joel Bodenmann freebsd_committer freebsd_triage 2023-10-03 11:11:09 UTC
Committed - thanks! :)