Remove -Werror from autotools/darwin build, enable it on CI for all platforms #636

veprbl · 2021-08-30T08:31:07Z

The -Werror was added very long time ago: c1e1d06
This flag is more suitable for running on CI or during development, but encumbers downstream packaging.

cc NixOS/nixpkgs#136060

Be-ing · 2021-09-01T15:33:56Z

I don't think there's any good reason to remove this upstream. If packagers are annoyed by it, I think that's their problem and they can remove it if they want.

veprbl · 2021-09-01T15:45:53Z

@Be-ing Would you accept a patch that adds -Werror for autotools on linux and for cmake builds?

Be-ing · 2021-09-01T15:58:30Z

Thank you for noticing that inconsistency in the build system, which is one more argument for removing autotools. I think we should use -Werror with CMAKE_BUILD_TYPE=Debug.

philburk · 2021-09-04T23:32:43Z

We definitely want -Werror somewhere in our CI to catch warnings.

@veprbl - Do you have more details on the problems -Werror causes for packagers?

r-burns · 2021-09-04T23:42:11Z

Werror is good to enable for development, but unless your CI checks every possible compiler out there, warnings can always sneak by. For example, Nixpkgs ran into some new warnings with gcc 10. I think the recommended approach is to keep Werror off by default, and enable it on the CI using CPPFLAGS.

veprbl · 2021-09-05T12:26:39Z

@veprbl - Do you have more details on the problems -Werror causes for packagers?

The compilation errors caused by -Werror are not so helpful to the end users. They are often seen as low importance and not reported upstream (One could say that upstream, in theory, has access to all the same compilers, so what is there to report). In the end work is put into silencing those errors without any productive output.

We definitely want -Werror somewhere in our CI to catch warnings.

That is understandable. How about adding it in the CI workflows?

philburk · 2021-09-05T23:54:03Z

I am in favor of merging this after we add -Werror to the CI.

philburk · 2021-09-06T00:39:30Z

Note that the Werror broke the MSVC CI.

veprbl · 2021-09-06T10:23:53Z

Note that the Werror broke the MSVC CI.

Correct. After using /WX it is still broken:

D:\a\portaudio\portaudio\build\asiosdk_2.3.3_2019-06-14\host\pc\asiolist.cpp(24,41): error C2220: the following warning is treated as an error [D:\a\portaudio\portaudio\build\PortAudio.vcxproj]
43
D:\a\portaudio\portaudio\build\asiosdk_2.3.3_2019-06-14\host\pc\asiolist.cpp(24,41): warning C4267: 'argument': conversion from 'size_t' to 'DWORD', possible loss of data [D:\a\portaudio\portaudio\build\PortAudio.vcxproj]
44
D:\a\portaudio\portaudio\build\asiosdk_2.3.3_2019-06-14\host\pc\asiolist.cpp(31,42): warning C4267: 'argument': conversion from 'size_t' to 'DWORD', possible loss of data [D:\a\portaudio\portaudio\build\PortAudio.vcxproj]

Any thoughts?

.github/workflows/autotools.yml

RossBencina · 2021-09-09T00:40:37Z

I think this is heading in the right direction. It's OK to limit -Werror to CI. Given that the problem with /WX on MSVC is with the ASIO SDK (something we can't fix), I think that we can move ahead with merging this without /WX.

Be-ing · 2021-09-11T02:47:32Z

.github/workflows/cmake.yml

@@ -19,6 +19,7 @@ jobs:
            cmake_generator: "Unix Makefiles"
            cmake_options:
              -DOSS=ON
+            werror_flags: "-Werror"


This should be done in CMakeLists.txt for ${CMAKE_BUILD_TYPE} STREQUAL "Debug" and change the cmake_build_type variable in this GH Actions workflow file to Debug.

The CI builds should use -Werror for both debug and release builds.

Be-ing · 2021-09-12T06:06:44Z

Please update the title of the pull request to reflect what it is actually doing now.

philburk · 2021-10-07T00:29:50Z

Ross and I discussed this and we are both OK with removing -Werror from configure and cmake.
So we need to have the -Werror in the CI builds.
But there is work remaining in this PR.

@veprbl - Would you like to make those changes and get this merged?

veprbl · 2021-10-07T00:47:21Z

The CI currently fails on GCC due to PortAudio using deprecated API's
https://github.com/veprbl/portaudio/pull/1/checks?check_run_id=3821876527

Build log

/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c: In function ‘BuildDeviceList’:
27
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:637:17: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
28
  637 |                 jack_port_get_latency( p ) / globalSampleRate;
29
      |                 ^~~~~~~~~~~~~~~~~~~~~
30
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
31
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
32
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
33
      |                ^~~~~~~~~~~~~~~~~~~~~
