[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Tal Zilcer]

From: Tal Zilcer <talz@ezchip.com>

When working with GCC initial at override source dir mode the
HOST_GCC_INITIAL_POST_PATCH_HOOKS is not called and compilation failes.
The solution is to use HOST_GCC_INITIAL_POST_RSYNC_HOOKS since this hook
is being called at override source dir mode.

Signed-off-by: Tal Zilcer <talz@ezchip.com>
---
 package/gcc/gcc-initial/gcc-initial.mk |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/package/gcc/gcc-initial/gcc-initial.mk b/package/gcc/gcc-initial/gcc-initial.mk
index 0fa2353..6b747d0 100644
--- a/package/gcc/gcc-initial/gcc-initial.mk
+++ b/package/gcc/gcc-initial/gcc-initial.mk
@@ -33,7 +33,11 @@ define HOST_GCC_INITIAL_DISABLE_LIBGMON
 	$(SED) 's/crtbeginS.o libgmon.a crtg.o/crtbeginS.o crtg.o/' \
 		$(@D)/libgcc/config.host
 endef
+ifeq ($(HOST_GCC_INITIAL_OVERRIDE_SRCDIR),)
 HOST_GCC_INITIAL_POST_PATCH_HOOKS += HOST_GCC_INITIAL_DISABLE_LIBGMON
+else
+HOST_GCC_INITIAL_POST_RSYNC_HOOKS += HOST_GCC_INITIAL_DISABLE_LIBGMON
+endif
 endif
 
 HOST_GCC_INITIAL_CONF_OPTS = \
-- 
1.7.1

[Buildroot] [PATCH 2/2] package/pkg-kconfig: define KCONFIG_EDITORS_MAKE_ENV

[Tal Zilcer]

From: Tal Zilcer <talz@ezchip.com>

When building a configuration editor we assume its for the HOST.
However some packages might define MAKE_ENV as TARGET_MAKE_NEV.
Using this variable adds to beginning of $(PATH),
the directory $(HOST_DIR)/usr/bin.

This cause us to not use pkg-config from our host machine but the one
from $(HOST_DIR)/usr/bin. This pkg-config does not support QT.
Now when we try to run for example linux-xconfig we fail since QT is not found.

To overcome this we defined a dedicated variable for the configuration editors

Signed-off-by: Tal Zilcer <talz@ezchip.com>
---
 package/pkg-kconfig.mk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 7a00fff..a066381 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -76,7 +76,7 @@ endif
 
 # Configuration editors (menuconfig, ...)
 $$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS)): $$($(2)_DIR)/.stamp_kconfig_fixup_done
-	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
+	$$($(2)_KCONFIG_EDITORS_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
 		$$($(2)_KCONFIG_OPTS) $$(subst $(1)-,,$$@)
 	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
 	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
-- 
1.7.1

[Buildroot] [PATCH 2/2] package/pkg-kconfig: define KCONFIG_EDITORS_MAKE_ENV

[Alexey Brodkin]

Hi Tal,

On Mon, 2015-06-29 at 15:50 +0300, Tal Zilcer wrote:
> From: Tal Zilcer <talz@ezchip.com>, 
> 
> When building a configuration editor we assume its for the HOST.
> However some packages might define MAKE_ENV as TARGET_MAKE_NEV.
> Using this variable adds to beginning of $(PATH),
> the directory $(HOST_DIR)/usr/bin.
> 
> This cause us to not use pkg-config from our host machine but the one
> from $(HOST_DIR)/usr/bin. This pkg-config does not support QT.
> Now when we try to run for example linux-xconfig we fail since QT is 
> not found.
> 
> To overcome this we defined a dedicated variable for the 
> configuration editors
> 
> Signed-off-by: Tal Zilcer <talz@ezchip.com>
> ---

I'm wondering why did you put 2 patches in one series?
 [1] https://patchwork.ozlabs.org/patch/489307/
 [2] https://patchwork.ozlabs.org/patch/489306/

I'd say these 2 patches solve completely different problems and so
should be posted as 2 separate patches compared to your [PATCH 1/2],
[PATCH 2/2].

The problem with series patches within series could not be applied
separately, that means chances are you may not get either applied
because:
 a) At least one patch could not be accepted by maintainer
 b) At least one patch required rework

