Message ID | 20160728193825.9199-1-maxime.ripard@free-electrons.com |
---|---|
State | Accepted |
Headers | show |
Maxime, All, On 2016-07-28 21:38 +0200, Maxime Ripard spake thusly: > KMS++ is a suite of library and test tools to interact with KMS drivers in > the linux kernel. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- [--SNIP--] > diff --git a/package/kmsxx/Config.in b/package/kmsxx/Config.in > new file mode 100644 > index 000000000000..db3c74b45310 > --- /dev/null > +++ b/package/kmsxx/Config.in > @@ -0,0 +1,26 @@ > +config BR2_PACKAGE_KMSXX > + bool "kmsxx" > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 > + depends on BR2_INSTALL_LIBSTDCPP > + depends on BR2_TOOLCHAIN_HAS_THREADS # libpthread-stubs The comment "# libpthread-stubs" seems to imply that the dependency on C++ is ihnerited from libpthread-stubs, but it is neither selected not depended on; it is also not in the build dependency. Is it an oversight? However, given the name of the package, I guess it needs C++ for itself. So, whether libpthread-stubs is needed or not, you should drop the comment altogether: C++ *is* needed by kms++. > + select BR2_PACKAGE_LIBDRM > + help > + libkms++ is a C++11 library for kernel mode setting. > + > + Also included are simple test tools for KMS and python wrapper for > + libkms++. Drop the comment about the Python bindings, since we don't support them (you dropped them sinc e v2 of the patch). > + https://github.com/tomba/kmsxx > + > +if BR2_PACKAGE_KMSXX > + > +config BR2_PACKAGE_KMSXX_INSTALL_TESTS > + bool "Install test programs" > + help > + This option allows to install the kmsxx test programs. > + > +endif Usually, when there is a single option to a package, we do not use an if-block, but just make that option depend on the package. But I'm fine with an if-block (I prefer it; it's just not what we usually do). > +comment "kmsxx needs a toolchain w/ threads" > + depends on !BR2_TOOLCHAIN_HAS_THREADS > + > diff --git a/package/kmsxx/kmsxx.mk b/package/kmsxx/kmsxx.mk > new file mode 100644 > index 000000000000..6eaa85452dfb > --- /dev/null > +++ b/package/kmsxx/kmsxx.mk > @@ -0,0 +1,45 @@ > +################################################################################ > +# > +# kmsxx > +# > +################################################################################ > + > +KMSXX_VERSION = a706f157b86e90696808025db01de99646d51a77 > +KMSXX_SITE = $(call github,tomba,kmsxx,$(KMSXX_VERSION)) > +KMSXX_LICENSE = MPLv2.0 > +KMSXX_LICENSE_FILES = LICENSE > + > +KMSXX_DEPENDENCIES += libdrm > +KMSXX_CONF_OPTS += -DKMSXX_ENABLE_PYTHON=OFF > + > +ifeq ($(BR2_PACKAGE_KMSXX_INSTALL_TESTS),y) > +define KMSXX_INSTALL_TARGET_TESTS > + $(INSTALL) -D -m 0755 $(@D)/bin/fbtestpat \ > + $(TARGET_DIR)/usr/bin/fbtestpat > + $(INSTALL) -D -m 0755 $(@D)/bin/kmsblank \ > + $(TARGET_DIR)/usr/bin/kmsblank > + $(INSTALL) -D -m 0755 $(@D)/bin/kmscapture \ > + $(TARGET_DIR)/usr/bin/kmscapture > + $(INSTALL) -D -m 0755 $(@D)/bin/kmsprint \ > + $(TARGET_DIR)/usr/bin/kmsprint > + $(INSTALL) -D -m 0755 $(@D)/bin/kmsview \ > + $(TARGET_DIR)/usr/bin/kmsview > + $(INSTALL) -D -m 0755 $(@D)/bin/testpat \ > + $(TARGET_DIR)/usr/bin/testpat > + $(INSTALL) -D -m 0755 $(@D)/bin/wbcap \ > + $(TARGET_DIR)/usr/bin/wbcap > + $(INSTALL) -D -m 0755 $(@D)/bin/wbm2m \ > + $(TARGET_DIR)/usr/bin/wbm2m KMSXX_TESTS = fbtestpat kmsblank kmscapture ... define KMSXX_INSTALL_TARGET_TESTS $(foreach t,$(KMSXX_TESTS),\ $(INSTALL) -D -m 0755 $(@D)/bin/$(t) $(TARGET_DIR/usr/bin/$(t)) ) endef > +endef > +endif > + > +define KMSXX_INSTALL_TARGET_CMDS > + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++.so \ > + $(TARGET_DIR)/usr/lib/libkms++.so > + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++util.so \ > + $(TARGET_DIR)/usr/lib/libkms++util.so > + No empty line above. > + $(KMSXX_INSTALL_TARGET_TESTS) > +endef Since it installs libraries, maybe it should be installed in staging as well? Regards, Yann E. MORIN. > +$(eval $(cmake-package)) > -- > 2.9.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Maxime, All, Well, I forgot a few nts in the previous mail... Here they are. ;-) On 2016-07-28 21:38 +0200, Maxime Ripard spake thusly: > KMS++ is a suite of library and test tools to interact with KMS drivers in > the linux kernel. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > diff --git a/package/kmsxx/Config.in b/package/kmsxx/Config.in > new file mode 100644 > index 000000000000..db3c74b45310 > --- /dev/null > +++ b/package/kmsxx/Config.in > @@ -0,0 +1,26 @@ > +config BR2_PACKAGE_KMSXX > + bool "kmsxx" We can call it by its real name in the prompt: kms++ . You should also provide a .hash file: Github provides reproducible archives. > diff --git a/package/kmsxx/kmsxx.mk b/package/kmsxx/kmsxx.mk > new file mode 100644 > index 000000000000..6eaa85452dfb > --- /dev/null > +++ b/package/kmsxx/kmsxx.mk > @@ -0,0 +1,45 @@ > +################################################################################ > +# > +# kmsxx > +# > +################################################################################ > + > +KMSXX_VERSION = a706f157b86e90696808025db01de99646d51a77 > +KMSXX_SITE = $(call github,tomba,kmsxx,$(KMSXX_VERSION)) > +KMSXX_LICENSE = MPLv2.0 > +KMSXX_LICENSE_FILES = LICENSE > + > +KMSXX_DEPENDENCIES += libdrm > +KMSXX_CONF_OPTS += -DKMSXX_ENABLE_PYTHON=OFF No need for "+=" here, just a plain "=". > +define KMSXX_INSTALL_TARGET_CMDS > + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++.so \ > + $(TARGET_DIR)/usr/lib/libkms++.so > + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++util.so \ > + $(TARGET_DIR)/usr/lib/libkms++util.so Aren't the libraries versioned? If they are, don't we need to provide the versioning symlinks? (Not that it matters, we're not supposed to have multiple versions of the same libs in Buildroot; it's just for consistency and customs.) Regards, Yann E. MORIN. > + $(KMSXX_INSTALL_TARGET_TESTS) > +endef > + > +$(eval $(cmake-package)) > -- > 2.9.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hello, On Thu, 28 Jul 2016 21:49:27 +0200, Yann E. MORIN wrote: > > + depends on BR2_INSTALL_LIBSTDCPP > > + depends on BR2_TOOLCHAIN_HAS_THREADS # libpthread-stubs > > The comment "# libpthread-stubs" seems to imply that the dependency on > C++ is ihnerited from libpthread-stubs, but it is neither selected not > depended on; it is also not in the build dependency. Is it an oversight? > > However, given the name of the package, I guess it needs C++ for itself. > So, whether libpthread-stubs is needed or not, you should drop the > comment altogether: C++ *is* needed by kms++. There is no comment on the C++ dependency, so I'm not sure what you mean here. The BR2_TOOLCHAIN_HAS_THREADS dependency is inherited from libdrm, which itself inherits it from libpthread-stubs. So the way we typically do it is: depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm -> libpthread-stubs Or just: depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm Thanks, Thomas
Thomas, All, On 2016-07-28 22:17 +0200, Thomas Petazzoni spake thusly: > On Thu, 28 Jul 2016 21:49:27 +0200, Yann E. MORIN wrote: > > > + depends on BR2_INSTALL_LIBSTDCPP > > > + depends on BR2_TOOLCHAIN_HAS_THREADS # libpthread-stubs > > > > The comment "# libpthread-stubs" seems to imply that the dependency on > > C++ is ihnerited from libpthread-stubs, but it is neither selected not > > depended on; it is also not in the build dependency. Is it an oversight? > > > > However, given the name of the package, I guess it needs C++ for itself. > > So, whether libpthread-stubs is needed or not, you should drop the > > comment altogether: C++ *is* needed by kms++. > > There is no comment on the C++ dependency, so I'm not sure what you > mean here. Grr... I read in diagonal, and in my eyes, the comment was on the BR2_INSTALL_LIBSTDCPP line... :-/ > The BR2_TOOLCHAIN_HAS_THREADS dependency is inherited from libdrm, > which itself inherits it from libpthread-stubs. So the way we typically > do it is: > > depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm -> libpthread-stubs > > Or just: > > depends on BR2_TOOLCHAIN_HAS_THREADS # libdrm Indeed. Sorry for the noise. Regards, Yann E. MORIN.
Hello, On Thu, 28 Jul 2016 21:38:25 +0200, Maxime Ripard wrote: > KMS++ is a suite of library and test tools to interact with KMS drivers in > the linux kernel. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> I've applied, after doing numerous updates: [Thomas: - rename prompt to kms++, suggested by Yann E. Morin - fixup the thread dependency comment - remove the mention of the python wrapper in the Config.in help text, since they are not installed - fix the Config.in comment to mention the C++ and gcc >= 4.8 dependencies - use = instead of += when appropriate - use a loop to install the test programs - use a loop to install the libraries - add installation to staging as well, both the libraries and header files - add missing dependency on host-pkgconf - add hash file.] Also, it would be nice to tell upstream that CMake can not only build stuff, but also install stuff. It is a bit silly to be using a well-known build system, but not use it to provide proper install targets. Thanks a lot! Thomas
diff --git a/package/Config.in b/package/Config.in index 0f0c3769ab33..9867a84db06c 100644 --- a/package/Config.in +++ b/package/Config.in @@ -941,6 +941,7 @@ menu "Graphics" source "package/imlib2/Config.in" source "package/jasper/Config.in" source "package/jpeg/Config.in" + source "package/kmsxx/Config.in" source "package/lcms2/Config.in" source "package/lesstif/Config.in" source "package/libart/Config.in" diff --git a/package/kmsxx/Config.in b/package/kmsxx/Config.in new file mode 100644 index 000000000000..db3c74b45310 --- /dev/null +++ b/package/kmsxx/Config.in @@ -0,0 +1,26 @@ +config BR2_PACKAGE_KMSXX + bool "kmsxx" + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 + depends on BR2_INSTALL_LIBSTDCPP + depends on BR2_TOOLCHAIN_HAS_THREADS # libpthread-stubs + select BR2_PACKAGE_LIBDRM + help + libkms++ is a C++11 library for kernel mode setting. + + Also included are simple test tools for KMS and python wrapper for + libkms++. + + https://github.com/tomba/kmsxx + +if BR2_PACKAGE_KMSXX + +config BR2_PACKAGE_KMSXX_INSTALL_TESTS + bool "Install test programs" + help + This option allows to install the kmsxx test programs. + +endif + +comment "kmsxx needs a toolchain w/ threads" + depends on !BR2_TOOLCHAIN_HAS_THREADS + diff --git a/package/kmsxx/kmsxx.mk b/package/kmsxx/kmsxx.mk new file mode 100644 index 000000000000..6eaa85452dfb --- /dev/null +++ b/package/kmsxx/kmsxx.mk @@ -0,0 +1,45 @@ +################################################################################ +# +# kmsxx +# +################################################################################ + +KMSXX_VERSION = a706f157b86e90696808025db01de99646d51a77 +KMSXX_SITE = $(call github,tomba,kmsxx,$(KMSXX_VERSION)) +KMSXX_LICENSE = MPLv2.0 +KMSXX_LICENSE_FILES = LICENSE + +KMSXX_DEPENDENCIES += libdrm +KMSXX_CONF_OPTS += -DKMSXX_ENABLE_PYTHON=OFF + +ifeq ($(BR2_PACKAGE_KMSXX_INSTALL_TESTS),y) +define KMSXX_INSTALL_TARGET_TESTS + $(INSTALL) -D -m 0755 $(@D)/bin/fbtestpat \ + $(TARGET_DIR)/usr/bin/fbtestpat + $(INSTALL) -D -m 0755 $(@D)/bin/kmsblank \ + $(TARGET_DIR)/usr/bin/kmsblank + $(INSTALL) -D -m 0755 $(@D)/bin/kmscapture \ + $(TARGET_DIR)/usr/bin/kmscapture + $(INSTALL) -D -m 0755 $(@D)/bin/kmsprint \ + $(TARGET_DIR)/usr/bin/kmsprint + $(INSTALL) -D -m 0755 $(@D)/bin/kmsview \ + $(TARGET_DIR)/usr/bin/kmsview + $(INSTALL) -D -m 0755 $(@D)/bin/testpat \ + $(TARGET_DIR)/usr/bin/testpat + $(INSTALL) -D -m 0755 $(@D)/bin/wbcap \ + $(TARGET_DIR)/usr/bin/wbcap + $(INSTALL) -D -m 0755 $(@D)/bin/wbm2m \ + $(TARGET_DIR)/usr/bin/wbm2m +endef +endif + +define KMSXX_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++.so \ + $(TARGET_DIR)/usr/lib/libkms++.so + $(INSTALL) -D -m 0755 $(@D)/lib/libkms++util.so \ + $(TARGET_DIR)/usr/lib/libkms++util.so + + $(KMSXX_INSTALL_TARGET_TESTS) +endef + +$(eval $(cmake-package))
KMS++ is a suite of library and test tools to interact with KMS drivers in the linux kernel. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- Changes from v2: - Dropped the python bindings - Rebased on top of master - Updated the version package/Config.in | 1 + package/kmsxx/Config.in | 26 ++++++++++++++++++++++++++ package/kmsxx/kmsxx.mk | 45 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 package/kmsxx/Config.in create mode 100644 package/kmsxx/kmsxx.mk