[v1] automation/alpine: add g++ to the list of build depends

[PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Roger Pau Monne 3 years, 1 month ago

clang++ relies on the C++ headers installed by g++, or else a clang
build will hit the following error:

<built-in>:3:10: fatal error: 'cstring' file not found
#include "cstring"
         ^~~~~~~~~
1 error generated.
make[10]: *** [Makefile:120: headers++.chk] Error 1

Reported-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
No real risk here from a release PoV, it's just pulling a package
required for the Alpine clang build. Worse that cold happen is that
the Alpine clang build broke, but it's already broken.
---
 automation/build/alpine/3.12.dockerfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/automation/build/alpine/3.12.dockerfile b/automation/build/alpine/3.12.dockerfile
index 2c02417ee6..94557e239b 100644
--- a/automation/build/alpine/3.12.dockerfile
+++ b/automation/build/alpine/3.12.dockerfile
@@ -21,6 +21,7 @@ RUN \
   apk add curl-dev && \
   apk add dev86 && \
   apk add gcc  && \
+  apk add g++ && \
   apk add clang  && \
   apk add gettext && \
   apk add git && \
-- 
2.30.1

Re: [PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Ian Jackson 3 years, 1 month ago

Roger Pau Monne writes ("[PATCH for-4.15] automation/alpine: add g++ to the list of build depends"):
> clang++ relies on the C++ headers installed by g++, or else a clang
> build will hit the following error:
> 
> <built-in>:3:10: fatal error: 'cstring' file not found
> #include "cstring"
>          ^~~~~~~~~
> 1 error generated.
> make[10]: *** [Makefile:120: headers++.chk] Error 1
> 
> Reported-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Jackson <iwj@xenproject.org>
> No real risk here from a release PoV, it's just pulling a package
> required for the Alpine clang build. Worse that cold happen is that
> the Alpine clang build broke, but it's already broken.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Re: [PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Andrew Cooper 3 years, 1 month ago

On 01/03/2021 09:58, Roger Pau Monne wrote:
> clang++ relies on the C++ headers installed by g++, or else a clang
> build will hit the following error:
>
> <built-in>:3:10: fatal error: 'cstring' file not found
> #include "cstring"
>          ^~~~~~~~~
> 1 error generated.
> make[10]: *** [Makefile:120: headers++.chk] Error 1
>
> Reported-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Jackson <iwj@xenproject.org>
> No real risk here from a release PoV, it's just pulling a package
> required for the Alpine clang build. Worse that cold happen is that
> the Alpine clang build broke, but it's already broken.

Shouldn't this be fixed upstream in Alpine?  Its clearly a packaging bug.

~Andrew

Re: [PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Andrew Cooper 3 years, 1 month ago

On 01/03/2021 17:59, Andrew Cooper wrote:
> On 01/03/2021 09:58, Roger Pau Monne wrote:
>> clang++ relies on the C++ headers installed by g++, or else a clang
>> build will hit the following error:
>>
>> <built-in>:3:10: fatal error: 'cstring' file not found
>> #include "cstring"
>>          ^~~~~~~~~
>> 1 error generated.
>> make[10]: *** [Makefile:120: headers++.chk] Error 1
>>
>> Reported-by: Stefano Stabellini <sstabellini@kernel.org>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> Cc: Ian Jackson <iwj@xenproject.org>
>> No real risk here from a release PoV, it's just pulling a package
>> required for the Alpine clang build. Worse that cold happen is that
>> the Alpine clang build broke, but it's already broken.
> Shouldn't this be fixed upstream in Alpine?  Its clearly a packaging bug.

Or (thinking about it), we've got a build system bug using g++ when it
should be using clang++.

How does this check work (without g++) on a non-clang build?

~Andrew

Re: [PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Roger Pau Monné 3 years, 1 month ago

On Mon, Mar 01, 2021 at 06:01:36PM +0000, Andrew Cooper wrote:
> On 01/03/2021 17:59, Andrew Cooper wrote:
> > On 01/03/2021 09:58, Roger Pau Monne wrote:
> >> clang++ relies on the C++ headers installed by g++, or else a clang
> >> build will hit the following error:
> >>
> >> <built-in>:3:10: fatal error: 'cstring' file not found
> >> #include "cstring"
> >>          ^~~~~~~~~
> >> 1 error generated.
> >> make[10]: *** [Makefile:120: headers++.chk] Error 1
> >>
> >> Reported-by: Stefano Stabellini <sstabellini@kernel.org>
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> Cc: Ian Jackson <iwj@xenproject.org>
> >> No real risk here from a release PoV, it's just pulling a package
> >> required for the Alpine clang build. Worse that cold happen is that
> >> the Alpine clang build broke, but it's already broken.
> > Shouldn't this be fixed upstream in Alpine?  Its clearly a packaging bug.
> 
> Or (thinking about it), we've got a build system bug using g++ when it
> should be using clang++.

No, the check is using clang++, the issue is that clang++ doesn't
install the standard c++ headers, and thus trying to use them (cstring
in this case) fails. Installing the g++ package solves the issue
because it installs the headers.

> How does this check work (without g++) on a non-clang build?

It works because on a non-clang Alpine build g++ wasn't installed, so
the check wasd just skipped. The headers++.chk target in
xen/include/Makefile has a explicit check for a functional CXX.

I do think this is some kind of error in the Alpine clang package, as
I assume installing clang++ without the standard c++ headers is an
error. Regardless of that, I also think installing g++ on the Alpine
container is fine, as we would then also perform the C++ header checks
with g++ on the normal build.

Thanks, Roger.

Re: [PATCH for-4.15] automation/alpine: add g++ to the list of build depends

Posted by Jan Beulich 3 years, 1 month ago

On 02.03.2021 09:14, Roger Pau Monné wrote:
> On Mon, Mar 01, 2021 at 06:01:36PM +0000, Andrew Cooper wrote:
>> On 01/03/2021 17:59, Andrew Cooper wrote:
>>> On 01/03/2021 09:58, Roger Pau Monne wrote:
>>>> clang++ relies on the C++ headers installed by g++, or else a clang
>>>> build will hit the following error:
>>>>
>>>> <built-in>:3:10: fatal error: 'cstring' file not found
>>>> #include "cstring"
>>>>          ^~~~~~~~~
>>>> 1 error generated.
>>>> make[10]: *** [Makefile:120: headers++.chk] Error 1
>>>>
>>>> Reported-by: Stefano Stabellini <sstabellini@kernel.org>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> ---
>>>> Cc: Ian Jackson <iwj@xenproject.org>
>>>> No real risk here from a release PoV, it's just pulling a package
>>>> required for the Alpine clang build. Worse that cold happen is that
>>>> the Alpine clang build broke, but it's already broken.
>>> Shouldn't this be fixed upstream in Alpine?  Its clearly a packaging bug.
>>
>> Or (thinking about it), we've got a build system bug using g++ when it
>> should be using clang++.
> 
> No, the check is using clang++, the issue is that clang++ doesn't
> install the standard c++ headers, and thus trying to use them (cstring
> in this case) fails. Installing the g++ package solves the issue
> because it installs the headers.

I have to admit that I consider this odd. The g++ package should
neither provide nor depend on the headers. It may recommend their
installation. On my distro (SLES) the headers come from the
libstdc++-devel package, as I would have expected. There
additionally is a dependency of libclang5 (no -devel suffix!) on
libstdc++-devel (I suppose this is an indication that things
aren't quite right here either; I haven't checked an up-to-date
version of the distro yet, though).

Jan