On 8/13/24 3:17 AM, Patrick Steinhardt wrote:
The git-gc(1) command knows to read a bunch of config keys to tweak its own behaviour. The values are parsed into global variables, which makes it hard to correctly manage the lifecycle of values that may require a memory allocation. Refactor the code to use a `struct gc_config` that gets populated and passed around. For one, this makes previously-implicit dependencies on these config values clear. Second, it will allow us to properly manage the lifecycle in the next commit.
I think this is a valuable goal.
-static const char *gc_log_expire = "1.day.ago"; -static const char *prune_expire = "2.weeks.ago"; -static const char *prune_worktrees_expire = "3.months.ago";
I was going to mention this in the previous patch where you change how these variables are cast into git_config_get_expiry(). They aren't changing to non-const here, either.
+struct gc_config {
...
+ const char *gc_log_expire; + const char *prune_expire; + const char *prune_worktrees_expire;
The fact that they are initialized to const strings makes it difficult to know if they've been updated. I wonder if we need to change them to have a "if NULL, then use a const default" somewhere. (And maybe you do this later in the series).
+static void gc_config(struct gc_config *cfg)
I appreciate that you are taking the step to make the structure a process parameter and not just another global. Thanks, -Stolee