From: Junio C Hamano <gitster@pobox•com>
To: Ayush Chandekar <ayu.chandekar@gmail•com>
Cc: christian.couder@gmail•com, git@vger•kernel.org,
shyamthakkar001@gmail•com, phillip.wood123@gmail•com,
ps@pks•im, ben.knoble@gmail•com
Subject: Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables
Date: Wed, 23 Jul 2025 15:14:07 -0700 [thread overview]
Message-ID: <xmqqcy9qlfm8.fsf@gitster.g> (raw)
In-Reply-To: <cover.1752882401.git.ayu.chandekar@gmail.com> (Ayush Chandekar's message of "Sat, 19 Jul 2025 05:41:25 +0530")
Ayush Chandekar <ayu.chandekar@gmail•com> writes:
> This patch series aims to remove global variables related to
> sparse-checkout from the global scope and to remove the definition
> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.
>
> It contains three patches:
>
> 1 - Remove the global variable 'core_apply_sparse_checkout' and
> move its setting to the 'struct repo_settings'. Also remove the
> definition '#define USE_THE_REPOSITORY_VARIABLE' from
> "builtin/backfill.c".
>
> 2 - Remove the global variable 'core_sparse_checkout_cone' and
> move its setting to the 'struct repo_settings'.
>
> 3 - Remove the global variable
> 'sparse_expect_files_outside_of_patterns` and move its setting to
> 'struct repo_settings'. Also remove the definition '#define
> USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"
> --
> Discussions since v5:
>
> * For 1/3 and 2/3, Junio told me that it was concerning to put so
> many calls to `prepare_repo_settings()` so I tried to minimize the
> calls and made sure that there's no useless calling.
I didn't mean that the number of places is the problem. What I
found troubling was that this is not done in any central place, so
it is hard to notice even if some random cmd_foo() failed to call
the function before doing its real work. For example, shouldn't we
be able to, at least for built-in commands that have RUN_SETUP bit
set, centrally call prepare_repo_settings() somewhere late in
git.c:run_builtin() after we figure out what should be in
the_repository? Now historically, setting up a repository may never
have involved opening and parsing tons of configuration files, so
such a change may be incurring extra overhead we did not have to
pay, so it needs a lot more thought than just trying to minimize the
number of calls, but some performance measurement.
> * For 3/3, Phillip told me that it broke user-facing as it will be
> parsed quite late in the callchain and might throw an error mid
> operation which we do not want.
So has the behaviour change caused by 3/3 been resolved?
A meta-level comment and a half.
* Please do not use "-- " (that is a line that has dash dash and a
single space and nothing else on it) lightly. It is called
signature line and often MUA pays attention to it when responding
to a message with such a line by omitting everything after it
(which is supposed to be your "who I am" advertisement) when
quoting the original. Since you had one before the "discussions
since v5" section and the range-diff, I had to manually resurrect
the part after the signature line while composing this message.
* This throws everything in repo_settings, but these settings are
inherently per repository and they are meaningful only when you
are working with a repository. What makes us choose to make them
new members in the repo_settings structure, not direct members in
the repository structure?
Not an objection and not a suggestion to move them out of the
repo_settings and to the repository proper. Just wanted to hear
the reasoning behind it (and have the rationale clearly
documented, preferrably in the proposed log messages).
next prev parent reply other threads:[~2025-07-23 22:14 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-03 13:18 [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-03 13:41 ` Patrick Steinhardt
2025-06-03 16:20 ` Ayush Chandekar
2025-06-04 2:20 ` Ben Knoble
2025-06-04 7:28 ` Patrick Steinhardt
2025-06-04 23:48 ` Ayush Chandekar
2025-06-08 0:31 ` [GSOC PATCH v2] " Ayush Chandekar
2025-06-08 6:39 ` Christian Couder
2025-06-11 17:34 ` [PATCH v3] " Ayush Chandekar
2025-06-11 21:06 ` Junio C Hamano
2025-06-11 21:33 ` Junio C Hamano
2025-06-13 6:57 ` Ayush Chandekar
2025-06-17 12:06 ` [GSOC PATCH v4 0/3] environment: remove sparse-checkout related global variables Ayush Chandekar
2025-06-17 12:06 ` [GSOC PATCH v4 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-17 16:15 ` Junio C Hamano
2025-06-17 12:06 ` [GSOC PATCH v4 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-06-17 16:29 ` Junio C Hamano
2025-06-17 12:06 ` [GSOC PATCH v4 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-06-17 16:30 ` Junio C Hamano
2025-06-30 19:27 ` [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables Ayush Chandekar
2025-06-30 19:27 ` [GSOC PATCH v5 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-30 19:27 ` [GSOC PATCH v5 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-06-30 19:27 ` [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-07-01 13:18 ` Phillip Wood
2025-07-01 23:53 ` Ayush Chandekar
2025-07-02 9:01 ` Phillip Wood
2025-07-11 19:24 ` Ayush Chandekar
2025-07-02 9:21 ` Junio C Hamano
2025-07-11 19:35 ` Ayush Chandekar
2025-06-30 21:08 ` [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables Junio C Hamano
2025-07-09 0:18 ` Junio C Hamano
2025-07-09 1:39 ` Ayush Chandekar
2025-07-19 0:11 ` [GSOC PATCH v6 " Ayush Chandekar
2025-07-19 0:11 ` [GSOC PATCH v6 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-07-19 0:11 ` [GSOC PATCH v6 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-07-19 0:11 ` [GSOC PATCH v6 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-07-23 22:14 ` Junio C Hamano [this message]
2025-07-24 13:25 ` [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables Derrick Stolee
2025-07-24 18:44 ` Junio C Hamano
2025-07-29 11:36 ` Ayush Chandekar
2025-07-29 12:19 ` Derrick Stolee
2025-07-29 12:53 ` Ayush Chandekar
2025-07-30 8:53 ` Phillip Wood
2025-07-30 15:52 ` Junio C Hamano
2025-07-26 23:55 ` Ayush Chandekar
2025-08-10 15:36 ` Ayush Chandekar
2025-08-26 12:20 ` Derrick Stolee
2025-08-27 21:31 ` Ayush Chandekar
2025-09-05 14:15 ` Junio C Hamano
2025-09-05 17:10 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcy9qlfm8.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=ayu.chandekar@gmail$(echo .)com \
--cc=ben.knoble@gmail$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=phillip.wood123@gmail$(echo .)com \
--cc=ps@pks$(echo .)im \
--cc=shyamthakkar001@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox