git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] wt-status: move #include "pathspec.h" to the header
Date: Fri, 21 Aug 2015 00:35:06 +0200	[thread overview]
Message-ID: <20150821003506.Horde.cIQHHynKCAfMFot8jSGGhg7@webmail.informatik.kit.edu> (raw)
In-Reply-To: <xmqq61497pc1.fsf@gitster.dls.corp.google.com>


Quoting Junio C Hamano <gitster@pobox.com>:

> SZEDER Gábor <szeder@ira.uka.de> writes:
>
>> The declaration of 'struct wt_status' requires the declararion of 'struct
>> pathspec'.
>
> I think this is fine.
>
> I am guessing that you are saying it is wrong to force wt-status.c
> to include pathspec.h before including wt-status.h; I am fine with
> that.

> This is a tangent, but the above is different from saying that with
> a single liner test.c that has
>
>   #include "wt-status.h"
>
> your compilation "cc -c test.c" should succeed.

Sort of, but not quite, rather a combination of both.  My point is a
builtin command, starting with #include "builtin.h" as it should, that
wanted to use only struct wt_status_state and wt_status_get_state()
from the wt-status machinery and has nothing to do with pathspecs, yet
it's not sufficient to #include "wt-status.h", because the required
pathspec.h is included in the .c file instead of the header itsef.
(Actually, that's exactly how I noticed.)


> But for that goal,
> direct inclusion of <stdio.h> to wt-status.h is also iffy.  We
> include the system headers from git-compat-util.h because some
> platforms are picky about order of inclusion of system header files
> and definitions of feature test macros.
>
> Right now, the codebase is correct only because it is NOT our goal
> to guarantee that such a single-liner test.c file compiles.
> Instead, everybody is instructed to #include "git-compat-util.h" as
> the first thing, either directly or indirectly.
>
> So in that sense, we should also remove that inclusion from
> wt-status.h, I think.
>
> Thanks.

      parent reply	other threads:[~2015-08-20 22:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-20 14:06 [PATCH] wt-status: move #include "pathspec.h" to the header SZEDER Gábor
2015-08-20 19:05 ` Junio C Hamano
2015-08-20 20:38   ` Jeff King
2015-08-20 22:35   ` SZEDER Gábor [this message]

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=20150821003506.Horde.cIQHHynKCAfMFot8jSGGhg7@webmail.informatik.kit.edu \
    --to=szeder@ira.uka.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).