Page MenuHomePhabricator
Differential D7480 Diff 23570 contrib/source-control-tools/land-revision.sh

Changeset View

Standalone View

View Options

contrib/source-control-tools/land-revision.sh

Show First 20 Lines • Show All 49 Lines • ▼ Show 20 Lines *)
;; ;;
esac esac
done done
REVISION="$1" REVISION="$1"
TOPLEVEL=$(git rev-parse --show-toplevel) TOPLEVEL=$(git rev-parse --show-toplevel)
SOURCE_CONTROL_TOOLS="${TOPLEVEL}"/contrib/source-control-tools SOURCE_CONTROL_TOOLS="${TOPLEVEL}"/contrib/source-control-tools
"${SOURCE_CONTROL_TOOLS}"/apply-revision.sh "${REVISION}"
# Copy the apply revision script out of the source tree. This ensures any
# modifications to that script do not interfere with checks on revision
# acceptance.
TEMP_SCRIPT="$(mktemp)"
cleanup() {
rm -f "${TEMP_SCRIPT}"
}
trap "cleanup" EXIT
cp -p "${SOURCE_CONTROL_TOOLS}"/apply-revision.sh "${TEMP_SCRIPT}"
"${TEMP_SCRIPT}" "${REVISION}"
deadalnixUnsubmitted
Not Done
Inline Actions

This is pretty bad. Either you want to allow patches to modify these script in a meaningful way - in which case it's all good - or you don't and in which case, you need to do this for every script, which is never going to work because it's fragile.

deadalnix: This is pretty bad. Either you want to allow patches to modify these script in a meaningful way…
jasonbcoxAuthorUnsubmitted
Done
Inline Actions

You don't need to do this for any other script. It's only necessary here to ensure the revision was actually accepted. Any changes that impact the behavior of the pipeline after this point would have been reviewed and accepted.

The current design has a similar restriction in that changes to check-revision-accepted.sh do not affect the pipeline until the change is landed.
The tradeoff here is:

  1. Current solution - More complexity and code.
  2. This patch - Slightly increased scope on the logic in this pipeline that does not get updated until the patch is successfully landed. I do not expect this logic to change often, though.
jasonbcox: You don't need to do this for any other script. It's only necessary here to ensure the revision…
# shellcheck source=sanitize-conduit-token.sh # shellcheck source=sanitize-conduit-token.sh
source "${SOURCE_CONTROL_TOOLS}"/sanitize-conduit-token.sh source "${SOURCE_CONTROL_TOOLS}"/sanitize-conduit-token.sh
# IMPORTANT NOTE: The patch is trusted past this point because it has been reviewed # IMPORTANT NOTE: The patch is trusted past this point because it has been reviewed
# and accepted. That includes any changes that may affect this workflow from this # and accepted. That includes any changes that may affect this workflow from this
# point onwards. # point onwards.
"${SOURCE_CONTROL_TOOLS}"/land-patch.sh "${LAND_PATCH_ARGS[@]}" "${SOURCE_CONTROL_TOOLS}"/land-patch.sh "${LAND_PATCH_ARGS[@]}"