Message ID | 20240430045945.2164313-1-mika.westerberg@linux.intel.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [v2] package/tbtools: new package | expand |
Hello, Thanks for your patch. I fixed up a few things, wanted to apply... but it fails to build for me. See below a number of comments, and details about the build failure. On Tue, 30 Apr 2024 07:59:44 +0300 Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > This is a collection of tools for Linux Thunderbolt/USB4 development, > debugging and validation. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Reviewed-by: Aapo Vienamo <aapo.vienamo@linux.intel.com> > --- > Changes from v1: > > - Added dependency to udev (libudev) and 64-bit builds. > - Added tag from Aapo. > > package/Config.in | 1 + > package/tbtools/Config.in | 11 +++++++++++ > package/tbtools/tbtools.mk | 12 ++++++++++++ > 3 files changed, 24 insertions(+) You need to add an entry to the DEVELOPERS file, like this: +N: Mika Westerberg <mika.westerberg@linux.intel.com> +F: package/tbtools/ (make sure to respect alphabetic ordering of developers) > diff --git a/package/tbtools/Config.in b/package/tbtools/Config.in > new file mode 100644 > index 000000000000..2627ea8bfcca > --- /dev/null > +++ b/package/tbtools/Config.in > @@ -0,0 +1,11 @@ > +config BR2_PACKAGE_TBTOOLS > + bool "tbtools" > + depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS > + depends on BR2_PACKAGE_HAS_UDEV > + depends on BR2_ARCH_IS_64 I think it was worth adding a comment about this: + # Fails to build on 32-bit systems > + select BR2_PACKAGE_HOST_RUSTC > + help > + tbtools is a collection of tools for Linux Thunderbolt/USB4 > + development, debugging and validation. > + > + https://github.com/intel/tbtools You need a Config.in comment: +comment "tbtools needs udev /dev management" + depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS + depends on BR2_ARCH_IS_64 + depends on !BR2_PACKAGE_HAS_UDEV > diff --git a/package/tbtools/tbtools.mk b/package/tbtools/tbtools.mk > new file mode 100644 > index 000000000000..61986ca8d311 > --- /dev/null > +++ b/package/tbtools/tbtools.mk > @@ -0,0 +1,12 @@ > +################################################################################ > +# > +# tbtools > +# > +################################################################################ > + > +TBTOOLS_VERSION = v0.3.2 > +TBTOOLS_SITE = $(call github,intel,tbtools,$(TBTOOLS_VERSION)) > +TBTOOLS_LICENSE = MIT > +TBTOOLS_LICENSE_FILES = LICENSE You're missing: +TBTOOLS_DEPENDENCIES = udev without which udev is not built before tbtools. Without this, it fails with a pkg-config detection issue. But even with udev built before, I get a build failure: = note: /home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/aarch64-buildroot-linux-musl/12.3.0/../../../../aarch64-buildroot-linux-musl/bin/ld: cannot find -ludev: No such file or directory collect2: error: ld returned 1 exit status You can reproduce the following defconfig: BR2_aarch64=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y BR2_TOOLCHAIN_EXTERNAL_BOOTLIN_AARCH64_MUSL_STABLE=y BR2_INIT_NONE=y BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y BR2_SYSTEM_BIN_SH_NONE=y # BR2_PACKAGE_BUSYBOX is not set BR2_PACKAGE_TBTOOLS=y # BR2_TARGET_ROOTFS_TAR is not set Could you have a look and submit a v2 ? Thanks a lot! Thomas
Hi, On Fri, May 10, 2024 at 01:44:11PM +0200, Thomas Petazzoni wrote: > Hello, > > Thanks for your patch. I fixed up a few things, wanted to apply... but > it fails to build for me. See below a number of comments, and details > about the build failure. Thanks for looking into it! > On Tue, 30 Apr 2024 07:59:44 +0300 > Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > > This is a collection of tools for Linux Thunderbolt/USB4 development, > > debugging and validation. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Reviewed-by: Aapo Vienamo <aapo.vienamo@linux.intel.com> > > --- > > Changes from v1: > > > > - Added dependency to udev (libudev) and 64-bit builds. > > - Added tag from Aapo. > > > > package/Config.in | 1 + > > package/tbtools/Config.in | 11 +++++++++++ > > package/tbtools/tbtools.mk | 12 ++++++++++++ > > 3 files changed, 24 insertions(+) > > You need to add an entry to the DEVELOPERS file, like this: > > +N: Mika Westerberg <mika.westerberg@linux.intel.com> > +F: package/tbtools/ > > (make sure to respect alphabetic ordering of developers) > > > diff --git a/package/tbtools/Config.in b/package/tbtools/Config.in > > new file mode 100644 > > index 000000000000..2627ea8bfcca > > --- /dev/null > > +++ b/package/tbtools/Config.in > > @@ -0,0 +1,11 @@ > > +config BR2_PACKAGE_TBTOOLS > > + bool "tbtools" > > + depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS > > + depends on BR2_PACKAGE_HAS_UDEV > > + depends on BR2_ARCH_IS_64 > > I think it was worth adding a comment about this: > > + # Fails to build on 32-bit systems > > > + select BR2_PACKAGE_HOST_RUSTC > > + help > > + tbtools is a collection of tools for Linux Thunderbolt/USB4 > > + development, debugging and validation. > > + > > + https://github.com/intel/tbtools > > You need a Config.in comment: > > +comment "tbtools needs udev /dev management" > + depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS > + depends on BR2_ARCH_IS_64 > + depends on !BR2_PACKAGE_HAS_UDEV > > > > diff --git a/package/tbtools/tbtools.mk b/package/tbtools/tbtools.mk > > new file mode 100644 > > index 000000000000..61986ca8d311 > > --- /dev/null > > +++ b/package/tbtools/tbtools.mk > > @@ -0,0 +1,12 @@ > > +################################################################################ > > +# > > +# tbtools > > +# > > +################################################################################ > > + > > +TBTOOLS_VERSION = v0.3.2 > > +TBTOOLS_SITE = $(call github,intel,tbtools,$(TBTOOLS_VERSION)) > > +TBTOOLS_LICENSE = MIT > > +TBTOOLS_LICENSE_FILES = LICENSE > > You're missing: > > +TBTOOLS_DEPENDENCIES = udev > > without which udev is not built before tbtools. Without this, it fails > with a pkg-config detection issue. > > But even with udev built before, I get a build failure: > > = note: /home/thomas/projets/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/aarch64-buildroot-linux-musl/12.3.0/../../../../aarch64-buildroot-linux-musl/bin/ld: cannot find -ludev: No such file or directory > collect2: error: ld returned 1 exit status > > You can reproduce the following defconfig: > > BR2_aarch64=y > BR2_TOOLCHAIN_EXTERNAL=y > BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y > BR2_TOOLCHAIN_EXTERNAL_BOOTLIN_AARCH64_MUSL_STABLE=y > BR2_INIT_NONE=y > BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y > BR2_SYSTEM_BIN_SH_NONE=y > # BR2_PACKAGE_BUSYBOX is not set > BR2_PACKAGE_TBTOOLS=y > # BR2_TARGET_ROOTFS_TAR is not set > > Could you have a look and submit a v2 ? I think Rust udev wants to have glibc as the C library. I will add dependency to that as well. I'll fix these up in the next version.
diff --git a/package/Config.in b/package/Config.in index cdbf3577f8e1..e346917649a8 100644 --- a/package/Config.in +++ b/package/Config.in @@ -157,6 +157,7 @@ menu "Debugging, profiling and benchmark" source "package/stress-ng/Config.in" source "package/sysdig/Config.in" source "package/sysprof/Config.in" + source "package/tbtools/Config.in" source "package/tcf-agent/Config.in" source "package/trace-cmd/Config.in" source "package/trinity/Config.in" diff --git a/package/tbtools/Config.in b/package/tbtools/Config.in new file mode 100644 index 000000000000..2627ea8bfcca --- /dev/null +++ b/package/tbtools/Config.in @@ -0,0 +1,11 @@ +config BR2_PACKAGE_TBTOOLS + bool "tbtools" + depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_ARCH_IS_64 + select BR2_PACKAGE_HOST_RUSTC + help + tbtools is a collection of tools for Linux Thunderbolt/USB4 + development, debugging and validation. + + https://github.com/intel/tbtools diff --git a/package/tbtools/tbtools.mk b/package/tbtools/tbtools.mk new file mode 100644 index 000000000000..61986ca8d311 --- /dev/null +++ b/package/tbtools/tbtools.mk @@ -0,0 +1,12 @@ +################################################################################ +# +# tbtools +# +################################################################################ + +TBTOOLS_VERSION = v0.3.2 +TBTOOLS_SITE = $(call github,intel,tbtools,$(TBTOOLS_VERSION)) +TBTOOLS_LICENSE = MIT +TBTOOLS_LICENSE_FILES = LICENSE + +$(eval $(cargo-package))