Re: [PATCH v2] Doc: Bundle file usage

From: Jeff King <peff@peff.net>
Date: Oct 16, 2019, 21:09 UTC
Message-ID: <20191016210957.GA28981@sigill.intra.peff.net>
In-Reply-To: <20191016095737.1588-1-philipoakley@iee.email>

On Wed, Oct 16, 2019 at 10:57:37AM +0100, Philip Oakley wrote:

Show 6 quoted lines

> From: Philip Oakley <philipoakley@iee.org>
> 
> Git URLs can accept bundle files for fetch, pull and clone, include
> in that section. Include git clone in the bundle usage description.
> Correct the quoting of <git-rev-list-args>.
> Detail the <git-rev-list-args> options for cloning a complete repo.

Thanks for picking this up again. :)

A few minor comments:

Show 10 quoted lines

> diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
> index 7d6c9dcd17..0498e4895d 100644
> --- a/Documentation/git-bundle.txt
> +++ b/Documentation/git-bundle.txt
> @@ -21,9 +21,9 @@ Some workflows require that one or more branches of development on one
>  machine be replicated on another machine, but the two machines cannot
>  be directly connected, and therefore the interactive Git protocols (git,
>  ssh, http) cannot be used.  This command provides support for
> -'git fetch' and 'git pull' to operate by packaging objects and references
> +'git fetch' and 'git pull' and 'git clone', to operate by packaging objects and references

Maybe:

  'git fetch', 'git pull', and 'git clone'

? Given the repetition below, though:

>  in an archive at the originating machine, then importing those into
> -another repository using 'git fetch' and 'git pull'
> +another repository using 'git fetch' and 'git pull' or 'git clone',

I wonder if we could rephrase this in a less awkward way. Perhaps:

  The 'git bundle' command packages objects and references in an archive
  at the originating machine, which can then be imported into another
  repository using 'git fetch', 'git pull', or 'git clone'.

Show 6 quoted lines

> @@ -35,7 +35,7 @@ OPTIONS
>  
>  create <file>::
>  	Used to create a bundle named 'file'.  This requires the
> -	'git-rev-list-args' arguments to define the bundle contents.
> +	'<git-rev-list-args>' arguments to define the bundle contents.

This hunk makes sense. I'd probably use backticks here instead of single-quotes, but I think we're pretty inconsistent across the documentation about this. It probably makes sense to match the existing text.

Show 7 quoted lines

> @@ -92,6 +92,10 @@ It is okay to err on the side of caution, causing the bundle file
>  to contain objects already in the destination, as these are ignored
>  when unpacking at the destination.
>  
> +To create a bundle for 'git clone', use `--branches --tags` for
> +the <git-rev-list-args>. The (inappropriate) use of `--all` would include
> +refs from refs/remotes/* hierarchy in the resulting bundle.

Should <git-rev-list-args> be in quotes or backticks?

Any bundle created without a negative revision would be appropriate for a clone. Maybe we could spell that out in more detail, like:

  Any bundle created without negative refspecs (e.g., `new` but not
  `old..new`) can be used on the receiving side with `git clone`. If you
  want to provide the same set of refs that a clone directly from the
  source repository would get, use `--branches --tags`. If you want to
  match `git clone --mirror`, which would clone other refs such as
  `refs/remotes/*`, use `--all`.

Show 10 quoted lines

> diff --git a/Documentation/urls.txt b/Documentation/urls.txt
> index bc354fe2dc..1c229d7581 100644
> --- a/Documentation/urls.txt
> +++ b/Documentation/urls.txt
> @@ -53,6 +53,9 @@ These two syntaxes are mostly equivalent, except the former implies
>  --local option.
>  endif::git-clone[]
>  
> +'git clone', 'git fetch' and 'git pull', but not 'git push', will also
> +accept a suitable bundle file. See linkgit:git-bundle[1].

This makes sense to mention here. It's a little funny because the user would see this included in "man git-clone" or whatever, but I don't think it hurts to just be exhaustive rather than trying to tailor it to each individual manpage.

-Peff