So in case of (a) you'll need to resend appropriate patch separately
and in case of (b) you'll need to resend all patches in the series even
if only one patch was modified.

As for this one I really like it. IMHO problem and solution are very
valid.

The only note this patch won't apply clean on current master due to
recent changes in "package/pkg-kconfig.mk". I.e. rebase is required
now.

Please rebase this patch on top of the current master and resend this
patch (preferably separately from another patch of the current series).

Feel free to add:
Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>

-Alexey

[Buildroot] [PATCH 2/2] package/pkg-kconfig: define KCONFIG_EDITORS_MAKE_ENV

[Thomas Petazzoni]

Dear Alexey Brodkin,

On Thu, 2 Jul 2015 07:44:05 +0000, Alexey Brodkin wrote:

> I'm wondering why did you put 2 patches in one series?
>  [1] https://patchwork.ozlabs.org/patch/489307/
>  [2] https://patchwork.ozlabs.org/patch/489306/
> 
> I'd say these 2 patches solve completely different problems and so
> should be posted as 2 separate patches compared to your [PATCH 1/2],
> [PATCH 2/2].

I agree they are not related.

> The problem with series patches within series could not be applied
> separately, that means chances are you may not get either applied
> because:

That's not really true. I tend to try to apply all patches I can from a
series, even if certain other patches of the series cannot be applied.
Look at the SELinux series for example. I have not applied some of the
first patches of the series, but already applied some patches later in
the series.

So while I agree it's a bit weird to mix completely independent stuff in
one single patch series, it's not actually that problematic.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/pkg-kconfig: define KCONFIG_EDITORS_MAKE_ENV

[Alexey Brodkin]

Hi Thomas,

On Thu, 2015-07-02 at 09:52 +0200, Thomas Petazzoni wrote:
> Dear Alexey Brodkin,
> 
> On Thu, 2 Jul 2015 07:44:05 +0000, Alexey Brodkin wrote:
> 
> > I'm wondering why did you put 2 patches in one series?
> >  [1] https://patchwork.ozlabs.org/patch/489307/
> >  [2] https://patchwork.ozlabs.org/patch/489306/
> > 
> > I'd say these 2 patches solve completely different problems and so
> > should be posted as 2 separate patches compared to your [PATCH 
> > 1/2],
> > [PATCH 2/2].
> 
> I agree they are not related.
> 
> > The problem with series patches within series could not be applied
> > separately, that means chances are you may not get either applied
> > because:
> 
> That's not really true. I tend to try to apply all patches I can from 
> a
> series, even if certain other patches of the series cannot be 
> applied.
> Look at the SELinux series for example. I have not applied some of 
> the
> first patches of the series, but already applied some patches later 
> in
> the series.
> 
> So while I agree it's a bit weird to mix completely independent stuff 
> in
> one single patch series, it's not actually that problematic.

Makes sense.

What about mentioned problem with patch rebase that is only required
because of line number changes?

In this particular case a few more lines were added in the file and so
"git am" fails while "patch" succeeds with message about offset like
that:
----------------->8-----------------
patching file package/pkg-kconfig.mk
Hunk #1 succeeded at 109 with fuzz 2 (offset 33 lines).
----------------->8-----------------

Is my proposal for resend is valid or you may handle it yourself?

-Alexey

[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Thomas Petazzoni]

Dear Tal Zilcer,

On Mon, 29 Jun 2015 15:50:26 +0300, Tal Zilcer wrote:

