Skip to content

Commit

Permalink
Give the user some control over progressbar invasiveness
Browse files Browse the repository at this point in the history
Progress reporting is useful but also has its drawbacks, like writing
to the calling terminal when it's not in the appropriate state, or
slowing down the process being reported on. This commit addresses both
situations:

1. a set of 3 configuration variables is introduced with which the user
can control (via macports.conf) after how long the progressbar should
appear, if it should also show indeterminate progress (aka "we're busy")
and what operations should report progress in addition to the traditional
ones (fetch, build, destroot). The defaults for these are the current
hardwired settings. The "also_for" setting is designed to be a list that
currently only accepts "de/activation" for the recent progress reporting
added to `port activate` and `port deactivate`. (Which causes a significant
increase in duration of those operations.)

2. a Pextlib function is introduced to determine whether `port` runs as a
foreground process connected to a terminal. This function is called to
check the current state before doing any actual progressbar output. It
thus becomes possible to push a long-running operation (say, a build) to
the background and do other work in the same terminal, without getting
frequent terminal and most likely illegible output.
  • Loading branch information
RJVB committed Aug 25, 2024
1 parent 96520f2 commit 7535dcc
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 17 deletions.
14 changes: 14 additions & 0 deletions doc/macports.conf.in
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,20 @@ variants_conf @MPCONFIGDIR_EXPANDED@/variants.conf
# Keep logs after successful installations.
#keeplogs no

# Progress reporting.
# MacPorts will show a progressbar during lengthy operations which take more
# than $progressbar_after milliseconds. The bar reports determinate progress
# when the total amount of work and the work done are quantified. It can also
# show an "indeterminate" indication of work being done (and switch to this
# display after a certain amount of time) if $progressbar_type is set to `both`.
# The operations that use this feature are downloads, builds and rev-upgrade,
# plus a list of additional operations $progressbar_also_for which can include
# `de/activation` (for `port deactivate` and `port activate`) and possibly more
# in the future. NB: no quotes for the string values!
# progressbar_after 500
# progressbar_type both
# progressbar_also_for de/activation

