diff mbox series

[v6,8/8] ci: add checking of includes to CI builds

Message ID 20210127173330.1671341-9-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series add checking of header includes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-testing fail Testing issues

Commit Message

Bruce Richardson Jan. 27, 2021, 5:33 p.m. UTC 
  For CI builds, turn on the checking of includes.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Aaron Conole <aconole@redhat.com>
---
 .ci/linux-build.sh | 1 +
 1 file changed, 1 insertion(+)

--
2.27.0

Comments

Thomas Monjalon Jan. 28, 2021, 11:12 a.m. UTC | #1 
27/01/2021 18:33, Bruce Richardson:
> For CI builds, turn on the checking of includes.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Aaron Conole <aconole@redhat.com>
> ---
>  .ci/linux-build.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index afa3689a09..fdbeb5a616 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -57,6 +57,7 @@ fi
>  OPTS="$OPTS -Dmachine=default"
>  OPTS="$OPTS --default-library=$DEF_LIB"
>  OPTS="$OPTS --buildtype=debugoptimized"
> +OPTS="$OPTS -Dcheck_includes=true"

Which percentage of time does it add on GHA?
Bruce Richardson Jan. 28, 2021, 11:41 a.m. UTC | #2 
On Thu, Jan 28, 2021 at 12:12:44PM +0100, Thomas Monjalon wrote:
> 27/01/2021 18:33, Bruce Richardson:
> > For CI builds, turn on the checking of includes.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by:
> > Aaron Conole <aconole@redhat.com> --- .ci/linux-build.sh | 1 + 1 file
> > changed, 1 insertion(+)
> > 
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > afa3689a09..fdbeb5a616 100755 --- a/.ci/linux-build.sh +++
> > b/.ci/linux-build.sh @@ -57,6 +57,7 @@ fi OPTS="$OPTS
> > -Dmachine=default" OPTS="$OPTS --default-library=$DEF_LIB" OPTS="$OPTS
> > --buildtype=debugoptimized" +OPTS="$OPTS -Dcheck_includes=true"
> 
> Which percentage of time does it add on GHA?
> 
I'd have to gather some before/after tests to know for sure, but to check
it wasn't too heavy-weight I was just monitoring the log at the end of a
build on GHA. The specific build I watched took nearly 15 minutes, and I
would guess-timate that the extra task took about 20 seconds of that.
Bruce Richardson Jan. 28, 2021, 6:34 p.m. UTC | #3 
On Thu, Jan 28, 2021 at 11:41:44AM +0000, Bruce Richardson wrote:
> On Thu, Jan 28, 2021 at 12:12:44PM +0100, Thomas Monjalon wrote:
> > 27/01/2021 18:33, Bruce Richardson:
> > > For CI builds, turn on the checking of includes.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by:
> > > Aaron Conole <aconole@redhat.com> --- .ci/linux-build.sh | 1 + 1 file
> > > changed, 1 insertion(+)
> > > 
> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > afa3689a09..fdbeb5a616 100755 --- a/.ci/linux-build.sh +++
> > > b/.ci/linux-build.sh @@ -57,6 +57,7 @@ fi OPTS="$OPTS
> > > -Dmachine=default" OPTS="$OPTS --default-library=$DEF_LIB" OPTS="$OPTS
> > > --buildtype=debugoptimized" +OPTS="$OPTS -Dcheck_includes=true"
> > 
> > Which percentage of time does it add on GHA?
> > 
> I'd have to gather some before/after tests to know for sure, but to check
> it wasn't too heavy-weight I was just monitoring the log at the end of a
> build on GHA. The specific build I watched took nearly 15 minutes, and I
> would guess-timate that the extra task took about 20 seconds of that.

Related to this, and to ensure we don't increase the GHA build time
massively, is there a reliably way to test this out? Looking at the daily
builds on GHA I see some successful builds completing in 7.5 mins, while
others take almost 18 minutes.
The extra build time from this chkincs takes very little time on my own
system but perhaps we want to limit it to a single build on GHA. I'm not
very familiar with configuring those builds, so perhaps someone could
suggest how to do so.

