Add archive options #3244

afhoffman · 2024-01-23T01:49:14Z

PR Description

Sometimes it's useful for me to create an archive or a bundle of the repository I'm working on. This PR adds menu options for creating an archive (branch, tag, commit) or a bundle (branch or tag).

Please check if the PR fulfills these requirements

Cheatsheets are up-to-date (run go generate ./...)
Code has been formatted (see here)
Tests have been added/updated (see here for the integration test guide)
Text is internationalised (see here)
Docs (specifically docs/Config.md) have been updated if necessary
You've read through your own file changes for silly mistakes etc

stefanhaller · 2024-01-25T09:25:56Z

This looks useful, thanks for the contribution! I have a lot of thoughts about this (mostly about the UX), but I need more time, and I don't have very much these days. Please bear with me if it takes a bit longer this time, I do plan to have a closer look and give feedback.

afhoffman · 2024-01-25T14:57:45Z

No worries! I appreciate you taking the time to check this out and look forward to hearing your feedback. I'll try to keep this branch up to date w/ master in the meantime if that helps. Thanks!

afhoffman · 2024-01-27T16:52:58Z

Nothing urgent here, just thought I would write down a couple of things on this one before I forget them.

For bundling:

I would like to include range select for branches so a user can choose which branches go into a bundle.
It would be nice to have range select for tags for the same reason.
It would be kind of nice to be able to select branches and tags when creating a bundle, but unless I'm misunderstanding something I think that would be more trouble to implement than it's probably worth.
In lieu of 3 (and probably more useful), it could include some combination of:
- git bundle create <foo> --all to get all refs (including branches/tags on remotes)
- git bundle create <foo> --branches --tags to get all local refs
  - there could be an option with --branches to just get branches, --tags to just get tags, or both.
- I realize the above collection of possibilities could make more clutter than useful features, but at least it's a range of possibilities we could pick a subset from.

stefanhaller · 2024-02-20T16:40:44Z

A few thoughts; these are not strong opinions, just input for discussion.

