feat: add e worktree for additional gclient working directories#836
feat: add e worktree for additional gclient working directories#836MarshallOfSound wants to merge 1 commit intomainfrom
e worktree for additional gclient working directories#836Conversation
MarshallOfSound
force-pushed
the
sam/ts-migration
branch
from
8610adf to
f5e38c3
Compare
MarshallOfSound
force-pushed
the
sattard/e-worktree
branch
2 times, most recently
from
e59a2b8 to
4a55cd5
Compare
MarshallOfSound
force-pushed
the
sam/ts-migration
branch
from
f5e38c3 to
54952a6
Compare
MarshallOfSound
force-pushed
the
sattard/e-worktree
branch
from
4a55cd5 to
d7a234e
Compare
src/e-worktree.ts
Outdated
| evmConfig.remove(name); | ||
| console.log(`Removed config ${color.config(name)}`); | ||
|
|
||
| console.log(`Deleting ${color.path(root)}...`); | ||
| deleteDir(root); |
There was a problem hiding this comment.
If deleteDir fails we've already nuked the config, so now you've got an orphaned worktree directory with no config pointing at it. Can we flip the order here?
There was a problem hiding this comment.
Good call, flipped — deleteDir runs first now and I hoisted the active-config check so we don't nuke the dir then fatal on that either.
| function isDerivedWorktree(root: string): boolean { | ||
| const gclient = path.join(root, '.gclient'); | ||
| const gitObjects = path.join(root, 'src', '.git', 'objects'); | ||
| return ( | ||
| fs.existsSync(gclient) && | ||
| fs.lstatSync(gclient).isSymbolicLink() && | ||
| fs.existsSync(gitObjects) && | ||
| fs.lstatSync(gitObjects).isSymbolicLink() | ||
| ); | ||
| } |
There was a problem hiding this comment.
If somebody runs git gc in the worktree, the .git/objects symlink can get replaced with a real directory, so this check would fail and they'd be unable to clean up via e worktree clean. Maybe add a --force escape hatch on clean?
There was a problem hiding this comment.
I don't think git gc actually replaces the symlink (it writes through it into pack/), but the wider point is fair. Rather than a --force that could take out a real checkout I've relaxed the check to just .gclient being a symlink — that's the one thing gclient-new-workdir.py always leaves and git never touches.
src/e-worktree.ts
Outdated
| console.log( | ||
| `Running ${color.cmd('e sync')} to fetch toolchains and apply patches in the new worktree...`, | ||
| ); | ||
| const e = path.resolve(__dirname, 'e'); |
There was a problem hiding this comment.
Yep, done — rebased on #838 and swapped this + the import extensions over.
Adds `e worktree add <name> <dir>` and `e worktree clean <name>`. `add` wraps depot_tools' gclient-new-workdir.py to create a second working tree that shares git object stores with an existing checkout, clones the source build config with the new root/buildtools path, and runs `e sync` so the worktree is immediately buildable. `clean` removes the worktree directory and its config. It refuses to touch a directory unless `.gclient` and `src/.git/objects` are symlinks, so it can never delete a primary checkout. Not supported on Windows (gclient-new-workdir.py requires symlinks).
MarshallOfSound
force-pushed
the
sattard/e-worktree
branch
from
d7a234e to
5b1dfc1
Compare
3 participants
Stacked on #834.
Summary
e worktree add <name> <dir>wrapsgclient-new-workdir.pyto create a second working tree that shares git objects with an existing checkout, writes a matching build config (cloned from--source, default current), and runse syncso it's immediately buildable.--no-sync/-o <out>/-favailable.e worktree clean <name> --yesremoves the worktree directory and its build config. Guarded: refuses unless.gclientandsrc/.git/objectsare symlinks (so a primary checkout can never be deleted), refuses if the config is currently active, and requires--yes.gclient-new-workdir.pyrequires symlinks.Pairs well with per-shell active configs:
Test plan
yarn buildcleanyarn lint:tscleanyarn test(63 passing)e worktree add testing2 ~/src/electron2 --no-synccreates an 18G workdir with symlinked.git/{objects,refs,config}, writesevm.testing2.json, switches to ite syncin the new workdir fetches gn/clang/node ande build --gen-onlyproducesbuild.ninjae worktree clean testing --yesrefuses (primary checkout)e worktree clean <active> --yesrefuses (config in use)e worktree clean testing2 --yesremoves dir + config