Skip to content

Commit b7bb721

Browse files
committed
Give the user some control over progressbar invasiveness
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.
1 parent 96520f2 commit b7bb721

6 files changed

Lines changed: 121 additions & 17 deletions

File tree

doc/macports.conf.in

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,20 @@ variants_conf @MPCONFIGDIR_EXPANDED@/variants.conf
136136
# Keep logs after successful installations.
137137
#keeplogs no
138138

139+
# Progress reporting.
140+
# MacPorts will show a progressbar during lengthy operations which take more
141+
# than $progressbar_after milliseconds. The bar reports determinate progress
142+
# when the total amount of work and the work done are quantified. It can also
143+
# show an "indeterminate" indication of work being done (and switch to this
144+
# display after a certain amount of time) if $progressbar_type is set to `both`.
145+
# The operations that use this feature are downloads, builds and rev-upgrade,
146+
# plus a list of additional operations $progressbar_also_for which can include
147+
# `de/activation` (for `port deactivate` and `port activate`) and possibly more
148+
# in the future. NB: no quotes for the string values!
149+
# progressbar_after 500
150+
# progressbar_type both
151+
# progressbar_also_for de/activation
152+
139153
# URLs that MacPorts attempts to download to find out whether a new version was
140154
# released. Multiple values, space-separated; only one of the URLs needs to be
141155
# available. Downloads will be attempted in the specified order.

