| Message ID | cover.1724923648.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | environment: guard reliance on `the_repository` | expand |
There may be more, but I know at least of these at the moment from
https://github.com/git/git/actions/runs/10619536685/job/29437358521
Perhaps this can become [0.5/21] of the series, before globals are
hidden behind the macro.
--- >8 ---
Subject: [PATCH] win32: mark the sources that depend on the_repository
These source files still need access to the global variables like
"ignore_case" and "protect_ntfs".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
compat/mingw.c | 2 ++
compat/win32/path-utils.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 29d3f09768..5c2080c04c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
#include "../git-compat-util.h"
#include "win32.h"
#include <aclapi.h>
diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c
index b658ca3f81..966ef779b9 100644
--- a/compat/win32/path-utils.c
+++ b/compat/win32/path-utils.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
#include "../../git-compat-util.h"
#include "../../environment.h"
On Thu, Aug 29, 2024 at 12:59:35PM -0700, Junio C Hamano wrote: > There may be more, but I know at least of these at the moment from > https://github.com/git/git/actions/runs/10619536685/job/29437358521 > > Perhaps this can become [0.5/21] of the series, before globals are > hidden behind the macro. Thanks, I'll add these. I really need to spend some time to finally get Win32 builds set up in GitLab CI. Patrick > --- >8 --- > Subject: [PATCH] win32: mark the sources that depend on the_repository > > These source files still need access to the global variables like > "ignore_case" and "protect_ntfs". > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > compat/mingw.c | 2 ++ > compat/win32/path-utils.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/compat/mingw.c b/compat/mingw.c > index 29d3f09768..5c2080c04c 100644 > --- a/compat/mingw.c > +++ b/compat/mingw.c > @@ -1,3 +1,5 @@ > +#define USE_THE_REPOSITORY_VARIABLE > + > #include "../git-compat-util.h" > #include "win32.h" > #include <aclapi.h> > diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c > index b658ca3f81..966ef779b9 100644 > --- a/compat/win32/path-utils.c > +++ b/compat/win32/path-utils.c > @@ -1,3 +1,5 @@ > +#define USE_THE_REPOSITORY_VARIABLE > + > #include "../../git-compat-util.h" > #include "../../environment.h" > > -- > 2.46.0-567-gbce23f156d >
Patrick Steinhardt <ps@pks.im> writes: > On Thu, Aug 29, 2024 at 12:59:35PM -0700, Junio C Hamano wrote: >> There may be more, but I know at least of these at the moment from >> https://github.com/git/git/actions/runs/10619536685/job/29437358521 >> >> Perhaps this can become [0.5/21] of the series, before globals are >> hidden behind the macro. > > Thanks, I'll add these. I really need to spend some time to finally get > Win32 builds set up in GitLab CI. Windows builds we can get from GitHub Actions already. It would be nice if somebody had Cygwin, which might have avoided a need for <d8c5e920-aff7-4e4b-af77-0d3193466b57@ramsayjones.plus.com> Thanks.
On Fri, Aug 30, 2024 at 09:32:35AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > On Thu, Aug 29, 2024 at 12:59:35PM -0700, Junio C Hamano wrote: > >> There may be more, but I know at least of these at the moment from > >> https://github.com/git/git/actions/runs/10619536685/job/29437358521 > >> > >> Perhaps this can become [0.5/21] of the series, before globals are > >> hidden behind the macro. > > > > Thanks, I'll add these. I really need to spend some time to finally get > > Win32 builds set up in GitLab CI. > > Windows builds we can get from GitHub Actions already. It would be > nice if somebody had Cygwin, which might have avoided a need for > <d8c5e920-aff7-4e4b-af77-0d3193466b57@ramsayjones.plus.com> We already have it, yeah. But as I typically only use GitLab CI, I tend to hit the failures on Windows systems when the series is already out there, which requires a roundtrip that would have otherwise been avoided. In any case, having Cygwin added to the mix in addition certainly would be nice. Patrick
Hi, this patch series is the next part in the epic to get rid of `the_repository`. This time around I focussed on the environment subsystem, which relies on `the_repository` both implicitly and explicitly. The series is structured as follows: - Patches 1 to 6 moves functions that belong to the repository subsystem. - Patches 7 and 8 adapt the config subsystem to stop depending on the `the_repository` again, which had to be added by the preceding patches. - Patches 9 to 11 move various functionality that is misplaced and that depends on `the_repository`. - Patches 12 and 13 guard functionality that relies on global state in the environment subsystem via `USE_THE_REPOSITORY_VARIBABLE`. - Patches 14 to 21 demonstrate how to get rid of global config variables part of the environment subsystem for a subset of them. Overall, I think that our reliance on global variables is broken because those variables may or may not point to the actual config of a specific repository we currently hold. They may not even hold state of `the_repository`, but might've been changed by subsequent logic where we parse another repository's config. I'm a bit unhappy that I wasn't able to adapt `is_bare_repository()` to take a `struct repository` yet, and as a consequence a bunch of code needs to declare `USE_THE_REPOSITORY_VARIBABLE` again. But it depends on another global variable `is_bare_repository_cfg` which is being modified quite heavily in multiple subsystems, and the resulting semantics are... confusing. I guess we'll need another biggish patch series to clean up this mess by making the setup subsystem not rely on such global state anymore. The series is built on top of 17d4b10aea (The tenth batch, 2024-08-28). It merges with `next` rather cleanish, except for a single merge conflict with headers. Thanks! Patrick Patrick Steinhardt (21): environment: make `get_git_dir()` accept a repository environment: make `get_git_common_dir()` accept a repository environment: make `get_object_directory()` accept a repository environment: make `get_index_file()` accept a repository environment: make `get_graft_file()` accept a repository environment: make `get_git_work_tree()` accept a repository config: document `read_early_config()` and `read_very_early_config()` config: make dependency on repo in `read_early_config()` explicit environment: move `odb_mkstemp()` into object layer environment: make `get_git_namespace()` self-contained environment: move `set_git_dir()` and related into setup layer environment: reorder header to split out `the_repository`-free section environment: guard state depending on a repository repo-settings: split out declarations into a standalone header branch: stop modifying `log_all_ref_updates` variable refs: stop modifying global `log_all_ref_updates` variable repo-settings: track defaults close to `struct repo_settings` environment: stop storing "core.logAllRefUpdates" globally environment: stop storing "core.preferSymlinkRefs" globally environment: stop storing "core.warnAmbiguousRefs" globally environment: stop storing "core.notesRef" globally alias.c | 6 +- apply.c | 2 +- branch.c | 5 +- builtin/am.c | 13 +- builtin/blame.c | 2 +- builtin/checkout.c | 4 +- builtin/commit-graph.c | 4 +- builtin/commit.c | 12 +- builtin/config.c | 4 +- builtin/count-objects.c | 2 +- builtin/difftool.c | 8 +- builtin/fsmonitor--daemon.c | 6 +- builtin/gc.c | 2 +- builtin/init-db.c | 4 +- builtin/merge.c | 17 +-- builtin/multi-pack-index.c | 2 +- builtin/notes.c | 22 ++-- builtin/pack-objects.c | 2 +- builtin/prune.c | 8 +- builtin/repack.c | 7 +- builtin/replace.c | 2 +- builtin/reset.c | 4 +- builtin/rev-parse.c | 9 +- builtin/stash.c | 16 +-- builtin/submodule--helper.c | 2 +- builtin/update-index.c | 4 +- builtin/worktree.c | 4 +- builtin/write-tree.c | 3 +- bulk-checkin.c | 4 +- bundle-uri.c | 1 + cache-tree.c | 3 +- commit.c | 4 +- config.c | 42 +------ config.h | 13 +- dir.c | 2 +- environment.c | 237 +++++------------------------------- environment.h | 142 ++++++++++----------- fetch-pack.c | 2 +- fsmonitor.c | 2 +- help.c | 2 +- http-backend.c | 2 +- name-hash.c | 3 + notes.c | 21 ++-- notes.h | 3 +- object-file.c | 37 +++++- object-name.c | 4 +- object-store-ll.h | 15 +++ pack-write.c | 2 +- packfile.c | 2 +- pager.c | 7 +- path.c | 2 + pathspec.c | 4 +- preload-index.c | 3 + prompt.c | 2 + prune-packed.c | 4 +- read-cache.c | 5 +- ref-filter.c | 2 +- refs.c | 7 +- refs.h | 4 +- refs/files-backend.c | 31 +++-- refs/reftable-backend.c | 22 ++-- repo-settings.c | 35 +++++- repo-settings.h | 75 ++++++++++++ repository.c | 40 ++++++ repository.h | 58 ++------- server-info.c | 3 +- setup.c | 152 +++++++++++++++++++---- setup.h | 5 +- sparse-index.c | 2 + statinfo.c | 2 + submodule.c | 2 +- t/helper/test-config.c | 3 +- t/helper/test-path-utils.c | 2 + tmp-objdir.c | 7 +- trace.c | 8 +- trace2/tr2_cfg.c | 4 +- transport-helper.c | 2 +- tree-diff.c | 3 + userdiff.c | 2 + worktree.c | 12 +- wt-status.c | 2 +- 81 files changed, 677 insertions(+), 554 deletions(-) create mode 100644 repo-settings.h