Bug 267777 - devel/cdialog: Makefile corrupts examples
Summary: devel/cdialog: Makefile corrupts examples
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: Robert Clausecker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-15 09:03 UTC by Philipp Engel
Modified: 2023-02-19 11:16 UTC (History)
4 users (show)

See Also:
fuz: maintainer-feedback? (jcpierri)


Attachments
Solution proposed by upstream author/maintainer (512 bytes, patch)
2022-11-20 23:57 UTC, Josmar Calin De Pierri
jcpierri: maintainer-approval+
Details | Diff
cdialog-1.3.20220728_1,2.patch (862 bytes, patch)
2022-11-21 16:21 UTC, takefu
takefu: maintainer-approval+
Details | Diff
Upgrade to version 1.3-20221229 (811 bytes, patch)
2022-12-31 13:31 UTC, Josmar Calin De Pierri
jcpierri: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Engel 2022-11-15 09:03:59 UTC
The Makefile of the port has the following pre-patch macro for the samples directory:

pre-patch-EXAMPLES-on:
	@${FIND} ${WRKSRC}/samples/ -type f -exec ${REINPLACE_CMD} -i "" "s|dialog|cdialog|g" {} \;

This leads to corrupted sample configurations (*.rc). The macro should be changed to:

pre-patch-EXAMPLES-on:
	@${FIND} ${WRKSRC}/samples/ -type f -not -name "*.rc" -exec ${REINPLACE_CMD} -i "" "s|dialog|cdialog|g" {} \;
Comment 1 Thomas E. Dickey 2022-11-16 09:13:19 UTC
For the specific problem (breakage of the ".rc" files),
that can be fixed by limiting the changes to full words,
e.g., using "\<" and "\>" delimiters in the sed command.
Comment 2 Josmar Calin De Pierri 2022-11-20 23:57:14 UTC
Created attachment 238199 [details]
Solution proposed by upstream author/maintainer
Comment 3 Josmar Calin De Pierri 2022-11-21 00:01:50 UTC
This way we resolve the issue keeping the intended behavior of the pre-patch (changing all occurrences of the name, including those in the RC files, without interfering with code)
Comment 4 takefu 2022-11-21 16:21:00 UTC
Created attachment 238216 [details]
cdialog-1.3.20220728_1,2.patch

