Do not disclose hidden deployment commits
ClosedPublic

Authored by demon on Dec 1 2016, 7:46 PM.

Details

Reviewers
thcipriani
mmodell
Group Reviewers
Release-Engineering-Team
Commits
rMSCA1b569c04348e: Do not disclose hidden deployment commits
rMSCAd9ec0247d4b9: Do not disclose hidden deployment commits
Patch without arc
git checkout -b D486 && curl -L https://phabricator.wikimedia.org/D486?download=true | git apply
Summary

Instead of displaying the exact sha1 of deployment, use the last common ancestor with the remote we're tracking. This is what we do in git.info() for MediaWiki already so reuse that data in scap3.

While we're here, clean up the logic for the failure case when we're not tracking an upstream. This has never worked, as "origin" is not a valid reference to anything, it's just a remote. For get_disclosable_head() to have any chance at finding an ancestor, we need an upstream branch or tag so accept one now.

git.info() is generally more robust to failure now.

Test Plan

Ran this with scap deploy a bunch of times with all kinds of bogus output. The default case should be mostly unchanged for MW, only more robust if we break things.

Diff Detail

Repository
rMSCA Scap
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
demon retitled this revision from to Do not disclose hidden deployment commits.Dec 1 2016, 7:46 PM
demon updated this object.
demon edited the test plan for this revision. (Show Details)
demon added a reviewer: thcipriani.
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
thcipriani accepted this revision.Dec 1 2016, 8:20 PM

Couple of inline nitpicks.

Overall this is much improved and needed -- accepted.

scap/git.py
123–124

You catch the exception for git remote in get_disclosable_head, but this will also raise if no remote is configured.

374

Could make except narrower: except subprocess.CalledProcessError:

This revision is now accepted and ready to land.Dec 1 2016, 8:20 PM
demon added inline comments.Dec 1 2016, 8:38 PM
scap/git.py
123–124

It should be basically impossible for this to fail assuming we got a sha1 back from get_disclosable_head()

374

Typo, will fix.

demon added inline comments.Dec 1 2016, 8:39 PM
scap/git.py
123–124

Duh, I misread you and was looking at the commit_date. You're right, will fix.

demon updated this revision to Diff 1292.Dec 1 2016, 8:44 PM
  • Better exception handling
demon marked 5 inline comments as done.Dec 1 2016, 8:44 PM

All inlines done.

This revision was automatically updated to reflect the committed changes.