[PATCH cygport] Add customization support for announce command

[PATCH cygport] Add customization support for announce command

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

The email generated by the cygport announce command is useful, but 
actual use cases are somewhat limited due to the hard-coded email 
submission.

The attached patch adds more flexibility. The patch is on top of the 
"Use correct wording if only one package is announced" patch.

Examples for cygport.conf settings:

ANNOUNCE_EDITOR='printf "\\nRegards,\\n$PN package maintainer\\n" >>"$1"'

ANNOUNCE_EDITOR='
n=$(wc -l <"$1") && cat >>"$1" <<EOF &&

   >>> This is an update to the latest upstream release.
   >>> This is a bugfix release.
   <<< PLEASE EDIT >>>

Regards,
$PN package maintainer
EOF
vim +$((n+2)) "$1" && ! grep -E "<<<|>>>" "$1"
'

ANNOUNCE_TO_CMD='cat "$1" >/dev/clipboard'

ANNOUNCE_TO_CMD='
sed "1,/^\$/d" "$1" >$PF-announcement.txt &&
echo "Announcement placed here: $(pwd)/$PF-announcement.txt"
'

ANNOUNCE_TO_CMD='
/usr/local/sbin/custom-mailer \
   --sender="$SMTP_SENDER" \
   --smarthost="$SMTP_SERVER" \
   ...more...options... \
   cygwin-announce@cygwin.com <"$1"
'

-- 
Regards,
Christian


[-- Attachment #2: 0001-Add-customization-support-for-announce-command.patch --]
[-- Type: text/plain, Size: 4637 bytes --]

From 1f13d54a40d639938fb67245eed4615be0a6e6c4 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Wed, 21 Feb 2024 15:14:53 +0100
Subject: [PATCH] Add customization support for announce command

Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_TO_CMD).
---
 data/cygport.conf      | 23 +++++++++++++++++
 lib/pkg_upload.cygpart | 57 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..48dc7bfe 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,6 +101,29 @@
 #PAGER=
 
 
+#****v* Configuration/ANNOUNCE_EDITOR
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command before sending the email.  If undefined, a text editor
+#  will be run, see EDITOR setting above.  If empty, nothing will be run.
+#  If not empty, '/bin/bash' will be launched with the command string passed
+#  with '-c' option and the path of the temporary email message file as '$1'.
+#  The working directory of the shell will be the directory of the cygport
+#  file.  The specified command string will be prepended by shell assignments
+#  of the cygport variables P, PF, PN, PR and PV and all SMTP_* settings
+#  described below.
+#ANNOUNCE_EDITOR=
+
+#****v* Configuration/ANNOUNCE_TO_CMD
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command after editing.  If undefined, the email will be sent
+#  using the builtin perl-based SMTP support.  If empty, nothing will be run.
+#  If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+#  described above.
+#ANNOUNCE_TO_CMD=
+
+
 #****v* Configuration/SMTP_SENDER
 #  DESCRIPTION
 #  Name and email address, in the form of "First Last <user@domain>" to be used
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 8039ec5c..b81bf3d5 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,28 @@ EOF
 	echo "Upload complete."
 }
 