Thanks,
/Bruce
David Marchand Jan. 29, 2021, 5:42 p.m. UTC | #4 
On Thu, Jan 28, 2021 at 7:34 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jan 28, 2021 at 11:41:44AM +0000, Bruce Richardson wrote:
> > On Thu, Jan 28, 2021 at 12:12:44PM +0100, Thomas Monjalon wrote:
> > > 27/01/2021 18:33, Bruce Richardson:
> > > > For CI builds, turn on the checking of includes.
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by:
> > > > Aaron Conole <aconole@redhat.com> --- .ci/linux-build.sh | 1 + 1 file
> > > > changed, 1 insertion(+)
> > > >
> > > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > > afa3689a09..fdbeb5a616 100755 --- a/.ci/linux-build.sh +++
> > > > b/.ci/linux-build.sh @@ -57,6 +57,7 @@ fi OPTS="$OPTS
> > > > -Dmachine=default" OPTS="$OPTS --default-library=$DEF_LIB" OPTS="$OPTS
> > > > --buildtype=debugoptimized" +OPTS="$OPTS -Dcheck_includes=true"
> > >
> > > Which percentage of time does it add on GHA?
> > >
> > I'd have to gather some before/after tests to know for sure, but to check
> > it wasn't too heavy-weight I was just monitoring the log at the end of a
> > build on GHA. The specific build I watched took nearly 15 minutes, and I
> > would guess-timate that the extra task took about 20 seconds of that.
>
> Related to this, and to ensure we don't increase the GHA build time
> massively, is there a reliably way to test this out? Looking at the daily
> builds on GHA I see some successful builds completing in 7.5 mins, while
> others take almost 18 minutes.

I guess it depends on the changes being tested.
We have a ccache regenerated once a week against the main branch.
That might help compilations for changes on drivers only.

And it might depend on luck too / the resources available on the
system that runs the tests.
We once hit a case with OVS GHA where the cached DPDK binaries were
built on a system that had a more recent cpu (and this is the reason
why we now build with -Dmachine=default).


> The extra build time from this chkincs takes very little time on my own
> system but perhaps we want to limit it to a single build on GHA. I'm not
> very familiar with configuring those builds, so perhaps someone could
> suggest how to do so.

From what I see in ovsrobot/dpdk/actions logs, the impact is small.
Aaron Conole Jan. 29, 2021, 9:38 p.m. UTC | #5 
David Marchand <david.marchand@redhat.com> writes:

> On Thu, Jan 28, 2021 at 7:34 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>>
>> On Thu, Jan 28, 2021 at 11:41:44AM +0000, Bruce Richardson wrote:
>> > On Thu, Jan 28, 2021 at 12:12:44PM +0100, Thomas Monjalon wrote:
>> > > 27/01/2021 18:33, Bruce Richardson:
>> > > > For CI builds, turn on the checking of includes.
>> > > >
>> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by:
>> > > > Aaron Conole <aconole@redhat.com> --- .ci/linux-build.sh | 1 + 1 file
>> > > > changed, 1 insertion(+)
>> > > >
>> > > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
>> > > > afa3689a09..fdbeb5a616 100755 --- a/.ci/linux-build.sh +++
>> > > > b/.ci/linux-build.sh @@ -57,6 +57,7 @@ fi OPTS="$OPTS
>> > > > -Dmachine=default" OPTS="$OPTS --default-library=$DEF_LIB" OPTS="$OPTS
>> > > > --buildtype=debugoptimized" +OPTS="$OPTS -Dcheck_includes=true"
>> > >
>> > > Which percentage of time does it add on GHA?
>> > >
>> > I'd have to gather some before/after tests to know for sure, but to check
>> > it wasn't too heavy-weight I was just monitoring the log at the end of a
>> > build on GHA. The specific build I watched took nearly 15 minutes, and I
>> > would guess-timate that the extra task took about 20 seconds of that.
>>
>> Related to this, and to ensure we don't increase the GHA build time
>> massively, is there a reliably way to test this out? Looking at the daily
>> builds on GHA I see some successful builds completing in 7.5 mins, while
>> others take almost 18 minutes.
>
> I guess it depends on the changes being tested.
> We have a ccache regenerated once a week against the main branch.
> That might help compilations for changes on drivers only.
>
> And it might depend on luck too / the resources available on the
> system that runs the tests.
> We once hit a case with OVS GHA where the cached DPDK binaries were
> built on a system that had a more recent cpu (and this is the reason
> why we now build with -Dmachine=default).
>
>
>> The extra build time from this chkincs takes very little time on my own
>> system but perhaps we want to limit it to a single build on GHA. I'm not
>> very familiar with configuring those builds, so perhaps someone could
>> suggest how to do so.
>
> From what I see in ovsrobot/dpdk/actions logs, the impact is small.

+1 - it seems negligible from what I can tell.
diff mbox series

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index afa3689a09..fdbeb5a616 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -57,6 +57,7 @@  fi
 OPTS="$OPTS -Dmachine=default"
 OPTS="$OPTS --default-library=$DEF_LIB"
 OPTS="$OPTS --buildtype=debugoptimized"
+OPTS="$OPTS -Dcheck_includes=true"
 meson build --werror $OPTS
 ninja -C build