# URLs that MacPorts attempts to download to find out whether a new version was
# released. Multiple values, space-separated; only one of the URLs needs to be
# available. Downloads will be attempted in the specified order.
Expand Down
15 changes: 15 additions & 0 deletions src/macports1.0/macports.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ namespace eval macports {
foreach opt [list binpath auto_path extra_env portdbformat \
portarchivetype portimage_mode hfscompression portautoclean \
porttrace portverbose keeplogs destroot_umask release_urls release_version_urls \
progressbar_after progressbar_type progressbar_also_for \
rsync_server rsync_options rsync_dir \
startupitem_autostart startupitem_type startupitem_install \
place_worksymlink xcodeversion xcodebuildcmd xcodecltversion xcode_license_unaccepted \
Expand All @@ -78,6 +79,7 @@ namespace eval macports {
registry.path registry.format user_home user_path user_ssh_auth_sock \
portarchivetype portarchive_hfscompression archivefetch_pubkeys \
portautoclean portimage_mode porttrace keeplogs portverbose destroot_umask \
progressbar_after progressbar_type progressbar_also_for \
rsync_server rsync_options rsync_dir startupitem_autostart startupitem_type startupitem_install \
place_worksymlink macportsuser sudo_user \
configureccache ccache_dir ccache_size configuredistcc configurepipe buildnicevalue buildmakejobs \
Expand Down Expand Up @@ -983,6 +985,9 @@ proc mportinit {{up_ui_options {}} {up_options {}} {up_variations {}}} {
macports::cxx_stdlib \
macports::hfscompression \
macports::portarchive_hfscompression \
macports::progressbar_after \
macports::progressbar_type \
macports::progressbar_also_for \
macports::host_cache \
macports::porturl_prefix_map \
macports::ui_options \
Expand Down Expand Up @@ -1460,6 +1465,16 @@ Please edit sources.conf and change '$url' to '[string range $url 0 26]macports/
}
set portarchive_hfscompression $hfscompression

if {![info exists progressbar_after]} {
set progressbar_after 500
}
if {![info exists progressbar_type]} {
set progressbar_type "both"
}
if {![info exists progressbar_also_for]} {
set progressbar_also_for "de/activation"
}

# Set rync options
if {![info exists rsync_server]} {
set rsync_server rsync.macports.org
Expand Down
36 changes: 36 additions & 0 deletions src/pextlib1.0/Pextlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,40 @@ int ClonefileCmd(ClientData clientData UNUSED, Tcl_Interp *interp, int objc, Tcl
return TCL_ERROR;
}

/*
processIsForeground
synopsis: processIsForeground
Returns true if the process is running in the foreground or false
if in the background.
*/
int IsProcessForegroundCmd(ClientData clientData UNUSED, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[])
{
/* Check the arg count */
if (objc != 1) {
Tcl_WrongNumArgs(interp, 1, objv, NULL);
return TCL_ERROR;
}

int fd, fdo = fileno(stdout);
if ((fd = isatty(fdo) ? fdo : open("/dev/tty", O_RDONLY)) != -1) {
const pid_t pgrp = getpgrp();
const pid_t tcpgrp = tcgetpgrp(fd);
if (fd != fdo) {
close(fd);
}
if (pgrp != -1 && tcpgrp != -1) {
Tcl_SetObjResult(interp, Tcl_NewBooleanObj(pgrp == tcpgrp));
return TCL_OK;
}
}
Tcl_SetErrno(errno);
Tcl_ResetResult(interp);
Tcl_AppendResult(interp, "processIsForeground: ", (char *)Tcl_PosixError(interp), NULL);
return TCL_ERROR;
}

int Pextlib_Init(Tcl_Interp *interp)
{
if (Tcl_InitStubs(interp, "8.4", 0) == NULL)
Expand Down Expand Up @@ -1214,6 +1248,8 @@ int Pextlib_Init(Tcl_Interp *interp)
Tcl_CreateObjCommand(interp, "fs_clone_capable", FSCloneCapableCmd, NULL, NULL);
Tcl_CreateObjCommand(interp, "clonefile", ClonefileCmd, NULL, NULL);

Tcl_CreateObjCommand(interp, "processIsForeground", IsProcessForegroundCmd, NULL, NULL);

if (Tcl_PkgProvide(interp, "Pextlib", "1.0") != TCL_OK)
return TCL_ERROR;

Expand Down
34 changes: 22 additions & 12 deletions src/port/port.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -4829,9 +4829,11 @@ namespace eval portclient::progress {
proc initDelay {} {
variable show
variable startTime
variable showTimeThreshold

set startTime [clock milliseconds]
set show no
set showTimeThreshold ${macports::progressbar_after}
}

##
Expand Down Expand Up @@ -4907,7 +4909,9 @@ namespace eval portclient::progress {
}
update {
lassign $args now total
if {[showProgress $now $total] eq "yes"} {
# Check on each update if we're still outputting to a tty - the user can
# have pushed us into the background.
if {[processIsForeground] && ([showProgress $now $total] eq "yes")} {
set barPrefix " "
set barPrefixLen [string length $barPrefix]
if {$total != 0} {
Expand All @@ -4919,11 +4923,13 @@ namespace eval portclient::progress {
}
intermission -
finish {
# erase to start of line
::term::ansi::send::esolch stderr
# return cursor to start of line
puts -nonewline stderr "\r"
flush stderr
if {[processIsForeground]} {
# erase to start of line
::term::ansi::send::esolch stderr
# return cursor to start of line
puts -nonewline stderr "\r"
flush stderr
}
}
}

Expand Down Expand Up @@ -4955,7 +4961,9 @@ namespace eval portclient::progress {
}
update {
lassign $args type total now speed
if {[showProgress $now $total] eq "yes"} {
# Check on each update if we're still outputting to a tty - the user can
# have pushed us into the background.
if {[processIsForeground] && ([showProgress $now $total] eq "yes")} {
set barPrefix " "
set barPrefixLen [string length $barPrefix]
if {$total != 0} {
Expand All @@ -4974,11 +4982,13 @@ namespace eval portclient::progress {
}
}
finish {
# erase to start of line
::term::ansi::send::esolch stderr
# return cursor to start of line
puts -nonewline stderr "\r"
flush stderr
if {[processIsForeground]} {
# erase to start of line
::term::ansi::send::esolch stderr
# return cursor to start of line
puts -nonewline stderr "\r"
flush stderr
}
}
}

Expand Down
12 changes: 9 additions & 3 deletions src/port1.0/portprogress.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ namespace eval portprogress {
variable indeterminate_timer 0

# If our progress callback should issue indeterminate progress updates
variable allow_indeterminate yes
variable indeterminate yes

# ninja ([<completed tasks>/<pending tasks>])
Expand All @@ -53,7 +54,8 @@ namespace eval portprogress {
# A SystemCmd callback that parses common target progress formats to display
# a progress bar
proc portprogress::target_progress_callback {event} {
global portverbose
global portverbose progressbar_type
variable allow_indeterminate
variable indeterminate
variable indeterminate_timer
variable indeterminate_threshold
Expand All @@ -66,7 +68,11 @@ proc portprogress::target_progress_callback {event} {

switch -- [dict get $event type] {
exec {
set indeterminate yes
# reset allow_indeterminate to the user/default-configured setting
if {${progressbar_type} ne "both"} {
set allow_indeterminate no
}
set indeterminate ${allow_indeterminate}
set indeterminate_timer 0
ui_progress_generic start
}
Expand Down Expand Up @@ -102,7 +108,7 @@ proc portprogress::target_progress_callback {event} {
set time_diff [expr { $time_now - $time_last }]

if {${time_diff} >= ${indeterminate_threshold}} {
set indeterminate yes
set indeterminate ${allow_indeterminate}
}
}

Expand Down
27 changes: 25 additions & 2 deletions src/registry2.0/portimage.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ namespace eval portimage {
variable force 0
variable noexec 0
variable UI_PREFIX {---> }
variable show_progressbar 1

# takes a composite version spec rather than separate version,revision,variants
proc activate_composite {name {v ""} {optionslist ""}} {
Expand Down Expand Up @@ -161,6 +162,7 @@ proc deactivate_composite {name {v ""} {optionslist ""}} {

proc deactivate {name {version ""} {revision ""} {variants 0} {options ""}} {
variable UI_PREFIX
variable show_progressbar

if {[dict exists $options ports_force] && [string is true -strict [dict get $options ports_force]] } {
# this not using the namespace variable is correct, since activate
Expand Down Expand Up @@ -247,6 +249,12 @@ proc deactivate {name {version ""} {revision ""} {variants 0} {options ""}} {
}
}

if {[lsearch -exact ${macports::progressbar_also_for} "de/activation"] < 0} {
set show_progressbar 0
# minimise the overhead of calling _progress: it wouldn't do a thing so make it a stub
proc _progress {args} {}
}

ui_msg "$UI_PREFIX [format [msgcat::mc "Deactivating %s @%s"] $name $specifier]"

try {
Expand Down Expand Up @@ -428,6 +436,8 @@ proc _activate_directories {dirs imageroot} {
# extract an archive to a directory
# returns: path to the extracted directory
proc extract_archive_to_imagedir {location} {
variable show_progressbar

set extractdir [file rootname $location]
if {[file exists $extractdir]} {
set extractdir [mkdtemp ${extractdir}XXXXXXXX]
Expand Down Expand Up @@ -586,7 +596,11 @@ proc extract_archive_to_imagedir {location} {
} else {
set cmdstring "${unarchive.pipe_cmd} ( ${unarchive.cmd} ${unarchive.pre_args} ${unarchive.args} )"
}
system -callback portimage::_extract_progress $cmdstring
if {${show_progressbar}} {
system -callback portimage::_extract_progress $cmdstring
} else {
system $cmdstring
}
} on error {_ eOptions} {
::file delete -force $extractdir
throw [dict get $eOptions -errorcode] [dict get $eOptions -errorinfo]
Expand Down Expand Up @@ -627,7 +641,9 @@ proc _extract_progress {event} {
}

proc _progress {args} {
if {[macports::ui_isset ports_verbose]} {
variable show_progressbar

if {!${show_progressbar} || [macports::ui_isset ports_verbose]} {
return
}

Expand Down Expand Up @@ -693,6 +709,7 @@ proc _activate_contents {port {rename_list {}}} {
variable keep_imagedir
variable progress_step
variable progress_total_steps
variable show_progressbar

set files [list]
set baksuffix .mp_[clock seconds]
Expand All @@ -701,6 +718,12 @@ proc _activate_contents {port {rename_list {}}} {
set imagefiles [$port imagefiles]
set num_imagefiles [llength $imagefiles]

if {[lsearch -exact ${macports::progressbar_also_for} "de/activation"] < 0} {
set show_progressbar 0
# minimise the overhead of calling _progress: it wouldn't do a thing so make it a stub
proc _progress {args} {}
}

set progress_step 0
if {[::file isfile $location]} {
set progress_total_steps [expr {$num_imagefiles * 3}]
Expand Down

0 comments on commit 7535dcc

Please sign in to comment.