diff mbox series

[v2,1/1] linux: Make dtc install step more reliable

Message ID 20181113155016.24022-1-anaumann@ultratronik.de
State Accepted
Headers show
Series [v2,1/1] linux: Make dtc install step more reliable | expand

Commit Message

Andreas Naumann Nov. 13, 2018, 3:50 p.m. UTC
Checking for the existence of the dtc binary built by the
non-dependent dtc package may cause instable behaviour when giving more
freedom on the order of how the packages are built (parallelization).

In addidion, when moving to per-package host/target method, the check
would always trigger in the isolated host, leading to linux-dtc always
being installed as dtc.
This in turn may lead to undesired overwriting of the real host-dtc binary
when finally assembling the global host dir.

Thus rework the linux-dtc install condition to be defined by configuration
rather than compile time order.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
---
Changes v1 -> v2: 
  - simplified by direct usage of make conditional almost as suggested by
    Arnout (test revealed that an additinal comma was needed for correct logic)
---
 linux/linux.mk | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Thomas Petazzoni Nov. 23, 2018, 9:29 p.m. UTC | #1
Hello,

On Tue, 13 Nov 2018 16:50:16 +0100, Andreas Naumann wrote:
> Checking for the existence of the dtc binary built by the
> non-dependent dtc package may cause instable behaviour when giving more
> freedom on the order of how the packages are built (parallelization).
> 
> In addidion, when moving to per-package host/target method, the check
> would always trigger in the isolated host, leading to linux-dtc always
> being installed as dtc.
> This in turn may lead to undesired overwriting of the real host-dtc binary
> when finally assembling the global host dir.
> 
> Thus rework the linux-dtc install condition to be defined by configuration
> rather than compile time order.
> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> ---
> Changes v1 -> v2: 
>   - simplified by direct usage of make conditional almost as suggested by
>     Arnout (test revealed that an additinal comma was needed for correct logic)
> ---
>  linux/linux.mk | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

I hesitated a bit, but in the end decided to apply this to master.
Indeed, with the current code, it's really a matter of luck whether
host-dtc gets built before/after linux that determines if the dtc ->
linux-dtc symlink will be created or not. With this patch, regardless
of whether host-dtc is built before or after linux, we'll have the same
result. To me, this was a sufficient motivation to apply this patch to
master.

Thanks!

Thomas
Andreas Naumann Nov. 26, 2018, 7:50 a.m. UTC | #2
Hi Thomas,

Am 23.11.18 um 22:29 schrieb Thomas Petazzoni:
> Hello,
> 
> On Tue, 13 Nov 2018 16:50:16 +0100, Andreas Naumann wrote:
>> Checking for the existence of the dtc binary built by the
>> non-dependent dtc package may cause instable behaviour when giving more
>> freedom on the order of how the packages are built (parallelization).
>>
>> In addidion, when moving to per-package host/target method, the check
>> would always trigger in the isolated host, leading to linux-dtc always
>> being installed as dtc.
>> This in turn may lead to undesired overwriting of the real host-dtc binary
>> when finally assembling the global host dir.
>>
>> Thus rework the linux-dtc install condition to be defined by configuration
>> rather than compile time order.
>>
>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>> ---
>> Changes v1 -> v2:
>>    - simplified by direct usage of make conditional almost as suggested by
>>      Arnout (test revealed that an additinal comma was needed for correct logic)
>> ---
>>   linux/linux.mk | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> I hesitated a bit, but in the end decided to apply this to master.
> Indeed, with the current code, it's really a matter of luck whether
> host-dtc gets built before/after linux that determines if the dtc ->
> linux-dtc symlink will be created or not. With this patch, regardless
> of whether host-dtc is built before or after linux, we'll have the same
> result. To me, this was a sufficient motivation to apply this patch to
> master.
> 

Thanks for applying it. May I ask why you hesitated, was the commit 
message clear enough? Or just too small of an issue?

regards,
Andreas



> Thanks!
> 
> Thomas
>
Thomas Petazzoni Nov. 26, 2018, 7:58 a.m. UTC | #3
Hello Andreas,

On Mon, 26 Nov 2018 08:50:35 +0100, Andreas Naumann wrote:

> > I hesitated a bit, but in the end decided to apply this to master.
> > Indeed, with the current code, it's really a matter of luck whether
> > host-dtc gets built before/after linux that determines if the dtc ->
> > linux-dtc symlink will be created or not. With this patch, regardless
> > of whether host-dtc is built before or after linux, we'll have the same
> > result. To me, this was a sufficient motivation to apply this patch to
> > master.
> 
> Thanks for applying it. May I ask why you hesitated, was the commit 
> message clear enough? Or just too small of an issue?

My hesitation was not whether to apply it or not, the explanation was
clear.

My hesitation was whether it should have been applied to our "master"
branch or to our "next" branch.

