Git development
 help / color / mirror / Atom feed
* Re: "git overlay" - command for overlaying branches
From: Oliver Bandel @ 2023-12-01  0:11 UTC (permalink / raw)
  To: Michal Suchánek; +Cc: git
In-Reply-To: <20231125105046.GB9696@kitsune.suse.cz>

What I basically want to do is:

- Working on more than one branch at the same time.
- Having all checked-out branches together in one working dir.
- When adding/committing, all checked-out files will be added/committed
  to those branches, from where they were checked out.

This way I could edit multiple branches simultaneously independently without the need of merging.
Merging can be done later on, if needed.

Possible alternatives?

For the multiple checkouts I could do a workaround and use a local bare-repo
and check out each branch of interest into a seperate directory.
I could then edit the files from the different dirs.

But for the overlaying thing, to get the files into one dir
(with the automatic hiding of equally named files from branches that
were overlayed before) it would need something like a FUSE-based
tool, so that all the branches could be blended into one working dir.

A "git overlay" would make this easier.
No extra bare-repo needed, no additional tool for blending the dirs
needed.

Ciao,
  Oliver

^ permalink raw reply

* Re: [PATCH] Mention default oldbranch in git-branch doc
From: Rubén Justo @ 2023-12-01  1:25 UTC (permalink / raw)
  To: Clarence "Sparr" Risher via GitGitGadget, git
  Cc: Clarence "Sparr" Risher
In-Reply-To: <pull.1613.git.git.1701303891791.gitgitgadget@gmail.com>

On 30-nov-2023 00:24:51, Clarence "Sparr" Risher via GitGitGadget wrote:
> From: "Clarence \"Sparr\" Risher" <sparr0@gmail.com>
> 
> `git branch -m` has an optional first argument, the branch to rename.
> If omitted, it defaults to the current branch. This behavior is not
> currently described in the documentation.

Good catch.

> While it can be determined
> relatively easily through experimentation, and less so through viewing
> the source code (builtin/branch.c:897)

Some people will find it easier by reading the code than through
experimentation ;-)

> it is not obvious what such a
> command will do when encountered in a less interactive context.

Certainly, I agree.

> 
> This patch adds one sentence to the git-branch documentation, with
> wording based on another optional argument described earlier in the
> same doc.
> 
> The same behavior applies to -M, -c, and -C

Good.

> 
> Signed-off-by: Clarence Risher <clarence@kira-learning.com>
> Signed-off-by: Clarence "Sparr" Risher <sparr0@gmail.com>

Are you both?  If that is the case, IMO it is unusual in this project to
have two different signatures for the same person in one commit.

> 
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 4395aa93543..affed1b17a4 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -73,6 +73,7 @@ overridden by using the `--track` and `--no-track` options, and
>  changed later using `git branch --set-upstream-to`.
>  
>  With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
> +If the <oldbranch> argument is missing it defaults to the current branch.

Sounds good.  But IMHO this information fits better below, where the
term "<oldbranch>" is described.  Something like:

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4395aa9354..9c17df9f4d 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -312,7 +312,8 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
        option is omitted, the current HEAD will be used instead.

 <oldbranch>::
-       The name of an existing branch to rename.
+       The name of an existing branch to rename or copy.  If this
+       option is omitted, the current branch will be used instead.

Completing while we're here, what we missed when -c was introduced in
52d59cc645 (branch: add a --copy (-c) option to go with --move (-m),
2017-06-18).

>  If <oldbranch> had a corresponding reflog, it is renamed to match
>  <newbranch>, and a reflog entry is created to remember the branch
>  renaming. If <newbranch> exists, -M must be used to force the rename
> 
> base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
> -- 
> gitgitgadget


Thank you for working on this.

^ permalink raw reply related

* Re: [PATCH 1/1] attr: add builtin objectmode values support
From: Joanna Wang @ 2023-12-01  4:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sunshine, tboegi
In-Reply-To: <xmqqil62qfvr.fsf@gitster.g>

> Cumulatively, aside from the removal of the t/#t* file, here is what
> I ended up with so far.

I want to double check if I should followup here.
I assumed that you had already applied these final fixes on my behalf,
similar to my patch for enabling attr for `git-add`. But if I was wrong,
I'm happy to send another update with all the fixes.


On Thu, Nov 16, 2023 at 4:08 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Other than the removal of that file, I locally applied the following
> > fix-up while checking the difference relative to the previous
> > iteration.
>
> Cumulatively, aside from the removal of the t/#t* file, here is what
> I ended up with so far.
>
> Subject: [PATCH] SQUASH???
>
> ---
>  Documentation/gitattributes.txt |  2 +-
>  neue                            |  0
>  t/t0003-attributes.sh           |  5 +++--
>  t/t6135-pathspec-with-attrs.sh  | 10 ++++++----
>  4 files changed, 10 insertions(+), 7 deletions(-)
>  create mode 100644 neue
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 784aa9d4de..201bdf5edb 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -108,7 +108,7 @@ user defined attributes under this namespace will be ignored and
>  trigger a warning.
>
>  `builtin_objectmode`
> -^^^^^^^^^^^^^^^^^^^^
> +~~~~~~~~~~~~~~~~~~~~
>  This attribute is for filtering files by their file bit modes (40000,
>  120000, 160000, 100755, 100644). e.g. ':(attr:builtin_objectmode=160000)'.
>  You may also check these values with `git check-attr builtin_objectmode -- <file>`.
> diff --git a/neue b/neue
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> index 86f8681570..774b52c298 100755
> --- a/t/t0003-attributes.sh
> +++ b/t/t0003-attributes.sh
> @@ -580,12 +580,13 @@ test_expect_success 'builtin object mode attributes work (dir and regular paths)
>  '
>
>  test_expect_success POSIXPERM 'builtin object mode attributes work (executable)' '
> -       >exec && chmod +x exec &&
> +       >exec &&
> +       chmod +x exec &&
>         attr_check_object_mode exec 100755
>  '
>
>  test_expect_success SYMLINKS 'builtin object mode attributes work (symlinks)' '
> -       >to_sym ln -s to_sym sym &&
> +       ln -s to_sym sym &&
>         attr_check_object_mode sym 120000
>  '
>
> diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
> index b08a32ea68..f6403ebbda 100755
> --- a/t/t6135-pathspec-with-attrs.sh
> +++ b/t/t6135-pathspec-with-attrs.sh
> @@ -295,22 +295,24 @@ test_expect_success 'reading from .gitattributes in a subdirectory (3)' '
>         test_cmp expect actual
>  '
>
> -test_expect_success 'pathspec with builtin_objectmode attr can be used' '
> +test_expect_success POSIXPERM 'pathspec with builtin_objectmode attr can be used' '
>         >mode_exec_file_1 &&
>
>         git status -s ":(attr:builtin_objectmode=100644)mode_exec_*" >actual &&
>         echo ?? mode_exec_file_1 >expect &&
>         test_cmp expect actual &&
>
> -       git add mode_exec_file_1 && chmod +x mode_exec_file_1 &&
> +       git add mode_exec_file_1 &&
> +       chmod +x mode_exec_file_1 &&
>         git status -s ":(attr:builtin_objectmode=100755)mode_exec_*" >actual &&
>         echo AM mode_exec_file_1 >expect &&
>         test_cmp expect actual
>  '
>
> -test_expect_success 'builtin_objectmode attr can be excluded' '
> +test_expect_success POSIXPERM 'builtin_objectmode attr can be excluded' '
>         >mode_1_regular &&
> -       >mode_1_exec  && chmod +x mode_1_exec &&
> +       >mode_1_exec  &&
> +       chmod +x mode_1_exec &&
>         git status -s ":(exclude,attr:builtin_objectmode=100644)" "mode_1_*" >actual &&
>         echo ?? mode_1_exec >expect &&
>         test_cmp expect actual &&
> --
> 2.43.0-rc2
>

^ permalink raw reply

* Re: [PATCH 3/4] refs: complete list of special refs
From: Patrick Steinhardt @ 2023-12-01  6:43 UTC (permalink / raw)
  To: phillip.wood; +Cc: git, hanwenn
In-Reply-To: <15f67e21-c05f-4a72-9557-2a09a1311f25@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4298 bytes --]

