diff mbox series

[1 of 3] host-acl: build fix on RHEL6 hosts (GCC 4.4.7)

Message ID 05bbe42a7a77f46daa8a.1532020842@cveaol6qa08.wv.mentorg.com
State Changes Requested
Headers show
Series [1 of 3] host-acl: build fix on RHEL6 hosts (GCC 4.4.7) | expand

Commit Message

Hollis Blanchard July 19, 2018, 5:20 p.m. UTC 
This didn't introduce any build warnings, FWIW.

Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>

Comments

Thomas Petazzoni July 25, 2018, 9:12 p.m. UTC | #1 
Hello,

On Thu, 19 Jul 2018 10:20:42 -0700, Hollis Blanchard wrote:
> This didn't introduce any build warnings, FWIW.
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> 
> diff --git a/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
> new file mode 100644
> --- /dev/null
> +++ b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
> @@ -0,0 +1,21 @@
> +build with old GCC versions
> +
> +GCC 4.4.7, as found in RHEL6, reports:
> +	libacl/acl_from_text.c:307: error: #pragma GCC diagnostic not allowed inside functions
> +
> +Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>

Are you sure removing this won't break the build with newer gcc
versions ?

In any case, for all three of your patches, could you please use Git to
generate the package patches themselves ? All three packages being
patched use Git upstream, and we very much like having Git formatted
patches in this case.

However, a larger question is: do you intend to contribute an
autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
just the few packages that are useful to you, but if we are going to
support something as old as RHEL6, then we need to do it properly and
have an autobuilder running on this distro. Do you think this is
something that could be done ?

Thanks!

Thomas
Hollis Blanchard July 25, 2018, 10:10 p.m. UTC | #2 
On 07/25/2018 02:12 PM, Thomas Petazzoni wrote:
> Hello,
>
> On Thu, 19 Jul 2018 10:20:42 -0700, Hollis Blanchard wrote:
>> This didn't introduce any build warnings, FWIW.
>>
>> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
>>
>> diff --git a/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
>> new file mode 100644
>> --- /dev/null
>> +++ b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
>> @@ -0,0 +1,21 @@
>> +build with old GCC versions
>> +
>> +GCC 4.4.7, as found in RHEL6, reports:
>> +	libacl/acl_from_text.c:307: error: #pragma GCC diagnostic not allowed inside functions
>> +
>> +Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> Are you sure removing this won't break the build with newer gcc
> versions ?
I've built it with host gcc 6.3.0 and 4.4.7 and saw no warnings in 
either case. (Aside: patchelf refused to build with 6.3.0, so I had to 
manually build that.)

If some toolchain emits a warning on this code, and if the package is 
built with -Werror somehow, I suppose it's possible to break the build. 
If you'd like, I will try relocating the pragma outside the whole 
function, which could only hide more warnings and thus be less likely to 
break the build.
> In any case, for all three of your patches, could you please use Git to
> generate the package patches themselves ? All three packages being
> patched use Git upstream, and we very much like having Git formatted
> patches in this case.
OK.
> However, a larger question is: do you intend to contribute an
> autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
> just the few packages that are useful to you, but if we are going to
> support something as old as RHEL6, then we need to do it properly and
> have an autobuilder running on this distro. Do you think this is
> something that could be done ?
Not certain, but possible. What does it entail, exactly? Do I just have 
to run 
https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run via 
cron?

Hollis Blanchard
Mentor Graphics Emulation Division
Thomas Petazzoni July 26, 2018, 7:43 a.m. UTC | #3 
Hello Hollis,

On Wed, 25 Jul 2018 15:10:56 -0700, Hollis Blanchard wrote:

> >> +Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>  
> > Are you sure removing this won't break the build with newer gcc
> > versions ?  
> I've built it with host gcc 6.3.0 and 4.4.7 and saw no warnings in 
> either case. (Aside: patchelf refused to build with 6.3.0, so I had to 
> manually build that.)
> 
> If some toolchain emits a warning on this code, and if the package is 
> built with -Werror somehow, I suppose it's possible to break the build. 