We have a release process that works as follows:

 1 After a release, say 2018.08, we have two months of active
   development. During this time period, all changes go to the master
   branch: both fixes and major changes (package updates, new packages,
   infrastructure changes, etc.)

 2 After two months of development, we cut the -rc1 of the next
   release, for example 2018.11-rc1. At this point, we stop merging new
   development in the "master" branch, and only merge
   bug/security/build fixes. In order to avoid completely stopping the
   merge of new features, we open a "next" branch.

 3 During one month, we have these split master/next branches, until
   the final 2018.11 is released, from the master branch.

 4 Once 2018.11 is cut, we merge "next" back into "master" and goto
   step 1.

We are currently in step (2), so for each change we have to decide
whether the change should be committed to the master branch or the next
branch. Some changes are obvious: a build fix goes to master, a new
package goes to next. Some other changes are less obvious, and for this
specific patch, I hesitated a bit between applying to master and next.
That's all what I meant to say.

Does this clarify the hesitation ?

Thanks!

Thomas
Andreas Naumann Nov. 26, 2018, 8:14 a.m. UTC | #4
Hi,

Am 26.11.18 um 08:58 schrieb Thomas Petazzoni:
> Hello Andreas,
> 
> On Mon, 26 Nov 2018 08:50:35 +0100, Andreas Naumann wrote:
> 
>>> I hesitated a bit, but in the end decided to apply this to master.
>>> Indeed, with the current code, it's really a matter of luck whether
>>> host-dtc gets built before/after linux that determines if the dtc ->
>>> linux-dtc symlink will be created or not. With this patch, regardless
>>> of whether host-dtc is built before or after linux, we'll have the same
>>> result. To me, this was a sufficient motivation to apply this patch to
>>> master.
>>
>> Thanks for applying it. May I ask why you hesitated, was the commit
>> message clear enough? Or just too small of an issue?
> 
> My hesitation was not whether to apply it or not, the explanation was
> clear.
> 
> My hesitation was whether it should have been applied to our "master"
> branch or to our "next" branch.
> 
> We have a release process that works as follows:
> 
>   1 After a release, say 2018.08, we have two months of active
>     development. During this time period, all changes go to the master
>     branch: both fixes and major changes (package updates, new packages,
>     infrastructure changes, etc.)
> 
>   2 After two months of development, we cut the -rc1 of the next
>     release, for example 2018.11-rc1. At this point, we stop merging new
>     development in the "master" branch, and only merge
>     bug/security/build fixes. In order to avoid completely stopping the
>     merge of new features, we open a "next" branch.
> 
>   3 During one month, we have these split master/next branches, until
>     the final 2018.11 is released, from the master branch.
> 
>   4 Once 2018.11 is cut, we merge "next" back into "master" and goto
>     step 1.
> 
> We are currently in step (2), so for each change we have to decide
> whether the change should be committed to the master branch or the next
> branch. Some changes are obvious: a build fix goes to master, a new
> package goes to next. Some other changes are less obvious, and for this
> specific patch, I hesitated a bit between applying to master and next.
> That's all what I meant to say.
> 
> Does this clarify the hesitation ?

Yes and thanks for applying to master then :-)

regards,
Andreas


> 
> Thanks!
> 
> Thomas
>
Peter Korsgaard Nov. 26, 2018, 5:48 p.m. UTC | #5
>>>>> "Andreas" == Andreas Naumann <anaumann@ultratronik.de> writes:

 > Checking for the existence of the dtc binary built by the
 > non-dependent dtc package may cause instable behaviour when giving more
 > freedom on the order of how the packages are built (parallelization).

 > In addidion, when moving to per-package host/target method, the check
 > would always trigger in the isolated host, leading to linux-dtc always
 > being installed as dtc.
 > This in turn may lead to undesired overwriting of the real host-dtc binary
 > when finally assembling the global host dir.

 > Thus rework the linux-dtc install condition to be defined by configuration
 > rather than compile time order.

 > Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
 > ---
 > Changes v1 -> v2: 
 >   - simplified by direct usage of make conditional almost as suggested by
 >     Arnout (test revealed that an additinal comma was needed for correct logic)

Committed to 2018.02.x and 2018.08.x, thanks.
diff mbox series

Patch

diff --git a/linux/linux.mk b/linux/linux.mk
index f2bca17328..66e11d2ab4 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -465,9 +465,7 @@  define LINUX_INSTALL_HOST_TOOLS
 	# Installing dtc (device tree compiler) as host tool, if selected
 	if grep -q "CONFIG_DTC=y" $(@D)/.config; then \
 		$(INSTALL) -D -m 0755 $(@D)/scripts/dtc/dtc $(HOST_DIR)/bin/linux-dtc ; \
-		if [ ! -e $(HOST_DIR)/bin/dtc ]; then \
-			ln -sf linux-dtc $(HOST_DIR)/bin/dtc ; \
-		fi \
+		$(if $(BR2_PACKAGE_HOST_DTC),,ln -sf linux-dtc $(HOST_DIR)/bin/dtc;) \
 	fi
 endef