From 29e205f8b94076e57fd06a25e157cbd53f053c31 Mon Sep 17 00:00:00 2001 From: Havard Eidnes Date: Sun, 24 Aug 2025 08:21:33 +0000 Subject: [PATCH 1/3] rust-installer/install-template.sh: improve efficiency, step 1. This round replaces repetitive pattern matching in the inner loop of this script using grep (which causes a fork() for each test) with built-in pattern matching in the bourne shell using the case / esac construct. This in reference to https://github.com/rust-lang/rust/issues/80684 and is a separated-out request from https://github.com/rust-lang/rust-installer/pull/111 which apparently never got any review. The forthcoming planned "step 2" change builds on top of this change, and replaces the inner-loops needless uses of sed (which again causes a fork() for each instance) with the suffix removal constructs from the bourne shell. Since this change touches lots of the same lines this change does, that pull request cannot be submitted before this one is accepted. Hopefully this first step is less controversial than the latter change. --- src/tools/rust-installer/install-template.sh | 106 +++++++++---------- 1 file changed, 52 insertions(+), 54 deletions(-) diff --git a/src/tools/rust-installer/install-template.sh b/src/tools/rust-installer/install-template.sh index 337aaa95b9a22..65480ba8c2e63 100644 --- a/src/tools/rust-installer/install-template.sh +++ b/src/tools/rust-installer/install-template.sh @@ -551,54 +551,45 @@ install_components() { # Decide the destination of the file local _file_install_path="$_dest_prefix/$_file" - if echo "$_file" | grep "^etc/" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^etc\///')" - _file_install_path="$CFG_SYSCONFDIR/$_f" - fi - - if echo "$_file" | grep "^bin/" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^bin\///')" - _file_install_path="$CFG_BINDIR/$_f" - fi - - if echo "$_file" | grep "^lib/" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^lib\///')" - _file_install_path="$CFG_LIBDIR/$_f" - fi - - if echo "$_file" | grep "^share" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^share\///')" - _file_install_path="$CFG_DATADIR/$_f" - fi - - if echo "$_file" | grep "^share/man/" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^share\/man\///')" - _file_install_path="$CFG_MANDIR/$_f" - fi - - # HACK: Try to support overriding --docdir. Paths with the form - # "share/doc/$product/" can be redirected to a single --docdir - # path. If the following detects that --docdir has been specified - # then it will replace everything preceding the "$product" path - # component. The problem here is that the combined rust installer - # contains two "products": rust and cargo; so the contents of those - # directories will both be dumped into the same directory; and the - # contents of those directories are _not_ disjoint. Since this feature - # is almost entirely to support 'make install' anyway I don't expect - # this problem to be a big deal in practice. - if [ "$CFG_DOCDIR" != "" ] - then - if echo "$_file" | grep "^share/doc/" > /dev/null - then - local _f="$(echo "$_file" | sed 's/^share\/doc\/[^/]*\///')" - _file_install_path="$CFG_DOCDIR/$_f" - fi - fi + case "$_file" in + etc/*) + local _f="$(echo "$_file" | sed 's/^etc\///')" + _file_install_path="$CFG_SYSCONFDIR/$_f" + ;; + bin/*) + local _f="$(echo "$_file" | sed 's/^bin\///')" + _file_install_path="$CFG_BINDIR/$_f" + ;; + lib/*) + local _f="$(echo "$_file" | sed 's/^lib\///')" + _file_install_path="$CFG_LIBDIR/$_f" + ;; + share/man/*) + local _f="$(echo "$_file" | sed 's/^share\/man\///')" + _file_install_path="$CFG_MANDIR/$_f" + ;; + share/doc/*) + # HACK: Try to support overriding --docdir. Paths with the form + # "share/doc/$product/" can be redirected to a single --docdir + # path. If the following detects that --docdir has been specified + # then it will replace everything preceding the "$product" path + # component. The problem here is that the combined rust installer + # contains two "products": rust and cargo; so the contents of those + # directories will both be dumped into the same directory; and the + # contents of those directories are _not_ disjoint. Since this feature + # is almost entirely to support 'make install' anyway I don't expect + # this problem to be a big deal in practice. + if [ "$CFG_DOCDIR" != "" ] + then + local _f="$(echo "$_file" | sed 's/^share\/doc\/[^/]*\///')" + _file_install_path="$CFG_DOCDIR/$_f" + fi + ;; + share/*) + local _f="$(echo "$_file" | sed 's/^share\///')" + _file_install_path="$CFG_DATADIR/$_f" + ;; + esac # Make sure there's a directory for it make_dir_recursive "$(dirname "$_file_install_path")" @@ -617,13 +608,20 @@ install_components() { maybe_backup_path "$_file_install_path" - if echo "$_file" | grep "^bin/" > /dev/null || test -x "$_src_dir/$_component/$_file" - then - run cp "$_src_dir/$_component/$_file" "$_file_install_path" - run chmod 755 "$_file_install_path" + if test -x "$_src_dir/$_component/$_file"; then + run cp "$_src_dir/$_component/$_file" "$_file_install_path" + run chmod 755 "$_file_install_path" else - run cp "$_src_dir/$_component/$_file" "$_file_install_path" - run chmod 644 "$_file_install_path" + case "$_file" in + bin/*) + run cp "$_src_dir/$_component/$_file" "$_file_install_path" + run chmod 755 "$_file_install_path" + ;; + *) + run cp "$_src_dir/$_component/$_file" "$_file_install_path" + run chmod 644 "$_file_install_path" + ;; + esac fi critical_need_ok "file creation failed" From 27a0720a03f731d5ec28b234a7bc7efa6aca851b Mon Sep 17 00:00:00 2001 From: Havard Eidnes Date: Sat, 18 Oct 2025 20:12:24 +0000 Subject: [PATCH 2/3] Avoid an extra case/esac: use a flag instead. Also factor out common commands to outside of test. --- src/tools/rust-installer/install-template.sh | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/tools/rust-installer/install-template.sh b/src/tools/rust-installer/install-template.sh index 65480ba8c2e63..3385baf17ad79 100644 --- a/src/tools/rust-installer/install-template.sh +++ b/src/tools/rust-installer/install-template.sh @@ -551,6 +551,7 @@ install_components() { # Decide the destination of the file local _file_install_path="$_dest_prefix/$_file" + local _is_bin=false case "$_file" in etc/*) local _f="$(echo "$_file" | sed 's/^etc\///')" @@ -558,6 +559,7 @@ install_components() { ;; bin/*) local _f="$(echo "$_file" | sed 's/^bin\///')" + _is_bin=true _file_install_path="$CFG_BINDIR/$_f" ;; lib/*) @@ -608,20 +610,11 @@ install_components() { maybe_backup_path "$_file_install_path" - if test -x "$_src_dir/$_component/$_file"; then - run cp "$_src_dir/$_component/$_file" "$_file_install_path" + run cp "$_src_dir/$_component/$_file" "$_file_install_path" + if $_is_bin || test -x "$_src_dir/$_component/$_file"; then run chmod 755 "$_file_install_path" else - case "$_file" in - bin/*) - run cp "$_src_dir/$_component/$_file" "$_file_install_path" - run chmod 755 "$_file_install_path" - ;; - *) - run cp "$_src_dir/$_component/$_file" "$_file_install_path" - run chmod 644 "$_file_install_path" - ;; - esac + run chmod 644 "$_file_install_path" fi critical_need_ok "file creation failed" From 924843586b1e6c2ec06420817e5ba5cbc7dd740a Mon Sep 17 00:00:00 2001 From: Havard Eidnes Date: Sat, 18 Oct 2025 21:21:15 +0000 Subject: [PATCH 3/3] Evidently tidy doesn't like tabs for indentation, even in shell scripts... --- src/tools/rust-installer/install-template.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/rust-installer/install-template.sh b/src/tools/rust-installer/install-template.sh index 3385baf17ad79..c4f0c618a52de 100644 --- a/src/tools/rust-installer/install-template.sh +++ b/src/tools/rust-installer/install-template.sh @@ -551,7 +551,7 @@ install_components() { # Decide the destination of the file local _file_install_path="$_dest_prefix/$_file" - local _is_bin=false + local _is_bin=false case "$_file" in etc/*) local _f="$(echo "$_file" | sed 's/^etc\///')" @@ -559,7 +559,7 @@ install_components() { ;; bin/*) local _f="$(echo "$_file" | sed 's/^bin\///')" - _is_bin=true + _is_bin=true _file_install_path="$CFG_BINDIR/$_f" ;; lib/*)