Page MenuHomeFreeBSD

bsdinstall: implement timezone with bsddialog
Needs ReviewPublic

Authored by khorben_defora.org on Apr 7 2024, 5:31 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 3:33 PM
Unknown Object (File)
Mon, Dec 9, 2:25 AM
Unknown Object (File)
Wed, Dec 4, 9:14 PM
Unknown Object (File)
Sun, Dec 1, 4:15 AM
Unknown Object (File)
Sun, Dec 1, 4:07 AM
Unknown Object (File)
Thu, Nov 28, 3:58 PM
Unknown Object (File)
Thu, Nov 28, 3:58 PM
Unknown Object (File)
Thu, Nov 28, 3:36 PM
Subscribers

Details

Summary

This effectively imports most of the timezone script from bsdconfig to bsdinstall. This allows:

  • invoking bsddialog(1) in line with bsdinstall(8)
  • keeping the backtitle consistent with that of the installer
  • eventually, direct support for graphical mode

Sponsored by: The FreeBSD Foundation

Test Plan
# bsdinstall finalconfig
# bsdinstall timezone

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Why do we need a copy of this code rather than just tweaking bsdconfig to support this?

Why do we need a copy of this code rather than just tweaking bsdconfig to support this?

My initial changes drifted a lot more from the original code of bsdconfig, like replacing every occurence of $DIALOG_BACKTITLE with $OSNAME Installer. I have tried to minimize the changes to the bare minimum for this series of reviews, and to restrict the changes to usr.sbin/bsdinstall for clarity.

However I can look into creating a different version of this replacement of timezone with some tweaking in bsdconfig instead.

However I can look into creating a different version of this replacement of timezone with some tweaking in bsdconfig instead.

Just my tip, if possible you could avoid to change bsdconfig, it is huge and heavy designed on dialog and xdialog. Furthermore a bsdconfig refactoring was in discussion, I would keep things as they are for future developments.

I ' ll propose another solution to add GUI timezone like a simple increment not a big change. So we can start discussing/understanding how to start with a multi frontend installer.

Probably the review title and description can be changed, because it wants to introduce GUI for timezone. (Otherwise, if its purpose is adding backtitle to tzsetup (timezone) we can add just 5-6 lines to tzsetup.c :D). Maybe:

Title. bsdinstall: prepare GUI timezone
Desc. Prepare timezone for GUI ...etc...

We are switching actually from a single frontend to a multi frontend installer: current TUI, new GUI, and probably new CLI for vision-impaired users; [bsd|gbsd|cli]dialog. Moreover, partially (and theoretically) we also have dialog and xdialog (to discuss in the future).

Furthermore we have an incomplete installer: direct calls to the (single) frontend and calls via bsdconfig "API" (to note bsdconfig is designed for dialog and xdialog). I wrote some public message in the past, ended with bikeshed and non-technical discussions. I' ll try to have a workgroup (eurobsdcon?) to discuss about bsdinstall, moreover some discussion exists for a refactoring/rewrite.

usr.sbin/bsdinstall/scripts/time
32

Clearly, here TUI solution and GUI solution are completely different. I'd keep things simple, instead to try to merge them. Other scripts use if/else to handle multi frontend. I would add something like:

if [ $DIALOG == "gbsddialog" ]
	timezone_gbsddialog
else
	tzsetup
fi

This should avoid to change finalconfig.

Some script uses $USE_XDIALOG, I would avoid this variable because it is related to bsdconfig to handle xdialog (another topic to discuss in the future); $DIALOG == "gbsddialog" is obvious and clear for future developments.

(I' ll add a new case for CLI in the future, something like if [ $DIALOG == "clidialog" ] tzsetup --cli)

usr.sbin/bsdinstall/scripts/timezone
1

This file can be renamed (maybe timezone_gbsddialog), so we can "isolate" GUI code; simple and clear for future changes/developments/improvements.

29

Here I would add a comment for rationale and future developments, maybe:

#Copy and adapt 'bsdconfig timezone' for gbsddialog because bsdconfig is
#designed for xdialog. ...etc...
#In the future we can add GUI lib to tzsetup ...etc....
#What you changed in this file ...etc...
#Rationale ...etc...

93

I would want to understand the gbsddialog autosizing and position.

Does gbsddialog provide autosizing? Does it need bsdconfig sizing API?
Is gbsddialog suitable for bsdconfig (that is xdialog) autosizing? For example xdialog has buttons below while gbsddialog in the title bar, how is this handled?

Does gbsddialog need bsdconfig auto position API? Does it provide auto position?

99

In this file gbsddialog instead of $DIALOG can be sufficient.
(As guideline for bsdinstall we can avoid to use var $DIALOG when we want to use only one frontend.)

213

The TUI code can be deleted, we could avoid reinventing the wheel (tzsetup). Obviously all the code unrelated to gbsddialog can be deleted.

usr.sbin/bsdinstall/scripts/time
32

Actually here my intention is not to separate the code between the TUI and GUI solutions. I am simply decoupling setting the date & time step from the one setting the timezone, for clarity. It happened to me once that I wanted to set the date & time in the finalconfig step, while knowing that the timezone was correct, and therefore I thought this was no longer possible.
Therefore there should be no need to detect GUI vs TUI in this patch.

usr.sbin/bsdinstall/scripts/timezone
93

I have implemented the auto-sizing of gbsddialog so that it works exactly like in bsdconfig. The algorithm is currently as follows:

  • obtain the current workarea from Gtk+: src/common.c, get_workarea()
  • obtain the default font size from Pango: src/common.c, get_font_size()
  • divide the height twice by the font size, remove 9 lines for the title and bottom
  • divide the width by the font size, remove 4 columns for a bit of margin on both sides

This can be seen in src/gbsddialog.c, where the handling of PRINT_MAXSIZE therefore returns a value in columns and rows available with the default font.
In my tests I am usually between 80+ and ~150 columns, and over 25 lines available, which should more than match the 80x25 of a VGA screen.

Regarding positioning, gbsddialog hints the window manager to position the window in the center of the primary screen, as per Gtk+' API gtk_window_set_position(GTK_WIN_POS_CENTER), see _builder_dialog() in src/builders.c.

My aim is to behave completely identically to bsddialog.

213

I would rather follow @jrtc27's advice and find a way to re-use the timezone code from bsdconfig, instead of keeping a slightly modified copy in bsdinstall as per the current version of this patch. I will look into that.
bsdinstall already depends heavily on bsdconfig, and there should be no perceptible performance impact in comparison with tzsetup.