[Mesa-dev] [Nine] 'meson: add -Werror=empty-body to disallow `if(x); `' - 'broke' Nine
Eric Engestrom
eric.engestrom at intel.com
Fri Oct 25 11:13:04 UTC 2019
On Friday, 2019-10-25 13:09:30 +0200, Timur Kristóf wrote:
> I think it depends on how much time it adds to the CI.
> If the overhead is negligible, then it makes a lot of sense to have CI
> make a release build in addition to the debug ones.
Exactly; see MR !2469 for that, I won't have time to look at it more for
a few days though.
>
> While we are at it:
>
> Would it be possible to add some CI tests to ensure that Nine doesn't
> break (even if it builds), similarly to how some drivers run their CTS
> tests in there? For instance, can we run Xnine or some other small
> testing tool in the CI?
Anything that doesn't need hardware can be tested directly in gitlab,
and anything that does should be added through eg. lava.
It's pretty much just a matter of knowing what to do and set it up in
the CI ;)
>
> What do you think about this, Axel?
>
> Thanks & best regards,
> Tim
>
> On Fri, 2019-10-25 at 11:48 +0100, Eric Engestrom wrote:
> > (for the record, the MR has been reviewed and will be merged when the
> > pipeline comes back all green)
> >
> > The problem is that the CI does debug builds, while this was
> > a release-build-only issue.
> >
> > We could perform multiple types of builds each time, but that would
> > multiply the CI time/cost; I'll take a look now to see the best way
> > to
> > do that and see if the time cost is reasonable.
> > (Meson has 6 build types, so testing all of them is not reasonable,
> > but
> > maybe we can test debug and release)
> >
> >
> > On Friday, 2019-10-25 10:49:04 +0200, Timur Kristóf wrote:
> > > Hi guys,
> > >
> > > How is it possible that the CI didn't detect this problem?
> > > Isn't there at least one build in the CI system which builds Nine?
> > >
> > > I've created a MR to deal with it:
> > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2465
> > >
> > > Best regards,
> > > Tim
> > >
> > >
> > > On Thu, 2019-10-24 at 23:31 +0200, Axel Davy wrote:
> > > > Hi Dieter,
> > > >
> > > > Maybe the best fix would be to change the definition of WARN and
> > > > DBG
> > > > when DEBUG is disabled.
> > > >
> > > > The definitions are in nine_debug.h
> > > >
> > > > I haven't tried by maybe using "(void)" instead of nothing would
> > > > work
> > > > ?
> > > >
> > > > Yours,
> > > >
> > > > Axel
> > > >
> > > > On 24/10/2019 16:34, Dieter Nützel wrote:
> > > > > Hello Eric,
> > > > >
> > > > > your mentioned commit
> > > > > (8d43e2b2ded0fe3c82d49561cdab9f208f9e64b6)
> > > > > broke
> > > > > building with NIne (-Dgallium-nine=true) for me.
> > > > >
> > > > > starting with
> > > > > [-]
> > > > > e_st at sta/cubetexture9.c.o' -c
> > > > > ../src/gallium/state_trackers/nine/cubetexture9.c
> > > > > ../src/gallium/state_trackers/nine/cubetexture9.c: In function
> > > > > ‘NineCubeTexture9_ctor’:
> > > > > ../src/gallium/state_trackers/nine/cubetexture9.c:108:43:
> > > > > error:
> > > > > suggest braces around empty body in an ‘if’ statement
> > > > > [-Werror=empty-body]
> > > > > 108 | "but this is unimplemented\n");
> > > > > | ^
> > > > > cc1: some warnings being treated as errors
> > > > >
> > > > > --
> > > > > Next
> > > > >
> > > > > /surface9.c.o' -c ../src/gallium/state_trackers/nine/surface9.c
> > > > > ../src/gallium/state_trackers/nine/surface9.c: In function
> > > > > ‘NineSurface9_GetContainer’:
> > > > > ../src/gallium/state_trackers/nine/surface9.c:334:40: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 334 | DBG("QueryInterface FAILED!\n");
> > > > > | ^
> > > > > cc1: some warnings being treated as errors
> > > > >
> > > > > --
> > > > >
> > > > > @sta/swapchain9.c.o' -c
> > > > > ../src/gallium/state_trackers/nine/swapchain9.c
> > > > > ../src/gallium/state_trackers/nine/swapchain9.c: In function
> > > > > ‘present’:
> > > > > ../src/gallium/state_trackers/nine/swapchain9.c:737:51: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 737 | pSourceRect->top, pSourceRect->bottom);
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/swapchain9.c:741:47: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 741 | pDestRect->top, pDestRect->bottom);
> > > > > | ^
> > > > > cc1: some warnings being treated as errors
> > > > >
> > > > > --
> > > > >
> > > > > evice9.c.o' -c ../src/gallium/state_trackers/nine/device9.c
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘NineDevice9_ctor’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:296:49: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 296 | DBG("\033[1;32mCSMT is active\033[0m\n");
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘create_zs_or_rt_surface’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:1221:87: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1221 | DBG("FIXME Used shared handle! This option isn't
> > > > > probably handled correctly!\n");
> > > > > > ^
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘NineDevice9_UpdateSurface’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:1307:53: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1307 | pSourceRect->right, pSourceRect->bottom);
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/device9.c:1309:68: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1309 | DBG("pDestPoint = (%u,%u)\n", pDestPoint->x,
> > > > > pDestPoint->y);
> > > > > > ^
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘NineDevice9_StretchRect’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:1588:53: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1588 | pSourceRect->right, pSourceRect->bottom);
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/device9.c:1591:49: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1591 | pDestRect->right, pDestRect->bottom);
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘NineDevice9_ColorFill’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:1786:41: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1786 | pRect->right, pRect->bottom);
> > > > > | ^
> > > > > ../src/gallium/state_trackers/nine/device9.c: In function
> > > > > ‘NineDevice9_CreateOffscreenPlainSurface’:
> > > > > ../src/gallium/state_trackers/nine/device9.c:1864:43: error:
> > > > > suggest
> > > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > > body]
> > > > > 1864 | DBG("Failed to create surface.\n");
> > > > > | ^
> > > > > cc1: some warnings being treated as errors
> > > > >
> > > > > --
> > > > >
> > > > > st at sta/nine_shader.c.o' -c
> > > > > ../src/gallium/state_trackers/nine/nine_shader.c
> > > > > ../src/gallium/state_trackers/nine/nine_shader.c: In function
> > > > > ‘tx_dst_param_as_src’:
> > > > > ../src/gallium/state_trackers/nine/nine_shader.c:1437:52:
> > > > > error:
> > > > > suggest braces around empty body in an ‘if’ statement
> > > > > [-Werror=empty-body]
> > > > > 1437 | WARN("mask is 0, using identity swizzle\n");
> > > > > |
> > > > >
> > > > > --
> > > > > Last
> > > > >
> > > > > ine_ff.c.o' -c ../src/gallium/state_trackers/nine/nine_ff.c
> > > > > ../src/gallium/state_trackers/nine/nine_ff.c: In function
> > > > > ‘nine_ff_get_vs’:
> > > > > ../src/gallium/state_trackers/nine/nine_ff.c:1610:72: error:
> > > > > suggest
> > > > > braces around empty body in an ‘else’ statement [-Werror=empty-
> > > > > body]
> > > > > 1610 | DBG("FF given texture coordinate >=
> > > > > 8.
> > > > > Ignoring\n");
> > > > > > ^
> > > > > cc1: some warnings being treated as errors
> > > > >
> > > > > Thanks,
> > > > > Dieter
> > > > >
> > > >
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list