Adam Dinwoodie via Cygwin-apps wrote:
On Tue, Apr 30, 2024 at 12:27:35PM +0200, Christian Franke via Cygwin-apps
wrote:
Jon Turney wrote:
On 10/03/2024 16:33, Christian Franke via Cygwin-apps wrote:
+ /bin/bash -c "cd ${top} || exit 1
+${HOMEPAGE+HOMEPAGE=${HOMEPAGE@Q}}
+P=${P@Q}; PF=${PF@Q}; PN=${PN@Q}; PR=${PR@Q}; PV=(${PV[*]@Q})
+${SMTP_SENDER+SMTP_SENDER=${SMTP_SENDER@Q}}
+${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
+${SMTP_SERVER_PORT+SMTP_SERVER_PORT=${SMTP_SERVER_PORT@Q}}
+${SMTP_ENCRYPTION+SMTP_ENCRYPTION=${SMTP_ENCRYPTION@Q}}
+${SMTP_USER+SMTP_USER=${SMTP_USER@Q}}
+${SMTP_PASS+SMTP_PASS=${SMTP_PASS@Q}}
+${cmd}
+" $0 ${msg} || error "Command '\${${cmdvar}} ${msg}'
(cwd=${top}) failed"
+}
Sorry I didn't notice this before, and I am terrible at writing shell,
but perhaps you could share the reasoning behind writing this as above,
and not as, e.g.
(cd ${top} && env BLAH ${cmd})
avoiding all the verbiage in the description of ANNOUNCE_EDITOR about it
being fed into 'bash -c' (and hence getting evaluated twice??) rather
than just run?
None of the mentioned variables are exported to the environment by cygport.
I wanted to keep this fact in the subshell. Therefore the assignments are
added to the script instead of passing via env(ironment). The latter won't
even work with the PV variable because arrays could not be exported.
Variables would not be evaluated twice. For example in the rare case that
someone uses something like
SMTP_SERVER="smtp.$(hostname -d)"
in cygport.conf, this would immediately expand to
SMTP_SERVER="smtp.some.domain". The above
${SMTP_SERVER+SMTP_SERVER=${SMTP_SERVER@Q}}
would expand to
SMTP_SERVER=${SMTP_SERVER@Q}
and then to
SMTP_SERVER='smtp.some.domain'
(The @Q bash extension ensures proper quoting).
Using a subshell created by ( ... ) would achieve the behaviour you're
after, without requiring nearly so much quote handling. The code Jon
has pulled out could be rewritten as below; using ( ... ) would mean
that everything happens in a subshell and the exports don't affect the
rest of the environment.
```
(
cd "$top"
export HOMEPAGE P PF PN PR PV SMTP_SENDER SMTP_SERVER SMTP_SERVER_PORT
SMTP_ENCRYPTION SMTP_USER SMTP_PASS
This unnecessarily exports all variables (except PV, see below) to the
subcommands run by $cmd.
"$cmd" "$msg" || error "Command $cmd $msg (cwd=${top}) failed"
This would limit $cmd to simple commands instead of multiline scripts.
This reduces flexibility and some of the examples I provided in my
original post would no longer work:
https://sourceware.org/pipermail/cygwin-apps/2024-February/043501.html
)
```
I've removed the array handling for $PV, as it's not an array; possibly
you've confused it with $PVP?
No. PV it is initialized as a regular shell variable but is later
changed to an array by appending the PVP array.
/bin/cygport:
...
declare PV=${VERSION}
...
PV=$(echo ${PF} | sed -e "s/${PN}\-\(.*\)\-${PR}$/\1/");
...
declare -r PV=(${PV} ${PVP[*]});
...
$ git blame bin/cygport.in | grep 'declare -r PV='
deb528a88 (Yaakov Selkowitz 2009-12-31 08:05:52 +0000 397) declare -r
PV=(${PV} ${PVP[*]});
Bash silently ignores 'export PV'.
In any case, there is no way to pass an
array to "$cmd" unless "$cmd" is itself a Bash function, as there's no
standard way to store anything other than strings in environment
variables.
That's why I use 'PV=(${PV[*]@Q})' as a prefix of the configured $cmd
string instead of passing any new environment to $cmd.
I've also removed the `|| return 1` part, since cygport runs with `set
-e` enabled so you only want to catch non-zero return codes if you want
specific error handling.
There is no 'return 1' is my patch.
Finally, I've also added "$msg" to the arguments to "$cmd"; that seems
to be missing and seems to be critical to this working at all!
$msg is not missing in my patch but passed to the launched /bin/bash as $1.
Alternatively, if you really wanted to avoid the export statement, the
below will achieve the same thing; the only use of the subshell at this
point is to avoid the `cd` changing the working directory for the rest
of the script.
```
(
cd "$top"
HOMEPAGE="$HOMEPAGE" P="$P" PF="$PF" PN="$PN" PR="$PR" PV="$PV" SMTP_SENDER="$SMTP_SENDER" SMTP_SERVER="$SMTP_SERVER"
SMTP_SERVER_PORT="$SMTP_SERVER_PORT" SMTP_ENCRYPTION="$SMTP_ENCRYPTION" SMTP_USER="$SMTP_USER" SMTP_PASS="$SMTP_PASS" "$cmd" "$msg" || error "Command $cmd $msg
(cwd=${top}) failed"
)
```
Same problem with missing flexibility for $cmd as above.
