CI build: Conditionally create the 'main' branch for pre-commit comparison #9825
Conversation
…lready there (i.e. when building the main branch itself!)
There was a problem hiding this comment.
We don't know if this solution will work before merging it into main, since the problem is there. However, I'd suggest to debug this problem a little more to understand what's happening and to find out the root cause of it. IMO, none of these git commands should be required since main (the default branch) should always exist and be up-to-date due the repository is cloned each time.
.circleci/config.yml
Outdated
| - run: git fetch origin main # needed for comparisson in pre-commit | ||
| - run: git branch -f --track main origin/main # needed for comparisson in pre-commit | ||
| # Meaning of below line: If a "main" branch does not exist, then create one. | ||
| - run: git rev-parse --verify main 2>/dev/null || git branch -f --track main origin/main |
There was a problem hiding this comment.
I don't understand why the main branch (the default) wouldn't exist after doing the checkout step in the first place. Also, I don't know why git fetch origin main is now required here for pre-commit. I seems you added in https://github.com/readthedocs/readthedocs.org/pull/9789/files#r1043369043 and you also weren't sure why this was needed.
Then we added the -f, but that seemed to not work as we expected (#9793). In this change it seems we are adding another small hack on top of the previous hack, but we still don't understand why this is happening and/or if this is gonna solve the problem.
My suggestion here is to debug this a little more and understand what is the root cause of the problem before adding more extra commands and keep complicating the CI process. Starting by removing all these git commands.
There was a problem hiding this comment.
If you are looking for an easier way out, it would be easier if we could run pre-commit run --all-files and have to bother with which branch that the check is being made against.
you also weren't sure why this was needed.
...
I don't understand why the main branch (the default) wouldn't exist after doing the checkout step in the first place.
Exactly, Circle CI clones and checks out a branch. So it requires a bit of work to understand what's going on. Not to say that something may have changed.
You can try to remove the entire thing and see if the problem has gone away, perhaps it was Circle CI momentarily doing shallow clones 🤷
There was a problem hiding this comment.
Btw. I have a main branch that I can build it on :) => https://app.circleci.com/pipelines/github/benjaoming/readthedocs.org/95/workflows/b98f810a-74e6-4dd9-8129-b788f2df7a58
There was a problem hiding this comment.
I have a
mainbranch that I can build it on
Cool! 💯 Can you try triggering a build on CircleCI for that repository by removing these git commands?
There was a problem hiding this comment.
There was a problem hiding this comment.
The old error is back by removing the git commands.
There was a problem hiding this comment.
Clicking on the "Checkout code" tab and reading the script, it seems these are the commands it executes to clone the repository and checkout the proper branch:
git clone --no-checkout [email protected]:benjaoming/readthedocs.org.git
git checkout --force -B ci-fix-for-pre-commit-base-branch e2c03dc3f4fb74659f75410eb58f11565dc4a3d5
Then, our pre-commit command executes:
git diff --name-only --no-ext-diff -z main..HEAD
There was a problem hiding this comment.
It seems this only happens in your github.com:benjaoming/readthedocs.org.git because it has master instead of main branch. If you create the PRs directly in the readthedocs/readthedocs.org repository you won't face this issue.
/tmp ⍉
▶ git clone --no-checkout [email protected]:benjaoming/readthedocs.org.git
Cloning into 'readthedocs.org'...
remote: Enumerating objects: 128358, done.
remote: Counting objects: 100% (120/120), done.
remote: Compressing objects: 100% (63/63), done.
remote: Total 128358 (delta 34), reused 97 (delta 25), pack-reused 128238
Receiving objects: 100% (128358/128358), 81.97 MiB | 9.05 MiB/s, done.
Resolving deltas: 100% (91342/91342), done.
Time: 0h:00m:12s
/tmp
▶ cd readthedocs.org
/tmp/readthedocs.org master ✗ 6y39d ✖
▶ git checkout --force -B ci-fix-for-pre-commit-base-branch e2c03dc3f4fb74659f75410eb58f11565dc4a3d5
Switched to a new branch 'ci-fix-for-pre-commit-base-branch'
/tmp/readthedocs.org ci-fix-for-pre-commit-base-branch ✔ 23m
▶ git diff --name-only --no-ext-diff -z main..HEAD
fatal: ambiguous argument 'main..HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
/tmp/readthedocs.org ci-fix-for-pre-commit-base-branch ✔ 23m ⍉
▶ git branch
error: cannot run delta: No such file or directory
* ci-fix-for-pre-commit-base-branch
master
There was a problem hiding this comment.
Yeah, it's been left like that for a while :)
This branch is 12455 commits behind readthedocs:main.
There was a problem hiding this comment.
😉 -- problem solved, then, and we can remove these git commands. You have to update your fork or, probably better, use readthedocs/readthedocs.org to push your code and open PR from there.
…lready there (i.e. when building the main branch itself!)
…fix-for-pre-commit-base-branch
|
It seems like we can make this issue go away by ensuring that the origin repository of a PR has Also, it would really be nice to run I'll open a PR to remove the addition |
No description provided.