34
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:658:17: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
35
  658 |                 jack_port_get_latency( p ) / globalSampleRate;
36
      |                 ^~~~~~~~~~~~~~~~~~~~~
37
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
38
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
39
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
40
      |                ^~~~~~~~~~~~~~~~~~~~~
41
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c: In function ‘OpenStream’:
42
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:1354:9: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
43
 1354 |         stream->streamRepresentation.streamInfo.inputLatency = (jack_port_get_latency( stream->remote_output_ports[0] )
44
      |         ^~~~~~
45
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
46
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
47
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
48
      |                ^~~~~~~~~~~~~~~~~~~~~
49
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:1358:9: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
50
 1358 |         stream->streamRepresentation.streamInfo.outputLatency = (jack_port_get_latency( stream->remote_input_ports[0] )
51
      |         ^~~~~~
52
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
53
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
54
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
55
      |                ^~~~~~~~~~~~~~~~~~~~~
56
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c: In function ‘RealProcess’:
57
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:1420:9: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
58
 1420 |         timeInfo.inputBufferAdcTime = timeInfo.currentTime - jack_port_get_latency( stream->remote_output_ports[0] )
59
      |         ^~~~~~~~
60
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
61
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
62
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
63
      |                ^~~~~~~~~~~~~~~~~~~~~
64
/home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:1423:9: error: ‘jack_port_get_latency’ is deprecated [-Werror=deprecated-declarations]
65
 1423 |         timeInfo.outputBufferDacTime = timeInfo.currentTime + jack_port_get_latency( stream->remote_input_ports[0] )
66
      |         ^~~~~~~~
67
In file included from /home/runner/work/portaudio/portaudio/src/hostapi/jack/pa_jack.c:63:
68
/home/runner/work/portaudio/portaudio/build/vcpkg_installed/x64-linux/include/jack/jack.h:1233:16: note: declared here
69
 1233 | jack_nframes_t jack_port_get_latency (jack_port_t *port) JACK_OPTIONAL_WEAK_DEPRECATED_EXPORT;
70
      |                ^~~~~~~~~~~~~~~~~~~~~
71
[ 10%] Building C object CMakeFiles/PortAudio.dir/src/os/unix/pa_unix_hostapis.c.o
72
[ 10%] Building C object CMakeFiles/PortAudio.dir/src/os/unix/pa_unix_util.c.o
73
[ 11%] Building C object CMakeFiles/PortAudio.dir/src/hostapi/alsa/pa_linux_alsa.c.o
74
/home/runner/work/portaudio/portaudio/src/hostapi/alsa/pa_linux_alsa.c:183:1: error: ‘snd_pcm_sw_params_set_xfer_align’ is deprecated [-Werror=deprecated-declarations]
75
  183 | _PA_DEFINE_FUNC(snd_pcm_sw_params_set_xfer_align);
76
      | ^~~~~~~~~~~~~~~
77
In file included from /usr/include/alsa/asoundlib.h:54,
78
                 from /home/runner/work/portaudio/portaudio/src/hostapi/alsa/pa_linux_alsa.c:52:
79
/usr/include/alsa/pcm.h:1331:5: note: declared here
80
 1331 | int snd_pcm_sw_params_set_xfer_align(snd_pcm_t *pcm, snd_pcm_sw_params_t *params, snd_pcm_uframes_t val) __attribute__((deprecated));
81
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
82
/home/runner/work/portaudio/portaudio/src/hostapi/alsa/pa_linux_alsa.c: In function ‘PaAlsa_LoadLibrary’:
83
/home/runner/work/portaudio/portaudio/src/hostapi/alsa/pa_linux_alsa.c:467:5: error: ‘snd_pcm_sw_params_set_xfer_align’ is deprecated [-Werror=deprecated-declarations]
84
  467 |     _PA_LOAD_FUNC(snd_pcm_sw_params_set_xfer_align);
85
      |     ^~~~~~~~~~~~~
86
In file included from /usr/include/alsa/asoundlib.h:54,
87
                 from /home/runner/work/portaudio/portaudio/src/hostapi/alsa/pa_linux_alsa.c:52:
88
/usr/include/alsa/pcm.h:1331:5: note: declared here
89
 1331 | int snd_pcm_sw_params_set_xfer_align(snd_pcm_t *pcm, snd_pcm_sw_params_t *params, snd_pcm_uframes_t val) __attribute__((deprecated));
90
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
91
cc1: all warnings being treated as errors
92
make[2]: *** [CMakeFiles/PortAudio.dir/build.make:230: CMakeFiles/PortAudio.dir/src/hostapi/jack/pa_jack.c.o] Error 1
93
make[2]: *** Waiting for unfinished jobs....
94
cc1: all warnings being treated as errors
95
make[2]: *** [CMakeFiles/PortAudio.dir/build.make:272: CMakeFiles/PortAudio.dir/src/hostapi/alsa/pa_linux_alsa.c.o] Error 1
96
make[1]: *** [CMakeFiles/Makefile2:254: CMakeFiles/PortAudio.dir/all] Error 2
97
make: *** [Makefile:136: all] Error 2
98
Error: Process completed with exit code 2.

.github/workflows/cmake.yml

RossBencina · 2021-11-04T00:48:00Z

Regarding the warnings that are breaking CI, we already have tickets for these:
jack_port_get_latency deprecated: #641
snd_pcm_sw_params_set_xfer_align deprecated: #213
_WIN32_WINNT redefined: #549 [FIXED]
error: 'OSMemoryBarrier' is deprecated: #639 [FIXED]

philburk · 2021-11-04T00:53:33Z

Thanks for adding the -Werror to the CI. They caused CI to fail on the deprecated functions, which is good.

We should fix the deprecated functions before we merge this PR.

dechamps · 2023-03-04T13:23:28Z

FYI I am proposing a slightly different approach in #780.

philburk requested a review from RossBencina September 5, 2021 23:54

RossBencina reviewed Sep 9, 2021

View reviewed changes

.github/workflows/autotools.yml Show resolved Hide resolved

Be-ing reviewed Sep 11, 2021

View reviewed changes

philburk assigned veprbl Oct 7, 2021

veprbl changed the title ~~Remove -Werror from autotools/darwin build~~ Remove -Werror from autotools/darwin build, enable it on CI for all platforms Oct 7, 2021

RossBencina reviewed Nov 4, 2021

View reviewed changes

.github/workflows/cmake.yml Outdated Show resolved Hide resolved

veprbl added 2 commits December 6, 2021 19:36

remove -Werror from autotools/darwin build

0fa9ff8

Add -Werror to CFLAGS and CXXFLAGS for CI workflows

1677998

This was referenced May 10, 2022

pa_jack.c uses deprecated jack_port_get_latency API #641

Closed

ALSA function snd_pcm_sw_params_set_xfer_align() deprecated #213

Open

dechamps mentioned this pull request Mar 4, 2023

Treat warnings as errors on CI #780

Merged

dechamps mentioned this pull request Mar 22, 2023

Remove -Werror from autotools darwin build #800

Merged

RossBencina added a commit to RossBencina/portaudio that referenced this pull request Mar 30, 2023

set CFLAGS and CXXFLAGS using environment vars as in PortAudio#636

b81d7b1

veprbl closed this by deleting the head repository Oct 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove -Werror from autotools/darwin build, enable it on CI for all platforms #636

Remove -Werror from autotools/darwin build, enable it on CI for all platforms #636

veprbl commented Aug 30, 2021 •

edited

Loading

Be-ing commented Sep 1, 2021

veprbl commented Sep 1, 2021

Be-ing commented Sep 1, 2021

philburk commented Sep 4, 2021

r-burns commented Sep 4, 2021

veprbl commented Sep 5, 2021

philburk commented Sep 5, 2021

philburk commented Sep 6, 2021

veprbl commented Sep 6, 2021

RossBencina commented Sep 9, 2021

Be-ing Sep 11, 2021

RossBencina Oct 7, 2021

Be-ing commented Sep 12, 2021

philburk commented Oct 7, 2021

veprbl commented Oct 7, 2021 •

edited

Loading

RossBencina commented Nov 4, 2021 •

edited

Loading

philburk commented Nov 4, 2021

dechamps commented Mar 4, 2023

Remove -Werror from autotools/darwin build, enable it on CI for all platforms #636

Remove -Werror from autotools/darwin build, enable it on CI for all platforms #636

Conversation

veprbl commented Aug 30, 2021 • edited Loading

Be-ing commented Sep 1, 2021

veprbl commented Sep 1, 2021

Be-ing commented Sep 1, 2021

philburk commented Sep 4, 2021

r-burns commented Sep 4, 2021

veprbl commented Sep 5, 2021

philburk commented Sep 5, 2021

philburk commented Sep 6, 2021

veprbl commented Sep 6, 2021

RossBencina commented Sep 9, 2021

Be-ing Sep 11, 2021

Choose a reason for hiding this comment

RossBencina Oct 7, 2021

Choose a reason for hiding this comment

Be-ing commented Sep 12, 2021

philburk commented Oct 7, 2021

veprbl commented Oct 7, 2021 • edited Loading

RossBencina commented Nov 4, 2021 • edited Loading

philburk commented Nov 4, 2021

dechamps commented Mar 4, 2023

veprbl commented Aug 30, 2021 •

edited

Loading

veprbl commented Oct 7, 2021 •

edited

Loading

RossBencina commented Nov 4, 2021 •

edited

Loading