On Thu, Nov 30, 2023 at 03:42:06PM +0000, Phillip Wood wrote:
> Hi Patrick
> 
> Thanks for working on this. I've left a couple of thought below.
> 
> On 29/11/2023 08:14, Patrick Steinhardt wrote:
> > +static int is_special_ref(const char *refname)
> > +{
> > +	/*
> > +	 * Special references get written and read directly via the filesystem
> > +	 * by the subsystems that create them. Thus, they must not go through
> > +	 * the reference backend but must instead be read directly. It is
> > +	 * arguable whether this behaviour is sensible, or whether it's simply
> > +	 * a leaky abstraction enabled by us only having a single reference
> > +	 * backend implementation. But at least for a subset of references it
> > +	 * indeed does make sense to treat them specially:
> > +	 *
> > +	 * - FETCH_HEAD may contain multiple object IDs, and each one of them
> > +	 *   carries additional metadata like where it came from.
> > +	 *
> > +	 * - MERGE_HEAD may contain multiple object IDs when merging multiple
> > +	 *   heads.
> > +	 *
> > +	 * - "rebase-apply/" and "rebase-merge/" contain all of the state for
> > +	 *   rebases, where keeping it closely together feels sensible.
> 
> I'd really like to get away from treating these files as refs. I think their
> use as refs is purely historic and predates the reflog and possibly
> ORIG_HEAD. These days I'm not sure there is a good reason to be running
> 
>     git rev-parse rebase-merge/orig-head
> 
> One reason for not wanting to treat them as refs is that we do not handle
> multi-level refs that do not begin with "refs/" consistently.
> 
>     git update-ref foo/bar HEAD
> 
> succeeds and creates .git/foo/bar but
> 
>     git update-ref -d foo/bar
> 
> fails with
> 
>     error: refusing to update ref with bad name 'foo/bar'
> 
> To me it would make sense to refuse to create 'foo/bar' but allow an
> existing ref named 'foo/bar' to be deleted but the current behavior is the
> opposite of that.
> 
> I'd be quite happy to see us refuse to treat anything that fails
> 
>     if (starts_with(refname, "refs/") || refname_is_safe(refname))
> 
> as a ref but I don't know how much pain that would cause.

Well, we already do use these internally as references, but I don't
disagree with you. I think the current state is extremely confusing,
which is why my first approach was to simply document what falls into
the category of these "special" references.

In my mind, this patch series here is a first step towards addressing
the problem more generally. For now it is more or less only documenting
_what_ is a special ref and why they are special, while also ensuring
that these refs are compatible with the reftable backend. But once this
lands, I'd certainly want to see us continue to iterate on this.

Most importantly, I'd love to see us address two issues:

  - Start to refuse writing these special refs via the refdb so that
    the rules I've now layed out are also enforced. This would also
    address your point about things being inconsistent.

  - Gradually reduce the list of special refs so that they are reduced
    to a bare minimum and so that most refs are simply that, a normal
    ref.

> > +	const char * const special_refs[] = {
> > +		"AUTO_MERGE",
> 
> Is there any reason to treat this specially in the long term? It points to a
> tree rather than a commit but unlike MERGE_HEAD and FETCH_HEAD it is
> effectively a "normal" ref.

No, I'd love to see this and others converted to become a normal ref
eventually. The goal of this patch series was mostly to document what we
already have, and address those cases which are inconsistent with the
new rules. But I'd be happy to convert more of these special refs to
become normal refs after it lands.

> > +		"BISECT_EXPECTED_REV",
> > +		"FETCH_HEAD",
> > +		"MERGE_AUTOSTASH",
> 
> Should we be treating this as a ref? I thought it was written as an
> implementation detail of the autostash implementation rather than to provide
> a ref for users and scripts.

Yes, we have to in the context of the reftable backend. There's a bunch
of tests that exercise our ability to parse this as a ref, and they
would otherwise fail with the reftable backend.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] git-prompt: stop manually parsing HEAD
From: Patrick Steinhardt @ 2023-12-01  7:34 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Eric Sunshine, git, Stan Hu
In-Reply-To: <20231124182803.GA11157@szeder.dev>

[-- Attachment #1: Type: text/plain, Size: 3976 bytes --]

On Fri, Nov 24, 2023 at 07:28:03PM +0100, SZEDER Gábor wrote:
> On Fri, Nov 24, 2023 at 01:09:03PM -0500, Eric Sunshine wrote:
> > On Fri, Nov 24, 2023 at 6:37 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > We're manually parsing the HEAD reference in git-prompt to figure out
> > > whether it is a symbolic or direct reference. This makes it intimately
> > > tied to the on-disk format we use to store references and will stop
> > > working once we gain additional reference backends in the Git project.
> > >
> > > Refactor the code to always use git-symbolic-ref(1) to read HEAD, which
> > > is both simpler and compatible with alternate reference backends.
> > 
> > This may get some push-back from Windows folks due to high
> > process-creation cost on that platform. As I recall, over the years, a
> > good deal of effort has been put into reducing the number of programs
> > run each time the prompt is displayed, precisely because invoking Git
> > (or other programs) multiple times became unbearably slow. In
> > particular, optimizations efforts have focussed on computing as much
> > as possible within the shell itself rather than invoking external
> > programs for the same purpose. Thus, this seems to be taking a step
> > backwards in that regard for the common or status quo case.
> > 
> > Would it be possible instead to, within shell, detect if the historic
> > file-based backend is being used in the current repository, thus
> > continue using the existing shell code for that case, and only employ
> > git-symbolic-ref if some other backend is in use?
> 
> Thanks for sharing my worries :)
> 
> I sent a patch a while ago to Han-Wen to make our Bash prompt script
> work with the reftable backend without incurring the overhead of extra
> subshells or processes when using the files based refs backend.  He
> picked it up and used to include it in rerolls of the reftable patch
> series; the last version of that patch is I believe at:
> 
>   https://public-inbox.org/git/patch-v4-21.28-443bdebfb5d-20210823T120208Z-avarab@gmail.com/

Fair enough, I'm sure I can roll something similar into my patch series.
I do wonder whether it's fine to already submit those patches now where
the reftable backend doesn't exist yet. But I'd hope so, because it
would make it significantly easier for us to upstream the backend if we
can only focus on the backend itself, whereas all the other parts were
already addressed in preliminary refactorings.

One question though is what the right way to detect the reference format
would be. Reading HEAD and comparing it to "ref: refs/heads/.invalid" is
okay for now, but doesn't really feel like a good fit in the long term
as there has been discussion around dropping the requirement for HEAD to
exist altogether [1] in the future. There are some alternatives:

  - Check for the existence of `reftables/` via `test -d`. This is easy
    enough to do, but also doesn't feel all that robust.

  - Extend git-rev-parse(1) to support a new `--show-reference-format`
    option. We already have `--show-object-format`, so this would be a
    natural fit.

In the long term I'd aim for the second solution -- it's the most robust
solution and would also scale if there ever were additional backends.
Furthermore, we already execute git-rev-parse(1) unconditionally anyway.
So there wouldn't be a performance hit here.

While I plan to introduce the `extensions.refStorage` format before
upstreaming the new backend itself, I think it's still going to be some
time until I submit that patch series. Until then, I'd say we simply use
the proposed way of parsing HEAD and second-guessing that it might
indicate the reftable backend, like Stan also does at [2] for our Bash
completion code. I'll make a mental note to refactor these once we have
the extension ready.

Patrick

[1]: <ZWcOvjGPVS_CMUAk@tanuki>
[2]: <20231130202404.89791-1-stanhu@gmail.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] completion: refactor and support reftables backend
From: Patrick Steinhardt @ 2023-12-01  7:49 UTC (permalink / raw)
  To: Stan Hu; +Cc: git, Christian Couder, Junio C Hamano
In-Reply-To: <20231130202404.89791-1-stanhu@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On Thu, Nov 30, 2023 at 12:24:02PM -0800, Stan Hu wrote:
> Hi,
> 
> This patch series refactors and updates git-completion.bash to become
> compatible with the upcoming reftables backend.

Hi Stan!

Disclaimer up front: I've already reviewed these patches internally at
GitLab.

Regarding the format of this patch submission: in the Git project we
typically use "shallow" threads, where every mail is a reply to the head
of the series (in this case the cover letter). You can configure these
by setting "format.thread=shallow" in your Git configuration.

That's an easy thing to miss though, as MyFirstContribution.txt doesn't
point this out at all. I wonder whether we want to amend it to say so?

Patrick

> Stan Hu (2):
>   completion: refactor existence checks for special refs
>   completion: stop checking for reference existence via `test -f`
> 
>  contrib/completion/git-completion.bash | 43 +++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> -- 
> 2.42.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] completion: stop checking for reference existence via `test -f`
From: Patrick Steinhardt @ 2023-12-01  7:49 UTC (permalink / raw)
  To: Stan Hu; +Cc: git, Christian Couder
In-Reply-To: <20231130202404.89791-3-stanhu@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Thu, Nov 30, 2023 at 12:24:04PM -0800, Stan Hu wrote:
> In contrib/completion/git-completion.bash, there are a bunch of
> instances where we read special refs like HEAD, MERGE_HEAD,
> REVERT_HEAD, and others via the filesystem. However, the upcoming
> reftable refs backend won't use '.git/HEAD' at all but instead will
> write an invalid refname as placeholder for backwards compatibility,
> which will break the git-completion script.
> 
> Update the '__git_ref_exists' function to:
> 
> 1. Recognize the placeholder '.git/HEAD' written by the reftable
>    backend (its content is specified in the reftable specs).
> 2. If reftable is in use, use 'git rev-parse' to determine whether the
>     given ref exists.
> 3. Otherwise, continue to use 'test -f' to check for the ref's filename.

Nit, not worth a reroll: you already document this in the code, but I
think it could help to also briefly explain why we're going through all
of these hoops here instead of just using git-rev-parse(1) everywhere in
the commit message.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 07/24] midx: implement `--retain-disjoint` mode
From: Patrick Steinhardt @ 2023-12-01  8:02 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZWjisIgiuziMvBph@nand.local>

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

On Thu, Nov 30, 2023 at 02:29:52PM -0500, Taylor Blau wrote:
> On Thu, Nov 30, 2023 at 11:18:51AM +0100, Patrick Steinhardt wrote:
> > > diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
> > > index d130e65b28..ac0c7b124b 100644
> > > --- a/Documentation/git-multi-pack-index.txt
> > > +++ b/Documentation/git-multi-pack-index.txt
> > > @@ -54,6 +54,14 @@ write::
> > >  		"disjoint". See the "`DISP` chunk and disjoint packs"
> > >  		section in linkgit:gitformat-pack[5] for more.
> > >
> > > +	--retain-disjoint::
> > > +		When writing a multi-pack index with a reachability
> > > +		bitmap, keep any packs marked as disjoint in the
> > > +		existing MIDX (if any) as such in the new MIDX. Existing
> > > +		disjoint packs which are removed (e.g., not listed via
> > > +		`--stdin-packs`) are ignored. This option works in
> > > +		addition to the '+' marker for `--stdin-packs`.
> >
> > I'm trying to understand when you're expected to pass this flag and when
> > you're expected not to pass it. This documentation could also help in
> > the documentation here so that the user can make a more informed
> > decision.
> 
> I think there are multiple reasons that you may or may not want to pass
> that flag. Certainly if you're not using disjoint packs (and instead
> only care about single-pack verbatim reuse over the MIDX's preferred
> packfile), then you don't need to pass it.
> 
> But if you are using disjoint packs, you may want to pass it if you are
> adding packs to the MIDX which are disjoint, _and_ you want to hold onto
> the existing set of disjoint packs.
> 
> But if you want to change the set of disjoint packs entirely, you would
> want to omit this flag (unless you knew a-priori that you were going to
> drop all of the currently marked disjoint packs from the new MIDX you
> are writing, e.g. with --stdin-packs).
> 
> If you think it would be useful, I could try and distill some of this
> down, but I think that there is likely too much detail here for it to be
> useful in user-facing documentation.

Yeah, this indeed feels too detailed to be added here. I was hoping for
a simple "Never do this if"-style rule that points out why it is unwise
under some circumstances, but seems like it's not as simple as that.

Well, so be it. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 08/24] pack-objects: implement `--ignore-disjoint` mode
From: Patrick Steinhardt @ 2023-12-01  8:17 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZWjjSOJHw6Q1qQ+y@nand.local>

[-- Attachment #1: Type: text/plain, Size: 3638 bytes --]

On Thu, Nov 30, 2023 at 02:32:24PM -0500, Taylor Blau wrote:
> On Thu, Nov 30, 2023 at 11:18:57AM +0100, Patrick Steinhardt wrote:
> > > Instead, teach `pack-objects` a special `--ignore-disjoint` which is the
> > > moral equivalent of marking the set of disjoint packs as kept, and
> > > ignoring their contents, even if it would have otherwise been packed. In
> > > fact, this similarity extends down to the implementation, where each
> > > disjoint pack is first loaded, then has its `pack_keep_in_core` bit set.
> > >
> > > With this in place, we can use the kept-pack cache from 20b031fede
> > > (packfile: add kept-pack cache for find_kept_pack_entry(), 2021-02-22),
> > > which looks up objects first in a cache containing just the set of kept
> > > (in this case, disjoint) packs. Assuming that the set of disjoint packs
> > > is a relatively small portion of the entire repository (which should be
> > > a safe assumption to make), each object lookup will be very inexpensive.
> >
> > This cought me by surprise a bit. I'd have expected that in the end,
> > most of the packfiles in a repository would be disjoint. Using for
> > example geometric repacks, my expectation was that all of the packs that
> > get written via geometric repacking would eventually become disjoint
> > whereas new packs added to the repository would initially not be.
> 
> Which part are you referring to here? If you're referring to the part
> where I say that the set of disjoint packs is relatively small in
> proposition to the rest of the packs, I think I know where the confusion
> is.

Yeah, that's what I was referring to.

> I'm not saying that the set of disjoint packs is small in comparison to
> the rest of the repository by object count, but rather by count of packs
> overall. You're right that packs from pushes will not be guaranteed to
> be disjoint upon entering the repository, but will become disjoint when
> geometrically repacked (assuming that the caller uses --ignore-disjoint
> when repacking).

I was actually thinking about it in the number of packfiles, not number
of objects. I'm mostly coming from the angle of geometric repacking
here, where it is totally expected that you have a comparatively large
number of packfiles when your repository is big. With a geometric factor
of 2, you'll have up to `log2($numobjects)` many packfiles in your repo
while keeping the geometric sequence intact.

In something like linux.git with almost 10M objects that boils down to
23 packfiles, and I'd assume that all of these would be disjoint in the
best case. So if you gain new packfiles by pushing into the repository
then I'd think that it's quite likely that the number of non-disjoint
packfiles is smaller than the number of disjoint ones.

I do realize though that in absolute numbers, this isn't all that many.
I was also thinking ahead though to a future where we have something
like geometric repacking with maximum packfile sizes working well
together so that we'll be able to merge packfiles together until they
reach a certain maximum size, and afterwards they are just left alone.
This would help to avoid those "surprise" repack cases where everything
is again packed into a single packfile for the biggest repositories out
there. But it would of course also lead to an increase in packfiles in
those huge repositories.

Anyway, I feel like I'm rambling. In the end it's probably going to be
fine, I was simply surprised by your assumption that the number of
disjoint packfiles should usually be much smaller than the number of
non-disjoint ones.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 00/24] pack-objects: multi-pack verbatim reuse
From: Patrick Steinhardt @ 2023-12-01  8:31 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZWjk/TYzsrdHefgy@nand.local>

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

On Thu, Nov 30, 2023 at 02:39:41PM -0500, Taylor Blau wrote:
> On Thu, Nov 30, 2023 at 11:18:19AM +0100, Patrick Steinhardt wrote:
[snip]
> Then you'd have five patch series, where each series does roughly the
> following:
> 
>   1. Preparatory clean-up.
>   2. Implementing the DISP chunk, as well as --retain-disjoint, without
>      a way to generate such packs.
>   3. Implement a way to generate such packs, but without actually being
>      able to reuse more than one of them.
>   4. Implement multi-pack reuse, but without actually reusing any packs.
>   5. Enable multi-pack reuse (and implement the required scaffolding to
>      do so), and test it.
> 
> That's the most sensible split that I could come up with, at least.

Looks sensible to me.

> But
> I still find it relatively unsatisfying for a couple of reasons:
> 
>   - With the exception of the last group of patches, none of the earlier
>     series enable any new, useful behavior on their own. IOW, if we just
>     merged the first three series and then forgot about this topic, we
>     wouldn't have done anything useful ;-).

Well, sometimes I wish we'd buy more into the iterative style of working
in the Git project, where it's fine to land patch series that only work
into the direction of a specific topic but don't yet do anything
interesting by themselves. The benefits would be both that individual
pieces land faster while also ensuring that the review load is kept at
bay.

But there's of course also downsides to this, especially in an open
source project like Git:

  - Contributors may go away in the middle of their endeavour, leaving
    behind dangling pieces that might have complicated some of our
    architecture without actually reaping the intended benefits.

  - Overall, it may take significantly longer to get all pieces into
    Git.

  - Contributors need to do a much better job to explain where they are
    headed when the series by itself doesn't do anything interesting
    yet.

So I understand why we typically don't work this way.

I wonder whether a compromise would be to continue sending complete
patch series, but explicitly point out "break points" in your patch
series. These break points could be an indicator to the maintainer that
it's fine to merge everything up to it while still keeping out the
remainder of the patch series.

>   - The fourth series (which actually implements multi-pack reuse) still
>     remains the most complicated, and would likely be the most difficult
>     to review. So I think you'd still have one difficult series to
>     review, plus four other series which are slightly less difficult to
>     review ;-).

That's fine in my opinion, there's no surprise here that a complicated
topic is demanding for both the author and the reviewer.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* git:// warn as connection not secure
From: Jonny Grant @ 2023-12-01 11:57 UTC (permalink / raw)
  To: git

Hello
May I ask if anyone has suggested adding a default warning that git:// is not a secure connection?

ie "warning: git:// is not a secure connection. https and ssh are secure."

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Cloning into 'linux'...
remote: Enumerating objects: 9863119, done.
remote: Counting objects: 100% (2360/2360), done.
remote: Compressing objects: 100% (1282/1282), done.
^Cceiving objects:   0% (8779/9863119), 3.21 MiB | 6.41 MiB/s

Kind regards
Jonny

^ permalink raw reply

* [PATCH v6] subtree: fix split processing with multiple subtrees present
From: Zach FettersMoore via GitGitGadget @ 2023-12-01 14:54 UTC (permalink / raw)
  To: git; +Cc: Christian Couder, Zach FettersMoore, Zach FettersMoore
In-Reply-To: <pull.1587.v5.git.1701206267300.gitgitgadget@gmail.com>

From: Zach FettersMoore <zach.fetters@apollographql.com>

When there are multiple subtrees present in a repository and they are
all using 'git subtree split', the 'split' command can take a
significant (and constantly growing) amount of time to run even when
using the '--rejoin' flag. This is due to the fact that when processing
commits to determine the last known split to start from when looking
for changes, if there has been a split/merge done from another subtree
there will be 2 split commits, one mainline and one subtree, for the
second subtree that are part of the processing. The non-mainline
subtree split commit will cause the processing to always need to search
the entire history of the given subtree as part of its processing even
though those commits are totally irrelevant to the current subtree
split being run.

To see this in practice you can use the open source GitHub repo
'apollo-ios-dev' and do the following in order:

-Make a changes to a file in 'apollo-ios' and 'apollo-ios-codegen'
 directories
-Create a commit containing these changes
-Do a split on apollo-ios-codegen
   - Do a fetch on the subtree repo
      - git fetch git@github.com:apollographql/apollo-ios-codegen.git
   - git subtree split --prefix=apollo-ios-codegen --squash --rejoin
   - Depending on the current state of the 'apollo-ios-dev' repo
     you may see the issue at this point if the last split was on
     apollo-ios
-Do a split on apollo-ios
   - Do a fetch on the subtree repo
      - git fetch git@github.com:apollographql/apollo-ios.git
   - git subtree split --prefix=apollo-ios --squash --rejoin
-Make changes to a file in apollo-ios-codegen
-Create a commit containing the change(s)
-Do a split on apollo-ios-codegen
   - git subtree split --prefix=apollo-ios-codegen --squash --rejoin
-To see that the patch fixes the issue you can use the custom subtree
 script in the repo so following the same steps as above, except
 instead of using 'git subtree ...' for the commands use
 'git-subtree.sh ...' for the commands

You will see that the final split is looking for the last split
on apollo-ios-codegen to use as it's starting point to process
commits. Since there is a split commit from apollo-ios in between the
2 splits run on apollo-ios-codegen, the processing ends up traversing
the entire history of apollo-ios which increases the time it takes to
do a split based on how long of a history apollo-ios has, while none
of these commits are relevant to the split being done on
apollo-ios-codegen.

So this commit makes a change to the processing of commits for the
split command in order to ignore non-mainline commits from other
subtrees such as apollo-ios in the above breakdown by adding a new
function 'should_ignore_subtree_commit' which is called during
'process_split_commit'. This allows the split/rejoin processing to
still function as expected but removes all of the unnecessary
processing that takes place currently which greatly inflates the
processing time. In the above example, previously the final split
would take ~10-12 minutes, while after this fix it takes seconds.

Added a test to validate that the proposed fix
solves the issue.

The test accomplishes this by checking the output
of the split command to ensure the output from
the progress of 'process_split_commit' function
that represents the 'extracount' of commits
processed remains at 0, meaning none of the commits
from the second subtree were processed.

This was tested against the original functionality
to show the test failed, and then with this fix
to show the test passes.

This illustrated that when using multiple subtrees,
A and B, when doing a split on subtree B, the
processing does not traverse the entire history
of subtree A which is unnecessary and would cause
the 'extracount' of processed commits to climb
based on the number of commits in the history of
subtree A.

Signed-off-by: Zach FettersMoore <zach.fetters@apollographql.com>
---
    subtree: fix split processing with multiple subtrees present
    
    When there are multiple subtrees in a repo and git subtree split
    --rejoin is being used for the subtrees, the processing of commits for a
    new split can take a significant (and constantly growing) amount of time
    because the split commits from other subtrees cause the processing to
    have to scan the entire history of the other subtree(s). This patch
    filters out the other subtree split commits that are unnecessary for the
    split commit processing.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1587%2FBobaFetters%2Fzf%2Fmulti-subtree-processing-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1587/BobaFetters/zf/multi-subtree-processing-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1587

Range-diff vs v5:

 1:  e7445a95f30 ! 1:  2a65ec0e4df subtree: fix split processing with multiple subtrees present
     @@ Commit message
          To see this in practice you can use the open source GitHub repo
          'apollo-ios-dev' and do the following in order:
      
     -    -Make a changes to a file in 'apollo-ios'A and 'apollo-ios-codegen'
     +    -Make a changes to a file in 'apollo-ios' and 'apollo-ios-codegen'
           directories
          -Create a commit containing these changes
          -Do a split on apollo-ios-codegen
     +       - Do a fetch on the subtree repo
     +          - git fetch git@github.com:apollographql/apollo-ios-codegen.git
             - git subtree split --prefix=apollo-ios-codegen --squash --rejoin
     +       - Depending on the current state of the 'apollo-ios-dev' repo
     +         you may see the issue at this point if the last split was on
     +         apollo-ios
          -Do a split on apollo-ios
     +       - Do a fetch on the subtree repo
     +          - git fetch git@github.com:apollographql/apollo-ios.git
             - git subtree split --prefix=apollo-ios --squash --rejoin
          -Make changes to a file in apollo-ios-codegen
          -Create a commit containing the change(s)
          -Do a split on apollo-ios-codegen
             - git subtree split --prefix=apollo-ios-codegen --squash --rejoin
     +    -To see that the patch fixes the issue you can use the custom subtree
     +     script in the repo so following the same steps as above, except
     +     instead of using 'git subtree ...' for the commands use
     +     'git-subtree.sh ...' for the commands
      
          You will see that the final split is looking for the last split
          on apollo-ios-codegen to use as it's starting point to process


 contrib/subtree/git-subtree.sh     | 30 +++++++++++++++++++++-
 contrib/subtree/t/t7900-subtree.sh | 40 ++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index e0c5d3b0de6..a0bf958ea66 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -778,6 +778,22 @@ ensure_valid_ref_format () {
 		die "fatal: '$1' does not look like a ref"
 }
 
+# Usage: check if a commit from another subtree should be
+# ignored from processing for splits
+should_ignore_subtree_split_commit () {
+	assert test $# = 1
+	local rev="$1"
+	if test -n "$(git log -1 --grep="git-subtree-dir:" $rev)"
+	then
+		if test -z "$(git log -1 --grep="git-subtree-mainline:" $rev)" &&
+			test -z "$(git log -1 --grep="git-subtree-dir: $arg_prefix$" $rev)"
+		then
+			return 0
+		fi
+	fi
+	return 1
+}
+
 # Usage: process_split_commit REV PARENTS
 process_split_commit () {
 	assert test $# = 2
@@ -963,7 +979,19 @@ cmd_split () {
 	eval "$grl" |
 	while read rev parents
 	do
-		process_split_commit "$rev" "$parents"
+		if should_ignore_subtree_split_commit "$rev"
+		then
+			continue
+		fi
+		parsedparents=''
+		for parent in $parents
+		do
+			if ! should_ignore_subtree_split_commit "$parent"
+			then
+				parsedparents="$parsedparents$parent "
+			fi
+		done
+		process_split_commit "$rev" "$parsedparents"
 	done || exit $?
 
 	latest_new=$(cache_get latest_new) || exit $?
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 49a21dd7c9c..ca4df5be832 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -385,6 +385,46 @@ test_expect_success 'split sub dir/ with --rejoin' '
 	)
 '
 
+# Tests that commits from other subtrees are not processed as
+# part of a split.
+#
+# This test performs the following:
+# - Creates Repo with subtrees 'subA' and 'subB'
+# - Creates commits in the repo including changes to subtrees
+# - Runs the following 'split' and commit' commands in order:
+# 	- Perform 'split' on subtree A
+# 	- Perform 'split' on subtree B
+# 	- Create new commits with changes to subtree A and B
+# 	- Perform split on subtree A
+# 	- Check that the commits in subtree B are not processed
+#			as part of the subtree A split
+test_expect_success 'split with multiple subtrees' '
+	subtree_test_create_repo "$test_count" &&
+	subtree_test_create_repo "$test_count/subA" &&
+	subtree_test_create_repo "$test_count/subB" &&
+	test_create_commit "$test_count" main1 &&
+	test_create_commit "$test_count/subA" subA1 &&
+	test_create_commit "$test_count/subA" subA2 &&
+	test_create_commit "$test_count/subA" subA3 &&
+	test_create_commit "$test_count/subB" subB1 &&
+	git -C "$test_count" fetch ./subA HEAD &&
+	git -C "$test_count" subtree add --prefix=subADir FETCH_HEAD &&
+	git -C "$test_count" fetch ./subB HEAD &&
+	git -C "$test_count" subtree add --prefix=subBDir FETCH_HEAD &&
+	test_create_commit "$test_count" subADir/main-subA1 &&
+	test_create_commit "$test_count" subBDir/main-subB1 &&
+	git -C "$test_count" subtree split --prefix=subADir \
+		--squash --rejoin -m "Sub A Split 1" &&
+	git -C "$test_count" subtree split --prefix=subBDir \
+		--squash --rejoin -m "Sub B Split 1" &&
+	test_create_commit "$test_count" subADir/main-subA2 &&
+	test_create_commit "$test_count" subBDir/main-subB2 &&
+	git -C "$test_count" subtree split --prefix=subADir \
+		--squash --rejoin -m "Sub A Split 2" &&
+	test "$(git -C "$test_count" subtree split --prefix=subBDir \
+		--squash --rejoin -d -m "Sub B Split 1" 2>&1 | grep -w "\[1\]")" = ""
+'
+
 test_expect_success 'split sub dir/ with --rejoin from scratch' '
 	subtree_test_create_repo "$test_count" &&
 	test_create_commit "$test_count" main1 &&

base-commit: bda494f4043963b9ec9a1ecd4b19b7d1cd9a0518
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH 08/24] pack-objects: implement `--ignore-disjoint` mode
From: Taylor Blau @ 2023-12-01 19:58 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZWmWojF6BlOTzkcc@tanuki>

On Fri, Dec 01, 2023 at 09:17:38AM +0100, Patrick Steinhardt wrote:
> In something like linux.git with almost 10M objects that boils down to
> 23 packfiles, and I'd assume that all of these would be disjoint in the
> best case. So if you gain new packfiles by pushing into the repository
> then I'd think that it's quite likely that the number of non-disjoint
> packfiles is smaller than the number of disjoint ones.

Right, although if you have 10M objects over 23 packs with a geometric
repacking factor of two, the last pack should have just around a single
object in it. In other words, as soon as you receive a push, your
geometric progression will collapse into a single pack.

So having a repository with 10M objects split across 23 packs is a
relatively short-lived state. And in general we should only be in that
state every time a repository doubles (again, assuming a factor of two).

In that sense, I'd expect relatively few packs to be disjoint, and for
each of those packs to have a relatively large number of objects,
accounting for most of the non-recent parts of the repository.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH 00/24] pack-objects: multi-pack verbatim reuse
From: Taylor Blau @ 2023-12-01 20:02 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <ZWmZ0pk6fr-8dmF1@tanuki>

On Fri, Dec 01, 2023 at 09:31:14AM +0100, Patrick Steinhardt wrote:
> > But
> > I still find it relatively unsatisfying for a couple of reasons:
> >
> >   - With the exception of the last group of patches, none of the earlier
> >     series enable any new, useful behavior on their own. IOW, if we just
> >     merged the first three series and then forgot about this topic, we
> >     wouldn't have done anything useful ;-).
>
> Well, sometimes I wish we'd buy more into the iterative style of working
> in the Git project, where it's fine to land patch series that only work
> into the direction of a specific topic but don't yet do anything
> interesting by themselves. The benefits would be both that individual
> pieces land faster while also ensuring that the review load is kept at
> bay.
>
> But there's of course also downsides to this, especially in an open
> source project like Git:

I tend to agree with the downsides you list. My biggest concern with
this series in particular is that we're trying to break down an
all-or-nothing change into smaller components. So if we landed four out
of the five of those series, it would be better to have landed none of
them, since the first four aren't really all that useful on their own.

I suppose if we're relatively confident that the last series will be
merged eventually, then that seems like less of a concern. But I'm not
sure that we're at that point yet.

> I wonder whether a compromise would be to continue sending complete
> patch series, but explicitly point out "break points" in your patch
> series. These break points could be an indicator to the maintainer that
> it's fine to merge everything up to it while still keeping out the
> remainder of the patch series.

I think that's a reasonable alternative. It does create a minor headache
for the maintainer[^1], though, so I'd like to avoid it if possible.

> >   - The fourth series (which actually implements multi-pack reuse) still
> >     remains the most complicated, and would likely be the most difficult
> >     to review. So I think you'd still have one difficult series to
> >     review, plus four other series which are slightly less difficult to
> >     review ;-).
>
> That's fine in my opinion, there's no surprise here that a complicated
> topic is demanding for both the author and the reviewer.

My preference is to avoid splitting the series if we can help it. But if
you feel strongly, or others feel similarly, I'm happy to take another
crack at breaking it up. Thanks for all of your review so far!

Thanks,
Taylor

^ permalink raw reply

* Re: git:// warn as connection not secure
From: Eric Wong @ 2023-12-01 21:24 UTC (permalink / raw)
  To: Jonny Grant; +Cc: git
In-Reply-To: <b6e0f49a-34ff-4e8e-9c3e-b8fd41d59f68@jguk.org>

Jonny Grant <jg@jguk.org> wrote:
> Hello
> May I ask if anyone has suggested adding a default warning that git:// is not a secure connection?
> 
> ie "warning: git:// is not a secure connection. https and ssh are secure."

To be accurate, that would need an exclusion list of hosts behind
already-encrypted and trusted networks.  So stuff like .onion hostnames
for Tor, and a user-configurable list of hosts in a private LAN/VPN.

^ permalink raw reply

* [ANNOUNCE] Git Rev News edition 105
From: Christian Couder @ 2023-12-01 22:44 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jakub Narebski, Markus Jansen,
	Štěpán Němec, Kaartic Sivaraam, Taylor Blau,
	Johannes Schindelin, Ævar Arnfjörð Bjarmason,
	Bruno Brito, Alexander Shopov, Luca Milanesio, René Scharfe,
	Philip Oakley, Jeff King, Jason Hatton, brian m. carlson,
	Eric Sunshine, lwn

Hi everyone,

The 105th edition of Git Rev News is now published:

  https://git.github.io/rev_news/2023/11/30/edition-105/

Thanks a lot to Alexander Shopov, Luca Milanesio, Bruno Brito, and
Štěpán Němec who helped this month!

Enjoy,
Christian, Jakub, Markus and Kaartic.

PS: An issue for the next edition is already opened and contributions
are welcome:

  https://github.com/git/git.github.io/issues/675

^ permalink raw reply

* [Patch] test-lib-functions.sh : change test_i18ngrep to test_grep
From: Shreyansh Paliwal @ 2023-12-02 17:24 UTC (permalink / raw)
  To: git

Recently the test_i18ngrep was deprecated from the source code and
test_grep was implemented but in the test-lib-functions.sh file , in
the test_grep() function definition,
it is written BUG "too few parameters to test_i18ngrep".
So the following patch solves the minor problem.

Signed-off-by: Shreyansh Paliwal <Shreyanshpaliwalcmsmn@gmail.com>
---
 t/test-lib-functions.sh | 2 +-
 1 file changed, 1 insertions(+), 1 deletions(-)

 t/test-lib-functions.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)diff --git
a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 9c3cf12b26..8737c95e0c 100644
--- a/t/test-lib-functions.sh
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1277,7 +1277,7 @@ test_grep () {
        if test $# -lt 2 ||
           { test "x!" = "x$1" && test $# -lt 3 ; }
        then
-               BUG "too few parameters to test_i18ngrep"
+               BUG "too few parameters to test_grep"
        fi

        if test "x!" = "x$1"
--
2.43

^ permalink raw reply

* [PATCH v3 1/4] completion: squelch stray errors in sparse-checkout completion
From: Elijah Newren via GitGitGadget @ 2023-12-03  5:57 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

If, in the root of a project, one types

    git sparse-checkout set --cone ../<TAB>

then an error message of the form

    fatal: ../: '../' is outside repository at '/home/newren/floss/git'

is written to stderr, which munges the users view of their own command.
Squelch such messages by using the __git() wrapper, designed for this
purpose; see commit e15098a314 (completion: consolidate silencing errors
from git commands, 2017-02-03) for more on the wrapper.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 13a39ebd2e7..b8661701718 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3084,7 +3084,7 @@ __gitcomp_directories ()
 			COMPREPLY+=("$c/")
 			_found=1
 		fi
-	done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
+	done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)
 
 	if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
 		# No possible further completions any deeper, so assume we're at
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 0/4] Sparse checkout completion fixes
From: Elijah Newren via GitGitGadget @ 2023-12-03  5:57 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren
In-Reply-To: <pull.1349.v2.git.1700985086.gitgitgadget@gmail.com>

This fixes a few issues with tab completion for the sparse-checkout command,
specifically with the "add" and "set" subcommands.

Changes since v2:

 * For patch 4, provide completion (and startings) of arguments into
   patterns that match files in the index -- as suggested by Junio.

Changes since v1:

 * Use __git wrapper function to squelch errors, as suggested by SZEDER
   Gábor
 * note that we could use the index or HEAD for the more involved solution
   in patch 4.

[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/

Elijah Newren (4):
  completion: squelch stray errors in sparse-checkout completion
  completion: fix logic for determining whether cone mode is active
  completion: avoid misleading completions in cone mode
  completion: avoid user confusion in non-cone mode

 contrib/completion/git-completion.bash | 123 ++++++++++++++++++++++++-
 t/t9902-completion.sh                  |   9 +-
 2 files changed, 127 insertions(+), 5 deletions(-)


base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1349

Range-diff vs v2:

 1:  97e20e3b99d = 1:  97e20e3b99d completion: squelch stray errors in sparse-checkout completion
 2:  212ba35ed46 = 2:  212ba35ed46 completion: fix logic for determining whether cone mode is active
 3:  1cbbcd9097c = 3:  1cbbcd9097c completion: avoid misleading completions in cone mode
 4:  604f21dc827 < -:  ----------- completion: avoid user confusion in non-cone mode
 -:  ----------- > 4:  89501b366ff completion: avoid user confusion in non-cone mode

-- 
gitgitgadget

^ permalink raw reply

* [PATCH v3 2/4] completion: fix logic for determining whether cone mode is active
From: Elijah Newren via GitGitGadget @ 2023-12-03  5:57 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

_git_sparse_checkout() was checking whether we were in cone mode by
checking whether either:

    A) core.sparseCheckoutCone was "true"
    B) "--cone" was specified on the command line

This code has 2 bugs I didn't catch in my review at the time

    1) core.sparseCheckout must be "true" for core.sparseCheckoutCone to
       be relevant (which matters since "git sparse-checkout disable"
       only unsets core.sparseCheckout, not core.sparseCheckoutCone)
    2) The presence of "--no-cone" should override any config setting

Further, I forgot to update this logic as part of 2d95707a02
("sparse-checkout: make --cone the default", 2022-04-22) for the new
default.

Update the code for the new default and make it be more careful in
determining whether to complete based on cone mode or non-cone mode.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b8661701718..7aa66c19ede 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3097,6 +3097,7 @@ _git_sparse_checkout ()
 {
 	local subcommands="list init set disable add reapply"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	local using_cone=true
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
 		return
@@ -3107,8 +3108,15 @@ _git_sparse_checkout ()
 		__gitcomp_builtin sparse-checkout_$subcommand "" "--"
 		;;
 	set,*|add,*)
-		if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
-		[ -n "$(__git_find_on_cmdline --cone)" ]; then
+		if [[ "$(__git config core.sparseCheckout)" == "true" &&
+		      "$(__git config core.sparseCheckoutCone)" == "false" &&
+		      -z "$(__git_find_on_cmdline --cone)" ]]; then
+			using_cone=false
+		fi
+		if [[ -n "$(__git_find_on_cmdline --no-cone)" ]]; then
+			using_cone=false
+		fi
+		if [[ "$using_cone" == "true" ]]; then
 			__gitcomp_directories
 		fi
 	esac
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 3/4] completion: avoid misleading completions in cone mode
From: Elijah Newren via GitGitGadget @ 2023-12-03  5:57 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

The "set" and "add" subcommands of "sparse-checkout", when in cone mode,
should only complete on directories.  For bash_completion in general,
when no completions are returned for any subcommands, it will often fall
back to standard completion of files and directories as a substitute.
That is not helpful here.  Since we have already looked for all valid
completions, if none are found then falling back to standard bash file
and directory completion is at best actively misleading.  In fact, there
are three different ways it can be actively misleading.  Add a long
comment in the code about how that fallback behavior can deceive, and
disable the fallback by returning a fake result as the sole completion.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7aa66c19ede..c614e5d4f07 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3090,6 +3090,26 @@ __gitcomp_directories ()
 		# No possible further completions any deeper, so assume we're at
 		# a leaf directory and just consider it complete
 		__gitcomp_direct_append "$cur "
+	elif [[ $_found == 0 ]]; then
+		# No possible completions found.  Avoid falling back to
+		# bash's default file and directory completion, because all
+		# valid completions have already been searched and the
+		# fallbacks can do nothing but mislead.  In fact, they can
+		# mislead in three different ways:
+		#    1) Fallback file completion makes no sense when asking
+		#       for directory completions, as this function does.
+		#    2) Fallback directory completion is bad because
+		#       e.g. "/pro" is invalid and should NOT complete to
+		#       "/proc".
+		#    3) Fallback file/directory completion only completes
+		#       on paths that exist in the current working tree,
+		#       i.e. which are *already* part of their
+		#       sparse-checkout.  Thus, normal file and directory
+		#       completion is always useless for "git
+		#       sparse-checkout add" and is also probelmatic for
+		#       "git sparse-checkout set" unless using it to
+		#       strictly narrow the checkout.
+		COMPREPLY=( "" )
 	fi
 }
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v3 4/4] completion: avoid user confusion in non-cone mode
From: Elijah Newren via GitGitGadget @ 2023-12-03  5:57 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, SZEDER Gábor, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1349.v3.git.1701583024.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

It is tempting to think of "files and directories" of the current
directory as valid inputs to the add and set subcommands of git
sparse-checkout.  However, in non-cone mode, they often aren't and using
them as potential completions leads to *many* forms of confusion:

Issue #1. It provides the *wrong* files and directories.

For
    git sparse-checkout add
we always want to add files and directories not currently in our sparse
checkout, which means we want file and directories not currently present
in the current working tree.  Providing the files and directories
currently present is thus always wrong.

For
    git sparse-checkout set
we have a similar problem except in the subset of cases where we are
trying to narrow our checkout to a strict subset of what we already
have.  That is not a very common scenario, especially since it often
does not even happen to be true for the first use of the command; for
years we required users to create a sparse-checkout via
    git sparse-checkout init
    git sparse-checkout set <args...>
(or use a clone option that did the init step for you at clone time).
The init command creates a minimal sparse-checkout with just the
top-level directory present, meaning the set command has to be used to
expand the checkout.  Thus, only in a special and perhaps unusual cases
would any of the suggestions from normal file and directory completion
be appropriate.

Issue #2: Suggesting patterns that lead to warnings is unfriendly.

If the user specifies any regular file and omits the leading '/', then
the sparse-checkout command will warn the user that their command is
problematic and suggest they use a leading slash instead.

Issue #3: Completion gets confused by leading '/', and provides wrong paths.

Users often want to anchor their patterns to the toplevel of the
repository, especially when listing individual files.  There are a
number of reasons for this, but notably even sparse-checkout encourages
them to do so (as noted above).  However, if users do so (via adding a
leading '/' to their pattern), then bash completion will interpret the
leading slash not as a request for a path at the toplevel of the
repository, but as a request for a path at the root of the filesytem.
That means at best that completion cannot help with such paths, and if
it does find any completions, they are almost guaranteed to be wrong.

Issue #4: Suggesting invalid patterns from subdirectories is unfriendly.

There is no per-directory equivalent to .gitignore with
sparse-checkouts.  There is only a single worktree-global
$GIT_DIR/info/sparse-checkout file.  As such, paths to files must be
specified relative to the toplevel of a repository.  Providing
suggestions of paths that are relative to the current working directory,
as bash completion defaults to, is wrong when the current working
directory is not the worktree toplevel directory.

Issue #5: Paths with special characters will be interpreted incorrectly

The entries in the sparse-checkout file are patterns, not paths.  While
most paths also qualify as patterns (though even in such cases it would
be better for users to not use them directly but prefix them with a
leading '/'), there are a variety of special characters that would need
special escaping beyond the normal shell escaping: '*', '?', '\', '[',
']', and any leading '#' or '!'.  If completion suggests any such paths,
users will likely expect them to be treated as an exact path rather than
as a pattern that might match some number of files other than 1.

However, despite the first four issues, we can note that _if_ users are
using tab completion, then they are probably trying to specify a path in
the index.  As such, we transform their argument into a top-level-rooted
pattern that matches such a file.  For example, if they type:
   git sparse-checkout add Make<TAB>
we could "complete" to
   git sparse-checkout add /Makefile
or, if they ran from the Documentation/technical/ subdirectory:
   git sparse-checkout add m<TAB>
we could "complete" it to:
   git sparse-checkout add /Documentation/technical/multi-pack-index.txt
Note in both cases I use "complete" in quotes, because we actually add
characters both before and after the argument in question, so we are
kind of abusing "bash completions" to be "bash completions AND
beginnings".

The fifth issue is a bit stickier, especially when you consider that we
not only need to deal with escaping issues because of special meanings
of patterns in sparse-checkout & gitignore files, but also that we need
to consider escaping issues due to ls-files needing to sometimes quote
or escape characters, and because the shell needs to escape some
characters.  The multiple interacting forms of escaping could get ugly;
this patch makes no attempt to do so and simply documents that we
decided to not deal with those corner cases for now but at least get the
common cases right.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 89 ++++++++++++++++++++++++++
 t/t9902-completion.sh                  |  9 ++-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c614e5d4f07..185b47d802f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3113,6 +3113,93 @@ __gitcomp_directories ()
 	fi
 }
 
+# In non-cone mode, the arguments to {set,add} are supposed to be
+# patterns, relative to the toplevel directory.  These can be any kind
+# of general pattern, like 'subdir/*.c' and we can't complete on all
+# of those.  However, if the user presses Tab to get tab completion, we
+# presume that they are trying to provide a pattern that names a specific
+# path.
+__gitcomp_slash_leading_paths ()
+{
+	local dequoted_word pfx="" cur_ toplevel
+
+	# Since we are dealing with a sparse-checkout, subdirectories may not
+	# exist in the local working copy.  Therefore, we want to run all
+	# ls-files commands relative to the repository toplevel.
+	toplevel="$(git rev-parse --show-toplevel)/"
+
+	__git_dequote "$cur"
+
+	# If the paths provided by the user already start with '/', then
+	# they are considered relative to the toplevel of the repository
+	# already.  If they do not start with /, then we need to adjust
+	# them to start with the appropriate prefix.
+	case "$cur" in
+	/*)
+		cur="${cur:1}"
+		;;
+	*)
+		pfx="$(__git rev-parse --show-prefix)"
+	esac
+
+	# Since sparse-index is limited to cone-mode, in non-cone-mode the
+	# list of valid paths is precisely the cached files in the index.
+	#
+	# NEEDSWORK:
+	#   1) We probably need to take care of cases where ls-files
+	#      responds with special quoting.
+	#   2) We probably need to take care of cases where ${cur} has
+	#      some kind of special quoting.
+	#   3) On top of any quoting from 1 & 2, we have to provide an extra
+	#      level of quoting for any paths that contain a '*', '?', '\',
+	#      '[', ']', or leading '#' or '!' since those will be
+	#      interpreted by sparse-checkout as something other than a
+	#      literal path character.
+	# Since there are two types of quoting here, this might get really
+	# complex.  For now, just punt on all of this...
+	completions="$(__git -C "${toplevel}" -c core.quotePath=false \
+			 ls-files --cached -- "${pfx}${cur}*" \
+			 | sed -e s%^%/% -e 's%$% %')"
+	# Note, above, though that we needed all of the completions to be
+	# prefixed with a '/', and we want to add a space so that bash
+	# completion will actually complete an entry and let us move on to
+	# the next one.
+
+	# Return what we've found.
+	if test -n "$completions"; then
+		# We found some completions; return them
+		local IFS=$'\n'
+		COMPREPLY=($completions)
+	else
+		# Do NOT fall back to bash-style all-local-files-and-dirs
+		# when we find no match.  Such options are worse than
+		# useless:
+		#     1. "git sparse-checkout add" needs paths that are NOT
+		#        currently in the working copy.  "git
+		#        sparse-checkout set" does as well, except in the
+		#        special cases when users are only trying to narrow
+		#        their sparse checkout to a subset of what they
+		#        already have.
+		#
+		#     2. A path like '.config' is ambiguous as to whether
+		#        the user wants all '.config' files throughout the
+		#        tree, or just the one under the current directory.
+		#        It would result in a warning from the
+		#        sparse-checkout command due to this.  As such, all
+		#        completions of paths should be prefixed with a
+		#        '/'.
+		#
+		#     3. We don't want paths prefixed with a '/' to
+		#        complete files in the system root directory, we
+		#        want it to complete on files relative to the
+		#        repository root.
+		#
+		# As such, make sure that NO completions are offered rather
+		# than falling back to bash's default completions.
+		COMPREPLY=( "" )
+	fi
+}
+
 _git_sparse_checkout ()
 {
 	local subcommands="list init set disable add reapply"
@@ -3138,6 +3225,8 @@ _git_sparse_checkout ()
 		fi
 		if [[ "$using_cone" == "true" ]]; then
 			__gitcomp_directories
+		else
+			 __gitcomp_slash_leading_paths
 		fi
 	esac
 }
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a7c3b4eb63a..bbd17f296e4 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1571,7 +1571,7 @@ test_expect_success FUNNYNAMES,!CYGWIN 'cone mode sparse-checkout completes dire
 	)
 '
 
-test_expect_success 'non-cone mode sparse-checkout uses bash completion' '
+test_expect_success 'non-cone mode sparse-checkout gives rooted paths' '
 	# reset sparse-checkout repo to non-cone mode
 	git -C sparse-checkout sparse-checkout disable &&
 	git -C sparse-checkout sparse-checkout set --no-cone &&
@@ -1581,7 +1581,12 @@ test_expect_success 'non-cone mode sparse-checkout uses bash completion' '
 		# expected to be empty since we have not configured
 		# custom completion for non-cone mode
 		test_completion "git sparse-checkout set f" <<-\EOF
-
+		/folder1/0/1/t.txt 
+		/folder1/expected 
+		/folder1/out 
+		/folder1/out_sorted 
+		/folder2/0/t.txt 
+		/folder3/t.txt 
 		EOF
 	)
 '
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode
From: Elijah Newren @ 2023-12-03  5:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <xmqq34ws3rfa.fsf@gitster.g>

On Sun, Nov 26, 2023 at 5:39 PM Junio C Hamano <gitster@pobox.com> wrote:
[...]
> >> Hmph, would an obvious alternative to (1) check against the HEAD (or
> >> the index) to see if the prefix string matches an entity at the
> >> current directory level, and then (2) to prefix the result of the
> >> previous step with "/$(git rev-parse --show-prefix)" work?  That is
> >> something like this:
> >>
> >>     $ cd t
> >>     $ git sparse-checkout add help<TAB>
> >>     ->
> >>     $ git sparse-checkout add /t/helper/
> >
> > I thought bash-completion was only for completions, not for startings
> > as well.  Was I mistaken?
>
> To my mind, the completion is what I as an end user get when I type
> <TAB> to help me formulate input that is acceptable by the command.
> As I said, I consider it a bug (or UI mistake) in the a command if
> it pretends to work inside a subdirecctory but complains when it is
> given a path relative to the current directory, so I'd rather prefer
> the approach to "fix" the underlying command, but if that is too
> much work or cannot be done for whatever reason, the second best
> would be to turn whatever we can do to help the end-user input into
> a form that is accepted by the command without changing what the
> input means.  If it takes more than "appending at the end", that is
> fine, at least by me as an end user.
>
> If you are saying "completion code can only append at the end
> because we can only return strings to be appended, not the entire
> strings, to the readline machinery, so mucking with the start of the
> string is not doable", then sorry---I accept that what we cannot do
> cannot be done, and in that case you are "not mistaken".

This was what I thought; that bash completion didn't support this.

> But from the existing use of COMPREPLY[], it didn't look that way
> (it seems __gitcomp is equipped to take fixed prefix to all
> candidates by passing it in $2 and used to complete names of
> configuration variables in a section, but it seems to me that it can
> be repurposed when prefixing "$(git rev-parse --show-prefix)" to a
> given pathname relative to the $cwd).

Ooh, that's really interesting; I had no idea it had this kind of
flexibility.  It does feel like we're abusing "bash completions" to be
both "bash completions AND startings", but I agree that this is a case
where it makes sense to do so.

I changed patch 4 to implement this for non-cone mode, and submitted
v3 with that change.

^ permalink raw reply

* [PATCH 01/12] treewide: remove unnecessary includes from header files
From: Elijah Newren via GitGitGadget @ 2023-12-03  6:41 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.git.1701585682.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

There are three kinds of unnecessary includes:
  * includes which aren't directly needed, but which include some other
    forgotten include
  * includes which could be replaced by a simple forward declaration of
    some structs
  * includes which aren't needed at all

Remove the third kind of include.  Subsequent commits (and a subsequent
series) will work on removing some of the other kinds of includes.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 fetch-pack.h       | 1 -
 midx.h             | 1 -
 ref-filter.h       | 1 -
 submodule-config.h | 1 -
 4 files changed, 4 deletions(-)

diff --git a/fetch-pack.h b/fetch-pack.h
index 8c7752fc821..6775d265175 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -2,7 +2,6 @@
 #define FETCH_PACK_H
 
 #include "string-list.h"
-#include "run-command.h"
 #include "protocol.h"
 #include "list-objects-filter-options.h"
 #include "oidset.h"
diff --git a/midx.h b/midx.h
index a5d98919c85..eb57a37519c 100644
--- a/midx.h
+++ b/midx.h
@@ -1,7 +1,6 @@
 #ifndef MIDX_H
 #define MIDX_H
 
-#include "repository.h"
 #include "string-list.h"
 
 struct object_id;
diff --git a/ref-filter.h b/ref-filter.h
index 1524bc463a5..4ecb6ab1c60 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -3,7 +3,6 @@
 
 #include "gettext.h"
 #include "oid-array.h"
-#include "refs.h"
 #include "commit.h"
 #include "string-list.h"
 #include "strvec.h"
diff --git a/submodule-config.h b/submodule-config.h
index 2a37689cc27..e8164cca3e4 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -2,7 +2,6 @@
 #define SUBMODULE_CONFIG_CACHE_H
 
 #include "config.h"
-#include "hashmap.h"
 #include "submodule.h"
 #include "strbuf.h"
 #include "tree-walk.h"
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 00/12] Additional header cleanups (removing unnecessary includes)
From: Elijah Newren via GitGitGadget @ 2023-12-03  6:41 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren

Several months ago, I sent in several series cleaning up headers, ultimately
removing cache.h. I had two other series ready to share, but...life
happened.

I've rebased and cleaned up these last two series, focusing on just reducing
unnecessary includes. Here's the first of the two.

Elijah Newren (12):
  treewide: remove unnecessary includes from header files
  treewide: remove unnecessary includes in source files
  archive.h: remove unnecessary include
  blame.h: remove unnecessary includes
  fsmonitor--daemon.h: remove unnecessary includes
  http.h: remove unnecessary include
  line-log.h: remove unnecessary include
  pkt-line.h: remove unnecessary include
  submodule-config.h: remove unnecessary include
  trace2/tr2_tls.h: remove unnecessary include
  treewide: add direct includes currently only pulled in transitively
  treewide: remove unnecessary includes in source files

 add-patch.c                          | 1 -
 apply.c                              | 1 -
 archive-tar.c                        | 1 +
 archive-zip.c                        | 1 +
 archive.c                            | 2 +-
 archive.h                            | 1 -
 attr.c                               | 1 -
 bisect.c                             | 1 -
 blame.c                              | 2 ++
 blame.h                              | 3 ---
 blob.c                               | 1 -
 bloom.c                              | 1 -
 builtin/add.c                        | 3 ---
 builtin/am.c                         | 4 ----
 builtin/apply.c                      | 1 -
 builtin/archive.c                    | 1 -
 builtin/bisect.c                     | 1 -
 builtin/blame.c                      | 1 -
 builtin/branch.c                     | 3 ---
 builtin/cat-file.c                   | 1 -
 builtin/checkout-index.c             | 1 -
 builtin/checkout.c                   | 3 ---
 builtin/clone.c                      | 1 -
 builtin/commit-graph.c               | 3 +--
 builtin/commit-tree.c                | 3 ---
 builtin/commit.c                     | 8 --------
 builtin/credential-cache.c           | 2 --
 builtin/describe.c                   | 2 --
 builtin/diff-files.c                 | 1 -
 builtin/diff-index.c                 | 2 --
 builtin/diff-tree.c                  | 1 -
 builtin/diff.c                       | 2 --
 builtin/difftool.c                   | 1 -
 builtin/fast-export.c                | 1 -
 builtin/fetch.c                      | 2 --
 builtin/for-each-ref.c               | 3 +--
 builtin/fsck.c                       | 3 ---
 builtin/fsmonitor--daemon.c          | 5 +++--
 builtin/get-tar-commit-id.c          | 1 -
 builtin/grep.c                       | 4 ----
 builtin/hash-object.c                | 1 -
 builtin/hook.c                       | 1 -
 builtin/index-pack.c                 | 2 --
 builtin/init-db.c                    | 1 -
 builtin/log.c                        | 2 --
 builtin/ls-files.c                   | 4 ----
 builtin/ls-remote.c                  | 1 -
 builtin/ls-tree.c                    | 2 --
 builtin/mailinfo.c                   | 1 -
 builtin/merge-base.c                 | 3 ---
 builtin/merge-recursive.c            | 3 ---
 builtin/merge-tree.c                 | 1 -
 builtin/merge.c                      | 4 ----
 builtin/mktag.c                      | 1 -
 builtin/mv.c                         | 1 -
 builtin/notes.c                      | 2 --
 builtin/pack-objects.c               | 3 ---
 builtin/pull.c                       | 5 -----
 builtin/push.c                       | 1 -
 builtin/range-diff.c                 | 1 -
 builtin/read-tree.c                  | 2 --
 builtin/rebase.c                     | 4 ----
 builtin/receive-pack.c               | 1 -
 builtin/repack.c                     | 1 -
 builtin/rerere.c                     | 1 -
 builtin/reset.c                      | 3 ---
 builtin/rev-list.c                   | 2 --
 builtin/revert.c                     | 2 --
 builtin/rm.c                         | 1 -
 builtin/send-pack.c                  | 5 -----
 builtin/show-ref.c                   | 1 -
 builtin/sparse-checkout.c            | 4 ----
 builtin/stash.c                      | 1 -
 builtin/submodule--helper.c          | 1 -
 builtin/tag.c                        | 1 -
 builtin/unpack-objects.c             | 4 ----
 builtin/update-ref.c                 | 1 -
 builtin/verify-commit.c              | 2 --
 builtin/verify-tag.c                 | 1 -
 bulk-checkin.c                       | 1 -
 bundle-uri.c                         | 1 -
 cache-tree.c                         | 1 -
 combine-diff.c                       | 1 -
 commit-graph.c                       | 3 +--
 commit-reach.c                       | 1 -
 commit.c                             | 2 --
 compat/fsmonitor/fsm-health-win32.c  | 1 +
 compat/fsmonitor/fsm-listen-darwin.c | 1 +
 compat/fsmonitor/fsm-listen-win32.c  | 1 +
 compat/simple-ipc/ipc-shared.c       | 3 ---
 compat/simple-ipc/ipc-unix-socket.c  | 1 -
 config.c                             | 3 ---
 delta-islands.c                      | 5 -----
 diff-lib.c                           | 1 -
 diff-no-index.c                      | 3 ---
 diff.c                               | 2 --
 diffcore-break.c                     | 1 -
 diffcore-delta.c                     | 1 -
 dir.c                                | 1 -
 entry.c                              | 1 -
 exec-cmd.c                           | 1 -
 fetch-pack.c                         | 2 --
 fetch-pack.h                         | 1 -
 fsck.c                               | 1 -
 fsmonitor--daemon.h                  | 4 +---
 fsmonitor-ipc.c                      | 1 -
 gettext.c                            | 2 --
 gpg-interface.c                      | 1 -
 grep.c                               | 1 -
 http-fetch.c                         | 2 +-
 http-push.c                          | 3 +--
 http-walker.c                        | 1 -
 http.c                               | 2 --
 http.h                               | 1 -
 imap-send.c                          | 2 --
 line-log.c                           | 4 +---
 line-log.h                           | 2 --
 line-range.c                         | 1 -
 list-objects-filter-options.c        | 5 -----
 list-objects-filter.c                | 5 -----
 log-tree.c                           | 1 +
 ls-refs.c                            | 1 -
 merge-blobs.c                        | 2 --
 merge-ort.c                          | 3 ---
 merge-recursive.c                    | 5 -----
 merge.c                              | 3 ---
 midx.h                               | 1 -
 negotiator/noop.c                    | 1 -
 notes-utils.c                        | 1 -
 notes.c                              | 2 --
 object-file.c                        | 8 --------
 object-name.c                        | 2 --
 pack-bitmap-write.c                  | 3 ---
 pack-check.c                         | 1 -
 pack-write.c                         | 1 -
 packfile.c                           | 1 -
 parse-options.c                      | 2 --
 patch-ids.c                          | 1 -
 pkt-line.c                           | 1 +
 pkt-line.h                           | 1 -
 protocol-caps.c                      | 1 -
 reachable.c                          | 1 -
 read-cache.c                         | 2 --
 ref-filter.c                         | 3 ---
 ref-filter.h                         | 1 -
 reflog.c                             | 1 -
 refs/files-backend.c                 | 2 --
 refs/packed-backend.c                | 1 -
 refs/ref-cache.c                     | 1 -
 reftable/dump.c                      | 2 --
 reftable/generic.c                   | 1 -
 reftable/merged.c                    | 1 -
 reftable/merged_test.c               | 1 -
 reftable/reader.c                    | 1 -
 reftable/readwrite_test.c            | 1 -
 reftable/refname_test.c              | 1 -
 reftable/stack_test.c                | 1 -
 reftable/test_framework.c            | 1 -
 reftable/tree_test.c                 | 2 --
 remote-curl.c                        | 3 +--
 remote.c                             | 1 -
 repo-settings.c                      | 1 -
 rerere.c                             | 2 --
 reset.c                              | 1 -
 revision.c                           | 2 --
 run-command.c                        | 2 --
 send-pack.c                          | 2 --
 sequencer.c                          | 3 ---
 setup.c                              | 1 -
 shallow.c                            | 1 -
 shell.c                              | 1 -
 submodule-config.h                   | 2 --
 submodule.c                          | 3 ---
 t/helper/test-bundle-uri.c           | 2 --
 t/helper/test-fast-rebase.c          | 1 -
 t/helper/test-pkt-line.c             | 1 +
 t/helper/test-reach.c                | 2 --
 t/helper/test-repository.c           | 2 --
 t/helper/test-simple-ipc.c           | 1 -
 t/helper/test-submodule.c            | 1 +
 t/helper/test-trace2.c               | 1 -
 tmp-objdir.c                         | 1 -
 trace2.c                             | 4 ----
 trace2/tr2_ctr.c                     | 1 -
 trace2/tr2_tgt_normal.c              | 1 +
 trace2/tr2_tls.c                     | 1 +
 trace2/tr2_tls.h                     | 1 -
 trace2/tr2_tmr.c                     | 1 -
 transport-helper.c                   | 2 --
 transport.c                          | 3 ---
 tree.c                               | 3 ---
 upload-pack.c                        | 6 ------
 wrapper.c                            | 1 -
 xdiff-interface.c                    | 2 --
 194 files changed, 25 insertions(+), 339 deletions(-)


base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1617%2Fnewren%2Fheader-cleanup-6-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1617/newren/header-cleanup-6-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1617
-- 
gitgitgadget

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox