Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop failing on toolstate changes #64977

Closed
wants to merge 2 commits into from
Closed

Stop failing on toolstate changes #64977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6096f82
Change toolstate to no longer fail on nightly
Mark-Simulacrum Oct 1, 2019
dbf6ac3
Fail toolstate like before on the PR builder
Mark-Simulacrum Oct 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ci/azure-pipelines/pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ jobs:
parameters:
only_on_updated_submodules: 'yes'
variables:
FORCE_FAIL_TOOLSTATE: true
IMAGE: x86_64-gnu-tools
23 changes: 11 additions & 12 deletions src/ci/docker/x86_64-gnu-tools/checktools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,16 @@ verify_submodule_changed() {
echo "If you do NOT intend to update '$1', please ensure you did not accidentally"
echo "change the submodule at '$2'. You may ask your reviewer for the"
echo "proper steps."
exit 3
# exit if we're in beta week, otherwise we don't want to,
# it's fine to land changes/regressions to toolstate
if [ $SIX_WEEK_CYCLE -ge 35 ]; then
exit 3
fi
# we want the PR builder to error out even though the auto branch
# builder will not
if [ -n "$FORCE_FAIL_TOOLSTATE" ]; then
exit 3
fi
fi
fi
}
Expand Down Expand Up @@ -105,21 +114,11 @@ status_check() {

status_check "submodule_changed"

CHECK_NOT="$(readlink -f "$(dirname $0)/checkregression.py")"
# This callback is called by `commit_toolstate_change`, see `repo.sh`.
change_toolstate() {
# only update the history
if python2.7 "$CHECK_NOT" "$OS" "$TOOLSTATE_FILE" "_data/latest.json" changed; then
echo 'Toolstate is not changed. Not updating.'
else
if [ $SIX_WEEK_CYCLE -ge 35 ]; then
# Reject any regressions during the week before beta cutoff.
python2.7 "$CHECK_NOT" "$OS" "$TOOLSTATE_FILE" "_data/latest.json" regressed
fi
sed -i "1 a\\
sed -i "1 a\\
$COMMIT\t$(cat "$TOOLSTATE_FILE")
" "history/$OS.tsv"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we not even do the "changed" test any more? That had nothing to do with regressing or not.
If I read this correctly, this would commit a new toolstate to the repo even for commits that do not change any toolstate. Is this intentional? That would be useful for #60301 but I was under the impression that we deliberately reduced traffic for that repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and also, doesn't this remove the check that tools do not regress in the beta week? We only run that check when submodules change now, instead of on all PRs, it seems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yeah, that might be right. To be honest I'm having a really hard time understanding this code - I thought we'd handled those cases up above but maybe not :)

fi
}

if [ "$RUST_RELEASE_CHANNEL" = nightly ]; then
Expand Down