Bug 251064

Summary: [NEW PORT] textproc/cast2gif: Tool to render Asciinema cast files to GIFs
Product: Ports & Packages Reporter: Nuno Teixeira <eduardo>
Component: Individual Port(s)Assignee: Rainer Hurling <rhurlin>
Status: Closed FIXED    
Severity: Affects Only Me CC: 0mp, mikael, rhurlin, zicklag
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Description Flags
Sugested shar archive
Sugested shar archive (corrected pkg-descr)
Sugested shar archive (corrected pkg-descr)
Patch with gmake correction
cast2gif with better version numbering and gmake none

Description Nuno Teixeira freebsd_committer 2020-11-12 08:34:30 UTC
Created attachment 219585 [details]
Sugested shar archive

New port: textproc/cast2gif

A tool to convert Asciinema cast file to Gif, animated PNG, or
SVG files without using Electron or a web browser.

WWW: https://github.com/katharostech/cast2gif


Nuno Teixeira
Comment 1 Zicklag 2020-11-12 16:36:02 UTC
We probably want to update the description to take out the reference to animated PNGs and SVG files. That comment was in the README but it was a little out-of-date. Cast2gif doesn't convert to SVG or APNG yet, even though it is possible that it will int the future.
Comment 2 Nuno Teixeira freebsd_committer 2020-11-13 08:04:38 UTC
Created attachment 219625 [details]
Sugested shar archive (corrected pkg-descr)