The package doesn't seem to be built with -Werror by default, so it
should be fine. Let's apply your patch, and see if it works fine on the
autobuilders.

> > However, a larger question is: do you intend to contribute an
> > autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
> > just the few packages that are useful to you, but if we are going to
> > support something as old as RHEL6, then we need to do it properly and
> > have an autobuilder running on this distro. Do you think this is
> > something that could be done ?  
> Not certain, but possible. What does it entail, exactly? Do I just have 
> to run 
> https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run via 
> cron?

It is about running this script yes, but it does not need to be run in
cron, the script itself is an infinite loop that continuously does some
builds and contribute the results to autobuild.buildroot.org.

Best regards,

Thomas
Arnout Vandecappelle July 26, 2018, 7:55 a.m. UTC | #4 
On 26-07-18 09:43, Thomas Petazzoni wrote:
> Hello Hollis,
> 
> On Wed, 25 Jul 2018 15:10:56 -0700, Hollis Blanchard wrote:
> 
>>>> +Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>  
[snip]
>>> However, a larger question is: do you intend to contribute an
>>> autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
>>> just the few packages that are useful to you, but if we are going to
>>> support something as old as RHEL6, then we need to do it properly and
>>> have an autobuilder running on this distro. Do you think this is
>>> something that could be done ?  
>> Not certain, but possible. What does it entail, exactly? Do I just have 
>> to run 
>> https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run via 
>> cron?
> 
> It is about running this script yes, but it does not need to be run in
> cron, the script itself is an infinite loop that continuously does some
> builds and contribute the results to autobuild.buildroot.org.

 You also need disk space, at least 100GB per job I think? That's the sole
reason why I'm not running an autobuilder myself. It's very annoying to get
spurious failures because of -ENOSPC.

 Regards,
 Arnout
Hollis Blanchard July 27, 2018, 7:30 p.m. UTC | #5 
On 07/26/2018 12:43 AM, Thomas Petazzoni wrote:
>
>>> However, a larger question is: do you intend to contribute an
>>> autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
>>> just the few packages that are useful to you, but if we are going to
>>> support something as old as RHEL6, then we need to do it properly and
>>> have an autobuilder running on this distro. Do you think this is
>>> something that could be done ?
>> Not certain, but possible. What does it entail, exactly? Do I just have
>> to run
>> https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run via
>> cron?
> It is about running this script yes, but it does not need to be run in
> cron, the script itself is an infinite loop that continuously does some
> builds and contribute the results to autobuild.buildroot.org.

OK, I set this up on a RHEL6.5 host, and it ran enough to hit the known 
libglib2 problem <https://patchwork.ozlabs.org/patch/947086/>. I 
encountered two issues along the way:

  * buildroot-test/buildroot-autobuild script uses the --detach option
    to 'git checkout', which RHEL6.5's git doesn't know about. I just
    deleted the option and everything's working fine. I can submit a
    patch if you like.
  * buildroot/utils/genrandconfig uses argparse, which doesn't exist in
    RHEL6.5's Python. I avoided the problem for now by supplying an
    alternate Python installation.

I guess I just need HTTP credentials to submit to 
http://autobuild.buildroot.org ?

Hollis Blanchard
Mentor Graphics Emulation Division
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 07/26/2018 12:43 AM, Thomas Petazzoni wrote:<br>
    <blockquote type="cite" cite="mid:20180726094334.4da615cf@windsurf"><br>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="">However, a larger question is: do you intend to contribute an
autobuilder that tests Buildroot under RHEL6 ? Indeed, you are fixing
just the few packages that are useful to you, but if we are going to
support something as old as RHEL6, then we need to do it properly and
have an autobuilder running on this distro. Do you think this is
something that could be done ?  
</pre>
        </blockquote>
        <pre wrap="">Not certain, but possible. What does it entail, exactly? Do I just have 