> diff --git a/package/gcc/gcc-initial/gcc-initial.mk b/package/gcc/gcc-initial/gcc-initial.mk
> index 0fa2353..6b747d0 100644
> --- a/package/gcc/gcc-initial/gcc-initial.mk
> +++ b/package/gcc/gcc-initial/gcc-initial.mk
> @@ -33,7 +33,11 @@ define HOST_GCC_INITIAL_DISABLE_LIBGMON
>  	$(SED) 's/crtbeginS.o libgmon.a crtg.o/crtbeginS.o crtg.o/' \
>  		$(@D)/libgcc/config.host
>  endef
> +ifeq ($(HOST_GCC_INITIAL_OVERRIDE_SRCDIR),)
>  HOST_GCC_INITIAL_POST_PATCH_HOOKS += HOST_GCC_INITIAL_DISABLE_LIBGMON
> +else
> +HOST_GCC_INITIAL_POST_RSYNC_HOOKS += HOST_GCC_INITIAL_DISABLE_LIBGMON
> +endif
>  endif
>  
>  HOST_GCC_INITIAL_CONF_OPTS = \

If you're using <pkg>_OVERRIDE_SRCDIR, then the assumption is that your
source tree is patched with whatever change is needed for the package
to build fine without applying any of the Buildroot patches.

In any case, I don't think it's worth the effort making that
conditional, just register the post-rsync hook unconditionally.

BTW, this problem should have been sorted out by the ARC developers,
maybe you should ping them about this?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Tal Zilcer]

From: Thomas Petazzoni [mailto:thomas.petazzoni at free-electrons.com] 
Sent: Monday, June 29, 2015 5:29 PM

> If you're using <pkg>_OVERRIDE_SRCDIR, then the assumption is that your source tree is patched with whatever change is needed for the package to build fine without applying any of the Buildroot patches.
We have one source tree for both build phases of GCC.
First phase must have this patch while second phase must NOT have it.
it doesn't seems reasonable to have two GCC source trees because of this patch.

> In any case, I don't think it's worth the effort making that conditional, just register the post-rsync hook unconditionally.
It is only to explain our intentions, do you prefer a comment or just to add the RSYNC hook?

> BTW, this problem should have been sorted out by the ARC developers, maybe you should ping them about this?
Adding Alexey to this correspondence

Thanks,

Tal

[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Thomas Petazzoni]

Hello,

On Mon, 29 Jun 2015 15:39:12 +0000, Tal Zilcer wrote:

> We have one source tree for both build phases of GCC.
> First phase must have this patch while second phase must NOT have it.
> it doesn't seems reasonable to have two GCC source trees because of this patch.

Ah right. I think there's probably a way to write the change so that it
does not try to use these object files during the initial gcc build.
This is really what the real fix is.

> > In any case, I don't think it's worth the effort making that conditional, just register the post-rsync hook unconditionally.
> It is only to explain our intentions, do you prefer a comment or just to add the RSYNC hook?

For now, we can just add the rsync hook, but...

> > BTW, this problem should have been sorted out by the ARC developers, maybe you should ping them about this?
> Adding Alexey to this correspondence

... I'd like to hear from Alexey about this, since I reported this
issue a while ago now.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/gcc: fix ARC failure to build in 2 phases.

[Alexey Brodkin]

Hi Thomas, Tal,

On Mon, 2015-06-29 at 17:49 +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 29 Jun 2015 15:39:12 +0000, Tal Zilcer wrote:
> 
> > We have one source tree for both build phases of GCC.
> > First phase must have this patch while second phase must NOT have 
> > it.
> > it doesn't seems reasonable to have two GCC source trees because of 
> > this patch.
> 
> Ah right. I think there's probably a way to write the change so that 
> it
> does not try to use these object files during the initial gcc build.
> This is really what the real fix is.
> 
> > > In any case, I don't think it's worth the effort making that 
> > > conditional, just register the post-rsync hook unconditionally.
> > It is only to explain our intentions, do you prefer a comment or 
> > just to add the RSYNC hook?
> 
> For now, we can just add the rsync hook, but...
> 
> > > BTW, this problem should have been sorted out by the ARC 
> > > developers, maybe you should ping them about this?
> > Adding Alexey to this correspondence
> 
> ... I'd like to hear from Alexey about this, since I reported this
> issue a while ago now.

Please pardon my late reply was super busy with internal activities.

As for known problem of gmon that includes libc headers unfortunately
nothing has been done on that front. That's because we have a
workaround with disabling libgmon in Buildroot.

I'll ping our gcc guy but I don't think things will change soon here
again because it doesn't look as a blocker.

So probably rsync hook is a way to go for now.

-Alexey