Conversation
repository.go
Outdated
There was a problem hiding this comment.
Can you write comments on all the methods? Include the all Worktree explaining which of the worktrees will be opened in case of multiple ones?
There was a problem hiding this comment.
All the discovered working trees will be opened in case of multiple ones. i.e. git worktree list
e40c661 to
826e731
Compare
|
Hey, I know |
|
Emm.. looks like this implementation does not support memoryFS. |
pjbgf
left a comment
There was a problem hiding this comment.
Some of the implementation could be pushed into the storer, making it more extensible. That should also resolve issues of having the force a given filesystem type.
|
|
||
| // CreateWorktree creates a linked working tree at the given path. | ||
| // Specify the branch for working tree files with CheckoutOptions. | ||
| func (r *Repository) CreateWorktree(path string, opts *CheckoutOptions) (*Worktree, error) { |
There was a problem hiding this comment.
A WorktreeOptions which contained a CheckoutOptions would make this more extensible.
| return nil, ErrWorktreeNotExists | ||
| } | ||
|
|
||
| _, err = os.Stat(filepath.Join(dot.Root(), "gitdir")) |
| } | ||
|
|
||
| common := fs.Filesystem() | ||
| pattern := filepath.Join(common.Root(), "worktrees", "*") |
| type fsBased interface { | ||
| Filesystem() billy.Filesystem | ||
| } | ||
|
|
||
| fs, isFSBased := r.Storer.(fsBased) | ||
| if !isFSBased { | ||
| return nil, ErrRepositoryNotFileBased | ||
| } |
There was a problem hiding this comment.
Did you encounter any specific problem trying to support in memory storer?
| wt := osfs.New(path) | ||
| dot := osfs.New(filepath.Join(common.Root(), "worktrees", name)) | ||
| repositoryFs := dotgit.NewRepositoryFilesystem(dot, common) | ||
| s := filesystem.NewStorage(repositoryFs, cache.NewObjectLRUDefault()) |
There was a problem hiding this comment.
I would add this as a new abstraction (e.g. storer.WorktreeStorer) within the Storer interface. It would be up to the storage to handle the implementation details.
| fs, isFSBased := worktree.Repository().Storer.(fsBased) | ||
| if !isFSBased { | ||
| return ErrRepositoryNotFileBased | ||
| } | ||
|
|
||
| if err = util.RemoveAll(worktree.Filesystem, ""); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return util.RemoveAll(fs.Filesystem(), "") |
There was a problem hiding this comment.
Same as above on the storer.WorktreeStorer.
|
This PR isn't stale. I'll look into the implementation at a future date. |
|
This work is really cool to see, when working with git repositories programmatically the ability to generate/play/destroy worktrees is super useful! I hope this is still something that's moving forward. It would be a great addition to the project. Thank you kindly for the effort here! +1 |
|
@beornf I was wondering if you planned on picking this back up and continuing it anytime soon. |
|
@clseibold I don't have the bandwidth to pick this up this year. If you are able to help rewrite this PR to use a |
|
This would also fix #1580. |
|
Closing as per linked PR above. |
This PR adds support for managing multiple worktrees. It is designed to be fully compatible with the
git worktree add,git worktree removeandgit worktree listcommands.Fixes #41, #285, #394.