I see archive and bundle as two sufficiently different concepts with different use cases that I'm not sure it makes sense to put them into one menu. It's also confusing that <c-a> opens a menu in the branches view (where bundle is applicable), but jumps straight to the archive command in the commits panel (where it's not); having two separate commands would solve that.
I'm not sure if archive needs the feature to specify a prefix. I found it confusing when I first tried it, not knowing what the prompt to enter a folder meant (although the prompt text is pretty clear about it, but still). In my opinion we can simplify this to always create an archive without a prefix.
We need to make it possible to create either "thick" or "thin" bundles. Personally I would almost always use thin bundles exclusively, because it's so much faster to create them for large repos (creating a thick bundle from our work repo takes almost two minutes), and it's sufficient for the scenario in which I need them (which is to transfer a few commits between machines that can't ssh into each other). Maybe it's good enough to decide between thick and thin based on the selection: if a range of commits is selected, make it a thin bundle, otherwise a thick one. However, that would make it look like you can select arbitrary commit ranges, which is not the case; it seems that git bundle always includes commits up to the ref that your bundling, and you can only specify the commit to start from. This is tricky, I don't have a good solution, but I wonder if we could do something similar to the shift-B feature that we use for "rebase onto". Or we could have separate menu entries for thick and thin bundles, where thin ones would only include the current branch (i.e. what you would get when pressing i in the commits panel).
Creating a bundle is only half the story; fetching from it is the other half, and I wonder if it's good to implement a feature that only solves the first one but not the second. Do you have any thoughts on what the UX could be for fetching from a bundle?

All in all I have the feeling that "archive" is straightforward enough that we can just add it, but "bundle" has enough questions to it that it would be good to do more of a requirements analysis first. Maybe it would be best to start with a description of what use cases or workflows you want to support with the feature.

afhoffman · 2024-02-20T17:21:48Z

Thanks for taking the time to give this a look! I really appreciate your feedback.

I see archive and bundle as two sufficiently different concepts with different use cases that I'm not sure it makes sense to put them into one menu. It's also confusing that <c-a> opens a menu in the branches view (where bundle is applicable), but jumps straight to the archive command in the commits panel (where it's not); having two separate commands would solve that.

I agree with this. I think my use case for the two may have a little overlap -- I pick one of these when I need to get my source to another machine that I can't ssh to, or if I need to get a copy of the source to someone else who can't clone from where the repo lives. I have to touch several different GitLab instances for work, and not everyone can get access to all of them. I pick a bundle if I can, but will use an archive if I know a bundle is going to confuse the person I'm trying to get the source to.

I do agree that they're sufficiently different that they should be two different menu options. I think I put them together because I didn't want to use two different keyboard shortcuts. Maybe <c-a> for archive and <c-b> for bundle where applicable?

I'm not sure if archive needs the feature to specify a prefix. I found it confusing when I first tried it, not knowing what the prompt to enter a folder meant (although the prompt text is pretty clear about it, but still). In my opinion we can simplify this to always create an archive without a prefix.

I did want to include this option to reduce the risk of dumping a ton of files in an unintended directory. If a prefix is supplied first, it saves the possibility of having to do clean-up after. I tend to always make a tmp directory before expanding any compressed archive since I've been bitten by that sort of thing plenty of times. I think archives should (almost) always have a folder at the top level, but that's just an opinion of mine. I would be happy to remove the option to supply an archive prefix if that's the way we want to go.

We need to make it possible to create either "thick" or "thin" bundles. Personally I would almost always use thin bundles exclusively, because it's so much faster to create them for large repos (creating a thick bundle from our work repo takes almost two minutes), and it's sufficient for the scenario in which I need them (which is to transfer a few commits between machines that can't ssh into each other). Maybe it's good enough to decide between thick and thin based on the selection: if a range of commits is selected, make it a thin bundle, otherwise a thick one. However, that would make it look like you can select arbitrary commit ranges, which is not the case; it seems that git bundle always includes commits up to the ref that your bundling, and you can only specify the commit to start from. This is tricky, I don't have a good solution, but I wonder if we could do something similar to the shift-B feature that we use for "rebase onto". Or we could have separate menu entries for thick and thin bundles, where thin ones would only include the current branch (i.e. what you would get when pressing i in the commits panel).

I totally agree. I can appreciate that use case, but unfortunately I almost always run into the situation where I need to transfer the repo and all its history to someone or some machine. I am very much in favor of adding the option to make a thin bundle where it's appropriate. I definitely have more learning to do about bundles. I'll try to make some time for that, and put together some notes so we can have a better conversation about them/their inclusion.

Creating a bundle is only half the story; fetching from it is the other half, and I wonder if it's good to implement a feature that only solves the first one but not the second. Do you have any thoughts on what the UX could be for fetching from a bundle?

You're right, we should also consider what it would take to fetch from a bundle. I'm not sure if I would hold up the bundle creation feature until the bundle fetch feature is done, but I wouldn't be opposed to it, either. As I understand it, to fetch from a bundle we'd need to:

Select a bundle from the filesystem
Run git bundle verify to make sure that the current repo is able to fetch from the selected bundle
Add the bundle as a remote so we can interactively see what's in there before we fetch from it

The first and second point don't seem too bad, but I think 3 will need some thought.

I don't think you can push to a bundle, so we'd either need to disable that option if the remote is a bundle or throw an error if the user tries to do it.

We'd also need to handle if the user deleted the bundle but still has it added as a remote. I think most of that logic is there already, but we'd want to spot check to make sure it applies for bundles, too.

All in all I have the feeling that "archive" is straightforward enough that we can just add it, but "bundle" has enough questions to it that it would be good to do more of a requirements analysis first. Maybe it would be best to start with a description of what use cases or workflows you want to support with the feature.

Yes the differences between archiving and bundling became more apparent as I was writing this, and I agree at this point that bundle needs a lot more thought than archive. I think I described my use cases/workflows, but please let me know if you have more questions!

What do you think about me removing the bundle portions of this PR? Then we can finish up the archive pieces separately, and I can make either a draft PR or an issue where we can discuss bundling a little more.

stefanhaller · 2024-02-22T19:43:17Z

Maybe <c-a> for archive and <c-b> for bundle where applicable?

Sounds good to me.

I'm not sure if archive needs the feature to specify a prefix.

I did want to include this option to reduce the risk of dumping a ton of files in an unintended directory.

I see. I didn't find this necessary because I usually unpack zip or even tar.gz files by double-clicking them, and both MacOS and Windows will take care of adding an enclosing folder (named after the archive file name) when unpacking when there isn't a single top-level folder in the archive. But I realize that users on the command line don't have this convenience, and I do remember being bitten by it once, so I'm fine with it.

However, I still like streamlined user interfaces, so we could also consider having just a prompt for the archive name and then using that for the prefix as well, unconditionally. What do you think about that?

We need to make it possible to create either "thick" or "thin" bundles.

I totally agree. I can appreciate that use case, but unfortunately I almost always run into the situation where I need to transfer the repo and all its history to someone or some machine.

Thinking about this more, I think we should have separate options for creating thick and thin bundles. If <c-b> is the command for "Create bundle", it could open a menu with these two options. A thin bundle would then include just the commits of the selected branch that are not included in the main branch, i.e. all the commits whose hash is yellow or red.

Add the bundle as a remote so we can interactively see what's in there before we fetch from it

Ah, you can add a bundle as a remote? I wasn't aware of that, that's cool. Yes, we'll want to prevent pushing to it, but it would probably just give an error if users try, so this might just be good enough.

What do you think about me removing the bundle portions of this PR? Then we can finish up the archive pieces separately, and I can make either a draft PR or an issue where we can discuss bundling a little more.

Sounds very good to me.

afhoffman · 2024-02-23T04:24:38Z

I see. I didn't find this necessary because I usually unpack zip or even tar.gz files by double-clicking them, and both MacOS and Windows will take care of adding an enclosing folder (named after the archive file name) when unpacking when there isn't a single top-level folder in the archive. But I realize that users on the command line don't have this convenience, and I do remember being bitten by it once, so I'm fine with it.

However, I still like streamlined user interfaces, so we could also consider having just a prompt for the archive name and then using that for the prefix as well, unconditionally. What do you think about that?

Yeah I still tend to extract archives from the command line for some reason. There is always -C for tar and -d for unzip. I do like the idea of prompting for the archive name and just using that as the folder that goes inside the archive.

I was thinking about this as I was cleaning some things up, and I'd also like to consider setting the title of that archive name prompt to include the branch name/tag name/commit sha that you're making an archive for so it's a little more clear what will go in the archive. Similar to how the worktree option shows "Create worktre from {{branch name}}".

Thinking about this more, I think we should have separate options for creating thick and thin bundles. If <c-b> is the command for "Create bundle", it could open a menu with these two options. A thin bundle would then include just the commits of the selected branch that are not included in the main branch, i.e. all the commits whose hash is yellow or red.

I like that idea a lot, I think we should do that.

Ah, you can add a bundle as a remote? I wasn't aware of that, that's cool. Yes, we'll want to prevent pushing to it, but it would probably just give an error if users try, so this might just be good enough.

Yes you can, but maybe doing it that way could get annoying if you're always having to delete the remote when you're done? I'm not sure how big a deal that is though, I doubt this is something people are doing 5x per day normally. Also I checked and it does throw an error if you try to push a branch to a bundle.

Here's how it looks when a bundle is added as a remote:

And here's the error that gets thrown when you try to push a new branch to a bundle:

What do you think about me removing the bundle portions of this PR? Then we can finish up the archive pieces separately, and I can make either a draft PR or an issue where we can discuss bundling a little more.

Sounds very good to me.

I've removed all of the bundle stuff from this branch and just pushed that. I still need to remove the prefix prompt and look at making it a little more clear as to what ref the archive is being created from. I'll let you know when I think that's ready for review again.

I put the bundle stuff on a different branch in my fork for now, and I'll plan to put a draft PR in for it around the time we get this one finished up.

afhoffman · 2024-03-02T20:40:48Z

@stefanhaller I think I have this in a state I'm comfortable with.

The title for archive name prompt is now "Choose name for archive of {{.ref}}" where ref is the branch name, tag name, or commit sha. That way it's a little more obvious what will go in the archive.
The prefix is now hard-set to be the same as the chosen archive name.
I enforced singleItemSelected in the tag & branch views.

Let me know if I missed anything.

stefanhaller

Very nice. A few comments inline below, most of them minor.

Apart from that, two things:

it's possible to leave the name empty, resulting in a file .zip being created; we should probably disallow that?
if the target file already exists, it is overwritten without warning. I find this a little drastic, I think we should check for that and add an additional prompt in this case.

stefanhaller · 2024-03-03T20:21:24Z

pkg/commands/git_commands/archive.go

+}
+
+func (self *ArchiveCommands) Archive(refName string, archiveName string, prefix string) error {
+	cmdArgs := NewGitCmd("archive").ArgIf(prefix != "", "--prefix", prefix).Arg("-o", archiveName, refName)


Most of the time we format commands so that each argument is on a separate line, so I'd format this something like:

cmdArgs := NewGitCmd("archive"). ArgIf(prefix != "", "--prefix", prefix). Arg("-o", archiveName). Arg(refName)

stefanhaller · 2024-03-03T20:23:40Z

pkg/commands/git_commands/archive.go

+
+	validExtensions, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
+	if err != nil {
+		return []string{""}, err


Does it make sense to return a list with a single empty string in the error case? I'd just say return nil, err.

stefanhaller · 2024-03-03T20:26:52Z

pkg/config/user_config.go

@@ -403,6 +403,7 @@ type KeybindingBranchesConfig struct {
 	SetUpstream            string `yaml:"setUpstream"`
 	FetchRemote            string `yaml:"fetchRemote"`
 	SortOrder              string `yaml:"sortOrder"`
+	Archive                string `yaml:"archive"`


We could consider putting it in Universal instead; it's "almost" universal (and we might consider offering it in the subcommits view too). I find it very unlikely that users will want to map it to different keys for branches and commits.

stefanhaller · 2024-03-03T20:29:13Z

pkg/gui/controllers/helpers/archive_helper.go

+}
+
+func (self *ArchiveHelper) runArchiveCommand(refName string, fileName string, prefix string, suffix string) error {
+	return self.c.WithWaitingStatus(self.c.Tr.ArchiveWaitingStatusMessage, func(gocui.Task) error {


It would be nice to show an inline status like we do for pulling and pushing. (And this will have the additional advantage that pulling/pushing will automatically be disabled while an archive is being created.)

stefanhaller · 2024-03-03T20:30:21Z

pkg/gui/controllers/helpers/archive_helper.go

+
+func (self *ArchiveHelper) runArchiveCommand(refName string, fileName string, prefix string, suffix string) error {
+	return self.c.WithWaitingStatus(self.c.Tr.ArchiveWaitingStatusMessage, func(gocui.Task) error {
+		return self.c.Git().Archive.Archive(refName, fileName+suffix, prefix)


Minor thing: Archive.Archive reads a bit funny. I wonder if the function should be renamed to Create.

stefanhaller · 2024-03-03T20:32:32Z

pkg/commands/git_commands/archive.go

+func (self *ArchiveCommands) Archive(refName string, archiveName string, prefix string) error {
+	cmdArgs := NewGitCmd("archive").ArgIf(prefix != "", "--prefix", prefix).Arg("-o", archiveName, refName)
+
+	return self.cmd.New(cmdArgs.ToArgv()).DontLog().Run()


Why DontLog()? I think it's nice to see the command in the log.

And call LogAction before, but probably not from here, but from the controller (or helper), I'm too lazy to look up how we usually do this.

feat: generate archive format menu from git output If someone has added a custom format via "git config --global tar.tar.bz2.command bzip2" or similar, that custom format will be selectable in the archive formats menu. feat: add i18n strings, assign keybindings test: add archive branch, tag, and commit tests

afhoffman force-pushed the feature/add-archive-and-bundle-options branch 2 times, most recently from b9e3dc5 to da35ada Compare January 25, 2024 03:04

afhoffman force-pushed the feature/add-archive-and-bundle-options branch from da35ada to 22b985f Compare January 25, 2024 14:18

afhoffman force-pushed the feature/add-archive-and-bundle-options branch from 22b985f to 700f1ab Compare January 26, 2024 14:15

afhoffman force-pushed the feature/add-archive-and-bundle-options branch 4 times, most recently from 81b1f54 to 3561654 Compare January 30, 2024 00:02

afhoffman force-pushed the feature/add-archive-and-bundle-options branch 4 times, most recently from ced9d7d to c32a4c2 Compare February 18, 2024 17:49

afhoffman force-pushed the feature/add-archive-and-bundle-options branch from c32a4c2 to b4a9fe1 Compare February 23, 2024 04:24

afhoffman changed the title ~~Add archive and bundle options~~ Add archive options Feb 23, 2024

afhoffman force-pushed the feature/add-archive-and-bundle-options branch from b4a9fe1 to 26a2930 Compare March 2, 2024 20:37

stefanhaller added the enhancement New feature or request label Mar 3, 2024

stefanhaller reviewed Mar 3, 2024

View reviewed changes

afhoffman force-pushed the feature/add-archive-and-bundle-options branch from 26a2930 to e6d4f3d Compare April 29, 2024 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add archive options #3244

Add archive options #3244

afhoffman commented Jan 23, 2024

stefanhaller commented Jan 25, 2024

afhoffman commented Jan 25, 2024

afhoffman commented Jan 27, 2024

stefanhaller commented Feb 20, 2024

afhoffman commented Feb 20, 2024

stefanhaller commented Feb 22, 2024

afhoffman commented Feb 23, 2024

afhoffman commented Mar 2, 2024

stefanhaller left a comment

stefanhaller Mar 3, 2024

stefanhaller Mar 3, 2024

stefanhaller Mar 3, 2024

stefanhaller Mar 3, 2024

stefanhaller Mar 3, 2024

stefanhaller Mar 3, 2024

Add archive options #3244

Are you sure you want to change the base?

Add archive options #3244

Conversation

afhoffman commented Jan 23, 2024

stefanhaller commented Jan 25, 2024

afhoffman commented Jan 25, 2024

afhoffman commented Jan 27, 2024

stefanhaller commented Feb 20, 2024

afhoffman commented Feb 20, 2024

stefanhaller commented Feb 22, 2024

afhoffman commented Feb 23, 2024

afhoffman commented Mar 2, 2024

stefanhaller left a comment

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment

stefanhaller Mar 3, 2024

Choose a reason for hiding this comment