Fancy --sub_dir and --keep_hash #1262
Conversation
This adds a fancy interaction between `--sub-dir` and `--keep_hash` that is meant to make sure we build *exactly* the right docs for pull requests. When we first deploy pull request builds we targeted them at the tip of the pull request branch. This caused trouble because the tip of the pull request branch was often behind the last successful build. Building the docs against that would often cause spurious failure from out of date links. Merging `master` into the PR branch usually fixed it but it was annoying. We fixed this problem by switching our target to the result of merging the pull request branch into the target branch. This was *better* because it had fewer spurious failures, but often caused us to think that the docs changes were larger than they actually were because the target branch often had changes in it that hadn't yet been picked up by the docs build. This was annoying because the `diff`s generated by the docs builds had changes that weren't part of the PR. Which made it difficult to track down what actually changed. This solves the problem by first merging the `--sub_dir`ed branch into the last successful docs build. Thus we only build *exactly* what is in the PR. We trigger this behavior by checking for both the `--keep_hash` and `--sub_dir` flags *and* checking for outstanding changes in the `--sub_dir`ed branch. This behavior might end up being slower sometimes because merging can be expensive, but it is usually less expensive than building a book we don't have to build. So maybe it'll all come out in the wash.
|
@olksdr This one kind of terrifies me a little, but I think it'll be ok. I did a fair bit of playing with it locally and everything seemed right. If we merge this then all of the PR builds will immediately start doing noop merges because we build against github's pre-merged hash. When we're good and ready we can switch them. |
lib/ES/Repo.pm
Outdated
| run qw(git diff-index --quiet HEAD --); | ||
| 1; | ||
| }; | ||
| unless ( $has_uncommitted_changes ) { |
There was a problem hiding this comment.
shouldn't it be if here ?
if ($has_uncommitted_changes ) {
# execute the code inside ?
}There was a problem hiding this comment.
Weird! I've got something backwards....
There was a problem hiding this comment.
OK - here is what happened --exit-code causes run to throw an exception if there are changes. Which causes eval to return 0. So, you could say that I named the variable backwards. Fixing.
There was a problem hiding this comment.
yeah 😄 the name was confusing a little bit.
|
@nik9000 before I finished this review you'd already fixed few things 😄 |
Sorry! |
It doesn't seem to be doing the right thing in PRs.... This reverts commit a28a83b.
False alarm. This reverts commit 311a4f5.
New, this time real alarm. Reverting. This reverts commit 0efe2bc.
|
I've reverted this because it looked like it was causing trouble with the PR builds that it was trying to help. I'll investigate further. |
…"""" Oh my. This reverts commit 69c9d78.
Revert "Revert "Revert "Revert "Fancy --sub_dir and --keep_hash (#1262)"""" Adds support building from references We try to do a ton of `git` operations inside the docker container while building docs. Without this change if one of those operations points to a git repo that was created with `--reference` but wasn't `--dissociate`d then we'd fail because with didn't mount the reference. This mounts the reference into the docker container *exactly* where they exist on the host system. Doing anything less makes git very unhappy.
This adds a fancy interaction between
--sub-dirand--keep_hashthatis meant to make sure we build exactly the right docs for pull
requests.
When we first deploy pull request builds we targeted them at the tip of
the pull request branch. This caused trouble because the tip of the pull
request branch was often behind the last successful build. Building the
docs against that would often cause spurious failure from out of date
links. Merging
masterinto the PR branch usually fixed it but it wasannoying.
We fixed this problem by switching our target to the result of merging
the pull request branch into the target branch. This was better
because it had fewer spurious failures, but often caused us to think
that the docs changes were larger than they actually were because the
target branch often had changes in it that hadn't yet been picked up by
the docs build. This was annoying because the
diffs generated by thedocs builds had changes that weren't part of the PR. Which made it
difficult to track down what actually changed.
This solves the problem by first merging the
--sub_dired branch intothe last successful docs build. Thus we only build exactly what is in
the PR. We trigger this behavior by checking for both the
--keep_hashand
--sub_dirflags and checking for outstanding changes in the--sub_dired branch.This behavior might end up being slower sometimes because merging can be
expensive, but it is usually less expensive than building a book we
don't have to build. So maybe it'll all come out in the wash.