(In reply to Josmar from comment #3)

With the submitted patch, the difference is overwhelmingly reduced when testing, but it doesn't disappear completely.


# make test
===>  Testing for cdialog-1.3.20220728_1,2
Verify that create-rc works
/bin/sh -c "DIALOG=./cdialog ./run_test.sh ./samples"
** ./samples/debian.rc
** ./samples/slackware.rc
--- ./samples/slackware.rc	2022-11-21 16:09:42.888301000 +0000
+++ ./samples/slackware.rc-test	2022-11-21 16:09:42.884590000 +0000
@@ -83,8 +83,8 @@

 border2_color = dialog_color

-inputbox_border2_color = border2_color
+inputbox_border2_color = dialog_color

-searchbox_border2_color = border2_color
+searchbox_border2_color = dialog_color

-menubox_border2_color = border2_color
+menubox_border2_color = dialog_color
** ./samples/sourcemage.rc
** ./samples/suse.rc
** ./samples/whiptail.rc


samples/slackware.rc is the color specification for slackware, so you don't have to worry about it.
Comment 5 Philipp Engel 2022-11-22 14:50:01 UTC
(In reply to takefu from comment #4)
> samples/slackware.rc is the color specification for slackware, so you don't have to worry about it.

Please think of the FreeBSD users who prefer the slackware theme over the default.
Comment 6 Josmar Calin De Pierri 2022-11-24 00:11:22 UTC
Yeah ...

I'm a long time user of Slackware too, been using it longer than FreeBSD, so I'll have to find a way to fix this glitch, without damaging anything else, of course ...
Comment 7 Josmar Calin De Pierri 2022-11-24 22:11:36 UTC
The (good|bad) news is that it happens on Slackware too, using the TAR pulled from upstream:

$ cat /etc/slackware-version
Slackware 15.0

$ sh -c "DIALOG=./dialog ./run_test.sh ./samples"
** ./samples/debian.rc
** ./samples/slackware.rc
--- ./samples/slackware.rc      2022-11-24 18:55:02.826023270 -0300
+++ ./samples/slackware.rc-test 2022-11-24 18:55:02.822023270 -0300
@@ -83,8 +83,8 @@

 border2_color = dialog_color

-inputbox_border2_color = border2_color
+inputbox_border2_color = dialog_color

-searchbox_border2_color = border2_color
+searchbox_border2_color = dialog_color

-menubox_border2_color = border2_color
+menubox_border2_color = dialog_color
** ./samples/sourcemage.rc
** ./samples/suse.rc
** ./samples/whiptail.rc
Comment 8 Thomas E. Dickey 2022-11-27 19:23:19 UTC
hmm - the reason for the difference is that dialog is using only the values
of the colors in those classes, and the assignment border2_color=dialog_color
isn't needed to obtain the same screen colors.

At the moment, dialog's 4th on a list of programs that I'm working on.
I'll keep this one in mind when I'm working on dialog, to see if/how
to improve it.

(note that "make check" doesn't return an error).
Comment 9 Josmar Calin De Pierri 2022-12-31 13:31:37 UTC
Created attachment 239159 [details]
Upgrade to version 1.3-20221229

https://invisible-island.net/dialog/CHANGES
Comment 10 Robert Clausecker freebsd_committer freebsd_triage 2023-02-15 13:48:56 UTC
Please note that REINPLACE_CMD must only be used for dynamic replacements.  Please replace this use with one or more patch files when you touch the port next time.

I will commit both the bug fix and the update to 1.3-20221229.  Is this correct?
Comment 11 Thomas E. Dickey 2023-02-16 00:08:17 UTC
apparently correct (the patch plus my change for slackware.rc).

In later changes I added an "install-examples" makefile rule,
thinking of this and some other packages, which would make the
sed command moot.  However, that makefile rule preserves the
executable permissions of the scripts which doesn't appear to
fall in the guidelines here.  (I'll probably change that...).

However, I see that this port still has the ftp url (which
my ISP said would go away -- seems to be gone now.

The MASTER_SITES should be

MASTER_SITES=   https://invisible-island.net/archives/${PORTNAME:S|^c||}/\
                https://invisible-mirror.net/archives/${PORTNAME:S|^c||}/
Comment 12 Robert Clausecker freebsd_committer freebsd_triage 2023-02-16 00:30:01 UTC
(In reply to Thomas E. Dickey from comment #11)

The changeset I plan to commit does not have any changes to slackware.rc.  Is this something you changed upstream?

Plan to change MASTER_SITES as advised.  Maintainer, do you approve of this change?

The full changeset I plan to commit is thus:

diff --git a/devel/cdialog/Makefile b/devel/cdialog/Makefile
index 49fd3d4d1a99..cd5f3cad2e55 100644
--- a/devel/cdialog/Makefile
+++ b/devel/cdialog/Makefile
@@ -1,8 +1,8 @@
 PORTNAME=      cdialog
-DISTVERSION=   1.3-20220728
+DISTVERSION=   1.3-20221229
 PORTEPOCH=     2
 CATEGORIES=    devel
-MASTER_SITES=  ftp://ftp.invisible-island.net/pub/${PORTNAME:S|^c||}/\
+MASTER_SITES=  https://invisible-island.net/archives/${PORTNAME:S|^c||}/\
                https://invisible-mirror.net/archives/${PORTNAME:S|^c||}/
 DISTNAME=      ${PORTNAME:S|^c||}-${PORTVERSION:R}-${PORTVERSION:E}
 
@@ -24,10 +24,11 @@ CONFIGURE_ARGS=     --enable-widec \
                --with-curses-dir=${NCURSESBASE}
 MAKEFILE=      makefile
 INSTALL_TARGET=        install-strip install-man install-lib
+TEST_TARGET=   check
 OPTIONS_DEFINE=        EXAMPLES
 
 pre-patch-EXAMPLES-on:
-       @${FIND} ${WRKSRC}/samples/ -type f -exec ${REINPLACE_CMD} -i "" "s|dialog|cdialog|g" {} \;
+       @${FIND} ${WRKSRC}/samples/ -type f -exec ${REINPLACE_CMD} -i "" "s|\<dialog\>|cdialog|g" {} \;
 
 pre-install:
        @${STRIP_CMD} ${WRKSRC}/.libs/libcdialog.so.15.0.0
diff --git a/devel/cdialog/distinfo b/devel/cdialog/distinfo
index 1022f9f87a54..809bff700f4b 100644
--- a/devel/cdialog/distinfo
+++ b/devel/cdialog/distinfo
@@ -1,3 +1,3 @@
-TIMESTAMP = 1662918811
-SHA256 (dialog-1.3-20220728.tgz) = 54418973d559a461b00695fafe68df62f2bc73d506b436821d77ca3df454190b
-SIZE (dialog-1.3-20220728.tgz) = 568086
+TIMESTAMP = 1672491666
+SHA256 (dialog-1.3-20221229.tgz) = d5663d016003e5260fa485f5e9c2ddffb386508f3bd0687d4fa3635ea9942b8e
+SIZE (dialog-1.3-20221229.tgz) = 567965
Comment 14 commit-hook freebsd_committer freebsd_triage 2023-02-19 11:03:16 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=346b5384e0807a1569d4c8ba18c1c8f3a7871444

commit 346b5384e0807a1569d4c8ba18c1c8f3a7871444
Author:     Josmar <jcpierri@gmail.com>
AuthorDate: 2023-02-15 13:46:39 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2023-02-19 11:01:41 +0000

    devel/cdialog: update to 1.3-20221229

     - fix MASTER_SITES as advised by upstream
     - wire up TEST_TARGET
     - bracket regex in REINPLACE_CMD to avoid replacing unrelated variables

    Changelog: https://invisible-island.net/dialog/CHANGES

    PR:             267777
    Reported by:    Philipp Engel <kidon@posteo.de>
    Approved by:    flo (mentor)
    Differential Revision: https://reviews.freebsd.org/D38637

 devel/cdialog/Makefile | 7 ++++---
 devel/cdialog/distinfo | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)
Comment 15 Robert Clausecker freebsd_committer freebsd_triage 2023-02-19 11:16:39 UTC
Thank you for your submission.