From: Jonas 'Sortie' Termansen <Sortie@Maxsi.org>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Cc: "Keller, Jacob E" <jacob.e.keller@intel.com>,
Johannes Sixt <j6t@kdbg.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
Date: Fri, 29 Aug 2014 20:54:52 +0200 [thread overview]
Message-ID: <5400CC7C.4010706@Maxsi.org> (raw)
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
next prev parent reply other threads:[~2014-08-29 18:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
2014-08-29 16:42 ` [PATCH 2/9] autoconf: Check for " Jacob Keller
2014-08-29 16:42 ` [PATCH 3/9] autoconf: Check for setitimer Jacob Keller
2014-08-29 16:42 ` [PATCH 4/9] autoconf: Check for timer_t Jacob Keller
2014-08-29 18:20 ` Jonas 'Sortie' Termansen
2014-08-29 16:42 ` [PATCH 5/9] autoconf: Check for struct timespec Jacob Keller
2014-08-29 16:42 ` [PATCH 6/9] autoconf: Check for struct sigevent Jacob Keller
2014-08-29 16:42 ` [PATCH 7/9] autoconf: Check for struct itimerspec Jacob Keller
2014-08-29 16:42 ` [PATCH 8/9] autoconf: Check for timer_settime Jacob Keller
2014-08-29 17:26 ` Johannes Sixt
2014-08-29 17:40 ` Keller, Jacob E
2014-09-10 15:33 ` Karsten Blees
2014-09-10 21:08 ` Junio C Hamano
2014-09-10 21:13 ` Keller, Jacob E
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
2014-08-29 18:02 ` Junio C Hamano
2014-08-29 18:09 ` Keller, Jacob E
2014-08-29 18:12 ` Junio C Hamano
2014-08-29 16:48 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Keller, Jacob E
2014-08-29 18:54 ` Jonas 'Sortie' Termansen [this message]
2014-08-29 19:07 ` Junio C Hamano
2014-08-29 19:43 ` Jonas 'Sortie' Termansen
2014-09-03 0:17 ` Keller, Jacob E
-- strict thread matches above, loose matches on Subject: below --
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jonas 'Sortie' Termansen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5400CC7C.4010706@Maxsi.org \
--to=sortie@maxsi.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jacob.e.keller@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).