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.

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

Allow checkout with git worktrees #917

Closed
wants to merge 4 commits into from
Closed

Conversation

lox
Copy link
Contributor

@lox lox commented Feb 14, 2019

Currently the agent keeps a copy of a git repository for each pipeline and agent installed on a host. This can lead to lots of copies of the repository, which can take up lots of space for large repositories and also means each copy must be cloned independently which can cause a lot of load on the upstream git provider.

This PR adds a new config flag repos-path, which is typically a directory at the same level as builds-path. The repository gets checked out there with a name based on the BUILDKITE_REPO variable, so something like /usr/local/var/buildkite-agent/repos/https---github-com-buildkite-bash-example-git. A new agent phase of repo is run before checkout that makes sure this repository is up to date, and then the checkout phase creates a lightweight git worktree for the job. This looks something like /usr/local/var/buildkite-agent/builds/85c4fd28-4e20-4993-9003-73cb49xxxxx.

Currently this feature is hidden behind an experiment flag of worktree, so should be able to run entirely in parallel.

Based on work that @sj26 did in https://github.com/sj26/git-worktree-buildkite-hooks.

@lox lox requested a review from a team February 14, 2019 20:43
@sj26
Copy link
Member

sj26 commented Feb 15, 2019

Yes yes yes, I'm so excited! 🚀

@matthewd
Copy link
Contributor

I'm still not sure what worktrees offer us over a mirror+clone setup 🤷‍♂️

Concretely, my concerns are that worktrees:

  • are tracked by the primary clone (which means the checkout operation needs to write to the mirror, and that they have to be removed with the right command, not just rmed)
  • cannot point to the same branch as another worktree
  • officially don't support submodules

To be clear, I'm super excited for any variant of this functionality... I just feel like worktrees are hard mode and I'm missing some reasoning.

@lox
Copy link
Contributor Author

lox commented Feb 15, 2019

@matthewd I totally agree! I've been asking all the folks that use worktrees "why worktrees vs reference clones". It seems too magical to me as well, and I'm also concerned about how submodules playout. @sj26 do you have feels?

98% of the work in this PR is for the repo phase. Perhaps I will re-cut this to just get that in with a less controversial set of decisions about the changes to the checkout phase.

@lox
Copy link
Contributor Author

lox commented Feb 15, 2019

I'd love any feedback on the naming or mechanics of the repo phase too. @toolmantim you might have feels on the addition of a new phase too, as it comes with pre-repo and post-repo hooks.

@lox
Copy link
Contributor Author

lox commented Feb 15, 2019

I actually hadn't grasped the distinction between --bare and --mirror either @matthewd, that makes a lot more sense I think. I wonder if the repo phase might better be called mirror? I don't love either.

@toolmantim
Copy link
Contributor

toolmantim commented Feb 15, 2019

This feature sounds great!

I’d have the phase match the configuration name “repository” perhaps?

If people have been using the pre-checkout hook for security, with the idea “before any code can end up on the agent”, is that still true? If not, we could just make it all part of the same phase for now? Or something 🤔

@lox
Copy link
Contributor Author

lox commented Feb 15, 2019

I’d have the phase match the configuration name “repository” perhaps?

I started there, but it's just very verbose, and we use BUILDKITE_REPO 🤷🏼‍♂️

If people have been using the pre-checkout hook for security, with the idea “before any code can end up on the agent”, is that still true? If not, we could just make it all part of the same phase for now? Or something 🤔

Nope, not true any more, was it ever? Isn't that an environment hook?

We could potentially just fold this stuff into the checkout phase, but it makes it even more complicated to overwrite the checkout hook. This way the repo phase can provide a BUILDKITE_MIRROR_REPO or similar env to the checkout hook that can be used for a reference clone.

@toolmantim
Copy link
Contributor

If it's behind the experimental flag, and destined to be v4 default behaviour, then it doesn't really matter if it changes assumptions behind https://buildkite.com/docs/agent/v3/hooks 👌🏼

@lox lox removed the request for review from a team February 27, 2019 03:34
@lox lox added the wip label Feb 27, 2019
@lox
Copy link
Contributor Author

lox commented Mar 10, 2019

Closing in favour of #936.

@lox lox closed this Mar 10, 2019
@keithduncan keithduncan deleted the checkout-with-git-worktrees branch September 22, 2021 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants