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

Fix deploy workflow (take 2) #1126

Merged
merged 6 commits into from
Nov 29, 2024
Merged

Fix deploy workflow (take 2) #1126

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
05ebcf2
Fix deploy workflow (take 2)
zcorpan Nov 21, 2024
34ebbb8
Address review comments
zcorpan Nov 21, 2024
629ebd1
Use --force-with-lease
zcorpan Nov 21, 2024
7513f7c
Use merge commits
zcorpan Nov 26, 2024
aca6caa
Use github.sha
zcorpan Nov 26, 2024
1b056a8
Avoid concurrent deploy jobs
zcorpan Nov 26, 2024
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
29 changes: 10 additions & 19 deletions .github/workflows/build-and-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,42 +35,33 @@ jobs:
python3 -m pip install --upgrade pip
pip install -r requirements.txt

- name: Generate Merged Data
run: |
python3 gh-data.py
python3 merge-data.py

- name: Switch to `gh-pages` and Preserve Changes
- name: Fetch and Check Out `gh-pages`
run: |
git config --global user.name "github-actions[bot]"
git config --global user.email "github-actions[bot]@users.noreply.github.com"
zcorpan marked this conversation as resolved.
Show resolved Hide resolved

# Stash changes to preserve them
git stash --include-untracked

# Fetch and switch to `gh-pages`
git fetch origin gh-pages:gh-pages || git checkout --orphan gh-pages
zcorpan marked this conversation as resolved.
Show resolved Hide resolved
git checkout gh-pages

# Rebase `gh-pages` onto `main` to ensure a linear history
- name: Rebase `gh-pages` onto `main` (if triggered by push to main)
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
run: |
git rebase main
Copy link
Member

Choose a reason for hiding this comment

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

I should have said: this is where things get a little whacky. It's not going to be possible to have a linear history like this. At some point, you will need to do something.

If you want the history of gh-pages to track main, then you will want merges. I suggest using -s theirs to resolve conflicts on the merge. That should work. However, you should probably have some sort of check that the files you are generating do not appear on main.

The other race condition considerations I outlined still apply.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, if you wanted to ensure that this gets the right commits, then you need to use ${{ github.sha }} rather than main here. Otherwise, if there are multiple pushes to main, you could end up with strange things happening. I think that GitHub's checkout action does funny things with ref names, so maybe that's not a genuine problem, but maybe avoid having to think about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, thanks. Fixed. The generated files are listed in .gitignore, do you think that's sufficient?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also fixed ${{ github.sha }}


# Apply stashed changes
git stash pop || echo "No changes to apply."
- name: Generate Merged Data
run: |
python3 gh-data.py
python3 merge-data.py

- name: Commit and Push Updates
run: |
# Add changes
git add -f merged-data.json
git add .

# Commit changes if there are any
if git diff --cached --quiet; then
echo "No changes to commit."
else
git commit -m "Update GitHub Pages with latest data"
fi
git commit -m "Update GitHub Pages with latest data"

# Push changes back to `gh-pages`
git remote set-url origin [email protected]:${{ github.repository }}.git
git push origin gh-pages
git push origin gh-pages --force
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that you want force pushes for this. If the build fails, that's likely because you are running multiple concurrent builds. Let it fail in that case. You'll get an email, but the next build should repair things.

On the other hand, force pushes can destroy history.

Copy link
Member Author

Choose a reason for hiding this comment

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

The rebase (line 50) means you have to force push, though, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe using merge commits is a better alternative?

Copy link
Member

Choose a reason for hiding this comment

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

Think about what leads to a conflict here. If this is the only process by which the gh-pages branch is updated, then you only end up here if there are two workflows running concurrently. That happens if it takes a little while and there are two runs triggered. The cron job would be fine, but pushes to main can conflict.

If there is a conflict then, it's most likely to be because you are the run associated with the second (or third...) push to main. A failure therefore likely results in the state from an old push to main being pushed to gh-pages.

I would suggest that you minimize the exposure by making the gh-pages checkout and the ensuing update be as close as possible together. Then, if it still fails, you can maybe check that you are the last commit on main (git ls-remote origin refs/heads/main should return the same hash as git show-ref refs/heads/main). If you are, then looping back to the point where you fetch and try to push is probably best. Do that at most a few times.

You shouldn't have to fetch many more commits if there is a mid-air collision, so make sure things go faster the second time by reusing the checkout. (I might suggest putting the gh-pages checkout in a subdirectory using actions/checkout, then refreshing the checkout in the loop.)

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 added concurrency group for this action to avoid mid-air collision issues.

Loading