src/macports1.0/macports.tcl

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ namespace eval macports {
5252
foreach opt [list binpath auto_path extra_env portdbformat \
5353
portarchivetype portimage_mode hfscompression portautoclean \
5454
porttrace portverbose keeplogs destroot_umask release_urls release_version_urls \
55+
progressbar_after progressbar_type progressbar_also_for \
5556
rsync_server rsync_options rsync_dir \
5657
startupitem_autostart startupitem_type startupitem_install \
5758
place_worksymlink xcodeversion xcodebuildcmd xcodecltversion xcode_license_unaccepted \
@@ -78,6 +79,7 @@ namespace eval macports {
7879
registry.path registry.format user_home user_path user_ssh_auth_sock \
7980
portarchivetype portarchive_hfscompression archivefetch_pubkeys \
8081
portautoclean portimage_mode porttrace keeplogs portverbose destroot_umask \
82+
progressbar_after progressbar_type progressbar_also_for \
8183
rsync_server rsync_options rsync_dir startupitem_autostart startupitem_type startupitem_install \
8284
place_worksymlink macportsuser sudo_user \
8385
configureccache ccache_dir ccache_size configuredistcc configurepipe buildnicevalue buildmakejobs \
@@ -983,6 +985,9 @@ proc mportinit {{up_ui_options {}} {up_options {}} {up_variations {}}} {
983985
macports::cxx_stdlib \
984986
macports::hfscompression \
985987
macports::portarchive_hfscompression \
988+
macports::progressbar_after \
989+
macports::progressbar_type \
990+
macports::progressbar_also_for \
986991
macports::host_cache \
987992
macports::porturl_prefix_map \
988993
macports::ui_options \
@@ -1460,6 +1465,16 @@ Please edit sources.conf and change '$url' to '[string range $url 0 26]macports/
14601465
}
14611466
set portarchive_hfscompression $hfscompression
14621467

1468+
if {![info exists progressbar_after]} {
1469+
set progressbar_after 500
1470+
}
1471+
if {![info exists progressbar_type]} {
1472+
set progressbar_type "both"
1473+
}
1474+
if {![info exists progressbar_also_for]} {
1475+
set progressbar_type "de/activation"
1476+
}
1477+
14631478
# Set rync options
14641479
if {![info exists rsync_server]} {
14651480
set rsync_server rsync.macports.org

src/pextlib1.0/Pextlib.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,40 @@ int ClonefileCmd(ClientData clientData UNUSED, Tcl_Interp *interp, int objc, Tcl
11501150
return TCL_ERROR;
11511151
}
11521152

1153+
/*
1154+
processIsForeground
1155+
1156+
synopsis: processIsForeground
1157+
1158+
Returns true if the process is running in the foreground or false
1159+
if in the background.
1160+
*/
1161+
int IsProcessForegroundCmd(ClientData clientData UNUSED, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[])
1162+
{
1163+
/* Check the arg count */
1164+
if (objc != 1) {
1165+
Tcl_WrongNumArgs(interp, 1, objv, NULL);
1166+
return TCL_ERROR;
1167+
}
1168+
1169+
int fd, fdo = fileno(stdout);
1170+
if ((fd = isatty(fdo) ? fdo : open("/dev/tty", O_RDONLY)) != -1) {
1171+
const pid_t pgrp = getpgrp();
1172+
const pid_t tcpgrp = tcgetpgrp(fd);
1173+
if (fd != fdo) {
1174+
close(fd);
1175+
}
1176+
if (pgrp != -1 && tcpgrp != -1) {
1177+
Tcl_SetObjResult(interp, Tcl_NewBooleanObj(pgrp == tcpgrp));
1178+
return TCL_OK;
1179+
}
1180+
}
1181+
Tcl_SetErrno(errno);
1182+
Tcl_ResetResult(interp);
1183+
Tcl_AppendResult(interp, "processIsForeground: ", (char *)Tcl_PosixError(interp), NULL);
1184+
return TCL_ERROR;
1185+
}
1186+
11531187
int Pextlib_Init(Tcl_Interp *interp)
11541188
{
11551189
if (Tcl_InitStubs(interp, "8.4", 0) == NULL)
@@ -1214,6 +1248,8 @@ int Pextlib_Init(Tcl_Interp *interp)
12141248
Tcl_CreateObjCommand(interp, "fs_clone_capable", FSCloneCapableCmd, NULL, NULL);
12151249
Tcl_CreateObjCommand(interp, "clonefile", ClonefileCmd, NULL, NULL);
12161250

1251+
Tcl_CreateObjCommand(interp, "processIsForeground", IsProcessForegroundCmd, NULL, NULL);
1252+
12171253
if (Tcl_PkgProvide(interp, "Pextlib", "1.0") != TCL_OK)
12181254
return TCL_ERROR;
12191255

src/port/port.tcl

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4829,9 +4829,11 @@ namespace eval portclient::progress {
48294829
proc initDelay {} {
48304830
variable show
48314831
variable startTime
4832+
variable showTimeThreshold
48324833

48334834
set startTime [clock milliseconds]
48344835
set show no
4836+
set showTimeThreshold ${macports::progressbar_after}
48354837
}
48364838

48374839
##
@@ -4907,7 +4909,9 @@ namespace eval portclient::progress {
49074909
}
49084910
update {
49094911
lassign $args now total
4910-
if {[showProgress $now $total] eq "yes"} {
4912+
# Check on each update if we're still outputting to a tty - the user can
4913+
# have pushed us into the background.
4914+
if {[processIsForeground] && ([showProgress $now $total] eq "yes")} {
49114915
set barPrefix " "
49124916
set barPrefixLen [string length $barPrefix]
49134917
if {$total != 0} {
@@ -4919,11 +4923,13 @@ namespace eval portclient::progress {
49194923
}
49204924
intermission -
49214925
finish {
4922-
# erase to start of line
4923-
::term::ansi::send::esolch stderr
4924-
# return cursor to start of line
4925-
puts -nonewline stderr "\r"
4926-
flush stderr
4926+
if {[processIsForeground]} {
4927+
# erase to start of line
4928+
::term::ansi::send::esolch stderr
4929+
# return cursor to start of line
4930+
puts -nonewline stderr "\r"
4931+
flush stderr
4932+
}
49274933
}
49284934
}
49294935

@@ -4955,7 +4961,9 @@ namespace eval portclient::progress {
49554961
}
49564962
update {
49574963
lassign $args type total now speed
4958-
if {[showProgress $now $total] eq "yes"} {
4964+
# Check on each update if we're still outputting to a tty - the user can
4965+
# have pushed us into the background.
4966+
if {[processIsForeground] && ([showProgress $now $total] eq "yes")} {
49594967
set barPrefix " "
49604968
set barPrefixLen [string length $barPrefix]
49614969
if {$total != 0} {
@@ -4974,11 +4982,13 @@ namespace eval portclient::progress {
49744982
}
49754983
}
49764984
finish {
4977-
# erase to start of line
4978-
::term::ansi::send::esolch stderr
4979-
# return cursor to start of line
4980-
puts -nonewline stderr "\r"
4981-
flush stderr
4985+
if {[processIsForeground]} {
4986+
# erase to start of line
4987+
::term::ansi::send::esolch stderr
4988+
# return cursor to start of line
4989+
puts -nonewline stderr "\r"
4990+
flush stderr
4991+
}
49824992
}
49834993
}
49844994

src/port1.0/portprogress.tcl

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ namespace eval portprogress {
4141
variable indeterminate_timer 0
4242

4343
# If our progress callback should issue indeterminate progress updates
44+
variable allow_indeterminate yes
4445
variable indeterminate yes
4546

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

6769
switch -- [dict get $event type] {
6870
exec {
69-
set indeterminate yes
71+
# reset allow_indeterminate to the user/default-configured setting
72+
if {${progressbar_type} ne "both"} {
73+
set allow_indeterminate no
74+
}
75+
set indeterminate ${allow_indeterminate}
7076
set indeterminate_timer 0
7177
ui_progress_generic start
7278
}
@@ -102,7 +108,7 @@ proc portprogress::target_progress_callback {event} {
102108
set time_diff [expr { $time_now - $time_last }]
103109

104110
if {${time_diff} >= ${indeterminate_threshold}} {
105-
set indeterminate yes
111+
set indeterminate ${allow_indeterminate}
106112
}
107113
}
108114

src/registry2.0/portimage.tcl

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ namespace eval portimage {
6363
variable force 0
6464
variable noexec 0
6565
variable UI_PREFIX {---> }
66+
variable show_progressbar 1
6667

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

162163
proc deactivate {name {version ""} {revision ""} {variants 0} {options ""}} {
163164
variable UI_PREFIX
165+
variable show_progressbar
164166

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

252+
if {[lsearch -exact ${macports::progressbar_also_for} "de/activation"] < 0} {
253+
set show_progressbar 0
254+
# minimise the overhead of calling _progress: it wouldn't do a thing so make it a stub
255+
proc _progress {args} {}
256+
}
257+
250258
ui_msg "$UI_PREFIX [format [msgcat::mc "Deactivating %s @%s"] $name $specifier]"
251259

252260
try {
@@ -428,6 +436,8 @@ proc _activate_directories {dirs imageroot} {
428436
# extract an archive to a directory
429437
# returns: path to the extracted directory
430438
proc extract_archive_to_imagedir {location} {
439+
variable show_progressbar
440+
431441
set extractdir [file rootname $location]
432442
if {[file exists $extractdir]} {
433443
set extractdir [mkdtemp ${extractdir}XXXXXXXX]
@@ -586,7 +596,11 @@ proc extract_archive_to_imagedir {location} {
586596
} else {
587597
set cmdstring "${unarchive.pipe_cmd} ( ${unarchive.cmd} ${unarchive.pre_args} ${unarchive.args} )"
588598
}
589-
system -callback portimage::_extract_progress $cmdstring
599+
if {${show_progressbar}} {
600+
system -callback portimage::_extract_progress $cmdstring
601+
} else {
602+
system $cmdstring
603+
}
590604
} on error {_ eOptions} {
591605
::file delete -force $extractdir
592606
throw [dict get $eOptions -errorcode] [dict get $eOptions -errorinfo]
@@ -627,7 +641,9 @@ proc _extract_progress {event} {
627641
}
628642

629643
proc _progress {args} {
630-
if {[macports::ui_isset ports_verbose]} {
644+
variable show_progressbar
645+
646+
if {!${show_progressbar} || [macports::ui_isset ports_verbose]} {
631647
return
632648
}
633649

@@ -693,6 +709,7 @@ proc _activate_contents {port {rename_list {}}} {
693709
variable keep_imagedir
694710
variable progress_step
695711
variable progress_total_steps
712+
variable show_progressbar
696713

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

721+
if {[lsearch -exact ${macports::progressbar_also_for} "de/activation"] < 0} {
722+
set show_progressbar 0
723+
# minimise the overhead of calling _progress: it wouldn't do a thing so make it a stub
724+
proc _progress {args} {}
725+
}
726+
704727
set progress_step 0
705728
if {[::file isfile $location]} {
706729
set progress_total_steps [expr {$num_imagefiles * 3}]

0 commit comments

Comments
 (0)