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

Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval

From
Jonas 'Sortie' Termansen <sortie@maxsi.org>
Date
Aug 29, 2014, 18:54 UTC
Message-ID
<5400CC7C.4010706@Maxsi.org>
In-Reply-To
<1409330916.18778.17.camel@jekeller-desk1.amr.corp.intel.com>
Hi,
Thanks for the interest. :)

There's a whole lot of emails being sent. I'll make a nice V2 shortly that takes your feedback into consideration. :)

But first let's discuss. I think we should define the intended criteria.
I expect to find these systems out there:
 * No setitimer and no timer_settime.
 * Has setitimer and no timer_settime.
 * Has setitimer and timer_settime (broken).
 * Has setitimer and timer_settime (works).
 * No setitimer and timer_settime (works).

Which of these do we want to support? Right we support the cases where systems have setitimer, the cases without it is slightly broken prior to my fixes.

Jake's modified patch set breaks the case where timer_settimer exists and is broken. As far as I know, that's only OpenBSD among the noticeable free software world, but could be more systems, perhaps in the future.

The progress bar displayed is rather non-essential. If we go with Jake's proposal, we support most non-broken platforms, and the broken platforms will start working when they add POSIX timers.

Ideally, I'd prefer to only support the systems with timer_settime that works, but real people use git on systems without and it is not too much work to support all of these combinations.

I see these approaches to the problem:
1) Only use setitimer (do nothing right now).
 * Disadvantage: We don't support modern systems without setitimer but that
   has timer_settime. Those are few, though, as setitimer is pretty much
   universal at the moment.
 * Disadvantage: We are using an older interface instead of the modern good
   practices.
2) Use setitimer (emulated with timer_create if needed).
 * Disadvantage: The core source code doesn't employ current best practices.
3) Use timer_create (emulated with setitimer if needed).
 * Disadvantage: The build system may have a false positive when checking
   whether timer_settime is available.
4) Use both (decision is made at runtime if both are available)
   If we do this well, the bulk of the compatibility code is isolated from
   the real source code (that just uses timer_settime naively) and it can
   be reduced when broken systems gets fixed.
 * Advantage: No regressions.
 * Disadvantage: The compatibility logic may be complicated.

I'm personally in favor of proposal 3, but it's more in git's spirit to pick proposal 4 as supports more of the real systems out there.

My first attempt was essentially proposal 4, but with no effort in trying to hide the fact that timer_settime might be broken. I'm going to develop a V2 that isolates this compatibility logic from the core code. I'm not convinced this approach is actually cleaner, but we'll see when it's done. Either way, isolated compatibility code today can be removed tomorrow when we no longer need it. :)

Jonas
Previous: Keller, Jacob ENext: Junio C Hamano
Message 20 of 23 in “git-compat-util.h: Add missing semicolon after struct itimerval”
  1. 1/9 git-compat-util.h: Add missing semicolon after struct itimervalJacob Keller, Aug 29, 2014
  2. 2/9 autoconf: Check for struct itimervalJacob Keller, Aug 29, 2014
  3. 3/9 autoconf: Check for setitimerJacob Keller, Aug 29, 2014
  4. 4/9 autoconf: Check for timer_tJacob Keller, Aug 29, 2014
  5. Jonas 'Sortie' TermansenAug 29, 2014
  6. 5/9 autoconf: Check for struct timespecJacob Keller, Aug 29, 2014
  7. 6/9 autoconf: Check for struct sigeventJacob Keller, Aug 29, 2014
  8. 7/9 autoconf: Check for struct itimerspecJacob Keller, Aug 29, 2014
  9. 8/9 autoconf: Check for timer_settimeJacob Keller, Aug 29, 2014
  10. Johannes SixtAug 29, 2014
  11. Keller, Jacob EAug 29, 2014
  12. Karsten BleesSep 10, 2014
  13. Junio C HamanoSep 10, 2014
  14. Keller, Jacob ESep 10, 2014
  15. 9/9 Use timer_settime for new platformsJacob Keller, Aug 29, 2014
  16. Junio C HamanoAug 29, 2014
  17. Keller, Jacob EAug 29, 2014
  18. Junio C HamanoAug 29, 2014
  19. Keller, Jacob EAug 29, 2014
  20. Jonas 'Sortie' TermansenAug 29, 2014
  21. Junio C HamanoAug 29, 2014
  22. Jonas 'Sortie' TermansenAug 29, 2014
  23. Keller, Jacob ESep 3, 2014

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.