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

Re: [PATCH 1/3] repository: move git_*_encoding configs to repo scope

From
shejialuo <shejialuo@gmail.com>
Date
Oct 17, 2024, 13:06 UTC
Message-ID
<ZxELulPlLQIs0sh0@ArchLinux>
In-Reply-To
<Zw9Z_xcHKcjKVUB4@pks.im>
On Wed, Oct 16, 2024 at 08:15:37AM +0200, Patrick Steinhardt wrote:
Show 53 quoted lines
> On Wed, Oct 16, 2024 at 12:05:05AM +0800, shejialuo wrote:
> > There are many builtins will execute this config setups by calling
> > "config.c::git_default_config" and then "git_default_i18n_config". If we
> > need to use "repo" pointer, we may need to wrap this pointer. (This is
> > not the problem and it is not hard).
> > 
> > But what if the "repo" pointer is NULL? We still need to set the value
> > of these environment variables. Because when using "git-mailinfo(1)"
> > outside of the repo, we still need to set "git_commit_encoding"
> > according to the user's config.
> > 
> > So, from this perspective, I don't think it's a good idea to put these
> > two configs into "struct repository". Because we can use these two
> > configs outside of the repo, if we put them into "struct repository", it
> > is strange.
> > 
> > However, I either don't know which way we would apply. So, I cannot give
> > accurate answer here.
> > 
> > ---
> > 
> > Patrick, I wanna ask you a question here. What's your envision here in
> > above situation. As you can see, if we put some configs into "struct
> > repository" and we run the builtins outside of the repo where we need to
> > set up configs, the "repo" is NULL. And we will get into trouble.
> > 
> > My idea is that if a config could be used outside of the repo, then we
> > should not put it into "struct repository".
> 
> I guess it would be nice to have a set of convenice functions in our
> config code that know to handle the case where the passed-in repository
> is `NULL`. If so, they'd only parse the global- and system-level config.
> If it is set, it would parse all three of them.
> 
> I also kind of agree that it should likely not be put into the `struct
> repository` in that case. I think where exactly to put things will
> always depend on the current usecase. I bet that in most cases, we
> should be able to get away with not storing the value anywhere global at
> all, which would be the best solution in my opinion:
> 
>   - It can either be stored on the stack if we don't have to pass it
>     around everywhere.
> 
>   - It can be passed around in a specific structure if we pass the value
>     within in a certain subsystem, only.
> 
>   - Or we can parse it on an as-needed basis if it happens deep down in
>     the calling stack when it's used essentially everwhere.
> 
> Now there will be situations where we used to store things globally as a
> caching mechanism, and not caching it may have performance impacts. But
> I guess that most cases do not fall into this category.
> 

Thanks for the direction. I will dive into code to think about how we could do this. Actually, I have tried to refactor "git-apply(1)" to remove "the_repository". And I found it hard to remove dependency "environment.c". It's a long journey.

> Patrick

Thanks, Jialuo

Previous: Patrick SteinhardtNext: Kousik Sanagavarapu
Message 7 of 13 in “pretty, builtin/mailinfo: don't rely on the_repository”
  1. 0/3 pretty, builtin/mailinfo: don't rely on the_repositoryKousik Sanagavarapu, Oct 15, 2024
  2. 1/3 repository: move git_*_encoding configs to repo scopeKousik Sanagavarapu, Oct 15, 2024
  3. shejialuoOct 15, 2024
  4. Patrick SteinhardtOct 16, 2024
  5. Kousik SanagavarapuOct 16, 2024
  6. Patrick SteinhardtOct 17, 2024
  7. shejialuoOct 17, 2024
  8. Kousik SanagavarapuOct 16, 2024
  9. 2/3 pretty: don't rely on "the_repository"Kousik Sanagavarapu, Oct 15, 2024
  10. 3/3 builtin/mailinfo: don't rely on "the_repository"Kousik Sanagavarapu, Oct 15, 2024
  11. shejialuoOct 15, 2024
  12. Taylor BlauOct 15, 2024
  13. Kousik SanagavarapuOct 16, 2024

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.