+__pkg_announce_run_cmd_on_msg() {
+	local cmdvar=$1
+	local msg=$2
+	local cmd
+
+	eval cmd="\${${cmdvar}}"
+
+	(
+		cd ${top} && /bin/bash -c "\
+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"
+}
+
 __pkg_announce() {
 	local msg=$(mktemp -t cygwin-announce-${PF}.XXXXXX)
 	local msgat=$(date +@%s)
@@ -178,10 +200,10 @@ __pkg_announce() {
 
 	cat > ${msg} <<_EOF
 From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
-From: ${SMTP_SENDER}
-To: cygwin-announce@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-announce@cygwin.com
 Date: $(date -R --date=${msgat})
-Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
+Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
 Subject: ${NAME} ${PVR}
 
 The following package${s} been uploaded to the Cygwin distribution:
@@ -199,7 +221,30 @@ _EOF
 ${DESCRIPTION}
 _EOF
 
-	${EDITOR:-vi} $msg || error "Editor exited abormally, aborting annoucement"
+	if [ "${ANNOUNCE_EDITOR+y}" = "y" ]
+	then
+		echo
+		inform "Launching '\${ANNOUNCE_EDITOR} ${msg}'"
+		[ -z "${ANNOUNCE_EDITOR}" ] ||
+			__pkg_announce_run_cmd_on_msg ANNOUNCE_EDITOR ${msg}
+	else
+		${EDITOR:-vi} ${msg} || error "Editor exited abnormally, aborting announcement"
+	fi
+
+	if [ "${ANNOUNCE_TO_CMD+y}" = "y" ]
+	then
+		echo
+		inform "Launching '\${ANNOUNCE_TO_CMD} ${msg}'"
+		[ -z "${ANNOUNCE_TO_CMD}" ] ||
+			__pkg_announce_run_cmd_on_msg ANNOUNCE_TO_CMD ${msg}
+	else
+		__pkg_announce_to_smtp ${msg}
+	fi
+	rm ${msg}
+}
+
+__pkg_announce_to_smtp() {
+	local msg=$1
 
 	perl <(cat <<EOF
 use strict;
@@ -242,8 +287,8 @@ if (defined \$smtp_user) {
 \$smtp->quit();
 print "Announcement sent\n";
 EOF
-) && rm $msg || error "Sending announcement failed, mbox is $msg"
+) || error "Sending announcement failed, mbox is $msg"
 }
 
 # protect functions
-readonly -f __pkg_upload __pkg_announce
+readonly -f __pkg_upload __pkg_announce __pkg_announce_run_cmd_on_msg __pkg_announce_to_smtp
-- 
2.43.0

Re: [PATCH cygport] Add customization support for announce command

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

Christian Franke wrote:
> The email generated by the cygport announce command is useful, but 
> actual use cases are somewhat limited due to the hard-coded email 
> submission.
>
> The attached patch adds more flexibility. The patch is on top of the 
> "Use correct wording if only one package is announced" patch.

Slightly changed patch attached. Also adjusted to new version of "Use 
correct wording if only one package is announced" patch.


>
> Examples for cygport.conf settings:
>
> ANNOUNCE_EDITOR='printf "\\nRegards,\\n$PN package maintainer\\n" >>"$1"'
>
> ANNOUNCE_EDITOR='
> n=$(wc -l <"$1") && cat >>"$1" <<EOF &&
>
>   >>> This is an update to the latest upstream release.
>   >>> This is a bugfix release.
>   <<< PLEASE EDIT >>>
>
> Regards,
> $PN package maintainer
> EOF
> vim +$((n+2)) "$1" && ! grep -E "<<<|>>>" "$1"
> '
>
> ANNOUNCE_TO_CMD='cat "$1" >/dev/clipboard'
>
> ANNOUNCE_TO_CMD='
> sed "1,/^\$/d" "$1" >$PF-announcement.txt &&
> echo "Announcement placed here: $(pwd)/$PF-announcement.txt"
> '
>
> ANNOUNCE_TO_CMD='
> /usr/local/sbin/custom-mailer \
>   --sender="$SMTP_SENDER" \
>   --smarthost="$SMTP_SERVER" \
>   ...more...options... \
>   cygwin-announce@cygwin.com <"$1"
> '
>

Possible (better?) alternative names for the new settings:
ANNOUNCEMENT_EDITOR
ANNOUNCEMENT_MAILER


[-- Attachment #2: 0001-Add-customization-support-for-announce-command.patch --]
[-- Type: text/plain, Size: 4586 bytes --]

From 14709f0a1ed19c7d00588fb2a1fa7273d47e00fd Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Fri, 23 Feb 2024 12:04:17 +0100
Subject: [PATCH] Add customization support for announce command

Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_TO_CMD).
---
 data/cygport.conf      | 23 +++++++++++++++++
 lib/pkg_upload.cygpart | 56 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..48dc7bfe 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,6 +101,29 @@
 #PAGER=
 
 
+#****v* Configuration/ANNOUNCE_EDITOR
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command before sending the email.  If undefined, a text editor
+#  will be run, see EDITOR setting above.  If empty, nothing will be run.
+#  If not empty, '/bin/bash' will be launched with the command string passed
+#  with '-c' option and the path of the temporary email message file as '$1'.
+#  The working directory of the shell will be the directory of the cygport
+#  file.  The specified command string will be prepended by shell assignments
+#  of the cygport variables P, PF, PN, PR and PV and all SMTP_* settings
+#  described below.
+#ANNOUNCE_EDITOR=
+
+#****v* Configuration/ANNOUNCE_TO_CMD
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command after editing.  If undefined, the email will be sent
+#  using the builtin perl-based SMTP support.  If empty, nothing will be run.
+#  If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+#  described above.
+#ANNOUNCE_TO_CMD=
+
+
 #****v* Configuration/SMTP_SENDER
 #  DESCRIPTION
 #  Name and email address, in the form of "First Last <user@domain>" to be used
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 37bc2d63..d38ea8b6 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,33 @@ EOF
 	echo "Upload complete."
 }
 
+__pkg_announce_run_cmd_on_msg() {
+	local cmdvar=$1
+	local msg=$2
+	local cmd
+
+	eval cmd="\${${cmdvar}}"
+
+	if [ "${cmd:+y}" != "y" ]
+	then
+		inform "\${${cmdvar}} is empty"
+		return 0
+	fi
+	echo
+	inform "Launching '\${${cmdvar}} ${msg}'"
+
+	/bin/bash -c "cd ${top} || exit 1
+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"
+}
+
 __pkg_announce() {
 	local msg=$(mktemp -t cygwin-announce-${PF}.XXXXXX)
 	local msgat=$(date +@%s)
@@ -178,10 +205,10 @@ __pkg_announce() {
 
 	cat > ${msg} <<_EOF
 From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
-From: ${SMTP_SENDER}
-To: cygwin-announce@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-announce@cygwin.com
 Date: $(date -R --date=${msgat})
-Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
+Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
 Subject: ${NAME} ${PVR}
 
 The following package${s_have} been uploaded to the Cygwin distribution:
@@ -199,7 +226,24 @@ _EOF
 ${DESCRIPTION}
 _EOF
 
-	${EDITOR:-vi} $msg || error "Editor exited abormally, aborting annoucement"
+	if [ "${ANNOUNCE_EDITOR+y}" = "y" ]
+	then
+		__pkg_announce_run_cmd_on_msg ANNOUNCE_EDITOR ${msg}
+	else
+		${EDITOR:-vi} ${msg} || error "Editor exited abnormally, aborting announcement"
+	fi
+
+	if [ "${ANNOUNCE_TO_CMD+y}" = "y" ]
+	then
+		__pkg_announce_run_cmd_on_msg ANNOUNCE_TO_CMD ${msg}
+	else
+		__pkg_announce_to_smtp ${msg}
+	fi
+	rm ${msg}
+}
+
+__pkg_announce_to_smtp() {
+	local msg=$1
 
 	perl <(cat <<EOF
 use strict;
@@ -242,8 +286,8 @@ if (defined \$smtp_user) {
 \$smtp->quit();
 print "Announcement sent\n";
 EOF
-) && rm $msg || error "Sending announcement failed, mbox is $msg"
+) || error "Sending announcement failed, mbox is $msg"
 }
 
 # protect functions
-readonly -f __pkg_upload __pkg_announce
+readonly -f __pkg_upload __pkg_announce __pkg_announce_run_cmd_on_msg __pkg_announce_to_smtp
-- 
2.43.0

Re: [PATCH cygport] Add customization support for announce command

[Jon Turney]

On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
> Christian Franke wrote:
>> The email generated by the cygport announce command is useful, but 
>> actual use cases are somewhat limited due to the hard-coded email 
>> submission.
>>
>> The attached patch adds more flexibility. The patch is on top of the 
>> "Use correct wording if only one package is announced" patch.
> 
> Slightly changed patch attached. Also adjusted to new version of "Use 
> correct wording if only one package is announced" patch.
> 
> 
[...]

Thanks for this.

> Possible (better?) alternative names for the new settings:
> ANNOUNCEMENT_EDITOR
> ANNOUNCEMENT_MAILER

Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
the best for clarity and conciseness.

> -From: ${SMTP_SENDER}
> -To: cygwin-announce@cygwin.com
> +${SMTP_SENDER:+From: ${SMTP_SENDER}
> +}To: cygwin-announce@cygwin.com
>  Date: $(date -R --date=${msgat})
> -Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
> +Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
>  Subject: ${NAME} ${PVR}

Can you also explain what this is doing in the commit message, since 
it's not immediately apparent.

Re: [PATCH cygport] Add customization support for announce command

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]

Jon Turney wrote:
> On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
>> Christian Franke wrote:
>>> The email generated by the cygport announce command is useful, but 
>>> actual use cases are somewhat limited due to the hard-coded email 
>>> submission.
>>>
>>> The attached patch adds more flexibility. The patch is on top of the 
>>> "Use correct wording if only one package is announced" patch.
>>
>> Slightly changed patch attached. Also adjusted to new version of "Use 
>> correct wording if only one package is announced" patch.
>>
>>
> [...]
>
> Thanks for this.
>
>> Possible (better?) alternative names for the new settings:
>> ANNOUNCEMENT_EDITOR
>> ANNOUNCEMENT_MAILER
>
> Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
> the best for clarity and conciseness.

New patch attached. Is still on top of "Use correct wording ..." patch.

I also added HOMEPAGE to the propagated variables as this should be 
included in an announcement.


>
>
>> -From: ${SMTP_SENDER}
>> -To: cygwin-announce@cygwin.com
>> +${SMTP_SENDER:+From: ${SMTP_SENDER}
>> +}To: cygwin-announce@cygwin.com
>>  Date: $(date -R --date=${msgat})
>> -Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo 
>> ${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
>> +Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo 
>> ${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
>>  Subject: ${NAME} ${PVR}
>
> Can you also explain what this is doing in the commit message, since 
> it's not immediately apparent.
>
>

If the mail infrastructure always replaces the "From:" line or the 
default one is sufficient, then there is no need to generate one. 
SMTP_SENDER could be left alone then. I added a related comment to 
cygport.conf


[-- Attachment #2: 0001-Add-customization-support-for-announce-command.patch --]
[-- Type: text/plain, Size: 5155 bytes --]

From 335cbde3c6c2450051cc739cee60a555b236843e Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sun, 10 Mar 2024 17:28:09 +0100
Subject: [PATCH] Add customization support for announce command

Two new configuration settings allow to override the launch of
a text editor (ANNOUNCE_EDITOR) and the builtin email submission
(ANNOUNCE_MAILER).  Don't create a "From:" header line if
SMTP_SENDER is undefined or empty.
---
 data/cygport.conf      | 27 +++++++++++++++++++-
 lib/pkg_upload.cygpart | 57 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/data/cygport.conf b/data/cygport.conf
index 34ccd291..3da744d9 100644
--- a/data/cygport.conf
+++ b/data/cygport.conf
@@ -101,10 +101,35 @@
 #PAGER=
 
 
+#****v* Configuration/ANNOUNCE_EDITOR
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command before sending the email.  If undefined, a text editor
+#  will be run, see EDITOR setting above.  If empty, nothing will be run.
+#  If not empty, '/bin/bash' will be launched with the command string passed
+#  with '-c' option and the path of the temporary email message file as '$1'.
+#  The working directory of the shell will be the directory of the cygport
+#  file.  The specified command string will be prepended by shell assignments
+#  of the cygport variables HOMEPAGE, P, PF, PN, PR and PV and all SMTP_*
+#  settings described below.
+#ANNOUNCE_EDITOR=
+
+#****v* Configuration/ANNOUNCE_MAILER
+#  DESCRIPTION
+#  Shell command string to process the email message created by cygport's
+#  announce command after editing.  If undefined, the email will be sent
+#  using the builtin perl-based SMTP support.  If empty, nothing will be run.
+#  If not empty, the command string will be handled similar to ANNOUNCE_EDITOR
+#  described above.
+#ANNOUNCE_MAILER=
+
+
 #****v* Configuration/SMTP_SENDER
 #  DESCRIPTION
 #  Name and email address, in the form of "First Last <user@domain>" to be used
-#  by cygport's announcement command.
+#  by cygport's announcement command.  If undefined or empty, no "From:" email
+#  header line will be generated.  The local mail tool or the mail provider may
+#  unconditionally replace this header line or only the "<user@domain>" part.
 #  NOTE
 #  Many webmail services do not allow using arbitrary sender address in SMTP
 #  mail, or may first require registering other email addresses as authorized
diff --git a/lib/pkg_upload.cygpart b/lib/pkg_upload.cygpart
index 37bc2d63..9ced1fb5 100644
--- a/lib/pkg_upload.cygpart
+++ b/lib/pkg_upload.cygpart
@@ -168,6 +168,34 @@ EOF
 	echo "Upload complete."
 }
 
+__pkg_announce_run_cmd_on_msg() {
+	local cmdvar=$1
+	local msg=$2
+	local cmd
+
+	eval cmd="\${${cmdvar}}"
+
+	if [ "${cmd:+y}" != "y" ]
+	then
+		inform "\${${cmdvar}} is empty"
+		return 0
+	fi
+	echo
+	inform "Launching '\${${cmdvar}} ${msg}'"
+
+	/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"
+}
+
 __pkg_announce() {
 	local msg=$(mktemp -t cygwin-announce-${PF}.XXXXXX)
 	local msgat=$(date +@%s)
@@ -178,10 +206,10 @@ __pkg_announce() {
 
 	cat > ${msg} <<_EOF
 From cygwin-announce-${PF} $(date '+%a %b %d %H:%M:%S %Y' --date=${msgat})
-From: ${SMTP_SENDER}
-To: cygwin-announce@cygwin.com
+${SMTP_SENDER:+From: ${SMTP_SENDER}
+}To: cygwin-announce@cygwin.com
 Date: $(date -R --date=${msgat})
-Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER} | sed 's|.*<\(.*\)>.*|\1|')>
+Message-Id: <$(date "+%Y%m%d%H%M%S.$$" --date=${msgat})-1-$(echo ${SMTP_SENDER:-cygport} | sed 's|.*<\(.*\)>.*|\1|')>
 Subject: ${NAME} ${PVR}
 
 The following package${s_have} been uploaded to the Cygwin distribution:
@@ -199,7 +227,24 @@ _EOF
 ${DESCRIPTION}
 _EOF
 
-	${EDITOR:-vi} $msg || error "Editor exited abormally, aborting annoucement"
+	if [ "${ANNOUNCE_EDITOR+y}" = "y" ]
+	then
+		__pkg_announce_run_cmd_on_msg ANNOUNCE_EDITOR ${msg}
+	else
+		${EDITOR:-vi} ${msg} || error "Editor exited abnormally, aborting announcement"
+	fi
+
+	if [ "${ANNOUNCE_MAILER+y}" = "y" ]
+	then
+		__pkg_announce_run_cmd_on_msg ANNOUNCE_MAILER ${msg}
+	else
+		__pkg_announce_to_smtp ${msg}
+	fi
+	rm ${msg}
+}
+
+__pkg_announce_to_smtp() {
+	local msg=$1
 
 	perl <(cat <<EOF
 use strict;
@@ -242,8 +287,8 @@ if (defined \$smtp_user) {
 \$smtp->quit();
 print "Announcement sent\n";
 EOF
-) && rm $msg || error "Sending announcement failed, mbox is $msg"
+) || error "Sending announcement failed, mbox is $msg"
 }
 
 # protect functions
-readonly -f __pkg_upload __pkg_announce
+readonly -f __pkg_upload __pkg_announce __pkg_announce_run_cmd_on_msg __pkg_announce_to_smtp
-- 
2.43.0

Re: [PATCH cygport] Add customization support for announce command

[Jon Turney]

On 10/03/2024 16:33, Christian Franke via Cygwin-apps wrote:
> Jon Turney wrote:
>> On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
>>> Christian Franke wrote:
>>>> The email generated by the cygport announce command is useful, but 
>>>> actual use cases are somewhat limited due to the hard-coded email 
>>>> submission.
>>>>
>>>> The attached patch adds more flexibility. The patch is on top of the 
>>>> "Use correct wording if only one package is announced" patch.
>>>
>>> Slightly changed patch attached. Also adjusted to new version of "Use 
>>> correct wording if only one package is announced" patch.
>>>
>>>
>> [...]
>>
>> Thanks for this.
>>
>>> Possible (better?) alternative names for the new settings:
>>> ANNOUNCEMENT_EDITOR
>>> ANNOUNCEMENT_MAILER
>>
>> Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
>> the best for clarity and conciseness.
> 
> New patch attached. Is still on top of "Use correct wording ..." patch.
> 
> I also added HOMEPAGE to the propagated variables as this should be 
> included in an announcement.

Thanks.

> +	/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?

Re: [PATCH cygport] Add customization support for announce command

[Christian Franke]

Jon Turney wrote:
> On 10/03/2024 16:33, Christian Franke via Cygwin-apps wrote:
>> Jon Turney wrote:
>>> On 23/02/2024 11:23, Christian Franke via Cygwin-apps wrote:
>>>> Christian Franke wrote:
>>>>> The email generated by the cygport announce command is useful, but 
>>>>> actual use cases are somewhat limited due to the hard-coded email 
>>>>> submission.
>>>>>
>>>>> The attached patch adds more flexibility. The patch is on top of 
>>>>> the "Use correct wording if only one package is announced" patch.
>>>>
>>>> Slightly changed patch attached. Also adjusted to new version of 
>>>> "Use correct wording if only one package is announced" patch.
>>>>
>>>>
>>> [...]
>>>
>>> Thanks for this.
>>>
>>>> Possible (better?) alternative names for the new settings:
>>>> ANNOUNCEMENT_EDITOR
>>>> ANNOUNCEMENT_MAILER
>>>
>>> Hmmm... I think "ANNOUNCE_EDITOR" and "ANNOUNCE_MAILER" would be
>>> the best for clarity and conciseness.
>>
>> New patch attached. Is still on top of "Use correct wording ..." patch.
>>
>> I also added HOMEPAGE to the propagated variables as this should be 
>> included in an announcement.
>
> Thanks.
>
>> +    /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).

Re: [PATCH cygport] Add customization support for announce command

[Adam Dinwoodie]

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
	"$cmd" "$msg" || error "Command $cmd $msg (cwd=${top}) failed"
)
```

I've removed the array handling for $PV, as it's not an array; possibly
you've confused it with $PVP?  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.

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.

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!

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"
)
```

