feat(clone): turbo clone #9904
feat(clone): turbo clone #9904
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -1274,7 +1285,6 @@ pub async fn run( | |||
| }; | |||
|
|
|||
| cli_args.command = Some(command); | |||
| cli_args.cwd = Some(repo_root.as_path().to_owned()); | |||
There was a problem hiding this comment.
as far as I can tell, we don't actually use the cwd as repo root anywhere. can correct if this isn't true
There was a problem hiding this comment.
Is this a necessary change for this PR? repo_root looks like a bad name since it can get it's value from cli_args.cwd. This value will now no longer be made absolute if it is given as a relative value.
There was a problem hiding this comment.
Unfortunately it is necessary, although I can just save the old cwd instead. We don't want the repo root to be our cwd and in fact, we probably don't want to be in a repo in the first place for turbo clone. This is a pretty big departure from our other commands.
| Make sure we allow partial clones | ||
|
|
||
| Do a blobless clone | ||
| $ ${TURBO} clone file://$(pwd)/basic-monorepo.t basic-monorepo-blobless --local |
There was a problem hiding this comment.
I'm not convinced we're actually doing a real blobless clone here. If anybody has ideas for better testing that doesn't involve hitting a remote git repo (unless we really have to ig), I'm open to ideas.
There was a problem hiding this comment.
Like the config claims it's blobless but my suspicion is that git only does a blobless clone if you really need it. For trivial repos it just doesn't make sense. I tried adding a test that adds a file, then deletes in in a second commit, and then does a blobless clone, with the idea that the file should not be included in the clone since it's not a blob that is reachable from HEAD. But the file was indeed cloned, so who knows.
There was a problem hiding this comment.
If you're not convinced this test is actually testing the behavior we want than we shouldn't include it. Only suggestion for testing would be to set up a local git server and clone from that, but that feels a bit like overkill.
There was a problem hiding this comment.
We could try to clone against GitHub but that's a little sketchy for a test
There was a problem hiding this comment.
I would prefer not to do that, but if we need a specific response from a git server we probably either need to hit GitHub or somehow run a similar server locally. Since we're still exploring this I think it's fine to test it out where we want to use it instead of investing into a testing strategy. I do still stand that if you don't think this test is testing the behavior, we should remove it.
There was a problem hiding this comment.
So I think this test is still worth doing because it's testing that we have indeed set up the repo to be a treeless/blobless clone. From there, it's up to the server to respect this config, but we can't control that.
| @@ -1274,7 +1285,6 @@ pub async fn run( | |||
| }; | |||
|
|
|||
| cli_args.command = Some(command); | |||
| cli_args.cwd = Some(repo_root.as_path().to_owned()); | |||
There was a problem hiding this comment.
Is this a necessary change for this PR? repo_root looks like a bad name since it can get it's value from cli_args.cwd. This value will now no longer be made absolute if it is given as a relative value.
| Make sure we allow partial clones | ||
|
|
||
| Do a blobless clone | ||
| $ ${TURBO} clone file://$(pwd)/basic-monorepo.t basic-monorepo-blobless --local |
There was a problem hiding this comment.
If you're not convinced this test is actually testing the behavior we want than we shouldn't include it. Only suggestion for testing would be to set up a local git server and clone from that, but that feels a bit like overkill.
NicholasLYang
force-pushed
the
nicholasyang/turbo-clone
branch
from
2538853 to
fa4591c
Compare
NicholasLYang
force-pushed
the
nicholasyang/turbo-clone
branch
from
eff8104 to
e895329
Compare
Description
Implements
turbo clone, essentially a small wrapper aroundgit clonethat does simple optimizations like blobless clones for local development and treeless clones for CI.Testing Instructions
Wrote some tests but not convinced they're actually effective. See comments below