[Buildroot] [PATCH v2 03/13] package/minetest: new package
Romain Naour
romain.naour at gmail.com
Wed Jul 12 16:37:39 UTC 2017
Hi Thomas,
Le 24/06/2017 à 17:19, Thomas Petazzoni a écrit :
> Hello,
>
> On Mon, 12 Jun 2017 22:54:00 +0200, Romain Naour wrote:
>
>> + message(STATUS "GetText enabled; locales found: ${GETTEXT_AVAILABLE_LOCALES}")
>> ++ # On some platforms, such as Linux with GNU libc, the gettext
>> ++ # functions are present in the C standard library and libintl
>> ++ # is not required. For other libc (uClibc-ng or musl) libintl
>
> Only uClibc-ng in fact. musl has stub gettext functionality.
So, with your series applied during the summer camp this is not true even with
uClibc-ng.
I think BR2_SYSTEM_ENABLE_NLS should be used in minetest.mk.
>
>> +if BR2_PACKAGE_MINETEST
>> +
>> +config BR2_PACKAGE_MINETEST_CLIENT
>> + bool
>> + select BR2_PACKAGE_BZIP2
>> + select BR2_PACKAGE_LIBPNG
>> + select BR2_PACKAGE_JPEG
>> + select BR2_PACKAGE_XLIB_LIBXXF86VM
>> +
>> +config BR2_PACKAGE_MINETEST_SERVER
>> + bool
>> +
>> +choice
>> + prompt "minetest build"
>> +
>> +config BR2_PACKAGE_MINETEST_CLIENT_ONLY
>> + bool "client only"
>> + select BR2_PACKAGE_MINETEST_CLIENT
>> + help
>> + Build Minetest client only.
>> +
>> +config BR2_PACKAGE_MINETEST_SERVER_ONLY
>> + bool "server only"
>> + select BR2_PACKAGE_MINETEST_SERVER
>> + help
>> + Build Minetest server only.
>> +
>> +config BR2_PACKAGE_MINETEST_CLIENT_SERVER
>> + bool "client and server"
>> + select BR2_PACKAGE_MINETEST_CLIENT
>> + select BR2_PACKAGE_MINETEST_SERVER
>> + help
>> + Build Minetest client and server.
>> +
>> +endchoice
>
> Why are you doing all this instead of having just two sub-options, one
> to enable the client, one to enable the server?
>
> Is it because you need at least one of them to be enabled?
Indeed.
If so, then
> just add a "select BR2_PACKAGE_MINETEST_CLIENT
> if !BR2_PACKAGE_MINETEST_SERVER" in the BR2_PACKAGE_MINETEST definition
Done.
>
>> +comment "minetest needs a toolchain w/ C++, gcc >= 4.7, threads"
>> + depends on !BR2_INSTALL_LIBSTDCPP \
>> + || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 \
>> + || !BR2_TOOLCHAIN_HAS_THREADS
>
> Missing depends on BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS>
>> +
>> +comment "minetest needs X11, an OpenGL provider, luajit"
>> + depends on (BR2_INSTALL_LIBSTDCPP \
>> + && BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 \
>> + && BR2_TOOLCHAIN_HAS_THREADS)
>> + depends on !BR2_PACKAGE_HAS_LIBGL || !BR2_PACKAGE_XORG7 \
>> + || !BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS
>
> Mentioning LuaJIT is useless, since you're selecting it. However, you
> shouldn't display this comment if BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS is
> not enabled.
>
> BR2_PACKAGE_LUAJIT_ARCH_SUPPORTS is an architecture dependency, very
> much like BR2_USE_MMU, so we don't add comments about it, since there's
> nothing the user can do about it.
Fixed.
>
>> +MINETEST_VERSION = 0.4.16
>> +MINETEST_SITE = $(call github,minetest,minetest,$(MINETEST_VERSION))
>> +MINETEST_LICENSE = LGPL-2.1+, CC-BY-SA-3.0
>
> Maybe mention what is under LGPL, what is under CC-BY-SA? if it's
> clearly understood.
Done.
>
>> +MINETEST_LICENSE_FILES = README.txt
>> +
>> +MINETEST_DEPENDENCIES = irrlicht gmp jpeg jsoncpp luajit sqlite zlib
>
> Alphabetic ordering would be nicer (j before g).
You mean g before i :)
Fixed.
>
>> +
>> +MINETEST_CONF_OPTS = \
>> + -DDEFAULT_RUN_IN_PLACE=OFF \
>> + -DENABLE_CURL=OFF \
>> + -DENABLE_GETTEXT=OFF \
>> + -DENABLE_SOUND=OFF \
>> + -DENABLE_GLES=OFF \
>> + -DENABLE_FREETYPE=OFF \
>> + -DENABLE_LUAJIT=ON \
>> + -DENABLE_CURSES=OFF \
>> + -DENABLE_POSTGRESQL=OFF \
>> + -DENABLE_LEVELDB=OFF \
>> + -DENABLE_REDIS=OFF \
>> + -DENABLE_SPATIAL=OFF \
>> + -DAPPLY_LOCALE_BLACKLIST=OFF \
>> + -DENABLE_SYSTEM_GMP=ON \
>> + -DENABLE_SYSTEM_JSONCPP=ON
>> +
>> +ifeq ($(BR2_PACKAGE_MINETEST_CLIENT),y)
>> +MINETEST_DEPENDENCIES += bzip2 jpeg libgl libpng xlib_libXxf86vm
>
> jpeg is already part of the unconditional dependencies.
Fixed.
Best regards,
Romain
>
> Best regards,
>
> Thomas
>
More information about the buildroot
mailing list