On 2015-09-08 20:44, Helmut Grohne wrote:
> Hi Aurelien,
> On Tue, Sep 08, 2015 at 07:58:13PM +0200, Aurelien Jarno wrote:
> > Thanks for the patch and the detailed explanation. The changes make sense,
> > so I have applied the patch.
> Thank you.
> > That said looking as this part of the code as a whole, it ends up being a
> > bit complicated. Basically we define defaults value before the case, but
> > then we basically handle all cases. Then we use a for loop as a if, as
> > $templates contains either zero or one value.
> I concur with that observation.
> > The complexity comes from the fact this piece of code has been forked from
> > the non-staged one. I therefore wonder if we should try to either:
> > 1) Simplify it.
> > 2) make it as common as possible with the non-stage code. I believe it's
> > not a problem if we generate debhelper files that we don't use in
> > practice, as long as the stage ones are correct.
> Maybe we should try 3) merge it into the non-stage code? Having these
> two cases separate has the disadvantage that they will go out of sync as
> the non-staged variant is adapted to the current needs. We should strive
> for minimal differences in stage1 to minimize its maintenance cost.

Your option 3), was actually the step I wanted to do after option 2) ;-)

> So what actually is the difference between these two implementations of
> the debhelper-common target? If I am not mistaken it is:
>  * Generate fewer debhelper templates. As you observed there is no harm
>    in just generating them anyway.
>  * Interpolate more variables, in particular RTLD_SO. They are not used
>    in the libc-dev templates. The computation of the shell variable
>    rtld_so would probably result in garbage as
>    debian/tmp-*/usr/bin/iconv will be missing, but maybe it still
>    succeeds (from an exit code pov).
>  * Remove lines containing LIBDIR in stage1. Unless I am missing
>    something, this is the only relevant difference.
> So maybe one could have something along the lines.
> ifeq ($(filter stage1,$(DEB_BUILD_PROFILES)),)
> else
> STAGE1_TEMPLATE_FILTER=sed -e "/LIBDIR.*\.a /d" $$t
> fi

You can even get rid of the call to sed there and just set
directly STAGE1_TEMPLATE_FILTER=-e "/LIBDIR.*\.a /d" $$t so
that it can be used in the same sed invocation. OTOH I don't
think we really care about performance

> and then add "$(STAGE1_TEMPLATE_FILTER);" to the pile of sed invocations
> in the non-stage1 code path while eliminating the stage1 block
> altogether. Do you think this idea would be worth trying and preparing a
> proper patch? 

Yes I think it would be a good idea.

> Do you have a better name for that strange make variable?

I think the name is fine.


Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurel...@aurel32.net                 http://www.aurel32.net

Reply via email to