On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > > Reminder, we have this new list dim-tools@xxxxxxxxxxxxxxxxxxxxx for > maintainer tools patches. Cc'd. > Ahh, cool. I didn't realize dim grew up! > On Mon, 30 Oct 2017, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote: >> Expanding on Jani's work to sign tags, this patch adds signing for git >> commit/am. > > I guess I'd like more rationale here. Is this something we should be > doing? Is anyone else doing this? > Sure thing. Signing commits allows Dave to use --verify-signatures when pulling. If something is not signed, we'll know it was either not applied with dim, or was altered on fdo (both warrant investigation). I suspect no one else is doing this since most trees are single maintainer, and it's not possible to sign commits via git send-email. Since we have the committer model, and a bunch of people with access to fdo and the tree, I think it's important to add this. Especially since we can do it in dim without overhead. >> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx> >> --- >> >> This has been lightly tested with dim apply-branch/dim push-branch. >> >> Sean >> >> dim | 78 +++++++++++++++++++++++++++++++++++++++++++++------------------------ >> 1 file changed, 51 insertions(+), 27 deletions(-) >> >> diff --git a/dim b/dim >> index 527989aff9ad..cd5e41f89a3a 100755 >> --- a/dim >> +++ b/dim >> @@ -67,9 +67,6 @@ DIM_TEMPLATE_SIGNATURE=${DIM_TEMPLATE_SIGNATURE:-$HOME/.dim.template.signature} >> # dim pull-request tag summary template >> DIM_TEMPLATE_TAG_SUMMARY=${DIM_TEMPLATE_TAG_SUMMARY:-$HOME/.dim.template.tagsummary} >> >> -# GPG key id for signing tags. If unset, don't sign. >> -DIM_GPG_KEYID=${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID} >> - >> # >> # Internal configuration. >> # >> @@ -104,6 +101,20 @@ test_request_recipients=( >> # integration configuration >> integration_config=nightly.conf >> >> +# GPG key id for signing tags. If unset, don't sign. >> +function gpg_keyid_for_tag >> +{ >> + echo "${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID}" >> + return 0 >> +} >> + >> +# GPG key id for committing (git commit/am). If unset, don't sign. >> +function gpg_keyid_for_commit >> +{ >> + echo "${DIM_GPG_KEYID:+-S$DIM_GPG_KEYID}" >> + return 0 >> +} > > This seems like an overly complicated way to achieve what you want. > > Just put these under "Internal configuration." instead: > > dim_gpg_sign_tag=${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID} > dim_gpg_sign_commit=${DIM_GPG_KEYID:+-S$DIM_GPG_KEYID} > > And use directly in git tag and commit, respectively? > Yep, sounds good. > Although... perhaps starting to sign tags should not force signing > commits? > Why would it be desirable to *not* sign tags? Sean > BR, > Jani. > > >> + >> function read_integration_config >> { >> # clear everything first to allow configuration reload >> @@ -473,12 +484,14 @@ EOF >> # append all arguments as tags at the end of the commit message of HEAD >> function dim_commit_add_tag >> { >> + local gpg_keyid >> + gpg_keyid=$(gpg_keyid_for_commit) >> for arg; do >> # the first sed deletes all trailing blank lines at the end >> git log -1 --pretty=%B | \ >> sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' | \ >> sed "\$a${arg}" | \ >> - git commit --amend -F- >> + git commit $gpg_keyid --amend -F- >> done >> } >> >> @@ -604,7 +617,7 @@ function update_rerere_cache >> >> function commit_rerere_cache >> { >> - local remote file commit_message >> + local remote file commit_message gpg_keyid >> >> echo -n "Updating rerere cache... " >> >> @@ -640,7 +653,8 @@ function commit_rerere_cache >> $(git --version) >> EOF >> >> - if git commit -F $commit_message >& /dev/null; then >> + gpg_keyid=$(gpg_keyid_for_commit) >> + if git commit $gpg_keyid -F $commit_message >& /dev/null; then >> echo -n "New commit. " >> else >> echo -n "Nothing changed. " >> @@ -653,13 +667,14 @@ function commit_rerere_cache >> >> function dim_rebuild_tip >> { >> - local integration_branch specfile first rerere repo remote >> + local integration_branch specfile first rerere repo remote gpg_keyid >> >> integration_branch=drm-tip >> specfile=$(mktemp) >> first=1 >> >> rerere=$DIM_PREFIX/drm-rerere >> + gpg_keyid=$(gpg_keyid_for_commit) >> >> cd $rerere >> if git status --porcelain | grep -q -v "^[ ?][ ?]"; then >> @@ -731,7 +746,7 @@ function dim_rebuild_tip >> >> # because we filter out fast-forward merges there will >> # always be something to commit >> - git commit --no-edit --quiet >> + git commit $gpg_keyid --no-edit --quiet >> echo "Done." >> fi >> >> @@ -743,7 +758,7 @@ function dim_rebuild_tip >> echo -n "Adding integration manifest $integration_branch: $dim_timestamp... " >> mv $specfile integration-manifest >> git add integration-manifest >> - git commit --quiet -m "$integration_branch: $dim_timestamp integration manifest" >> + git commit $gpg_keyid --quiet -m "$integration_branch: $dim_timestamp integration manifest" >> echo "Done." >> >> remote=$(repo_to_remote drm-tip) >> @@ -848,7 +863,7 @@ function dim_push >> >> function apply_patch #patch_file >> { >> - local patch message_id committer_email patch_from sob rv >> + local patch message_id committer_email patch_from sob rv gpg_keyid >> >> patch="$1" >> shift >> @@ -860,7 +875,8 @@ function apply_patch #patch_file >> sob=-s >> fi >> >> - git am --scissors -3 $sob "$@" $patch >> + gpg_keyid=$(gpg_keyid_for_commit) >> + git am --scissors -3 $sob $gpg_keyid "$@" $patch >> >> if [ -n "$message_id" ]; then >> dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id" >> @@ -911,7 +927,7 @@ function dim_apply_branch >> >> function dim_apply_pull >> { >> - local branch file message_id pull_branch rv >> + local branch file message_id pull_branch rv gpg_keyid >> >> branch=${1:?$usage} >> file=$(mktemp) >> @@ -929,7 +945,8 @@ function dim_apply_pull >> >> message_id=$(message_get_id $file) >> >> - git commit --amend -s --no-edit >> + gpg_keyid=$(gpg_keyid_for_commit) >> + git commit $gpg_keyid --amend -s --no-edit >> if [ -n "$message_id" ]; then >> dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id" >> else >> @@ -945,7 +962,7 @@ function dim_apply_pull >> >> function dim_backmerge >> { >> - local branch upstream patch_file >> + local branch upstream patch_file gpg_keyid >> >> branch=${1:?$usage} >> upstream=${2:?$usage} >> @@ -990,8 +1007,9 @@ function dim_backmerge >> echoerr " git commit -a" >> fi >> >> + gpg_keyid=$(gpg_keyid_for_commit) >> git add -u >> - git commit -s >> + git commit $gpg_keyid -s >> } >> >> function dim_add_link >> @@ -1227,7 +1245,7 @@ function dim_magic_patch >> >> function dim_create_branch >> { >> - local branch repo remote >> + local branch repo remote gpg_keyid >> >> branch=${1:?$usage} >> start=${2:-HEAD} >> @@ -1250,13 +1268,14 @@ function dim_create_branch >> cd $DIM_PREFIX/drm-rerere >> $DRY sed -i "s/^\() # DO NOT CHANGE THIS LINE\)$/\t\"$repo\t\t${branch//\//\\\/}\"\n\1/" $integration_config >> >> + gpg_keyid=$(gpg_keyid_for_commit) >> $DRY git add $integration_config >> - $DRY git commit --quiet -m "Add $repo $branch to $integration_config" >> + $DRY git commit $gpg_keyid --quiet -m "Add $repo $branch to $integration_config" >> } >> >> function dim_remove_branch >> { >> - local branch repo remote >> + local branch repo remote gpg_keyid >> >> branch=${1:?$usage} >> >> @@ -1288,8 +1307,9 @@ function dim_remove_branch >> cd $DIM_PREFIX/drm-rerere >> $DRY sed -i "/^[[:space:]]*\"${repo}[[:space:]]\+${branch//\//\\\/}.*$/d" $integration_config >> >> + gpg_keyid=$(gpg_keyid_for_commit) >> $DRY git add $integration_config >> - $DRY git commit --quiet -m "Remove $repo $branch from $integration_config" >> + $DRY git commit $gpg_keyid --quiet -m "Remove $repo $branch from $integration_config" >> >> dim_rebuild_tip >> } >> @@ -1579,7 +1599,7 @@ function dim_for_each_workdir >> >> function dim_update_next >> { >> - local remote >> + local remote gpg_keyid >> >> assert_branch drm-intel-next-queued >> >> @@ -1597,12 +1617,13 @@ function dim_update_next >> exit 2 >> fi >> >> + gpg_keyid=$(gpg_keyid_for_commit) >> driver_date=$(date +%Y%m%d) >> driver_timestamp=$(date +%s) >> $DRY sed -i -e "s/^#define DRIVER_DATE.*\"[0-9]*\"$/#define DRIVER_DATE\t\t\"$driver_date\"/; s/^#define DRIVER_TIMESTAMP.*/#define DRIVER_TIMESTAMP\t$driver_timestamp/" \ >> drivers/gpu/drm/i915/i915_drv.h >> $DRY git add drivers/gpu/drm/i915/i915_drv.h >> - git commit $DRY_RUN -sm "drm/i915: Update DRIVER_DATE to $driver_date" >> + git commit $DRY_RUN $gpg_keyid -sm "drm/i915: Update DRIVER_DATE to $driver_date" >> >> gitk drm-intel-next-queued ^$(repo_to_remote drm-upstream)/drm-next & >> >> @@ -1614,7 +1635,7 @@ function dim_update_next >> >> function dim_update_next_continue >> { >> - local remote intel_remote req_file suffix tag tag_testing >> + local remote intel_remote req_file suffix tag tag_testing gpg_keyid >> >> assert_branch drm-intel-next-queued >> >> @@ -1630,7 +1651,8 @@ function dim_update_next_continue >> tag_testing="drm-intel-testing-$dim_today-$((++suffix))" >> done >> >> - $DRY git tag -a $DIM_GPG_KEYID $tag $intel_remote/drm-intel-next >> + gpg_keyid=$(gpg_keyid_for_tag) >> + $DRY git tag -a $gpg_keyid $tag $intel_remote/drm-intel-next >> git push $DRY_RUN $intel_remote $tag >> >> echo "Updating drm-intel-testing to latest drm-tip" >> @@ -1655,7 +1677,7 @@ function dim_update_next_continue >> >> function dim_tag_next >> { >> - local intel_remote tag suffix >> + local intel_remote tag suffix gpg_keyid >> >> cd $DIM_PREFIX/$DIM_REPO >> >> @@ -1670,7 +1692,8 @@ function dim_tag_next >> tag="drm-intel-next-$dim_today-$((++suffix))" >> done >> >> - $DRY git tag -a $DIM_GPG_KEYID $tag $intel_remote/drm-intel-next >> + gpg_keyid=$(gpg_keyid_for_tag) >> + $DRY git tag -a $gpg_keyid $tag $intel_remote/drm-intel-next >> git push $DRY_RUN $intel_remote $tag >> else >> echo "drm-intel-next not up-to-date, aborting" >> @@ -1700,7 +1723,7 @@ function prep_pull_tag_summary >> # dim_pull_request branch upstream >> function dim_pull_request >> { >> - local branch upstream remote repo req_file url_list git_url suffix tag >> + local branch upstream remote repo req_file url_list git_url suffix tag gpg_keyid >> >> branch=${1:?$usage} >> upstream=${2:?$usage} >> @@ -1731,7 +1754,8 @@ function dim_pull_request >> done >> gitk "$branch@{upstream}" ^$upstream & >> prep_pull_tag_summary | $DRY git tag -F- $tag "$branch@{upstream}" >> - $DRY git tag -a $DIM_GPG_KEYID -f $tag >> + gpg_keyid=$(gpg_keyid_for_tag) >> + $DRY git tag -a $gpg_keyid -f $tag >> $DRY git push $remote $tag >> prep_pull_mail $req_file $tag > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx