| Message ID | 1452252619-13802-1-git-send-email-patrickdepinguin@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Thomas, In the subject: is -> if On 08-01-16 12:30, Thomas De Schampheleire wrote: > From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > > Commit ab74e09eb4e28dab8bed8d783c5f0464d39a32e7 renamed the dtc host tool > provided by linux to linux-dtc to avoid clashes with the dtc host tool > provided by host-dtc. > > However, external scripting may well rely on the existence of a device tree > compiler as $(HOST_DIR)/usr/bin/dtc, regardless of its source. Changing > these external scripts to use linux-dtc means that the scripts need to be > aware of the buildroot release they are working with, which is not very > nice. > > Add a symlink dtc->linux-dtc when no $(HOST_DIR)/usr/bin/dtc is present. > When host-dtc is not enabled, the end result will be dtc and > linux-dtc representing the same thing. > When host-dtc is enabled, either it is build before linux and no symlink > is created at any time, or it is build after linux, and the 'install' > command in host-dtc will overwrite the symlink with a proper dtc. In both > cases, the end result will be dtc and linux-dtc representing a different > thing. Or, when we eventually enable top-level parallel build, dtc and linux-dtc may be installed in parallel... Why do you always have these controversial patches, Thomas :-) How about a simple: ln -s linux-dtc $(HOST_DIR)/usr/bin/dtc 2>/dev/null i.e. just try to create the symlink and ignore errors. Creating a symlink is atomic. Argh, no, that leaves another race condition: the install of the dtc package will do stat, (unlink), open, so between the stat or unlink and the open there is a window in which the symlink will succeed. Then the version from host-dtc will overwrite linux-dtc, which is not what we want... I just love race conditions :-) Perhaps a better solution is to revisit ab74e09e completely. The intention of that commit was to have the linux version of dtc available for scripts. But is there ever any reason for a script _not_ to use the linux version? So what we would actually like is to keep a single dtc: the linux one if it is available, that one will be used, and host-dtc is a NOP. If not, then host-dtc kicks into action. So we'd add a dependency from host-dtc to linux if BR2_LINUX_KERNEL=y, and in the build and install commands test if dtc is available in the kernel (testing $(HOST_DIR)/usr/bin/dtc isn't sufficient, because it's possible that we rebuilt host-dtc and then we do want to overwrite it). It's a bit kludgy but it would work I think. PeterS, PeterK, what do you think? That said, we don't have parallel build at the moment, and my proposed alternative is definitely going to be controversial, so we should probably accept this one as is and revisit later. So: Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Regards, Arnout > > Cc: Peter Korsgaard <peter@korsgaard.com> > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > --- > linux/linux.mk | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/linux/linux.mk b/linux/linux.mk > index 045294b..f5c629c 100644 > --- a/linux/linux.mk > +++ b/linux/linux.mk > @@ -339,6 +339,9 @@ 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)/usr/bin/linux-dtc ; \ > + if [ ! -e $(HOST_DIR)/usr/bin/dtc ]; then \ > + ln -sf linux-dtc $(HOST_DIR)/usr/bin/dtc ; \ > + fi \ > fi > endef > >
On Sat, Jan 9, 2016 at 1:52 AM, Arnout Vandecappelle <arnout@mind.be> wrote: > Hi Thomas, > > In the subject: is -> if > > On 08-01-16 12:30, Thomas De Schampheleire wrote: >> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> >> >> Commit ab74e09eb4e28dab8bed8d783c5f0464d39a32e7 renamed the dtc host tool >> provided by linux to linux-dtc to avoid clashes with the dtc host tool >> provided by host-dtc. >> >> However, external scripting may well rely on the existence of a device tree >> compiler as $(HOST_DIR)/usr/bin/dtc, regardless of its source. Changing >> these external scripts to use linux-dtc means that the scripts need to be >> aware of the buildroot release they are working with, which is not very >> nice. >> >> Add a symlink dtc->linux-dtc when no $(HOST_DIR)/usr/bin/dtc is present. >> When host-dtc is not enabled, the end result will be dtc and >> linux-dtc representing the same thing. >> When host-dtc is enabled, either it is build before linux and no symlink >> is created at any time, or it is build after linux, and the 'install' >> command in host-dtc will overwrite the symlink with a proper dtc. In both >> cases, the end result will be dtc and linux-dtc representing a different >> thing. > > Or, when we eventually enable top-level parallel build, dtc and linux-dtc may > be installed in parallel... > > Why do you always have these controversial patches, Thomas :-) > > How about a simple: > > ln -s linux-dtc $(HOST_DIR)/usr/bin/dtc 2>/dev/null > > i.e. just try to create the symlink and ignore errors. Creating a symlink is > atomic. > > Argh, no, that leaves another race condition: the install of the dtc package > will do stat, (unlink), open, so between the stat or unlink and the open there > is a window in which the symlink will succeed. Then the version from host-dtc > will overwrite linux-dtc, which is not what we want... > > I just love race conditions :-) > > > Perhaps a better solution is to revisit ab74e09e completely. The intention of > that commit was to have the linux version of dtc available for scripts. But is > there ever any reason for a script _not_ to use the linux version? So what we > would actually like is to keep a single dtc: the linux one if it is available, > that one will be used, and host-dtc is a NOP. If not, then host-dtc kicks into > action. > > So we'd add a dependency from host-dtc to linux if BR2_LINUX_KERNEL=y, and in > the build and install commands test if dtc is available in the kernel (testing > $(HOST_DIR)/usr/bin/dtc isn't sufficient, because it's possible that we rebuilt > host-dtc and then we do want to overwrite it). It's a bit kludgy but it would > work I think. This would mean that a simple 'make host-dtc' would result in the kernel being built first, which in the case of a large package like the kernel is quite cumbersome. We can get rid of any race condition by making the symlink in a sequential part of the build, say target-finalize or a separate step. It's a bit odd (which is why I did not do that in the first place) but would work too. > > PeterS, PeterK, what do you think? > > > That said, we don't have parallel build at the moment, and my proposed > alternative is definitely going to be controversial, so we should probably > accept this one as is and revisit later. So: > > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Thanks :) /Thomas
Hello, On Fri, 8 Jan 2016 12:30:19 +0100, Thomas De Schampheleire wrote: > From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > > Commit ab74e09eb4e28dab8bed8d783c5f0464d39a32e7 renamed the dtc host tool > provided by linux to linux-dtc to avoid clashes with the dtc host tool > provided by host-dtc. > > However, external scripting may well rely on the existence of a device tree > compiler as $(HOST_DIR)/usr/bin/dtc, regardless of its source. Changing > these external scripts to use linux-dtc means that the scripts need to be > aware of the buildroot release they are working with, which is not very > nice. > > Add a symlink dtc->linux-dtc when no $(HOST_DIR)/usr/bin/dtc is present. > When host-dtc is not enabled, the end result will be dtc and > linux-dtc representing the same thing. > When host-dtc is enabled, either it is build before linux and no symlink > is created at any time, or it is build after linux, and the 'install' > command in host-dtc will overwrite the symlink with a proper dtc. In both > cases, the end result will be dtc and linux-dtc representing a different > thing. > > Cc: Peter Korsgaard <peter@korsgaard.com> > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> I've applied this patch to next, mainly because Arnout gave his Reviewed-by, and because it has been waiting for a loooong time. However, I don't quite get why you simply don't include host-dtc in your build if you want to be sure to have something in $(HOST_DIR)/usr/bin/dtc. We could even push the thing further and not installed the Linux DTC at all in $(HOST_DIR) and simply ask people to add host-dtc to their build if they need it. But oh well, if we decide to move in this direction, it can be reworked later, which is why I've applied. Thanks! Thomas
On 05/31/16 22:50, Thomas Petazzoni wrote: > Hello, > > On Fri, 8 Jan 2016 12:30:19 +0100, Thomas De Schampheleire wrote: >> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> >> >> Commit ab74e09eb4e28dab8bed8d783c5f0464d39a32e7 renamed the dtc host tool >> provided by linux to linux-dtc to avoid clashes with the dtc host tool >> provided by host-dtc. >> >> However, external scripting may well rely on the existence of a device tree >> compiler as $(HOST_DIR)/usr/bin/dtc, regardless of its source. Changing >> these external scripts to use linux-dtc means that the scripts need to be >> aware of the buildroot release they are working with, which is not very >> nice. >> >> Add a symlink dtc->linux-dtc when no $(HOST_DIR)/usr/bin/dtc is present. >> When host-dtc is not enabled, the end result will be dtc and >> linux-dtc representing the same thing. >> When host-dtc is enabled, either it is build before linux and no symlink >> is created at any time, or it is build after linux, and the 'install' >> command in host-dtc will overwrite the symlink with a proper dtc. In both >> cases, the end result will be dtc and linux-dtc representing a different >> thing. >> >> Cc: Peter Korsgaard <peter@korsgaard.com> >> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com> > > I've applied this patch to next, mainly because Arnout gave his > Reviewed-by, and because it has been waiting for a loooong time. > > However, I don't quite get why you simply don't include host-dtc in > your build if you want to be sure to have something in > $(HOST_DIR)/usr/bin/dtc. We could even push the thing further and not > installed the Linux DTC at all in $(HOST_DIR) and simply ask people to > add host-dtc to their build if they need it. I could be wrong, but don't we have situations where we really need linux-dtc and not host-dtc because linux-dtc is patched for e.g. DT overlays? Regards, Arnout > > But oh well, if we decide to move in this direction, it can be reworked > later, which is why I've applied. > > Thanks! > > Thomas >
Hello, On Wed, 1 Jun 2016 16:41:51 +0200, Arnout Vandecappelle wrote: > I could be wrong, but don't we have situations where we really need linux-dtc > and not host-dtc because linux-dtc is patched for e.g. DT overlays? linux-dtc is not patched for DT overlays. You need to use patched DTC versions, which are available on numerous random Github repositories, but neither the DTC in the Linux kernel tree nor in its official upstream Git repository are overlay capable. Thomas
diff --git a/linux/linux.mk b/linux/linux.mk index 045294b..f5c629c 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -339,6 +339,9 @@ 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)/usr/bin/linux-dtc ; \ + if [ ! -e $(HOST_DIR)/usr/bin/dtc ]; then \ + ln -sf linux-dtc $(HOST_DIR)/usr/bin/dtc ; \ + fi \ fi endef