git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

From
Patrick Steinhardt <ps@pks.im>
Date
Jun 4, 2025, 07:28 UTC
Message-ID
<aD_1tD-H74SOh1Xx@pks.im>
In-Reply-To
<4F074544-4E25-472D-A42C-C50A1E1CAC69@gmail.com>
On Tue, Jun 03, 2025 at 10:20:38PM -0400, Ben Knoble wrote:
Show 33 quoted lines
> > Le 3 juin 2025 à 12:21, Ayush Chandekar <ayu.chandekar@gmail.com> a écrit :
> >>> +{
> >>> +     return repo->settings.core_apply_sparse_checkout;
> >>> +}
> >>> +
> >>> +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> >>> +{
> >>> +     repo->settings.core_apply_sparse_checkout = value;
> >>> +}
> >> Getters and setters only really help in the case where they actually
> >> provide a benefit. These don't though, so it's dubious whether we should
> >> have them.
> 
> My thoughts exactly; see below.
> 
> >> Also, shouldn't these functions call `prepare_repo_settings()`?
> >> Otherwise we cannot guarantee that those settings have already been
> >> parsed at all. And for the setter it could happen that the settings get
> >> overwritten by the next caller of `prepare_repo_settings()`.
> > 
> > Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in
> > them, wouldn't
> > it be better to use getter and setter functions? Otherwise, I'd have to call
> > `prepare_repo_settings()` everywhere I'm using the setting.
> 
> Aren’t most of the consumers builtins? And from a recent look, don’t
> they (all?) initialize the repo settings? I agree it is relatively
> painful to require developers to make sure that prepare_repo_settings
> has been called on each (new) code path that reads this variable, but
> OTOH I would expect that to be a straightforward audit during this
> change and then (see following) relatively easy to catch going
> forward. Is already a code convention that reading things in
> repo->settings depends on having prepared them?
Yes, it is a code convention. We have two patterns though:
  - Those that access the repo settings fields directly _always_ call
    `prepare_repo_settings()` manually beforehand.
  - Those that use a getter/setter rely on those to call
    `prepare_repo_settings()`.

So if you add the call to `prepare_repo_settings()` the getter and setter do provide additional value. So in that case it may be sensible to retain them indeed.

Patrick
Previous: Ben KnobleNext: Ayush Chandekar
Message 5 of 50 in “environment: move access to "core.sparsecheckout" into repo_settings”
  1. environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jun 3, 2025
  2. Patrick SteinhardtJun 3, 2025
  3. Ayush ChandekarJun 3, 2025
  4. Ben KnobleJun 4, 2025
  5. Patrick SteinhardtJun 4, 2025
  6. Ayush ChandekarJun 4, 2025
  7. environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jun 8, 2025
  8. Christian CouderJun 8, 2025
  9. environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jun 11, 2025
  10. Junio C HamanoJun 11, 2025
  11. Junio C HamanoJun 11, 2025
  12. Ayush ChandekarJun 13, 2025
  13. 0/3 environment: remove sparse-checkout related global variablesAyush Chandekar, Jun 17, 2025
  14. 1/3 environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jun 17, 2025
  15. Junio C HamanoJun 17, 2025
  16. 2/3 environment: move access to "core.sparsecheckoutcone" into repo_settingsAyush Chandekar, Jun 17, 2025
  17. Junio C HamanoJun 17, 2025
  18. 3/3 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'Ayush Chandekar, Jun 17, 2025
  19. Junio C HamanoJun 17, 2025
  20. 0/3 environment: remove sparse-checkout related global variablesAyush Chandekar, Jun 30, 2025
  21. 1/3 environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jun 30, 2025
  22. 2/3 environment: move access to "core.sparsecheckoutcone" into repo_settingsAyush Chandekar, Jun 30, 2025
  23. 3/3 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'Ayush Chandekar, Jun 30, 2025
  24. Phillip WoodJul 1, 2025
  25. Ayush ChandekarJul 1, 2025
  26. Phillip WoodJul 2, 2025
  27. Ayush ChandekarJul 11, 2025
  28. Junio C HamanoJul 2, 2025
  29. Ayush ChandekarJul 11, 2025
  30. Junio C HamanoJun 30, 2025
  31. Junio C HamanoJul 9, 2025
  32. Ayush ChandekarJul 9, 2025
  33. 0/3 environment: remove sparse-checkout related global variablesAyush Chandekar, Jul 19, 2025
  34. 1/3 environment: move access to "core.sparsecheckout" into repo_settingsAyush Chandekar, Jul 19, 2025
  35. 2/3 environment: move access to "core.sparsecheckoutcone" into repo_settingsAyush Chandekar, Jul 19, 2025
  36. 3/3 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'Ayush Chandekar, Jul 19, 2025
  37. Junio C HamanoJul 23, 2025
  38. Derrick StoleeJul 24, 2025
  39. Junio C HamanoJul 24, 2025
  40. Ayush ChandekarJul 29, 2025
  41. Derrick StoleeJul 29, 2025
  42. Ayush ChandekarJul 29, 2025
  43. Phillip WoodJul 30, 2025
  44. Junio C HamanoJul 30, 2025
  45. Ayush ChandekarJul 26, 2025
  46. Ayush ChandekarAug 10, 2025
  47. Derrick StoleeAug 26, 2025
  48. Ayush ChandekarAug 27, 2025
  49. Junio C HamanoSep 5, 2025
  50. Junio C HamanoSep 5, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.