to run 
<a class="moz-txt-link-freetext" href="https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run">https://git.buildroot.net/buildroot-test/tree/scripts/autobuild-run</a> via 
cron?
</pre>
      </blockquote>
      <pre wrap="">
It is about running this script yes, but it does not need to be run in
cron, the script itself is an infinite loop that continuously does some
builds and contribute the results to autobuild.buildroot.org.
</pre>
    </blockquote>
    <br>
    OK, I set this up on a RHEL6.5 host, and it ran enough to hit the <a
      moz-do-not-send="true"
      href="https://patchwork.ozlabs.org/patch/947086/">known libglib2
      problem</a>. I encountered two issues along the way:<br>
    <ul>
      <li>buildroot-test/buildroot-autobuild script uses the --detach
        option to 'git checkout', which RHEL6.5's git doesn't know
        about. I just deleted the option and everything's working fine.
        I can submit a patch if you like.<br>
      </li>
      <li>buildroot/utils/genrandconfig uses argparse, which doesn't
        exist in RHEL6.5's Python. I avoided the problem for now by
        supplying an alternate Python installation.<br>
      </li>
    </ul>
    <p>I guess I just need HTTP credentials to submit to
      <a class="moz-txt-link-freetext" href="http://autobuild.buildroot.org">http://autobuild.buildroot.org</a> ?<br>
    </p>
    <pre class="moz-signature" cols="72">Hollis Blanchard
Mentor Graphics Emulation Division</pre>
  </body>
</html>
Thomas Petazzoni July 30, 2018, 12:28 p.m. UTC | #6 
Hello Hollis,

+Arnout in Cc about the --detach option.

On Fri, 27 Jul 2018 12:30:15 -0700, Hollis Blanchard wrote:

> OK, I set this up on a RHEL6.5 host, and it ran enough to hit the known 
> libglib2 problem <https://patchwork.ozlabs.org/patch/947086/>. I 
> encountered two issues along the way:

Great!

>   * buildroot-test/buildroot-autobuild script uses the --detach option
>     to 'git checkout', which RHEL6.5's git doesn't know about. I just
>     deleted the option and everything's working fine. I can submit a
>     patch if you like.

This option was added in commit
66f91eb4409e5efacb1871b8ab2b336c4873b796, and Arnout gave the following
reason:

    Since switching repositories may also switch branches, we use a git
    fetch/checkout sequence instead of doing a git pull. With git pull, the
    branches would be merged instead of switched. To avoid polluting the
    log with the long git message about a detached head, while still
    getting some useful output from git, pass the --detach option to
    git checkout.

Since it's apparently just about logging, perhaps we can just remove it. Arnout ?

>   * buildroot/utils/genrandconfig uses argparse, which doesn't exist in
>     RHEL6.5's Python. I avoided the problem for now by supplying an
>     alternate Python installation.

Sounds good.

> I guess I just need HTTP credentials to submit to 
> http://autobuild.buildroot.org ?

I'll reply to you privately about this.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
new file mode 100644
--- /dev/null
+++ b/package/acl/0001-pragma-gcc-diagnostic-in-fn.patch
@@ -0,0 +1,21 @@ 
+build with old GCC versions
+
+GCC 4.4.7, as found in RHEL6, reports:
+	libacl/acl_from_text.c:307: error: #pragma GCC diagnostic not allowed inside functions
+
+Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
+
+--- host-acl-2.2.53/libacl/acl_from_text.c.orig	2018-07-19 09:15:40.425940094 -0700
++++ host-acl-2.2.53/libacl/acl_from_text.c	2018-07-19 09:15:50.777940093 -0700
+@@ -304,11 +304,8 @@
+ create_entry:
+ 	if (acl_create_entry(acl_p, &entry_d) != 0)
+ 		return -1;
+-#pragma GCC diagnostic push
+-#pragma GCC diagnostic ignored "-Waddress"
+ 	if (acl_copy_entry(entry_d, int2ext(&entry_obj)) != 0)
+ 		return -1;
+-#pragma GCC diagnostic pop
+ 	return 0;
+ 
+ fail: