[v2,1/2] Strip binaries in the rootfs creation instead of in target-finalize

From: Carlos Santos <unixmania@gmail.com>

Since commit 118534fe54 the root filesystem image is generated from a
temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
only.

This allows us to easily find the non-stripped executables to debug with
gdbserver, as they are at the same relative path in TARGET_DIR as in the
target device, rather than searching inside the build directory.

Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386

Signed-off-by: Carlos Santos <unixmania@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
CC: Ciro Santilli <ciro.santilli@gmail.com>
---
Changes v1->v2:
- Strip before running the fakeroot script, as suggested by Arnout
  Vandecappelle
- Change commit message accordingly. Removed paragraph about setting
  sysroot to TARGET_DIR in gdb, which is done in the next commit.
---
 Makefile     | 34 ----------------------------------
 fs/common.mk | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 35 deletions(-)

On Mon, 30 Sep 2019 22:36:53 -0300
unixmania@gmail.com wrote:

> From: Carlos Santos <unixmania@gmail.com>
> 
> Since commit 118534fe54 the root filesystem image is generated from a
> temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
> only.
> 
> This allows us to easily find the non-stripped executables to debug with
> gdbserver, as they are at the same relative path in TARGET_DIR as in the
> target device, rather than searching inside the build directory.
> 
> Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

I see Arnout reviewed-by this, and while I understand the goal, I am
not sure this is the real direction/solution we want for this problem.
I believe the real solution we have been talking about for a long time
is to install everything in STAGING_DIR (not just libraries). This way,
unstripped binaries with debugging symbols with all be in STAGING_DIR.

To me, the change being proposed here makes TARGET_DIR really different
from what ends up in the final rootfs image. While we admittely already
have a few differences, I personally like to keep the idea of
TARGET_DIR being really what's on the target root filesystem, with as
few differences as possible.

Thomas

On Tue, Oct 1, 2019 at 3:52 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 30 Sep 2019 22:36:53 -0300
> unixmania@gmail.com wrote:
>
> > From: Carlos Santos <unixmania@gmail.com>
> >
> > Since commit 118534fe54 the root filesystem image is generated from a
> > temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
> > only.
> >
> > This allows us to easily find the non-stripped executables to debug with
> > gdbserver, as they are at the same relative path in TARGET_DIR as in the
> > target device, rather than searching inside the build directory.
> >
> > Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386
> >
> > Signed-off-by: Carlos Santos <unixmania@gmail.com>
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> I see Arnout reviewed-by this, and while I understand the goal, I am
> not sure this is the real direction/solution we want for this problem.
> I believe the real solution we have been talking about for a long time
> is to install everything in STAGING_DIR (not just libraries). This way,
> unstripped binaries with debugging symbols with all be in STAGING_DIR.
>
> To me, the change being proposed here makes TARGET_DIR really different
> from what ends up in the final rootfs image. While we admittely already
> have a few differences, I personally like to keep the idea of
> TARGET_DIR being really what's on the target root filesystem, with as
> few differences as possible.

We could make it optional, keeping the the current behavior by default.

Thomas, Carlos, Arnout, All,

On 2019-10-01 08:52 +0200, Thomas Petazzoni spake thusly:
> On Mon, 30 Sep 2019 22:36:53 -0300
> unixmania@gmail.com wrote:
> 
> > From: Carlos Santos <unixmania@gmail.com>
> > 
> > Since commit 118534fe54 the root filesystem image is generated from a
> > temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
> > only.
> > 
> > This allows us to easily find the non-stripped executables to debug with
> > gdbserver, as they are at the same relative path in TARGET_DIR as in the
> > target device, rather than searching inside the build directory.
> > 
> > Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386
> > 
> > Signed-off-by: Carlos Santos <unixmania@gmail.com>
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> I see Arnout reviewed-by this, and while I understand the goal, I am
> not sure this is the real direction/solution we want for this problem.
> I believe the real solution we have been talking about for a long time
> is to install everything in STAGING_DIR (not just libraries). This way,
> unstripped binaries with debugging symbols with all be in STAGING_DIR.

I agree with Thomas here.

The dichotomy between staging/ and target/ has no raison-d'être nowadays.

The idea would be to ensure that all packages install in staging/,
always. Then, identify packages that have different commands for target/
and staging/ and make them identical. Finally, remove the target install
commands, and genreate it as a copy of staging at the beginning of the
target-finalize step.

Yes, the real patches will be a bit more gory than the ideal view above.
But except for some special snowflakes (qt5, I'm looking at you), it
should be fairly straghtforward...

> To me, the change being proposed here makes TARGET_DIR really different
> from what ends up in the final rootfs image. While we admittely already
> have a few differences, I personally like to keep the idea of
> TARGET_DIR being really what's on the target root filesystem, with as
> few differences as possible.

I beg to slightly disagree here, and with a much more controversial view
on the topic: we should just get rid of target/ altogether. The only
valid artefacts we generates are the images in images/

But that one is definitely not on the table. ;-)

Regards,
Yann E. MORIN.

> Thomas
> -- 
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Hello Carlos,

On Mon, 30 Sep 2019 22:36:53 -0300
unixmania@gmail.com wrote:

> From: Carlos Santos <unixmania@gmail.com>
> 
> Since commit 118534fe54 the root filesystem image is generated from a
> temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
> only.
> 
> This allows us to easily find the non-stripped executables to debug with
> gdbserver, as they are at the same relative path in TARGET_DIR as in the
> target device, rather than searching inside the build directory.
> 
> Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386
> 
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> CC: Ciro Santilli <ciro.santilli@gmail.com>
> ---
> Changes v1->v2:
> - Strip before running the fakeroot script, as suggested by Arnout
>   Vandecappelle
> - Change commit message accordingly. Removed paragraph about setting
>   sysroot to TARGET_DIR in gdb, which is done in the next commit.
> ---
>  Makefile     | 34 ----------------------------------
>  fs/common.mk | 36 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 35 insertions(+), 35 deletions(-)

