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

Re: [PATCH v2 2/4] Consolidate code to close a pack's file descriptor

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Oct 6, 2015, 13:42 UTC
Message-ID
<fb0ee65afcb4087de1137d27991dc896@dscho.org>
In-Reply-To
<xmqq1td9x8pz.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On 2015-10-06 00:15, Junio C Hamano wrote:
Show 31 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
>>>> +				&& !p->do_not_close)
>>>> +				close_pack_fd(p);
>>>
>>> I wonder how this do_not_close bit should interact with "we are
>>> shutting down (or we are spawning another to do the real work, while
>>> we won't do anything useful anymore), so close everything down".
>>
>> The `close_all_packs()` function is meant for the use case where you
>> really close all the packs, no question asked.
>>
>> I cannot think of a use case where it would make sense to try to be
>> gentle, yet still ask for *all* of the packs to be closed. Maybe you
>> can think of one to convince me that it might make sense to respect
>> the `do_not_close` flag in such a function? Because so far, I am
>> totally unconvinced.
> 
> Do not wait for being convinced forever, as I am not interested in
> convincing you either way.  Decision by made-up examples or lack of
> imagination is a waste of time ;-).
> 
> My earlier "I wonder" leads me to totally different conclusion,
> which is "we declare that it is a bug for any caller to call
> close-all-packs while marking any open pack with do_not_close bit".
> 
> Which merely means that while iterating over packs in the
> "close-all" loop, we should throw a die("BUG") at the caller if that
> bit is on.  That way, we won't have to rely on "we did not think of
> a good use case so we unconditionally closed the packs without
> telling the remainder of the system, hopefully we broke nothing."
I just sent another iteration out that adds this sanity check.

Ciao, Dscho

Previous: Junio C HamanoNext: Johannes Schindelin
Message 12 of 23 in “clone --dissociate: avoid locking pack files”
  1. clone --dissociate: avoid locking pack filesJohannes Schindelin, Sep 28, 2015
  2. Max KirillovSep 30, 2015
  3. Junio C HamanoSep 30, 2015
  4. Max KirillovOct 1, 2015
  5. Johannes SchindelinOct 5, 2015
  6. 0/4 Fix locking issues on Windows with `git clone --dissociate`Johannes Schindelin, Oct 5, 2015
  7. 1/4 Demonstrate a Windows file locking issue with `git clone --dissociate`Johannes Schindelin, Oct 5, 2015
  8. 2/4 Consolidate code to close a pack's file descriptorJohannes Schindelin, Oct 5, 2015
  9. Junio C HamanoOct 5, 2015
  10. Johannes SchindelinOct 5, 2015
  11. Junio C HamanoOct 5, 2015
  12. Johannes SchindelinOct 6, 2015
  13. 3/4 Add a function to release all packsJohannes Schindelin, Oct 5, 2015
  14. 4/4 clone --dissociate: avoid locking pack filesJohannes Schindelin, Oct 5, 2015
  15. Junio C HamanoOct 5, 2015
  16. 0/4 Fix locking issues on Windows with `git clone --dissociate`Johannes Schindelin, Oct 6, 2015
  17. 1/4 Demonstrate a Windows file locking issue with `git clone --dissociate`Johannes Schindelin, Oct 6, 2015
  18. 2/4 Consolidate code to close a pack's file descriptorJohannes Schindelin, Oct 6, 2015
  19. 3/4 Add a function to release all packsJohannes Schindelin, Oct 6, 2015
  20. Junio C HamanoOct 7, 2015
  21. Johannes SchindelinOct 8, 2015
  22. 4/4 clone --dissociate: avoid locking pack filesJohannes Schindelin, Oct 6, 2015
  23. Max KirillovOct 11, 2015

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.