[Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in addition to tags
Daniel Vetter
daniel at ffwll.ch
Tue Oct 31 09:02:29 UTC 2017
On Tue, Oct 31, 2017 at 10:27:24AM +0200, Jani Nikula wrote:
>
> Reminder, we have this new list dim-tools at lists.freedesktop.org for
> maintainer tools patches. Cc'd.
>
> On Mon, 30 Oct 2017, Sean Paul <seanpaul at chromium.org> 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?
Same here. I get why signing tags makes sense, because email is an
entirely unsecured protocol. Signing commits otoh seems mildly silly.
What's the threat/attack model that prevents?
And if we go with signed commits, shouldn't we encourage contributors to
sign their mails and have some means to add the verification of the same
to the commit? This is what happens when you merge a signed patch at least
...
I'm confused about this.
-Daniel
>
> > Signed-off-by: Sean Paul <seanpaul at chromium.org>
> > ---
> >
> > 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?
>
> Although... perhaps starting to sign tags should not force signing
> commits?
>
> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list