Correct pkg description
Comment 3 Nuno Teixeira freebsd_committer 2020-11-13 08:11:41 UTC
(In reply to Zicklag from comment #1)


I've updated pkg-descr:

A tool to convert Asciinema cast file to GIFs without using
Electron or a web browser.

Comment 4 Nuno Teixeira freebsd_committer 2020-11-13 08:18:26 UTC
Created attachment 219627 [details]
Sugested shar archive (corrected pkg-descr)

replace file -> files
Comment 5 Mateusz Piotrowski freebsd_committer 2020-11-14 11:46:35 UTC
The port fails to build on 11.4 amd64, there is some problem with the servo-fontconfig-sys.

The error message: http://paste.debian.net/1172415/
Comment 6 Nuno Teixeira freebsd_committer 2020-11-15 05:46:27 UTC
(In reply to Mateusz Piotrowski from comment #5)

Could you try MAKE_JOBS_UNSAFE?

Or should I ask upstream to update cargo.lock to latest dependencies if possible?
Comment 7 Rainer Hurling freebsd_committer 2020-11-15 08:26:20 UTC
(In reply to Nuno Teixeira from comment #6)

I think this is the part causing the error in an 11.4 amd64 jail (with MAKE_JOBS_UNSAFE=yes):

     Running `/wrkdirs/usr/ports/textproc/cast2gif/work/target/release/build/servo-fontconfig-sys-8a6e63b13ef68bdc/build-script-build`
error: failed to run custom build command for `servo-fontconfig-sys v4.0.9`

Caused by:
  process didn't exit successfully: `/wrkdirs/usr/ports/textproc/cast2gif/work/target/release/build/servo-fontconfig-sys-8a6e63b13ef68bdc/build-script-build` (exit code: 101)
  --- stderr
  make[1]: illegal argument to -j -- must be positive integer!
  thread 'main' panicked at 'assertion failed: Command::new("make").env("MAKEFLAGS",
                                                                                   "makefile.cargo"]).status().unwrap().success()', /wrkdirs/usr/ports/textproc/cast2gif/work/cast2gif-pre-release/cargo-crates/servo-fontconfig-sys-4.0.9/build.rs:20:5
  stack backtrace:
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
*** Error code 101
Comment 8 Zicklag 2020-11-15 17:09:02 UTC
(In reply to Rainer Hurling from comment #7)

That's interesting. It looks like there is something wrong with the contents of the `CARGO_MAKEFLAGS` environment variable. Like something tried to add `-j[SomethingNotAPostiveInteger]` to `CARGO_MAKEFLAGS`.
Comment 9 Mikael Urankar freebsd_committer 2020-11-29 18:07:17 UTC
make is too old on 11.4 and AFAIU is not "jobserver" compatible: https://github.com/rust-lang/rust/pull/42682#issue-125886139

On 11.4, you can install gmake and patch cargo-crates/servo-fontconfig-sys-4.0.9/makefile.cargo:

--- ./cargo-crates/servo-fontconfig-sys-4.0.9/makefile.cargo.orig       2020-11-29 19:04:42.745374000 +0100
+++ ./cargo-crates/servo-fontconfig-sys-4.0.9/makefile.cargo    2020-11-29 19:02:38.491280000 +0100
@@ -58,7 +58,7 @@ SRC_DIR = $(shell pwd)
 all: $(OUT_DIR)/libfontconfig.a
 $(OUT_DIR)/libfontconfig.a: $(OUT_DIR)/Makefile
-       cd $(OUT_DIR) && make -j$(NUM_JOBS)
+       cd $(OUT_DIR) && gmake -j$(NUM_JOBS)
        cp $(OUT_DIR)/src/.libs/libfontconfig.a $(OUT_DIR)

--- ./cargo-crates/servo-fontconfig-sys-4.0.9/build.rs.orig     2020-11-29 19:06:19.604660000 +0100
+++ ./cargo-crates/servo-fontconfig-sys-4.0.9/build.rs  2020-11-29 19:06:26.037062000 +0100
@@ -17,7 +17,7 @@ fn main() {
-    assert!(Command::new("make")
+    assert!(Command::new("gmake")
         .env("MAKEFLAGS", env::var("CARGO_MAKEFLAGS").unwrap_or_default())
         .args(&["-R", "-f", "makefile.cargo"])
Comment 10 Rainer Hurling freebsd_committer 2020-11-29 20:07:14 UTC
Created attachment 220080 [details]
Patch with gmake correction

Thanks to Mikael,

there seems to be a solution for the problem from comment #7.

The new patch added two patches under files, like suggested by Mikael. Adding USES=gmake now does the job.

Nuno, could you please try, if the new shar file works for you? Thanks in advance.
Comment 11 Nuno Teixeira freebsd_committer 2020-11-30 08:51:33 UTC
(In reply to Rainer Hurling from comment #10)

It compiles ok with gmake patches.

I didn't know about 11.4 make not -j compatible.
This was a great lesson that I've learned and next time I will be more carefull testing on both 11.x and 12.x versions.

Please commit "Patch with gmake correction" shar.

Thanks very much
Comment 12 Mikael Urankar freebsd_committer 2020-11-30 09:13:42 UTC
(In reply to Rainer Hurling from comment #10)
How hard would it be to make the dep on gmake conditionnal and only installed on 11.4?
If it's too complicated, can you put a comment that says the gmake dep can be removed when 11.4 goes eol?
Comment 13 Rainer Hurling freebsd_committer 2020-11-30 10:03:28 UTC
(In reply to Mikael Urankar from comment #12)
Hi Mikael,

Thanks again for the suggestion. I will try to find a way to use gmake only optionally for 11.x and report back here ;)
Comment 14 Nuno Teixeira freebsd_committer 2020-11-30 12:14:26 UTC
Looks like I'm feeling some butterfly effect in here.
The port is mine!
All mine!
(portuguese humor!)
Comment 15 Rainer Hurling freebsd_committer 2020-11-30 15:44:30 UTC
(In reply to Nuno Teixeira from comment #14)

> Looks like I'm feeling some butterfly effect in here.
Better a butterfly effect than a tsunami ;)

> The port is mine!
> All mine!
> (portuguese humor!)
> Thanks!
It also remains yours :))

It is a bit tricky to use gmake only for FreeBSD 11.x. For example, it could be used in a condition like

.if ${OPSYS} == FreeBSD && ${OSVERSION} < 1200000
EXTRA_PATCHES=${FILESDIR}/extra-patch-cargo-crates_servo-fontconfig-sys-4.0.9_build.rs ${FILESDIR}/extra-patch-cargo-crates_servo-fontconfig-sys-4.0.9_makefile.cargo
Comment 16 Mikael Urankar freebsd_committer 2020-11-30 16:23:32 UTC
(In reply to Rainer Hurling from comment #15)
Ok, forget about it, just put a comment why it's needed and that it can be removed when 11.4 goes eol
Comment 17 Rainer Hurling freebsd_committer 2020-12-01 08:10:29 UTC
Created attachment 220132 [details]
cast2gif with better version numbering and gmake

Hi Nuno,

After a more systematic investigation I am now quite sure that the problem described in comment #5 and comment #7 does not only occur with FreeBSD 11.4, but in all versions 11.4, 12.2 and HEAD.

In all cases the solution is to use gmake. Affected is the cargo package servo-fontconfig-sys, as found by Mikael. In ${PORTS}/Mk/Uses/cargo.mk:143 there is an example of a similar case with the cargo package jemalloc-sys.

So I suggest to use gmake in the port in general.

cast2gif currently has pre-release status on github. The internal version number in the sources is 0.1.0. The current naming of the port 'cast2gif-p-r' is not only unattractive. At the latest when version numbers are used in the next update, there may be problems which can only be solved by using PORTEPOCH. Therefore I suggest to use the DISTVERSION 0.1.0-pre1. This will not cause problems with later updates.

Since the pre-release there are two more commits on Github which should be considered in the port [1]. I have already integrated them via GH_TAGNAME=4f76eeb.

[1] https://github.com/katharostech/cast2gif/compare/pre-release...master

A new patch (shar file) is attached, which contains all changes mentioned. This patch is tested under Poudriere for 11.4, 12.2 and HEAD, both amd64 and i386. If you, Nuno, agree, I would like to commit it like this for you :)
Comment 18 Nuno Teixeira freebsd_committer 2020-12-01 08:54:01 UTC
(In reply to Rainer Hurling from comment #17)

I've used "pre-release" like I normaly use "g20201201" thinking in the future versions and to avoid PORTEPOCH.

But it is a better version numbering like you said and I will use it as example for future ports.

Please commit "cast2gif with better version numbering and gmake" shar.

Thanks all for help that I receive in this port.
I've learned alot.

Comment 19 Rainer Hurling freebsd_committer 2020-12-01 09:43:00 UTC
(In reply to Nuno Teixeira from comment #18)

Since I am a new, mentored committer, I created a review in Phabricator [1]. If you are interested, you can follow up there if there are further ideas and remarks of the mentors ;)

[1] https://reviews.freebsd.org/D27435
Comment 20 Nuno Teixeira freebsd_committer 2020-12-01 09:52:29 UTC
(In reply to Rainer Hurling from comment #19)

I will follow.
Comment 21 Rainer Hurling freebsd_committer 2020-12-01 13:24:48 UTC
(In reply to Rainer Hurling from comment #19)

I had a first reaction from Gleb Popov, one of my mentors. He mentioned a problem with the usage of PORTDOCS, INSTALL_MAN etc.

Because there is no man page ATM, I will correct the code for README.md.

Another point: There are two gif files in the sources doc dir. I would like to install these both in a ports examples dir, like shown in my updated review. Hope, this is ok.
Comment 22 Nuno Teixeira freebsd_committer 2020-12-01 15:08:43 UTC
(In reply to Rainer Hurling from comment #21)

Porters handbook says: "INSTALL_MAN is a command to install manpages and other documentation (it does not compress anything)."

Comment 23 Rainer Hurling freebsd_committer 2020-12-01 15:11:12 UTC
(In reply to Nuno Teixeira from comment #22)

The review is already accepted now and ready to commit! 

And yes, with INSTALL_DATA instead of INSTALL_MAN. And with EXAMPLES (the both gif files), see review please.

If you agree with the latest changes (gif examples), I would commit your port now :)

Comment 24 Nuno Teixeira freebsd_committer 2020-12-02 07:22:38 UTC
(In reply to Rainer Hurling from comment #23)

Nice work!

Please commit.

I will review all the ports that I maintain and correct those.

Thanks very much,

Comment 25 commit-hook freebsd_committer 2020-12-02 08:15:21 UTC
A commit references this bug:

Author: rhurlin
Date: Wed Dec  2 08:15:02 UTC 2020
New revision: 556806
URL: https://svnweb.freebsd.org/changeset/ports/556806

  textproc/cast2gif: Add port 0.1.0 pre-release

  Tool to render Asciinema cast files to GIFs without
  using Electron or a web browser

  The port contains the pre-release [1] plus two commits,
  added since then.

  [1] https://github.com/katharostech/cast2gif/releases

  PR:		251064
  Submitted by:	Nuno Teixeira <ed.arrakis@gmail.com> (maintainer)
  Approved by:	arrowd (mentor)
  Differential Revision:	https://reviews.freebsd.org/D27435

Comment 26 Rainer Hurling freebsd_committer 2020-12-02 08:23:29 UTC
Committed, thanks :)
Comment 27 Zicklag 2020-12-02 16:06:02 UTC
Awesome! Thanks for working on this all, and for starting this initiative Nuno. I don't use FreeBSD myself, but it's cool to have my tool in the FreeBSD ports. Hopefully this will help cast2gif help more people out. :)