Jon suggested using `env`, although I don't think it provides any
function here that isn't already provided by built-in Bash function.

Personally, I'd rewrite the whole __pkg_announce_run_cmd_on_msg function
as below, although some of this is just my own stylistic preferences (in
particular, I try to avoid `eval` if at all possible, and here `local -n`
works just fine):

```
__pkg_announce_run_cmd_on_msg() {
	local cmdvar="$1"
	local -n cmd="$1"
	local msg="$2"

	if [[ ! "$cmd" ]]; then
		inform "$cmdvar is empty"
		return 0
	fi

	echo
	inform "Launching '$cmd $msg'"

	(
		cd "$top"
		export HOMEPAGE P PF PN PR PV SMTP_SENDER SMTP_SERVER \
			SMTP_SERVER_PORT SMTP_ENCRYPTION SMTP_USER \
			SMTP_PASS
		"$cmd" "$msg" || error "Command '$cmd $msg' (cwd=${top}) failed"
	)
}
```

Re: [PATCH cygport] Add customization support for announce command

[Christian Franke]

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.

Re: [PATCH cygport] Add customization support for announce command

[Adam Dinwoodie]

On Wed, May 01, 2024 at 04:49:10PM +0200, Christian Franke via Cygwin-apps wrote:
> 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

Ah, right!  Apologies, I'd missed/forgotten that context.

I still think this is the wrong approach.  Cygport has a very well
established design paradigm for running non-trivial custom code: there's
a default Bash function in the Cygport library files, and an individual
cygport file can override that function with its own version.  If
running something more than a simple command is required here, I'd have
thought following that established paradigm would be a much more
sensible approach.  This would also completely avoid the need for any
special variable handling, because the function will run in an
environment where the variables are already set.

I'd suggest a pair of functions that _aren't_ read-only, say
`announce_edit` and `announce_send`, that default to the current code
for editing and sending the announcement, but which a maintainer can
override in their cygport file should they so desire.

I really don't like passing strings around to be eval'd, particularly
when they're expected to contain non-trivial code.  Given how difficult
Bash quoting can be to get right, I worry that even if your patch is
correct now, it'll be a ripe site for bugs when other people try to
amend the code.

> > )
> > ```
> > 
> > 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[*]});

Oh.  Oh I don't like that.  Clearly your code is correct in this
context, but that existing code is deeply unintuitive when other arrays
are declared explicitly.

> 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.

Which wouldn't work if -- as I'd thought -- $cmd was only going to be a
simple command, since there was no reference to $msg, but it makes sense
if $cmd is actually a complete script in its own right, including being
able to reference arguments.

> > 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.
>