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

Re: [PATCH v1 3/4] config: factor out global config file retrievalync-mailbox>

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 25, 2023, 05:38 UTC
Message-ID
<ZTip7JWm-WRWTImU@tanuki>
In-Reply-To
<87badbe0-de18-4f8a-9589-314cea46065e@app.fastmail.com>
On Tue, Oct 24, 2023 at 03:23:11PM +0200, Kristoffer Haugsbakk wrote:
Show 36 quoted lines
> Hi Taylor and Patrick
> 
> On Mon, Oct 23, 2023, at 19:40, Taylor Blau wrote:
> >> Nit: we don't know about the intent of the caller, so they may not want
> >> to write to the file but only read it.
> >
> > I was going to suggest that we allow the caller to pass in the flags
> > that they wish for git_global_config() to pass down to access(2), but
> > was surprised to see that we always use R_OK.
> >
> > But thinking on it for a moment longer, I realized that we don't care
> > about write-level permissions for the config, since we want to instead
> > open $GIT_DIR/config.lock for writing, and then rename() it into place,
> > meaning we only care about whether or not we have write permissions on
> > $GIT_DIR itself.
> >
> > I think in the existing location of this code, the "if we should write"
> > portion of the comment is premature, since we don't know for sure
> > whether or not we are writing. So I'd be fine with leaving it as-is, but
> > changing the comment seems easy enough to do...
> >
> >> > +		 * location; error out even if XDG_CONFIG_HOME
> >> > +		 * is set and points at a sane location.
> >> > +		 */
> >> > +		die(_("$HOME not set"));
> >>
> >> Is it sensible to `die()` here in this new function that behaves more
> >> like a library function? I imagine it would be more sensible to indicate
> >> the error to the user and let them handle it accordingly.
> >
> > Agreed.
> >
> > Thanks,
> > Taylor
> 
> What do you guys think the signature of `git_global_config` should be?
Either of the following:
    - `int git_global_config(char **out_pat)`
    - `char **git_global_config(void)`

In the first case you'd signal error via a non-zero return value, whereas in the second case you would signal it via a `NULL` return value.

To decide which one to go with I'd recommend to check whether there is any similar precedent in "config.h" and what style that precedent uses.

Patrick
Previous: Kristoffer HaugsbakkNext: Kristoffer Haugsbakk
Message 8 of 33 in “maintenance: use XDG config if it exists”
  1. 0/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Oct 18, 2023
  2. 1/4 config: format newlinesKristoffer Haugsbakk, Oct 18, 2023
  3. 2/4 config: rename global config functionKristoffer Haugsbakk, Oct 18, 2023
  4. 3/4 config: factor out global config file retrievalKristoffer Haugsbakk, Oct 18, 2023
  5. Patrick SteinhardtOct 23, 2023
  6. Taylor BlauOct 23, 2023
  7. Kristoffer HaugsbakkOct 24, 2023
  8. Patrick SteinhardtOct 25, 2023
  9. Kristoffer HaugsbakkOct 25, 2023
  10. Patrick SteinhardtOct 25, 2023
  11. Junio C HamanoOct 27, 2023
  12. 4/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Oct 18, 2023
  13. Patrick SteinhardtOct 23, 2023
  14. Eric SunshineOct 23, 2023
  15. 0/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Jan 14, 2024
  16. 1/4 config: format newlinesKristoffer Haugsbakk, Jan 14, 2024
  17. 2/4 config: rename global config functionKristoffer Haugsbakk, Jan 14, 2024
  18. 3/4 config: factor out global config file retrievalKristoffer Haugsbakk, Jan 14, 2024
  19. Junio C HamanoJan 16, 2024
  20. Kristoffer HaugsbakkJan 16, 2024
  21. Patrick SteinhardtJan 19, 2024
  22. Kristoffer HaugsbakkJan 19, 2024
  23. Patrick SteinhardtJan 19, 2024
  24. Junio C HamanoJan 19, 2024
  25. Junio C HamanoJan 19, 2024
  26. rsbecker@nexbridge.comJan 19, 2024
  27. 4/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Jan 14, 2024
  28. Junio C HamanoJan 16, 2024
  29. 0/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Jan 18, 2024
  30. 1/4 config: format newlinesKristoffer Haugsbakk, Jan 18, 2024
  31. 2/4 config: rename global config functionKristoffer Haugsbakk, Jan 18, 2024
  32. 3/4 config: factor out global config file retrievalKristoffer Haugsbakk, Jan 18, 2024
  33. 4/4 maintenance: use XDG config if it existsKristoffer Haugsbakk, Jan 18, 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.