There's been feedback from both Yann and me on this patch, and both of
us think this is not the approach we want to take. Instead, we'd rather
see everything installed to STAGING_DIR as the way of fixing the
original issue.

So I've marked both patches as Rejected in patchwork. Of course, if
other people disagree with this decision, we can always revisit and
rediscuss the matter.

Thanks!

Thomas

On Mon, Apr 13, 2020 at 11:02 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Carlos,
>
> On Mon, 30 Sep 2019 22:36:53 -0300
> unixmania@gmail.com wrote:
>
> > From: Carlos Santos <unixmania@gmail.com>
> >
> > Since commit 118534fe54 the root filesystem image is generated from a
> > temporary copy of TARGET_DIR, so we can strip the binaries in the copy,
> > only.
> >
> > This allows us to easily find the non-stripped executables to debug with
> > gdbserver, as they are at the same relative path in TARGET_DIR as in the
> > target device, rather than searching inside the build directory.
> >
> > Fixes: https://bugs.busybox.net/show_bug.cgi?id=10386
> >
> > Signed-off-by: Carlos Santos <unixmania@gmail.com>
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > ---
> > CC: Ciro Santilli <ciro.santilli@gmail.com>
> > ---
> > Changes v1->v2:
> > - Strip before running the fakeroot script, as suggested by Arnout
> >   Vandecappelle
> > - Change commit message accordingly. Removed paragraph about setting
> >   sysroot to TARGET_DIR in gdb, which is done in the next commit.
> > ---
> >  Makefile     | 34 ----------------------------------
> >  fs/common.mk | 36 +++++++++++++++++++++++++++++++++++-
> >  2 files changed, 35 insertions(+), 35 deletions(-)
>
> There's been feedback from both Yann and me on this patch, and both of
> us think this is not the approach we want to take. Instead, we'd rather
> see everything installed to STAGING_DIR as the way of fixing the
> original issue.
>
> So I've marked both patches as Rejected in patchwork. Of course, if
> other people disagree with this decision, we can always revisit and
> rediscuss the matter.

Just to remind you, the problem was reported two and a half years ago
and the situation is still the same. Perfect is the enemy of good and
later easily becomes never. Anyway, your project, your rules.

Hello Carlos,

On Mon, 13 Apr 2020 20:41:11 -0300
Carlos Santos <unixmania@gmail.com> wrote:

> > There's been feedback from both Yann and me on this patch, and both of
> > us think this is not the approach we want to take. Instead, we'd rather
> > see everything installed to STAGING_DIR as the way of fixing the
> > original issue.
> >
> > So I've marked both patches as Rejected in patchwork. Of course, if
> > other people disagree with this decision, we can always revisit and
> > rediscuss the matter.  
> 
> Just to remind you, the problem was reported two and a half years ago
> and the situation is still the same. Perfect is the enemy of good and
> later easily becomes never. Anyway, your project, your rules.

Why do you have to be so aggressive immediately ? Don't you have in
your palette of expression, some intermediate feelings, where you can
express concern and possibly frustration, without the agression ? Be
constructive instead of aggressive ?

It is deeply annoying that we can never say anything about your patches
and the approach you're taking without getting back a missile. This is
not how open-source works. We discuss, understand each others concerns,
find good trade-offs and make progress.

Have you read our concerns ? As far as I can see, your only reply was
to make it optional, which really only makes the whole thing even more
complicated.

I would really like to build a more constructive and collaborative work
relationship with you. You are always sending very good and valuable
contributions of high-quality, but there's a total impossibility of
discussing them if we have some fundamental concerns about them. All of
us maintainers always fear to reply negatively to one of your
contributions, because we know we are going to get back an aggression.
This is not how it should work. This is not why we all want to
contribute together to the same project.

Best regards,

Thomas

ing Tue, Apr 14, 2020 at 2:36 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Carlos,
>
> On Mon, 13 Apr 2020 20:41:11 -0300
> Carlos Santos <unixmania@gmail.com> wrote:
>
> > > There's been feedback from both Yann and me on this patch, and both of
> > > us think this is not the approach we want to take. Instead, we'd rather
> > > see everything installed to STAGING_DIR as the way of fixing the
> > > original issue.
> > >
> > > So I've marked both patches as Rejected in patchwork. Of course, if
> > > other people disagree with this decision, we can always revisit and
> > > rediscuss the matter.
> >
> > Just to remind you, the problem was reported two and a half years ago
> > and the situation is still the same. Perfect is the enemy of good and
> > later easily becomes never. Anyway, your project, your rules.

---8<---
> Have you read our concerns ? As far as I can see, your only reply was
> to make it optional, which really only makes the whole thing even more
> complicated.
---8<---

I have read the concerns. They sound reasonable but at the same time
there was an assumption (or promise) that the problem would vanish
once the dichotomy between staging/ and target/ be eliminated. Fair
enough, but no ETA was provided and the fact that the problem still
exists should be an alert about how difficult the solution is.

So two questions must be answered:

1. Is the problem reported by Ciro Santilli in bug 10386 real? If the
answer is "no", there's nothing else to argue about. Otherwise, jump
to question 2.

2. Is anybody able to estimate how long it will take to provide a solution?