*-pull-request: cleanup and overhaul
cover *-pull-request: cleanup and overhaul
After some discussion on the oe-core list regarding the *-pull-request scripts I provided a proposal, received some feedback, and these patches are the result of that discussion. The Cc list varies from patch to patch a bit more wildly than is typical as a final sanity test, although it does reflect who commented and expressed interest in which parts of the overhaul.
Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.
The following changes since commit 65e5ebc1d27b0a6cb2e33596d6d32c536b857f0d:
tcmode-default: fix a typo (2011-05-13 11:02:20 +0100)
are available in the git repository at:
git://git.pokylinux.org/poky-contrib.git dvhart/git-pull
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dvhart/git-pull
Darren Hart (10):
create-pull-request: alphabetize arguments
create-pull-request: whitespace cleanup
create-pull-request: use git request-pull and arbitrary remotes
create-pull-request: rewrite known private URLs to public URLs
create-pull-request: provide an RFC mode via -c argument
send-pull-request: whitespace cleanup
send-pull-request: remove local mta support
send-pull-request: fix greedy auto-cc regex
send-pull-request: don't send all patches to everyone even with -a
send-pull-request: verify git sendemail config
scripts/create-pull-request | 176 +++++++++++++++++++---------- scripts/send-pull-request | 265 ++++++++++++++++++------------------------- 2 files changed, 228 insertions(+), 213 deletions(-)
RFC PATCH 1/10 create-pull-request: alphabetize arguments
Some initial cleanup prior to a significant overhaul.
--- scripts/create-pull-request | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
scripts/create-pull-request +5 -5
@@ -9,14 +9,14 @@ usage() { CMD=$(basename $0) cat <<EOM Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -b contrib_branch + -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree -h Display this help message - -o output_dir Specify the output directory for the messages (default: pull-PID) + -i commit_id Ending commit (default: HEAD) -m msg_body_file The file containing a blurb to be inserted into the summary email + -o output_dir Specify the output directory for the messages (default: pull-PID) + -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix -r relative_to Starting commit (default: master) - -i commit_id Ending commit (default: HEAD) - -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree -s subject The subject to be inserted into the summary email - -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix Examples: $CMD -b nitin/basic
@@ -27,7 +27,7 @@ EOM } # Parse and validate arguments -while getopts "b:hi:m:o:r:s:p:" OPT; do +while getopts "b:hi:m:o:p:r:s:" OPT; do case $OPT in b) CONTRIB_BRANCH="$OPTARG"
Signature · 1 line
1.7.1
RFC PATCH 2/10 create-pull-request: whitespace cleanup
In reply to Darren HartIndent with tabs, not spaces, to be consistent with other bash scripts.
--- scripts/create-pull-request | 78 +++++++++++++++++++++--------------------- 1 files changed, 39 insertions(+), 39 deletions(-)
scripts/create-pull-request +39 -39
@@ -28,42 +28,42 @@ EOM # Parse and validate arguments while getopts "b:hi:m:o:p:r:s:" OPT; do - case $OPT in - b) - CONTRIB_BRANCH="$OPTARG" - ;; - h) - usage - exit 0 - ;; - i) - COMMIT_ID="$OPTARG" - ;; - m) - BODY="$OPTARG" - if [ ! -e "$BODY" ]; then - echo "ERROR: Body file does not exist" - exit 1 - fi - ;; - o) - ODIR="$OPTARG" - ;; - p) - PREFIX="$OPTARG" - ;; - r) - RELATIVE_TO="$OPTARG" - ;; - s) - SUBJECT="$OPTARG" - ;; - esac + case $OPT in + b) + CONTRIB_BRANCH="$OPTARG" + ;; + h) + usage + exit 0 + ;; + i) + COMMIT_ID="$OPTARG" + ;; + m) + BODY="$OPTARG" + if [ ! -e "$BODY" ]; then + echo "ERROR: Body file does not exist" + exit 1 + fi + ;; + o) + ODIR="$OPTARG" + ;; + p) + PREFIX="$OPTARG" + ;; + r) + RELATIVE_TO="$OPTARG" + ;; + s) + SUBJECT="$OPTARG" + ;; + esac done if [ -z "$CONTRIB_BRANCH" ]; then - usage - exit 1 + usage + exit 1 fi
@@ -72,9 +72,9 @@ fi WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$CONTRIB_BRANCH" wget -q $WEB_URL -O /dev/null if [ $? -ne 0 ]; then - echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree." - echo " Please check your contrib-branch parameter before sending." - echo "" + echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree." + echo " Please check your contrib-branch parameter before sending." + echo "" fi if [ -e $ODIR ]; then
@@ -106,13 +106,13 @@ EOM # If the user specified a message body, insert it into the cover letter and # remove the BLURB token. if [ -n "$BODY" ]; then - sed -i "/BLURB HERE/ r $BODY" "$CL" - sed -i "/BLURB HERE/ d" "$CL" + sed -i "/BLURB HERE/ r $BODY" "$CL" + sed -i "/BLURB HERE/ d" "$CL" fi # If the user specified a subject, replace the SUBJECT token with it. if [ -n "$SUBJECT" ]; then - sed -i -e "s/\*\*\* SUBJECT HERE \*\*\*/$SUBJECT/" "$CL" + sed -i -e "s/\*\*\* SUBJECT HERE \*\*\*/$SUBJECT/" "$CL" fi
Signature · 1 line
1.7.1
RFC PATCH 3/10 create-pull-request: use git request-pull and arbitrary remotes
In reply to Darren HartAllow for arbitrary remotes via a new -u argument. Remove the hard coded references to the pokylinux repositories. Create the WEB_URL from known remotes. Future patches can add additional WEB_URL mappings for remotes like Git Hub, Gitorious, and kernel.org.
Rather than duplicating the git request-pull command ourselves, just use the existing one.
--- scripts/create-pull-request | 74 ++++++++++++++++++++++++++++-------------- 1 files changed, 49 insertions(+), 25 deletions(-)
scripts/create-pull-request +49 -25
@@ -2,14 +2,13 @@ ODIR=pull-$$ RELATIVE_TO="master" COMMIT_ID="HEAD" -PULL_URL="git://git.pokylinux.org/poky-contrib.git" PREFIX="PATCH" usage() { CMD=$(basename $0) cat <<EOM -Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -b contrib_branch - -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree +Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -u remote -b branch + -b branch Branch name in the specified remote -h Display this help message -i commit_id Ending commit (default: HEAD) -m msg_body_file The file containing a blurb to be inserted into the summary email
@@ -17,20 +16,21 @@ Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix -r relative_to Starting commit (default: master) -s subject The subject to be inserted into the summary email + -u remote The git remote where the branch is located Examples: - $CMD -b nitin/basic - $CMD -r distro/master -i nitin/distro -b nitin/distro - $CMD -r master -i misc -b nitin/misc -o pull-misc - $CMD -p "RFC PATCH" -b nitin/experimental + $CMD -u contrib -b nitin/basic + $CMD -u contrib -r distro/master -i nitin/distro -b nitin/distro + $CMD -u contrib -r master -i misc -b nitin/misc -o pull-misc + $CMD -u contrib -p "RFC PATCH" -b nitin/experimental EOM } # Parse and validate arguments -while getopts "b:hi:m:o:p:r:s:" OPT; do +while getopts "b:hi:m:o:p:r:s:u:" OPT; do case $OPT in b) - CONTRIB_BRANCH="$OPTARG" + BRANCH="$OPTARG" ;; h) usage
@@ -58,23 +58,42 @@ while getopts "b:hi:m:o:p:r:s:" OPT; do s) SUBJECT="$OPTARG" ;; + u) + REMOTE="$OPTARG" + REMOTE_URL=$(git config remote.$REMOTE.url) + if [ $? -ne 0 ]; then + echo "ERROR: git config failed to find a url for '$REMOTE'" + exit 1 + fi + ;; esac done -if [ -z "$CONTRIB_BRANCH" ]; then +if [ -z "$BRANCH" ] || [ -z "$REMOTE_URL" ]; then usage exit 1 fi +# Set WEB_URL from known remotes +case "$REMOTE_URL" in + *git.yoctoproject.org*) + WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; + *git.pokylinux.org*) + WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; +esac + # Perform a sanity test on the web URL. Issue a warning if it is not # accessible, but do not abort as users may want to run offline. -WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$CONTRIB_BRANCH" -wget -q $WEB_URL -O /dev/null -if [ $? -ne 0 ]; then - echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree." - echo " Please check your contrib-branch parameter before sending." - echo "" +if [ -n "$WEB_URL" ]; then + wget -q $WEB_URL -O /dev/null + if [ $? -ne 0 ]; then + echo "WARNING: Branch '$BRANCH' was not found on the contrib git tree." + echo " Please check your remote and branch parameter before sending." + echo "" + fi fi if [ -e $ODIR ]; then
@@ -90,18 +109,23 @@ git format-patch -M --subject-prefix="$PREFIX" -n -o $ODIR --thread=shallow --co # Customize the cover letter CL="$ODIR/0000-cover-letter.patch" -(cat <<EOM +PM="$ODIR/pull-msg" +git request-pull $RELATIVE_TO $REMOTE_URL $COMMIT_ID > "$PM" +if [ $? -ne 0 ]; then + echo "ERROR: git request-pull reported an error" + exit 1 +fi -Pull URL: $PULL_URL - Branch: $CONTRIB_BRANCH - Browse: $WEB_URL +# The cover letter already has a diffstat, remove it from the pull-msg +# before inserting it. +sed -n "0,\#$REMOTE_URL# p" "$PM" | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" +rm "$PM" -Thanks, - $(git config user.name) <$(git config user.email)> ---- +# Insert the WEB_URL if there is one +if [ -n "$WEB_URL" ]; then + echo " $WEB_URL" | sed -i "\#$REMOTE_URL# r /dev/stdin" "$CL" +fi -EOM -) | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" # If the user specified a message body, insert it into the cover letter and # remove the BLURB token.
Signature · 1 line
1.7.1
RFC PATCH 4/10 create-pull-request: rewrite known private URLs to public URLs
In reply to Darren HartRather than requiring users to have public remotes and private remotes when their development remotes are ssh based (and therefor unsuitable for a pull request URL), rewrite the ones we know about from ssh://git@ to git://.
With this infrastructure in place, future patches can augment the list of known remotes for things like Git Hub, Gitorious, kernel.org, etc.
--- scripts/create-pull-request | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
scripts/create-pull-request +10 -0
@@ -65,6 +65,16 @@ while getopts "b:hi:m:o:p:r:s:u:" OPT; do echo "ERROR: git config failed to find a url for '$REMOTE'" exit 1 fi + + # Rewrite known private URLs to public URLs + case "$REMOTE_URL" in + *ssh://[email protected]*) + REMOTE_URL=${REMOTE_URL/'ssh://git@'/'git://'} + ;; + *ssh://[email protected]*) + REMOTE_URL=${REMOTE_URL/"ssh://git@"/"git://"} + ;; + esac ;; esac done
Signature · 1 line
1.7.1
RFC PATCH 5/10 create-pull-request: provide an RFC mode via -c argument
In reply to Darren HartCurrently it is difficult to know if a pull request is being sent for review or just to be pulled.
Add a -c argument to add RFC to the subject prefix and a blurb requesting review to the cover letter.
--- scripts/create-pull-request | 24 ++++++++++++++++++++++-- 1 files changed, 22 insertions(+), 2 deletions(-)
scripts/create-pull-request +22 -2
@@ -3,12 +3,14 @@ ODIR=pull-$$ RELATIVE_TO="master" COMMIT_ID="HEAD" PREFIX="PATCH" +RFC=0 usage() { CMD=$(basename $0) cat <<EOM Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -u remote -b branch -b branch Branch name in the specified remote + -c Create an RFC (Request for Comment) patch series -h Display this help message -i commit_id Ending commit (default: HEAD) -m msg_body_file The file containing a blurb to be inserted into the summary email
@@ -27,11 +29,14 @@ EOM } # Parse and validate arguments -while getopts "b:hi:m:o:p:r:s:u:" OPT; do +while getopts "b:chi:m:o:p:r:s:u:" OPT; do case $OPT in b) BRANCH="$OPTARG" ;; + c) + RFC=1 + ;; h) usage exit 0
@@ -84,6 +89,10 @@ if [ -z "$BRANCH" ] || [ -z "$REMOTE_URL" ]; then exit 1 fi +if [ $RFC -eq 1 ]; then + PREFIX="RFC $PREFIX" +fi + # Set WEB_URL from known remotes case "$REMOTE_URL" in
@@ -120,7 +129,7 @@ git format-patch -M --subject-prefix="$PREFIX" -n -o $ODIR --thread=shallow --co # Customize the cover letter CL="$ODIR/0000-cover-letter.patch" PM="$ODIR/pull-msg" -git request-pull $RELATIVE_TO $REMOTE_URL $COMMIT_ID > "$PM" +git request-pull $RELATIVE_TO $REMOTE_URL $COMMIT_ID >> "$PM" if [ $? -ne 0 ]; then echo "ERROR: git request-pull reported an error" exit 1
@@ -131,6 +140,17 @@ fi sed -n "0,\#$REMOTE_URL# p" "$PM" | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" rm "$PM" +# If this is an RFC, make that clear in the cover letter +if [ $RFC -eq 1 ]; then +(cat <<EOM +Please review the following changes for suitability for inclusion. If you have +any objections or suggestions for improvement, please respond to the patches. If +you agree with the changes, please provide your Acked-by. + +EOM +) | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" +fi + # Insert the WEB_URL if there is one if [ -n "$WEB_URL" ]; then echo " $WEB_URL" | sed -i "\#$REMOTE_URL# r /dev/stdin" "$CL"
Signature · 1 line
1.7.1
RFC PATCH 6/10 send-pull-request: whitespace cleanup
In reply to Darren HartIndent with tabs, not spaces, for consistency with other bash scripts.
--- scripts/send-pull-request | 250 ++++++++++++++++++++++---------------------- 1 files changed, 125 insertions(+), 125 deletions(-)
scripts/send-pull-request +125 -125
@@ -31,72 +31,72 @@ EOM # $2: The regex to match and strip from the line with email addresses harvest_recipients() { - TO_CC=$1 - REGX=$2 - export IFS=$',\n' - for PATCH in $PDIR/*.patch; do - # Grab To addresses - for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do - if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then - if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi - elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then - if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi - fi - done - done - unset IFS + TO_CC=$1 + REGX=$2 + export IFS=$',\n' + for PATCH in $PDIR/*.patch; do + # Grab To addresses + for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do + if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then + if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi + elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then + if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi + fi + done + done + unset IFS } # Parse and verify arguments while getopts "af:ghp:t:" OPT; do - case $OPT in - a) - AUTO=1 - ;; - f) - FROM="$OPTARG" - ;; - g) - PULL_MTA="git" - ;; - h) - usage - exit 0 - ;; - p) - PDIR=${OPTARG%/} - if [ ! -d $PDIR ]; then - echo "ERROR: pull-dir \"$PDIR\" does not exist." - usage - exit 1 - fi - ;; - t) - if [ -n "$TO" ]; then - TO="$TO,$OPTARG" - else - TO="$OPTARG" - fi - ;; - esac + case $OPT in + a) + AUTO=1 + ;; + f) + FROM="$OPTARG" + ;; + g) + PULL_MTA="git" + ;; + h) + usage + exit 0 + ;; + p) + PDIR=${OPTARG%/} + if [ ! -d $PDIR ]; then + echo "ERROR: pull-dir \"$PDIR\" does not exist." + usage + exit 1 + fi + ;; + t) + if [ -n "$TO" ]; then + TO="$TO,$OPTARG" + else + TO="$OPTARG" + fi + ;; + esac done if [ -z "$PDIR" ]; then - echo "ERROR: you must specify a pull-dir." - usage - exit 1 + echo "ERROR: you must specify a pull-dir." + usage + exit 1 fi # Verify the cover letter is complete and free of tokens CL="$PDIR/0000-cover-letter.patch" for TOKEN in SUBJECT BLURB; do - grep -q "*** $TOKEN HERE ***" "$CL" - if [ $? -eq 0 ]; then - echo "ERROR: Please edit $CL and try again (Look for '*** $TOKEN HERE ***')." - exit 1 - fi + grep -q "*** $TOKEN HERE ***" "$CL" + if [ $? -eq 0 ]; then + echo "ERROR: Please edit $CL and try again (Look for '*** $TOKEN HERE ***')." + exit 1 + fi done
@@ -104,39 +104,39 @@ done # In addition to To and CC headers/lines, the common Signed-off-by, Tested-by, # etc. (*-by) will be added to CC. if [ $AUTO -eq 1 ]; then - harvest_recipients TO "^[Tt][Oo]: *" - harvest_recipients CC "^[Cc][Cc]: *" - harvest_recipients CC "^.*-[Bb][Yy]: *" + harvest_recipients TO "^[Tt][Oo]: *" + harvest_recipients CC "^[Cc][Cc]: *" + harvest_recipients CC "^.*-[Bb][Yy]: *" fi case "$PULL_MTA" in - git) - FROM="$(git config sendemail.from)" +git) + FROM="$(git config sendemail.from)" AUTO_TO="$(git config sendemail.to)" if [ -n "$AUTO_TO" ]; then - if [ -n "$TO" ]; then - TO="$TO,$AUTO_TO" - else - TO="$AUTO_TO" - fi + if [ -n "$TO" ]; then + TO="$TO,$AUTO_TO" + else + TO="$AUTO_TO" + fi + fi + ;; +sendmail) + if [ -z "$FROM" ]; then + FROM="$(git config user.name) <$(git config user.email)>" + if [ -z "$FROM" ]; then + echo "ERROR: unable to determine a FROM address" + usage + exit 1 + fi fi - ;; - sendmail) - if [ -z "$FROM" ]; then - FROM="$(git config user.name) <$(git config user.email)>" - if [ -z "$FROM" ]; then - echo "ERROR: unable to determine a FROM address" - usage - exit 1 - fi - fi - ;; + ;; esac if [ -z "$TO" ] && [ -z "$CC" ]; then - echo "ERROR: you have not specified any recipients." - usage - exit 1 + echo "ERROR: you have not specified any recipients." + usage + exit 1 fi
@@ -155,55 +155,55 @@ echo "Continue? [y/N] " read cont if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then - ERROR=0 - case "$PULL_MTA" in - git) - export IFS=$',' - GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) - GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) - unset IFS - for PATCH in $PDIR/*patch; do - # We harvest the emails manually, so force git not to. - eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" - if [ $? -eq 1 ]; then - ERROR=1 - fi - done - ;; - sendmail) - for PATCH in $PDIR/*patch; do - # Insert To and CC headers via formail to keep them separate and - # appending them to the sendmail command as -- $TO $CC has - # proven to be an exercise in futility. - # - # Clear the From header, leaving it up to sendmail to insert an - # appropriate one. Insert the original sender (per git) into the - # body of the message. - # - # Use tail to remove the email envelope from git or formail as - # msmtp (sendmail) would choke on them. - # - # Modify the patch date for sequential delivery, but retain the - # original date as "Old-Date". - DATE=$(date +"%a, %d %b %Y %k:%M:%S %z") - GIT_FROM=$(cat $PATCH | formail -X "From:") - cat $PATCH | formail -I "To: $TO" -I "CC: $CC" -I "From: $FROM" -i "Date: $DATE" | sed "0,/^$/s/^$/\n$GIT_FROM\n/" | tail -n +2 | sendmail -t - if [ $? -eq 1 ]; then - ERROR=1 - fi - done - ;; - *) - echo "ERROR: unknown MTA: $PULL_MTA" - usage - exit 1 - ;; - esac - - if [ $ERROR -eq 1 ]; then - echo "ERROR: Failed to send one or more messages. Check your MTA log for details." - fi + ERROR=0 + case "$PULL_MTA" in + git) + export IFS=$',' + GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) + GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) + unset IFS + for PATCH in $PDIR/*patch; do + # We harvest the emails manually, so force git not to. + eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" + if [ $? -eq 1 ]; then + ERROR=1 + fi + done + ;; + sendmail) + for PATCH in $PDIR/*patch; do + # Insert To and CC headers via formail to keep them separate and + # appending them to the sendmail command as -- $TO $CC has + # proven to be an exercise in futility. + # + # Clear the From header, leaving it up to sendmail to insert an + # appropriate one. Insert the original sender (per git) into the + # body of the message. + # + # Use tail to remove the email envelope from git or formail as + # msmtp (sendmail) would choke on them. + # + # Modify the patch date for sequential delivery, but retain the + # original date as "Old-Date". + DATE=$(date +"%a, %d %b %Y %k:%M:%S %z") + GIT_FROM=$(cat $PATCH | formail -X "From:") + cat $PATCH | formail -I "To: $TO" -I "CC: $CC" -I "From: $FROM" -i "Date: $DATE" | sed "0,/^$/s/^$/\n$GIT_FROM\n/" | tail -n +2 | sendmail -t + if [ $? -eq 1 ]; then + ERROR=1 + fi + done + ;; + *) + echo "ERROR: unknown MTA: $PULL_MTA" + usage + exit 1 + ;; + esac + + if [ $ERROR -eq 1 ]; then + echo "ERROR: Failed to send one or more messages. Check your MTA log for details." + fi else - echo "Send aborted." + echo "Send aborted." fi
Signature · 1 line
1.7.1
RFC PATCH 7/10 send-pull-request: remove local mta support
In reply to Darren HartThere is no real value in supporting sendmail directly when git can be configured to use it. The script used to generate the pull request mails relies heavily on git, so doing so here does not impose any additional dependencies and it greatly reduces the complexity of this script.
--- scripts/send-pull-request | 108 +++++++++------------------------------------ 1 files changed, 21 insertions(+), 87 deletions(-)
scripts/send-pull-request +21 -87
@@ -1,15 +1,9 @@ #!/bin/bash AUTO=0 -# Check env for any default settings, command line options will override these. -if [ -z "$PULL_MTA" ]; then - PULL_MTA="sendmail" -fi - # Prevent environment leakage to these vars. unset TO unset CC -# allow the user to set FROM in the environment usage() {
@@ -18,10 +12,6 @@ Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir -t email Explicitly add email to the recipients -a Automatically harvest recipients from "*-by: email" lines in the patches in the pull-dir - -f Specify a FROM address, you can also use the FROM environment - variable. If you do not specify one, it will try to use the one - from your git config. This is ignored if -g is used. - -g Use git-send-email to send mail instead of sendmail -p pull-dir Directory containing summary and patch files EOM }
@@ -49,17 +39,11 @@ harvest_recipients() # Parse and verify arguments -while getopts "af:ghp:t:" OPT; do +while getopts "ahp:t:" OPT; do case $OPT in a) AUTO=1 ;; - f) - FROM="$OPTARG" - ;; - g) - PULL_MTA="git" - ;; h) usage exit 0
@@ -109,29 +93,14 @@ if [ $AUTO -eq 1 ]; then harvest_recipients CC "^.*-[Bb][Yy]: *" fi -case "$PULL_MTA" in -git) - FROM="$(git config sendemail.from)" - AUTO_TO="$(git config sendemail.to)" - if [ -n "$AUTO_TO" ]; then - if [ -n "$TO" ]; then - TO="$TO,$AUTO_TO" - else - TO="$AUTO_TO" - fi +AUTO_TO="$(git config sendemail.to)" +if [ -n "$AUTO_TO" ]; then + if [ -n "$TO" ]; then + TO="$TO,$AUTO_TO" + else + TO="$AUTO_TO" fi - ;; -sendmail) - if [ -z "$FROM" ]; then - FROM="$(git config user.name) <$(git config user.email)>" - if [ -z "$FROM" ]; then - echo "ERROR: unable to determine a FROM address" - usage - exit 1 - fi - fi - ;; -esac +fi if [ -z "$TO" ] && [ -z "$CC" ]; then echo "ERROR: you have not specified any recipients."
@@ -145,10 +114,7 @@ cat <<EOM The following patches: $(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done) -will be sent with the following headers: - From: $FROM - To: $TO - CC: $CC +will now be sent via the git send-email command. EOM echo "Continue? [y/N] "
@@ -156,52 +122,20 @@ read cont if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then ERROR=0 - case "$PULL_MTA" in - git) - export IFS=$',' - GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) - GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) - unset IFS - for PATCH in $PDIR/*patch; do - # We harvest the emails manually, so force git not to. - eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" - if [ $? -eq 1 ]; then - ERROR=1 - fi - done - ;; - sendmail) - for PATCH in $PDIR/*patch; do - # Insert To and CC headers via formail to keep them separate and - # appending them to the sendmail command as -- $TO $CC has - # proven to be an exercise in futility. - # - # Clear the From header, leaving it up to sendmail to insert an - # appropriate one. Insert the original sender (per git) into the - # body of the message. - # - # Use tail to remove the email envelope from git or formail as - # msmtp (sendmail) would choke on them. - # - # Modify the patch date for sequential delivery, but retain the - # original date as "Old-Date". - DATE=$(date +"%a, %d %b %Y %k:%M:%S %z") - GIT_FROM=$(cat $PATCH | formail -X "From:") - cat $PATCH | formail -I "To: $TO" -I "CC: $CC" -I "From: $FROM" -i "Date: $DATE" | sed "0,/^$/s/^$/\n$GIT_FROM\n/" | tail -n +2 | sendmail -t - if [ $? -eq 1 ]; then - ERROR=1 - fi - done - ;; - *) - echo "ERROR: unknown MTA: $PULL_MTA" - usage - exit 1 - ;; - esac + export IFS=$',' + GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) + GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) + unset IFS + for PATCH in $PDIR/*patch; do + # We harvest the emails manually, so force git not to. + eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" + if [ $? -eq 1 ]; then + ERROR=1 + fi + done if [ $ERROR -eq 1 ]; then - echo "ERROR: Failed to send one or more messages. Check your MTA log for details." + echo "ERROR: Failed to send one or more messages." fi else echo "Send aborted."
Signature · 1 line
1.7.1
RFC PATCH 8/10 send-pull-request: fix greedy auto-cc regex
In reply to Darren HartA greedy regular expression caused emails to be harvested from patches that were quoted in the commit message. Ensure only tags that start at the beginning of the line are considered for harvesting.
NOTE: users are still responsible for verifying the recipients list and to
ensure they do not spam people!
--- scripts/send-pull-request | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
scripts/send-pull-request +1 -1
@@ -90,7 +90,7 @@ done if [ $AUTO -eq 1 ]; then harvest_recipients TO "^[Tt][Oo]: *" harvest_recipients CC "^[Cc][Cc]: *" - harvest_recipients CC "^.*-[Bb][Yy]: *" + harvest_recipients CC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *" fi AUTO_TO="$(git config sendemail.to)"
Signature · 1 line
1.7.1
RFC PATCH 9/10 send-pull-request: don't send all patches to everyone even with -a
In reply to Darren HartRather than sending every patch to every recipient of the entire series when -a is used, only send the cover letter to everyone and use git's --signed-off-by-cc feature to generate an auto cc list for the individual patches.
Using git to harvest the Cc list means only collecting Signed-off-by and Cc lines, rather than the more generic *-by lines previously. This is a fair trade-off for significantly reduced complexity. If users want to add Acked-by and Tested-by lines and want to use the -a feature, they should include those recipients as Cc lines as well.
Now that we rely on git for auto-cc for the individual patches, make sure the user is prompted before sending each patch by forcing --confirm=always.
--- scripts/send-pull-request | 60 +++++++++++++++++++++++++++------------------ 1 files changed, 36 insertions(+), 24 deletions(-)
scripts/send-pull-request +36 -24
@@ -4,33 +4,34 @@ AUTO=0 # Prevent environment leakage to these vars. unset TO unset CC +unset AUTO_CC usage() { cat <<EOM Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir -t email Explicitly add email to the recipients - -a Automatically harvest recipients from "*-by: email" lines - in the patches in the pull-dir + -a Automatically harvest recipients from Cc and Signed-off-by lines + in the patches in the pull-dir. These will be used on a per + patch basis. The cover-letter will be sent to all addresses. -p pull-dir Directory containing summary and patch files EOM } -# Collect To and CC addresses from the patch files if they exist -# $1: Which header to add the recipients to, "TO" or "CC" -# $2: The regex to match and strip from the line with email addresses +# Collect addresses from a patch into AUTO_CC +# $1: a patch file harvest_recipients() { - TO_CC=$1 - REGX=$2 + PATCH=$1 export IFS=$',\n' - for PATCH in $PDIR/*.patch; do - # Grab To addresses + for REGX in "^[Cc][Cc]: *" "^[Ss]igned-[Oo]ff-[Bb]y: *"; do for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do - if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then - if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi - elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then - if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi + if [ "${AUTO_CC/$EMAIL/}" == "$AUTO_CC" ] && [ -n "$EMAIL" ]; then + if [ -z "$AUTO_CC" ]; then + AUTO_CC=$EMAIL; + else + AUTO_CC="$AUTO_CC,$EMAIL"; + fi fi done done
@@ -84,13 +85,11 @@ for TOKEN in SUBJECT BLURB; do done -# Harvest emails from the generated patches and populate the TO and CC variables -# In addition to To and CC headers/lines, the common Signed-off-by, Tested-by, -# etc. (*-by) will be added to CC. +# Harvest emails from the generated patches and populate AUTO_CC. if [ $AUTO -eq 1 ]; then - harvest_recipients TO "^[Tt][Oo]: *" - harvest_recipients CC "^[Cc][Cc]: *" - harvest_recipients CC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *" + for PATCH in $PDIR/*.patch; do + harvest_recipients $PATCH + done fi AUTO_TO="$(git config sendemail.to)"
@@ -102,7 +101,7 @@ if [ -n "$AUTO_TO" ]; then fi fi -if [ -z "$TO" ] && [ -z "$CC" ]; then +if [ -z "$TO" ] && [ -z "$AUTO_CC" ]; then echo "ERROR: you have not specified any recipients." usage exit 1
@@ -114,7 +113,8 @@ cat <<EOM The following patches: $(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done) -will now be sent via the git send-email command. +will now be sent via the git send-email command. Git will prompt you before +sending any email. EOM echo "Continue? [y/N] "
@@ -124,11 +124,23 @@ if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then ERROR=0 export IFS=$',' GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) - GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) + GIT_CC=$(for R in $AUTO_CC; do echo -n "--cc='$R' "; done) unset IFS for PATCH in $PDIR/*patch; do - # We harvest the emails manually, so force git not to. - eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" + if [ $AUTO -eq 1 ]; then + if [ $PATCH == "$CL" ]; then + # Send the cover letter to every recipient, both + # specified as well as harvested. + eval "git send-email $GIT_TO $GIT_CC --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + else + # Send the patch to the specified recipients and + # those git finds in this specific patch. + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --signed-off-by-cc $PATCH" + fi + else + # Only send to the explicitly specified recipients + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + fi if [ $? -eq 1 ]; then ERROR=1 fi
Signature · 1 line
1.7.1
RFC PATCH 10/10 send-pull-request: verify git sendemail config
In reply to Darren HartPerform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
--- scripts/send-pull-request | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
scripts/send-pull-request +15 -0
@@ -38,6 +38,18 @@ harvest_recipients() unset IFS } +check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from" + exit 1 + fi +} # Parse and verify arguments while getopts "ahp:t:" OPT; do
@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do esac done +# Abort early if git-send-email is not properly configured +check_git_sendemail_config + if [ -z "$PDIR" ]; then echo "ERROR: you must specify a pull-dir." usage
Signature · 1 line
1.7.1
Re: [RFC PATCH 09/10] send-pull-request: don't send all patches to everyone even with -a
In reply to Darren HartQuoted 11 lines · Darren Hart
Rather than sending every patch to every recipient of the entire series when -a is used, only send the cover letter to everyone and use git's --signed-off-by-cc feature to generate an auto cc list for the individual patches.
Using git to harvest the Cc list means only collecting Signed-off-by and Cc lines, rather than the more generic *-by lines previously. This is a fair trade-off for significantly reduced complexity. If users want to add Acked-by and Tested-by lines and want to use the -a feature, they should include those recipients as Cc lines as well.
Quoted 3 lines
Now that we rely on git for auto-cc for the individual patches, make sure the user is prompted before sending each patch by forcing --confirm=always.
it can be tedious for large patchsets say 30-40 patches. and moreover if something is wrong in middle of sending series of patches say a CC list is wrong then what to do as some of the patches would have already been sent. So I would suggest a dry pass
where it will show you the email headers and CC list so sender can be assured that all is well
and then he can send them all at once.
Quoted 128 lines
Signed-off-by Darren Hart <[email protected]>Cc Khem Raj <[email protected]>Cc Koen Kooi <[email protected]>---
scripts/send-pull-request | 60 +++++++++++++++++++++++++++------------------1 files changed, 36 insertions(+), 24 deletions(-)scripts/send-pull-request +0
-0@@ -4,33 +4,34 @@ AUTO=0 # Prevent environment leakage to these vars.unset TO
unset CC
+unsetAUTO_CCusage()
{
cat <<EOM
Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir
-t email Explicitly add email to the recipients
- -a Automatically harvest recipients from "*-by: email" lines
- in the patches in the pull-dir
+ -a Automatically harvest recipients from Cc and Signed-off-by lines
+ in the patches in the pull-dir. These will be used on a per
+ patch basis. The cover-letter will be sent to all addresses. -p pull-dir Directory containing summary and patch files
EOM
}scripts/send-pull-request +2
-3-# Collect To and CC addresses from the patch files if they exist -# $1: Which header to add the recipients to, "TO" or "CC" -# $2: The regex to match and strip from the line with email addresses +# Collect addresses from a patch into AUTO_CC +# $1: a patch fileharvest_recipients()
{
- TO_CC=$1
- REGX=$2
+ PATCH=$1
export IFS=$',\n'
- for PATCH in $PDIR/*.patch; do
- # Grab To addresses
+ for REGX in "^[Cc][Cc]: *" "^[Ss]igned-[Oo]ff-[Bb]y: *"; do
for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do
- if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then
- if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi
- elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then
- if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi
+ if [ "${AUTO_CC/$EMAIL/}" == "$AUTO_CC" ] && [ -n "$EMAIL" ]; then
+ if [ -z "$AUTO_CC" ]; then
+ AUTO_CC=$EMAIL;
+ else
+ AUTO_CC="$AUTO_CC,$EMAIL";
+ fi
fi
done
donescripts/send-pull-request +0
-0@@ -84,13 +85,11 @@ for TOKEN in SUBJECT BLURB; dodone
scripts/send-pull-request +1
-3-# Harvest emails from the generated patches and populate the TO and CC variables -# In addition to To and CC headers/lines, the common Signed-off-by, Tested-by, -# etc. (*-by) will be added to CC. +# Harvest emails from the generated patches and populate AUTO_CC.if [ $AUTO -eq 1 ]; then
-harvest_recipientsTO "^[Tt][Oo]: *"
-harvest_recipientsCC "^[Cc][Cc]: *"
-harvest_recipientsCC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *"
+ for PATCH in $PDIR/*.patch; do
+harvest_recipients$PATCH
+ done
fiAUTO_TO="$(git config sendemail.to)"
scripts/send-pull-request +0
-0@@ -102,7 +101,7 @@ if [ -n "$AUTO_TO" ]; thenfi
fiscripts/send-pull-request +1
-1-if [ -z "$TO" ] && [ -z "$CC" ]; then +if [ -z "$TO" ] && [ -z "$AUTO_CC" ]; thenecho "ERROR: you have not specified any recipients." usage
exit 1scripts/send-pull-request +0
-0@@ -114,7 +113,8 @@ cat <<EOMThe following patches:
$(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done)scripts/send-pull-request +2
-1-will now be sent via the git send-email command. +will now be sent via the git send-email command. Git will prompt you before +sending any email.EOM
echo "Continue? [y/N] "scripts/send-pull-request +0
-0@@ -124,11 +124,23 @@ if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; thenERROR=0
export IFS=$','
GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done)
- GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done)
+ GIT_CC=$(for R in $AUTO_CC; do echo -n "--cc='$R' "; done)
unset IFS
for PATCH in $PDIR/*patch; do
- # We harvest the emails manually, so force git not to.
- eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH"
+ if [ $AUTO -eq 1 ]; then
+ if [ $PATCH == "$CL" ]; then
+ # Send the cover letter to every recipient, both
+ # specified as well as harvested.
+ eval "git send-email $GIT_TO $GIT_CC --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH"
+ else
+ # Send the patch to the specified recipients and
+ # those git finds in this specific patch.
+ eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --signed-off-by-cc $PATCH"
+ fi
+ else
+ # Only send to the explicitly specified recipients
+ eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH"
+ fi
if [ $? -eq 1 ]; then
ERROR=1
fi
--
1.7.1
Re: [RFC PATCH 09/10] send-pull-request: don't send all patches to everyone even with -a
Quoted 16 lines · Khem Raj
Quoted 11 lines · Darren Hart
Rather than sending every patch to every recipient of the entire series when -a is used, only send the cover letter to everyone and use git's --signed-off-by-cc feature to generate an auto cc list for the individual patches.
Using git to harvest the Cc list means only collecting Signed-off-by and Cc lines, rather than the more generic *-by lines previously. This is a fair trade-off for significantly reduced complexity. If users want to add Acked-by and Tested-by lines and want to use the -a feature, they should include those recipients as Cc lines as well.
Quoted 3 lines
Now that we rely on git for auto-cc for the individual patches, make sure the user is prompted before sending each patch by forcing --confirm=always.
Quoted 7 lines
it can be tedious for large patchsets say 30-40 patches. and moreover if something is wrong in middle of sending series of patches say a CC list is wrong then what to do as some of the patches would have already been sent. So I would suggest a dry pass
where it will show you the email headers and CC list so sender can be assured that all is well
and then he can send them all at once.
Agree with Khem here, unless you're trying to force people to send smaller patch series?
Quoted 134 lines
Quoted 126 lines
Signed-off-by Darren Hart <[email protected]>Cc Khem Raj <[email protected]>Cc Koen Kooi <[email protected]>--- scripts/send-pull-request | 60 +++++++++++++++++++++++++++------------------ 1 files changed, 36 insertions(+), 24 deletions(-)scripts/send-pull-request +36
-25@@ -4,33 +4,34 @@ AUTO=0 # Prevent environment leakage to these vars. unset TO unset CC +unset AUTO_CC usage() { cat <<EOM Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir -t email Explicitly add email to the recipients - -a Automatically harvest recipients from "*-by: email" lines - in the patches in the pull-dir + -a Automatically harvest recipients from Cc and Signed-off-by lines + in the patches in the pull-dir. These will be used on a per + patch basis. The cover-letter will be sent to all addresses. -p pull-dir Directory containing summary and patch files EOM } -# Collect To and CC addresses from the patch files if they exist -# $1: Which header to add the recipients to, "TO" or "CC" -# $2: The regex to match and strip from the line with email addresses +# Collect addresses from a patch into AUTO_CC +# $1: a patch file harvest_recipients() { - TO_CC=$1 - REGX=$2 + PATCH=$1 export IFS=$',\n' - for PATCH in $PDIR/*.patch; do - # Grab To addresses + for REGX in "^[Cc][Cc]: *" "^[Ss]igned-[Oo]ff-[Bb]y: *"; do for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do - if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then - if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi - elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then - if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi + if [ "${AUTO_CC/$EMAIL/}" == "$AUTO_CC" ] && [ -n "$EMAIL" ]; then + if [ -z "$AUTO_CC" ]; then + AUTO_CC=$EMAIL; + else + AUTO_CC="$AUTO_CC,$EMAIL"; + fi fi done done@@ -84,13 +85,11 @@ for TOKEN in SUBJECT BLURB; do done -# Harvest emails from the generated patches and populate the TO and CC variables -# In addition to To and CC headers/lines, the common Signed-off-by, Tested-by, -# etc. (*-by) will be added to CC. +# Harvest emails from the generated patches and populate AUTO_CC. if [ $AUTO -eq 1 ]; then - harvest_recipients TO "^[Tt][Oo]: *" - harvest_recipients CC "^[Cc][Cc]: *" - harvest_recipients CC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *" + for PATCH in $PDIR/*.patch; do + harvest_recipients $PATCH + done fi AUTO_TO="$(git config sendemail.to)"@@ -102,7 +101,7 @@ if [ -n "$AUTO_TO" ]; then fi fi -if [ -z "$TO" ] && [ -z "$CC" ]; then +if [ -z "$TO" ] && [ -z "$AUTO_CC" ]; then echo "ERROR: you have not specified any recipients." usage exit 1@@ -114,7 +113,8 @@ cat <<EOM The following patches: $(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done) -will now be sent via the git send-email command. +will now be sent via the git send-email command. Git will prompt you before +sending any email. EOM echo "Continue? [y/N] "@@ -124,11 +124,23 @@ if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then ERROR=0 export IFS=$',' GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) - GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) + GIT_CC=$(for R in $AUTO_CC; do echo -n "--cc='$R' "; done) unset IFS for PATCH in $PDIR/*patch; do - # We harvest the emails manually, so force git not to. - eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" + if [ $AUTO -eq 1 ]; then + if [ $PATCH == "$CL" ]; then + # Send the cover letter to every recipient, both + # specified as well as harvested. + eval "git send-email $GIT_TO $GIT_CC --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + else + # Send the patch to the specified recipients and + # those git finds in this specific patch. + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --signed-off-by-cc $PATCH" + fi + else + # Only send to the explicitly specified recipients + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + fi if [ $? -eq 1 ]; then ERROR=1 fi --Quoted 3 lines
1.7.1
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Signature · 3 lines
Joshua Lock
Yocto Build System Monkey
Intel Open Source Technology CentreRe: [RFC PATCH 03/10] create-pull-request: use git request-pull and arbitrary remotes
In reply to Darren HartQuoted 87 lines · Darren Hart
Allow for arbitrary remotes via a new -u argument. Remove the hard coded references to the pokylinux repositories. Create the
WEB_URLfrom known remotes. Future patches can add additionalWEB_URLmappings for remotes like Git Hub, Gitorious, and kernel.org.Rather than duplicating the git request-pull command ourselves, just use the existing one.
Signed-off-by Darren Hart <[email protected]>Cc Tom Rini <[email protected]>Cc Martin Jansa <[email protected]>Cc Otavio Salvador <[email protected]>Cc Richard Purdie <[email protected]>--- scripts/create-pull-request | 74 ++++++++++++++++++++++++++++-------------- 1 files changed, 49 insertions(+), 25 deletions(-)scripts/create-pull-request +18
-10@@ -2,14 +2,13 @@ ODIR=pull-$$ RELATIVE_TO="master" COMMIT_ID="HEAD" -PULL_URL="git://git.pokylinux.org/poky-contrib.git" PREFIX="PATCH" usage() { CMD=$(basename $0) cat <<EOM -Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -b contrib_branch - -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree +Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -u remote -b branch + -b branch Branch name in the specified remote -h Display this help message -i commit_id Ending commit (default: HEAD) -m msg_body_file The file containing a blurb to be inserted into the summary email@@ -17,20 +16,21 @@ Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix -r relative_to Starting commit (default: master) -s subject The subject to be inserted into the summary email + -u remote The git remote where the branch is located Examples: - $CMD -b nitin/basic - $CMD -r distro/master -i nitin/distro -b nitin/distro - $CMD -r master -i misc -b nitin/misc -o pull-misc - $CMD -p "RFC PATCH" -b nitin/experimental + $CMD -u contrib -b nitin/basic + $CMD -u contrib -r distro/master -i nitin/distro -b nitin/distro + $CMD -u contrib -r master -i misc -b nitin/misc -o pull-misc + $CMD -u contrib -p "RFC PATCH" -b nitin/experimental EOM } # Parse and validate arguments -while getopts "b:hi:m:o:p:r:s:" OPT; do +while getopts "b:hi:m:o:p:r:s:u:" OPT; do case $OPT in b) - CONTRIB_BRANCH="$OPTARG" + BRANCH="$OPTARG" ;; h) usage@@ -58,23 +58,42 @@ while getopts "b:hi:m:o:p:r:s:" OPT; do s) SUBJECT="$OPTARG" ;; + u) + REMOTE="$OPTARG" + REMOTE_URL=$(git config remote.$REMOTE.url) + if [ $? -ne 0 ]; then + echo "ERROR: git config failed to find a url for '$REMOTE'" + exit 1 + fi + ;; esac done -if [ -z "$CONTRIB_BRANCH" ]; then +if [ -z "$BRANCH" ] || [ -z "$REMOTE_URL" ]; then usage exit 1 fi
scripts/create-pull-request while getopts "b:hi:m:o:p:r:s:" OPT; do · line 86
Quoted 10 lines
scripts/create-pull-request +9
-0+# Set WEB_URL from known remotes +case "$REMOTE_URL" in + *git.yoctoproject.org*) + WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; + *git.pokylinux.org*) + WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; +esac
Nit: what about if this is a remote branch on that server but not for that repo?
Quoted 52 lines
scripts/create-pull-request +22
-15+ # Perform a sanity test on the web URL. Issue a warning if it is not # accessible, but do not abort as users may want to run offline. -WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$CONTRIB_BRANCH" -wget -q $WEB_URL -O /dev/null -if [ $? -ne 0 ]; then - echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree." - echo " Please check your contrib-branch parameter before sending." - echo "" +if [ -n "$WEB_URL" ]; then + wget -q $WEB_URL -O /dev/null + if [ $? -ne 0 ]; then + echo "WARNING: Branch '$BRANCH' was not found on the contrib git tree." + echo " Please check your remote and branch parameter before sending." + echo "" + fi fi if [ -e $ODIR ]; then@@ -90,18 +109,23 @@ git format-patch -M --subject-prefix="$PREFIX" -n -o $ODIR --thread=shallow --co # Customize the cover letter CL="$ODIR/0000-cover-letter.patch" -(cat <<EOM +PM="$ODIR/pull-msg" +git request-pull $RELATIVE_TO $REMOTE_URL $COMMIT_ID > "$PM" +if [ $? -ne 0 ]; then + echo "ERROR: git request-pull reported an error" + exit 1 +fi -Pull URL: $PULL_URL - Branch: $CONTRIB_BRANCH - Browse: $WEB_URL +# The cover letter already has a diffstat, remove it from the pull-msg +# before inserting it. +sed -n "0,\#$REMOTE_URL# p" "$PM" | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" +rm "$PM" -Thanks, - $(git config user.name) <$(git config user.email)> ---- +# Insert the WEB_URL if there is one +if [ -n "$WEB_URL" ]; then + echo " $WEB_URL" | sed -i "\#$REMOTE_URL# r /dev/stdin" "$CL" +fi -EOM -) | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" # If the user specified a message body, insert it into the cover letter and # remove the BLURB token.
Signature · 3 lines
Joshua Lock
Yocto Build System Monkey
Intel Open Source Technology CentreRe: [RFC PATCH 09/10] send-pull-request: don't send all patches to everyone even with -a
In reply to Khem RajQuoted 12 lines · Khem Raj
Quoted 11 lines · Darren Hart
Rather than sending every patch to every recipient of the entire series when -a is used, only send the cover letter to everyone and use git's --signed-off-by-cc feature to generate an auto cc list for the individual patches.
Using git to harvest the Cc list means only collecting Signed-off-by and Cc lines, rather than the more generic *-by lines previously. This is a fair trade-off for significantly reduced complexity. If users want to add Acked-by and Tested-by lines and want to use the -a feature, they should include those recipients as Cc lines as well.
Quoted 5 lines
Quoted 3 lines
Now that we rely on git for auto-cc for the individual patches, make sure the user is prompted before sending each patch by forcing --confirm=always.
it can be tedious for large patchsets say 30-40 patches. and moreover
or 10.... <cough>
Quoted 3 lines
if something is wrong in middle of sending series of patches say a CC list is wrong then what to do as some of the patches would have already been sent.
We already break the process up by creating the patches in a directory for people review. Do we really need a third stage?
Quoted 4 lines
So I would suggest a dry pass
where it will show you the email headers and CC list so sender can be assured that all is well
and then he can send them all at once.
I was pondering this one, wondering who would catch it first ;-)
We currently loop through all the patches and send them individually, this is an artifact of the old code. We could send the cover letter with one git command, and all the rest with one more. This would much more closer match a typical git-send-email session, and would allow for using the "all" option from git-send-email.
As for a dry run followed by a send... that would be selecting no for all the send requests and validating them, then:
$ yes | ./send-pull-request ...
We could add a --dry-run (or -d I guess) option to make the first pass easier.
THoughts?
--
Darren
Quoted 129 lines
Quoted 129 lines
Signed-off-by Darren Hart <[email protected]>Cc Khem Raj <[email protected]>Cc Koen Kooi <[email protected]>--- scripts/send-pull-request | 60 +++++++++++++++++++++++++++------------------ 1 files changed, 36 insertions(+), 24 deletions(-)scripts/send-pull-request +36
-25@@ -4,33 +4,34 @@ AUTO=0 # Prevent environment leakage to these vars. unset TO unset CC +unset AUTO_CC usage() { cat <<EOM Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir -t email Explicitly add email to the recipients - -a Automatically harvest recipients from "*-by: email" lines - in the patches in the pull-dir + -a Automatically harvest recipients from Cc and Signed-off-by lines + in the patches in the pull-dir. These will be used on a per + patch basis. The cover-letter will be sent to all addresses. -p pull-dir Directory containing summary and patch files EOM } -# Collect To and CC addresses from the patch files if they exist -# $1: Which header to add the recipients to, "TO" or "CC" -# $2: The regex to match and strip from the line with email addresses +# Collect addresses from a patch into AUTO_CC +# $1: a patch file harvest_recipients() { - TO_CC=$1 - REGX=$2 + PATCH=$1 export IFS=$',\n' - for PATCH in $PDIR/*.patch; do - # Grab To addresses + for REGX in "^[Cc][Cc]: *" "^[Ss]igned-[Oo]ff-[Bb]y: *"; do for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do - if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then - if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi - elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then - if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi + if [ "${AUTO_CC/$EMAIL/}" == "$AUTO_CC" ] && [ -n "$EMAIL" ]; then + if [ -z "$AUTO_CC" ]; then + AUTO_CC=$EMAIL; + else + AUTO_CC="$AUTO_CC,$EMAIL"; + fi fi done done@@ -84,13 +85,11 @@ for TOKEN in SUBJECT BLURB; do done -# Harvest emails from the generated patches and populate the TO and CC variables -# In addition to To and CC headers/lines, the common Signed-off-by, Tested-by, -# etc. (*-by) will be added to CC. +# Harvest emails from the generated patches and populate AUTO_CC. if [ $AUTO -eq 1 ]; then - harvest_recipients TO "^[Tt][Oo]: *" - harvest_recipients CC "^[Cc][Cc]: *" - harvest_recipients CC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *" + for PATCH in $PDIR/*.patch; do + harvest_recipients $PATCH + done fi AUTO_TO="$(git config sendemail.to)"@@ -102,7 +101,7 @@ if [ -n "$AUTO_TO" ]; then fi fi -if [ -z "$TO" ] && [ -z "$CC" ]; then +if [ -z "$TO" ] && [ -z "$AUTO_CC" ]; then echo "ERROR: you have not specified any recipients." usage exit 1@@ -114,7 +113,8 @@ cat <<EOM The following patches: $(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done) -will now be sent via the git send-email command. +will now be sent via the git send-email command. Git will prompt you before +sending any email. EOM echo "Continue? [y/N] "@@ -124,11 +124,23 @@ if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then ERROR=0 export IFS=$',' GIT_TO=$(for R in $TO; do echo -n "--to='$R' "; done) - GIT_CC=$(for R in $CC; do echo -n "--cc='$R' "; done) + GIT_CC=$(for R in $AUTO_CC; do echo -n "--cc='$R' "; done) unset IFS for PATCH in $PDIR/*patch; do - # We harvest the emails manually, so force git not to. - eval "git send-email $GIT_TO $GIT_CC --no-chain-reply-to --suppress-cc=all $PATCH" + if [ $AUTO -eq 1 ]; then + if [ $PATCH == "$CL" ]; then + # Send the cover letter to every recipient, both + # specified as well as harvested. + eval "git send-email $GIT_TO $GIT_CC --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + else + # Send the patch to the specified recipients and + # those git finds in this specific patch. + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --signed-off-by-cc $PATCH" + fi + else + # Only send to the explicitly specified recipients + eval "git send-email $GIT_TO --confirm=always --no-chain-reply-to --suppress-cc=all $PATCH" + fi if [ $? -eq 1 ]; then ERROR=1 fi --1.7.1
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Re: [RFC PATCH 03/10] create-pull-request: use git request-pull and arbitrary remotes
In reply to Joshua LockQuoted 89 lines · Joshua Lock
Quoted 88 lines · Darren Hart
Allow for arbitrary remotes via a new -u argument. Remove the hard coded references to the pokylinux repositories. Create the
WEB_URLfrom known remotes. Future patches can add additionalWEB_URLmappings for remotes like Git Hub, Gitorious, and kernel.org.Rather than duplicating the git request-pull command ourselves, just use the existing one.
Signed-off-by Darren Hart <[email protected]>Cc Tom Rini <[email protected]>Cc Martin Jansa <[email protected]>Cc Otavio Salvador <[email protected]>Cc Richard Purdie <[email protected]>--- scripts/create-pull-request | 74 ++++++++++++++++++++++++++++-------------- 1 files changed, 49 insertions(+), 25 deletions(-)scripts/create-pull-request +18
-10@@ -2,14 +2,13 @@ ODIR=pull-$$ RELATIVE_TO="master" COMMIT_ID="HEAD" -PULL_URL="git://git.pokylinux.org/poky-contrib.git" PREFIX="PATCH" usage() { CMD=$(basename $0) cat <<EOM -Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -b contrib_branch - -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree +Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -u remote -b branch + -b branch Branch name in the specified remote -h Display this help message -i commit_id Ending commit (default: HEAD) -m msg_body_file The file containing a blurb to be inserted into the summary email@@ -17,20 +16,21 @@ Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix -r relative_to Starting commit (default: master) -s subject The subject to be inserted into the summary email + -u remote The git remote where the branch is located Examples: - $CMD -b nitin/basic - $CMD -r distro/master -i nitin/distro -b nitin/distro - $CMD -r master -i misc -b nitin/misc -o pull-misc - $CMD -p "RFC PATCH" -b nitin/experimental + $CMD -u contrib -b nitin/basic + $CMD -u contrib -r distro/master -i nitin/distro -b nitin/distro + $CMD -u contrib -r master -i misc -b nitin/misc -o pull-misc + $CMD -u contrib -p "RFC PATCH" -b nitin/experimental EOM } # Parse and validate arguments -while getopts "b:hi:m:o:p:r:s:" OPT; do +while getopts "b:hi:m:o:p:r:s:u:" OPT; do case $OPT in b) - CONTRIB_BRANCH="$OPTARG" + BRANCH="$OPTARG" ;; h) usage@@ -58,23 +58,42 @@ while getopts "b:hi:m:o:p:r:s:" OPT; do s) SUBJECT="$OPTARG" ;; + u) + REMOTE="$OPTARG" + REMOTE_URL=$(git config remote.$REMOTE.url) + if [ $? -ne 0 ]; then + echo "ERROR: git config failed to find a url for '$REMOTE'" + exit 1 + fi + ;; esac done -if [ -z "$CONTRIB_BRANCH" ]; then +if [ -z "$BRANCH" ] || [ -z "$REMOTE_URL" ]; then usage exit 1 fi
Quoted 12 lines
scripts/create-pull-request while getopts "b:hi:m:o:p:r:s:" OPT; do · line 86
Quoted 9 lines
scripts/create-pull-request +9
-0+# Set WEB_URL from known remotes +case "$REMOTE_URL" in + *git.yoctoproject.org*) + WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; + *git.pokylinux.org*) + WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; +esacNit: what about if this is a remote branch on that server but not for that repo?
Doh, that isn't a nit, that's a bug. Thanks for catching it!
--
Darren
Quoted 54 lines
Quoted 53 lines
scripts/create-pull-request +22
-15+ # Perform a sanity test on the web URL. Issue a warning if it is not # accessible, but do not abort as users may want to run offline. -WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$CONTRIB_BRANCH" -wget -q $WEB_URL -O /dev/null -if [ $? -ne 0 ]; then - echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree." - echo " Please check your contrib-branch parameter before sending." - echo "" +if [ -n "$WEB_URL" ]; then + wget -q $WEB_URL -O /dev/null + if [ $? -ne 0 ]; then + echo "WARNING: Branch '$BRANCH' was not found on the contrib git tree." + echo " Please check your remote and branch parameter before sending." + echo "" + fi fi if [ -e $ODIR ]; then@@ -90,18 +109,23 @@ git format-patch -M --subject-prefix="$PREFIX" -n -o $ODIR --thread=shallow --co # Customize the cover letter CL="$ODIR/0000-cover-letter.patch" -(cat <<EOM +PM="$ODIR/pull-msg" +git request-pull $RELATIVE_TO $REMOTE_URL $COMMIT_ID > "$PM" +if [ $? -ne 0 ]; then + echo "ERROR: git request-pull reported an error" + exit 1 +fi -Pull URL: $PULL_URL - Branch: $CONTRIB_BRANCH - Browse: $WEB_URL +# The cover letter already has a diffstat, remove it from the pull-msg +# before inserting it. +sed -n "0,\#$REMOTE_URL# p" "$PM" | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" +rm "$PM" -Thanks, - $(git config user.name) <$(git config user.email)> ---- +# Insert the WEB_URL if there is one +if [ -n "$WEB_URL" ]; then + echo " $WEB_URL" | sed -i "\#$REMOTE_URL# r /dev/stdin" "$CL" +fi -EOM -) | sed -i "/BLURB HERE/ r /dev/stdin" "$CL" # If the user specified a message body, insert it into the cover letter and # remove the BLURB token.
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Re: [RFC PATCH 03/10] create-pull-request: use git request-pull and arbitrary remotes
In reply to Joshua LockQuoted 13 lines
Review
Quoted 10 lines
(quoted diff) +9
-0+# Set WEB_URL from known remotes +case "$REMOTE_URL" in + *git.yoctoproject.org*) + WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; + *git.pokylinux.org*) + WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; +esacNit: what about if this is a remote branch on that server but not for that repo?
I'll roll a fix into V2, how about this:
scripts/create-pull-request +3 -2
@@ -70,6 +70,7 @@ while getopts "b:chi:m:o:p:r:s:u:" OPT; do echo "ERROR: git config failed to find a url for '$REMOTE'" exit 1 fi + REMOTE_REPO=$(echo $REMOTE_URL | sed "s#.*/\(.*\)\(.git\)#\1#") # Rewrite known private URLs to public URLs case "$REMOTE_URL" in
@@ -97,10 +98,10 @@ fi # Set WEB_URL from known remotes case "$REMOTE_URL" in *git.yoctoproject.org*) - WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + WEB_URL="http://git.yoctoproject.org/cgit.cgi/$REMOTE_REPO/log/?h=$BRANCH" ;; *git.pokylinux.org*) - WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + WEB_URL="http://git.pokylinux.org/cgit.cgi/$REMOTE_REPO/log/?h=$BRANCH" ;; esac
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Note that oe-core has one additional commit to create-pull-request which I didn't have in poky. I guess the scripts dirs aren't kept in sync the same way meta is.
If you want to apply these as they are and not wait for v2, you just need to revert the following commit from oe-core:
commit 84e07ef76bf4f484a2d5e0180632e7a4981a1af7
Author: Martin Jansa <[email protected]> Date: Tue Mar 15 11:07:22 2011 +0100
create-pull-request: switch URL from git.pokylinux.org/poky-contrib
to git.o
There is nothing wrong with the patch, I just didn't have it and this series obsoletes it, so reverting it was easier than dealing with the merge conflicts ;-)
--
Darren
Quoted 34 lines · Darren Hart
After some discussion on the oe-core list regarding the *-pull-request scripts I provided a proposal, received some feedback, and these patches are the result of that discussion. The Cc list varies from patch to patch a bit more wildly than is typical as a final sanity test, although it does reflect who commented and expressed interest in which parts of the overhaul.
Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.
The following changes since commit
65e5ebc1d27b0a6cb2e33596d6d32c536b857f0d:tcmode-default: fix a typo (2011-05-13 11:02:20 +0100)
are available in the git repository at:
git://git.pokylinux.org/poky-contrib.gitdvhart/git-pull
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dvhart/git-pullDarren Hart (10):
create-pull-request: alphabetize arguments
create-pull-request: whitespace cleanup
create-pull-request: use git request-pull and arbitrary remotes
create-pull-request: rewrite known private URLs to public URLs
create-pull-request: provide an RFC mode via -c argument
send-pull-request: whitespace cleanup
send-pull-request: remove local mta support
send-pull-request: fix greedy auto-cc regex
send-pull-request: don't send all patches to everyone even with -a
send-pull-request: verify git sendemail configscripts/create-pull-request | 176 +++++++++++++++++++---------- scripts/send-pull-request | 265 ++++++++++++++++++------------------------- 2 files changed, 228 insertions(+), 213 deletions(-)
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Re: [RFC PATCH 08/10] send-pull-request: fix greedy auto-cc regex
In reply to Darren HartQuoted 6 lines · Darren Hart
A greedy regular expression caused emails to be harvested from patches that were quoted in the commit message. Ensure only tags that start at the beginning of the line are considered for harvesting.
NOTE: users are still responsible for verifying the recipients list and to ensure they do not spam people!
This is key!
But a change for the better.
Bruce
Quoted 31 lines
Signed-off-by Darren Hart <[email protected]>Cc Richard Purdie <[email protected]>Cc Saul Wold <[email protected]>Cc Bruce Ashfield <[email protected]>---
scripts/send-pull-request | 2 +-1 files changed, 1 insertions(+), 1 deletions(-)scripts/send-pull-request +0
-0@@ -90,7 +90,7 @@ doneif [ $AUTO -eq 1 ]; then
harvest_recipients TO "^[Tt][Oo]: *"harvest_recipientsCC "^[Cc][Cc]: *"
-harvest_recipientsCC "^.*-[Bb][Yy]: *"
+harvest_recipientsCC "^[A-Z][A-Za-z-]*-[Bb][Yy]: *" fiAUTO_TO="$(git config sendemail.to)"
--
1.7.1_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"
Re: [RFC PATCH 10/10] send-pull-request: verify git sendemail config
In reply to Darren HartQuoted 16 lines · Darren Hart
Perform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
Signed-off-by Darren Hart <[email protected]>---
scripts/send-pull-request | 15 +++++++++++++++1 files changed, 15 insertions(+), 0 deletions(-)scripts/send-pull-request +0
-0@@ -38,6 +38,18 @@ harvest_recipients()unset IFS
}
scripts/send-pull-request harvest_recipients() · line 49
Quoted 9 lines
scripts/send-pull-request +9
-0+check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from"
I was going to ask for this to be optional, since I always pass the smtp settings on the command line .. but then I realized that I invoke git send-email directly anyway, so I don't need it to be optional. :)
Regardless, reading this from git config is good idea, so the above is only a comment.
Bruce
Quoted 25 lines
+ exit 1
+ fi
+}# Parse and verify arguments
while getopts "ahp:t:" OPT; doscripts/send-pull-request +0
-0@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; doesac
donescripts/send-pull-request +3
-0+# Abort early if git-send-email is not properly configured +check_git_sendemail_config +if [ -z "$PDIR" ]; then
echo "ERROR: you must specify a pull-dir."
usage
--
1.7.1_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"
Re: [RFC PATCH 10/10] send-pull-request: verify git sendemail config
Quoted 31 lines · Bruce Ashfield
Quoted 15 lines · Darren Hart
Perform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
Signed-off-by Darren Hart <[email protected]>--- scripts/send-pull-request | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)scripts/send-pull-request +0
-0@@ -38,6 +38,18 @@ harvest_recipients() unset IFS }scripts/send-pull-request harvest_recipients() · line 49
Quoted 10 lines
scripts/send-pull-request +9
-0+check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from"I was going to ask for this to be optional, since I always pass the smtp settings on the command line .. but then I realized that I invoke git send-email directly anyway, so I don't need it to be optional. :)
Quoted 2 lines
Regardless, reading this from git config is good idea, so the above is only a comment.
This only impacts the send-pull-request script which requires git-send-email to be configured in order to work, so it really can't be an optional thing.
--
Darren
Quoted 31 lines
Bruce
Quoted 25 lines
scripts/send-pull-request +6
-1+ exit 1 + fi +} # Parse and verify arguments while getopts "ahp:t:" OPT; do@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do esac done +# Abort early if git-send-email is not properly configured +check_git_sendemail_config + if [ -z "$PDIR" ]; then echo "ERROR: you must specify a pull-dir." usage --1.7.1
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Re: [RFC PATCH 10/10] send-pull-request: verify git sendemail config
Quoted 34 lines · Darren Hart
Quoted 31 lines · Bruce Ashfield
Quoted 16 lines · Darren Hart
Perform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
Signed-off-by Darren Hart <[email protected]>---
scripts/send-pull-request | 15 +++++++++++++++1 files changed, 15 insertions(+), 0 deletions(-)scripts/send-pull-request +0
-0@@ -38,6 +38,18 @@ harvest_recipients()unset IFS
}scripts/send-pull-request harvest_recipients() · line 49
Quoted 9 lines
scripts/send-pull-request +9
-0+check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from"I was going to ask for this to be optional, since I always pass the smtp settings on the command line .. but then I realized that I invoke git send-email directly anyway, so I don't need it to be optional. :)
Quoted 6 lines
Quoted 2 lines
Regardless, reading this from git config is good idea, so the above is only a comment.
This only impacts the send-pull-request script which requires git-send-email to be configured in order to work, so it really can't be an optional thing.
I meant making it optional from being in your .gitconfig. I use git send-email with a smtp server almost every day .. and it's not in my .gitconfig :)
Bruce
Quoted 41 lines
--
DarrenQuoted 23 lines
Bruce
Quoted 20 lines
+ exit 1
+ fi
+}# Parse and verify arguments
while getopts "ahp:t:" OPT; doscripts/send-pull-request +0
-0@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; doesac
donescripts/send-pull-request +3
-0+# Abort early if git-send-email is not properly configured +check_git_sendemail_config +if [ -z "$PDIR" ]; then
echo "ERROR: you must specify a pull-dir."
usage
--
1.7.1Quoted 8 lines
Quoted 5 lines
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"
Re: [RFC PATCH 10/10] send-pull-request: verify git sendemail config
Quoted 38 lines · Bruce Ashfield
Quoted 37 lines · Darren Hart
Quoted 33 lines · Bruce Ashfield
Quoted 15 lines · Darren Hart
Perform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
Signed-off-by Darren Hart <[email protected]>--- scripts/send-pull-request | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)scripts/send-pull-request +0
-0@@ -38,6 +38,18 @@ harvest_recipients() unset IFS }scripts/send-pull-request harvest_recipients() · line 49
Quoted 10 lines
scripts/send-pull-request +9
-0+check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from"I was going to ask for this to be optional, since I always pass the smtp settings on the command line .. but then I realized that I invoke git send-email directly anyway, so I don't need it to be optional. :)
Regardless, reading this from git config is good idea, so the above is only a comment.
Quoted 7 lines
Quoted 3 lines
This only impacts the send-pull-request script which requires git-send-email to be configured in order to work, so it really can't be an optional thing.
I meant making it optional from being in your .gitconfig. I use git send-email with a smtp server almost every day .. and it's not in my .gitconfig :)
Right, duh. OK, so it order to make this optional, we would have to add all the necessary options to pass through from the script to git-send-email. I'd really rather avoid that.
What is your objection to having a default smtp/from in your git config? You can always override it on the command line, and with confirm=always in your git config (you're sane right?) there is little risk of sending from the wrong account accidentally.
--
Darren
Quoted 47 lines
Bruce
Quoted 41 lines
--
DarrenQuoted 23 lines
Bruce
Quoted 20 lines
scripts/send-pull-request +6
-1+ exit 1 + fi +} # Parse and verify arguments while getopts "ahp:t:" OPT; do@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do esac done +# Abort early if git-send-email is not properly configured +check_git_sendemail_config + if [ -z "$PDIR" ]; then echo "ERROR: you must specify a pull-dir." usage --1.7.1
Quoted 8 lines
Quoted 5 lines
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
Signature · 3 lines
Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel
Re: [RFC PATCH 10/10] send-pull-request: verify git sendemail config
Quoted 43 lines · Darren Hart
Quoted 42 lines · Bruce Ashfield
Quoted 40 lines · Darren Hart
Quoted 31 lines · Bruce Ashfield
Quoted 16 lines · Darren Hart
Perform a quick sanity check to be able to direct users to configure git.sendemail if they haven't yet.
Signed-off-by Darren Hart <[email protected]>---
scripts/send-pull-request | 15 +++++++++++++++1 files changed, 15 insertions(+), 0 deletions(-)scripts/send-pull-request +0
-0@@ -38,6 +38,18 @@ harvest_recipients()unset IFS
}scripts/send-pull-request harvest_recipients() · line 49
Quoted 9 lines
scripts/send-pull-request +9
-0+check_git_sendemail_config() +{ + GIT_SMTP=$(git config sendemail.smtpserver) + GIT_FROM=$(git config sendemail.from) + if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then + echo "ERROR: git sendemail is not configured." + echo "Please read GIT-SEND-EMAIL(1) and configure:" + echo " sendemail.smtpserver" + echo " sendemail.from"I was going to ask for this to be optional, since I always pass the smtp settings on the command line .. but then I realized that I invoke git send-email directly anyway, so I don't need it to be optional. :)
Quoted 2 lines
Regardless, reading this from git config is good idea, so the above is only a comment.
This only impacts the send-pull-request script which requires git-send-email to be configured in order to work, so it really can't be an optional thing.
Quoted 7 lines
Quoted 3 lines
I meant making it optional from being in your .gitconfig. I use git send-email with a smtp server almost every day .. and it's not in my .gitconfig :)
Right, duh. OK, so it order to make this optional, we would have to add all the necessary options to pass through from the script to git-send-email. I'd really rather avoid that.
Exactly. that's why I dropped my idea of asking for it to be optional, you'd need to chain it down, and that's a bad idea.
Quoted 5 lines
What is your objection to having a default smtp/from in your git config? You can always override it on the command line, and with confirm=always in your git config (you're sane right?) there is little risk of sending from the wrong account accidentally.
I just have a zillion boxes, some with different connectivity, so that is typically left out of my config. The hard error would cause me minimal pain, but I invoke git send-email by hand, so it's no problem for me in the long run.
So there's nothing here for you to change, if I needed something, I could do it locally.
Cheers,
Bruce
Quoted 57 lines
--
DarrenQuoted 47 lines
Bruce
Quoted 41 lines
--
DarrenQuoted 23 lines
Bruce
Quoted 20 lines
+ exit 1
+ fi
+}# Parse and verify arguments
while getopts "ahp:t:" OPT; doscripts/send-pull-request +0
-0@@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; doesac
donescripts/send-pull-request +3
-0+# Abort early if git-send-email is not properly configured +check_git_sendemail_config +if [ -z "$PDIR" ]; then
echo "ERROR: you must specify a pull-dir."
usage
--
1.7.1Quoted 8 lines
Quoted 5 lines
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
Signature · 2 lines
"Thou shalt not follow the NULL pointer, for chaos and madness await thee at its end"
Joshua Lock
Acked-byRe: [RFC PATCH 03/10] create-pull-request: use git request-pull and arbitrary remotes
In reply to Darren HartQuoted 31 lines · Darren Hart
Quoted 4 lines
Quoted 4 lines
(quoted diff) +4
-0+# Set WEB_URL from known remotes +case "$REMOTE_URL" in + *git.yoctoproject.org*) + WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH"Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ ;; + *git.pokylinux.org*) + WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + ;; +esacNit: what about if this is a remote branch on that server but not for that repo?
I'll roll a fix into V2, how about this:
scripts/create-pull-request +1
-0@@ -70,6 +70,7 @@ while getopts "b:chi:m:o:p:r:s:u:" OPT; do echo "ERROR: git config failed to find a url for '$REMOTE'" exit 1 fi + REMOTE_REPO=$(echo $REMOTE_URL | sed "s#.*/\(.*\)\(.git\)#\1#") # Rewrite known private URLs to public URLs case "$REMOTE_URL" in@@ -97,10 +98,10 @@ fi # Set WEB_URL from known remotes case "$REMOTE_URL" in
scripts/create-pull-request fi · line 107
Quoted 10 lines
scripts/create-pull-request +2
-2*git.yoctoproject.org*) - WEB_URL="http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + WEB_URL="http://git.yoctoproject.org/cgit.cgi/$REMOTE_REPO/log/?h=$BRANCH" ;; *git.pokylinux.org*) - WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$BRANCH" + WEB_URL="http://git.pokylinux.org/cgit.cgi/$REMOTE_REPO/log/?h=$BRANCH" ;; esac
Looks good to me.
Cheers,
Joshua
Signature · 3 lines
Joshua Lock
Yocto Build System Monkey
Intel Open Source Technology Centre