Re: git: 18f20d5d967a - main - bsddialog: convert bsdinstall/distextract

On Sat, 1 Jan 2022 22:40:02 +0100
Baptiste Daroussin <bapt@FreeBSD.org> wrote:

> On Sat, Jan 01, 2022 at 09:35:50PM +0000, Jessica Clarke wrote:
> > On 1 Jan 2022, at 20:52, Baptiste Daroussin <bapt@FreeBSD.org> wrote:
> > > 
> > > The branch main has been updated by bapt:
> > > 
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=18f20d5d967ae790f121963e1fcee68d729a529e
> > > 
> > > commit 18f20d5d967ae790f121963e1fcee68d729a529e
> > > Author:     Alfonso Siciliano <alfsiciliano@gmail.com>
> > > AuthorDate: 2022-01-01 20:50:44 +0000
> > > Commit:     Baptiste Daroussin <bapt@FreeBSD.org>
> > > CommitDate: 2022-01-01 20:51:23 +0000
> > > 
> > >    bsddialog: convert bsdinstall/distextract
> > > 
> > >    Differential Revision:  https://reviews.freebsd.org/D33581
> > > ---
> > > usr.sbin/bsdinstall/distextract/Makefile      |   3 +-
> > > usr.sbin/bsdinstall/distextract/distextract.c | 180 +++++++++++++-------------
> > > 2 files changed, 92 insertions(+), 91 deletions(-)
> > > 
> > > diff --git a/usr.sbin/bsdinstall/distextract/Makefile b/usr.sbin/bsdinstall/distextract/Makefile
> > > index 2b7180e28ea0..5e9f2b9e1473 100644
> > > --- a/usr.sbin/bsdinstall/distextract/Makefile
> > > +++ b/usr.sbin/bsdinstall/distextract/Makefile
> > > @@ -2,7 +2,8 @@
> > > 
> > > BINDIR= ${LIBEXECDIR}/bsdinstall
> > > PROG=	distextract
> > > -LIBADD=	archive dpv dialog m
> > > +CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib
> > > +LIBADD=	archive bsddialog m
> > > 
> > > MAN=
> > > 
> > > diff --git a/usr.sbin/bsdinstall/distextract/distextract.c b/usr.sbin/bsdinstall/distextract/distextract.c
> > > index 8ad6c7b2c64b..df10c299ab7f 100644
> > > --- a/usr.sbin/bsdinstall/distextract/distextract.c
> > > +++ b/usr.sbin/bsdinstall/distextract/distextract.c
> > > @@ -33,11 +33,12 @@ __FBSDID("$FreeBSD$");
> > > #include <sys/param.h>
> > > #include <archive.h>
> > > #include <ctype.h>
> > > -#include <dialog.h>
> > > -#include <dpv.h>
> > > +#include <bsddialog.h>
> > > +#include <bsddialog_progressview.h>
> > > #include <err.h>
> > > #include <errno.h>
> > > #include <limits.h>
> > > +#include <signal.h>
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > #include <string.h>
> > > @@ -46,30 +47,27 @@ __FBSDID("$FreeBSD$");
> > > /* Data to process */
> > > static char *distdir = NULL;
> > > static struct archive *archive = NULL;
> > > -static struct dpv_file_node *dists = NULL;
> > > 
> > > /* Function prototypes */
> > > static void	sig_int(int sig);
> > > static int	count_files(const char *file);
> > > -static int	extract_files(struct dpv_file_node *file, int out);
> > > +static int	extract_files(struct bsddialog_fileminibar *file);
> > > 
> > > -#define _errx(...) (end_dialog(), errx(__VA_ARGS__))
> > > +#define _errx(...) (bsddialog_end(), errx(__VA_ARGS__))
> > > 
> > > int
> > > main(void)
> > > {
> > > 	char *chrootdir;
> > > 	char *distributions;
> > > +	unsigned int i;
> > > 	int retval;
> > > -	size_t config_size = sizeof(struct dpv_config);
> > > -	size_t file_node_size = sizeof(struct dpv_file_node);
> > > +	size_t minibar_size = sizeof(struct bsddialog_fileminibar);
> > > 	size_t span;
> > > -	struct dpv_config *config;
> > > -	struct dpv_file_node *dist = dists;
> > > -	static char backtitle[] = "FreeBSD Installer";
> > > -	static char title[] = "Archive Extraction";
> > > -	static char aprompt[] = "\n  Overall Progress:";
> > > -	static char pprompt[] = "Extracting distribution files...\n";
> > > +	unsigned int nminibars;
> > > +	struct bsddialog_fileminibar *dists;
> > > +	struct bsddialog_progviewconf pvconf;
> > > +	struct bsddialog_conf conf;
> > > 	struct sigaction act;
> > > 	char error[PATH_MAX + 512];
> > > 
> > > @@ -78,17 +76,17 @@ main(void)
> > > 	if ((distdir = getenv("BSDINSTALL_DISTDIR")) == NULL)
> > > 		distdir = __DECONST(char *, "");
> > > 
> > > -	/* Initialize dialog(3) */
> > > -	init_dialog(stdin, stdout);
> > > -	dialog_vars.backtitle = backtitle;
> > > -	dlg_put_backtitle();
> > > -
> > > -	dialog_msgbox("",
> > > -	    "Checking distribution archives.\nPlease wait...", 4, 35, FALSE);
> > > -
> > > -	/*
> > > -	 * Parse $DISTRIBUTIONS into dpv(3) linked-list
> > > -	 */
> > > +	if (bsddialog_init() == BSDDIALOG_ERROR)
> > > +		errx(EXIT_FAILURE, "Cannot init libbsdialog");
> > > +	bsddialog_initconf(&conf);
> > > +	bsddialog_backtitle(&conf, __DECONST(char *, "FreeBSD Installer"));
> > 
> > Can all these interfaces really not just use a const char * rather than
> > having to scatter __DECONST everywhere if you want to pass a string
> > literal to functions like this? There are quite a few __DECONST?s of
> > string literals in this patch alone...
> > 
> > Jess
> 
> Adding Alfonso to CC.
> 
> Yes I agree, the __DECONST are from me, not from Alfonso, I added them to avoid
> having to play with WARNS and make progress in the conversion anyway. Then start
> the discussion with Alfonso to see how we can improve this.
> 
> Best regrads,
> Bapt

Hi Jessica,

Thank you for your report,

Briefly:
I know the problem and it is in the TODO list, I hope to improve soon,
it was very tedious "__DECONST" each string in bsdinstall.

Rationale: 
libbsddialog is a new library without real world use cases, indeed it is
private in BASE; so far we are adding it "for steps" in PORTS and BASE.

I implemented the features to replace LGPL libdialog, now we are working
mainly to test/fix/improve the dialogs (infobox, checklist, form, etc)
to complete the "library-migration" process.
Fortunately this phase is ending (only 2 reviews in progress), then
I' ll improve the lib/API: char*/const char*, mixedgauge, wrapping
text autosize algorithm, re-update bsdinstall.
Finally, the last step: bsddialog utility in bsdinstall/scripts.

Please let me know any problem or if you need some new or GPL-dialog
feature in bsddialog.

Best regards,
Alfonso

--- 
Alfonso S. Siciliano 
   http://alfonsosiciliano.gitlab.io