Skip to content

Add create-diff-link.sh script #3837

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.

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

Merged
merged 1 commit into from
Aug 20, 2021
Merged
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions script/create-diff-link.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash

set -e

echo "fetching updates from production app…"
git fetch heroku-prod

prod_sha=`git rev-parse --short heroku-prod/master`
Copy link
Member

@jtgeibel jtgeibel Aug 19, 2021

Choose a reason for hiding this comment

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

One thing to note is that if code is pushed via non-git means, then the fetched branch doesn't reflect the actual commit that is deployed. I've seen this before on staging with deploys done by the ops bot. On production that shouldn't be an issue, but on the rare occasion that we rollback a deploy, the remote repo state and deployed state will diverge.

Also, I wonder if we should parse heroku-prod/HEAD, as at some point we may switch to a main branch which Heroku now supports.

Alternatively, we could extract the deployed commit from the server by fetching https://crates.io/api/v1/site_metadata. This should always reflect the latest deployed commit hash.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've seen this before on staging with deploys done by the ops bot

does that bot not use git to deploy?

Also, I wonder if we should parse heroku-prod/HEAD

let me try if that works, but generally sounds reasonable

Alternatively, we could extract the deployed commit from the server by fetching https://crates.io/api/v1/site_metadata. This should always reflect the latest deployed commit hash.

seems like that would require something like https://stedolan.github.io/jq/ and I was hoping to avoid external dependencies for this script to keep it simple :)

Copy link
Member Author

Choose a reason for hiding this comment

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

$ git rev-parse --short heroku-prod/HEAD
fatal: Needed a single revision

$ git rev-parse --short heroku-prod/master
86ded9db0

unfortunately it doesn't :-/

Copy link
Member

Choose a reason for hiding this comment

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

does that bot not use git to deploy?

I'm not too familiar with that codebase, but this looks like the relevant logic. I believe it makes an API request to Heroku which then fetches from GitHub. From what I recall, that method doesn't update the master branch of Heroku's version of the repo.

let me try if that works, but generally sounds reasonable
unfortunately it doesn't :-/

It looks like git remote set-head heroku-prod -a would lookup the remote HEAD and cache it as heroku-prod/HEAD, however I think that is a one-time operation and would need to either be run every time by the script, or every user would have to remember to run it if we change to a different HEAD branch. Therefore, it seems fine to me to just reference master directly and update the script if we ever rename the branch to main.

seems like that would require something like https://stedolan.github.io/jq/ and I was hoping to avoid external dependencies for this script to keep it simple :)

I agree that it doesn't seem worth adding a dev dependency for this. Also, given that there are only a few of us who will run this script, I think we can remember to tweak the generated URL as necessary if we're end up rolling back a deployment.

This looks good to me, but I'll defer to @pietroalbini for the final r+.

Copy link
Member Author

Choose a reason for hiding this comment

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

From what I recall, that method doesn't update the master branch of Heroku's version of the repo.

I see, thanks :)

master_sha=`git rev-parse --short master`

echo ""
echo "production app is at: ${prod_sha}"
echo "local \`master\` branch is at: ${master_sha}"
echo ""
echo "https://github.com/rust-lang/crates.io/compare/${prod_sha}...${master_sha}"