All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: TIAN Yuanhao <tianyuanhao3@163.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH/next 6/6] package/micromamba: new package
Date: Tue, 7 Feb 2023 17:26:15 +0100	[thread overview]
Message-ID: <20230207172615.48ac9871@windsurf> (raw)
In-Reply-To: <20221128123418.2197-6-tianyuanhao3@163.com>

Hello,

On Mon, 28 Nov 2022 12:34:18 +0000
TIAN Yuanhao <tianyuanhao3@163.com> wrote:

> Signed-off-by: TIAN Yuanhao <tianyuanhao3@163.com>

Thanks for this submission. Could you clarify what is the motivation
for this package? Indeed having a package manager on the target feels a
bit strange in the context of Buildroot. Buildroot is normally there
precisely to generate a ready-to-use rootfs, not a rootfs which then
downloads random stuff from the Internet.

Of course, we do have npm or pip, but really their usage is not
recommended.


> diff --git a/package/micromamba/Config.in b/package/micromamba/Config.in
> new file mode 100644
> index 0000000000..998809f452
> --- /dev/null
> +++ b/package/micromamba/Config.in
> @@ -0,0 +1,43 @@
> +config BR2_PACKAGE_MICROMAMBA_ARCH_SUPPORTS
> +	bool
> +	# See libmamba/include/mamba/core/context.hpp
> +	default y if BR2_arm && (BR2_ARM_CPU_ARMV6 || BR2_ARM_CPU_ARMV7A)
> +	default y if BR2_aarch64
> +	default y if BR2_i386
> +	default y if BR2_powerpc64
> +	default y if BR2_powerpc64le
> +	default y if BR2_s390x
> +	default y if BR2_x86_64
> +	depends on BR2_USE_MMU # fork()

Always a bit annoying to have architecture-specific things in something
that isn't hardware-related in the first place, but OK.

> +config BR2_PACKAGE_MICROMAMBA
> +	bool "micromamba"
> +	depends on BR2_PACKAGE_MICROMAMBA_ARCH_SUPPORTS
> +	depends on BR2_USE_WCHAR # fmt, reproc

libarchive also needs wchar, spdlog as well (through fmt, admittedly)

> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # pthread
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_8 # C++17 filesystem
> +	select BR2_PACKAGE_LIBARCHIVE
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_JSON_FOR_MODERN_CPP
> +	select BR2_PACKAGE_YAML_CPP
> +	select BR2_PACKAGE_SPDLOG
> +	select BR2_PACKAGE_LIBCURL
> +	select BR2_PACKAGE_LIBSOLV
> +	select BR2_PACKAGE_REPROC
> +	select BR2_PACKAGE_TL_EXPECTED
> +	select BR2_PACKAGE_CLI11
> +	select BR2_PACKAGE_FMT
> +	select BR2_PACKAGE_TERMCOLOR

These select should be sorted alphabetically to look a bit nicer.

> +	help
> +	  micromamba is a small, pure-C++ reimplementation of
> +	  mamba/conda. It strives to be a full replacement for mamba and
> +	  conda.
> +
> +	  https://github.com/mamba-org/mamba
> +
> +comment "micromamba needs a toolchain w/ wchar, threads, C++, gcc >= 8"
> +	depends on BR2_PACKAGE_MICROMAMBA_ARCH_SUPPORTS
> +	depends on !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP || \
> +		!BR2_TOOLCHAIN_HAS_THREADS || \
> +		!BR2_TOOLCHAIN_GCC_AT_LEAST_8
> diff --git a/package/micromamba/micromamba.hash b/package/micromamba/micromamba.hash
> new file mode 100644
> index 0000000000..e584c3bd28
> --- /dev/null
> +++ b/package/micromamba/micromamba.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  7303d983b49a1a52b302ceae355af1c05afef3a07aa3ad6dd27c36d64c43f991  micromamba-1.0.0.tar.gz
> +sha256  41fd98a468e39d319911bd94f4e65d6ad6a7ea66559dd5aa4112f138ff9b629a  LICENSE
> diff --git a/package/micromamba/micromamba.mk b/package/micromamba/micromamba.mk
> new file mode 100644
> index 0000000000..188864444c
> --- /dev/null
> +++ b/package/micromamba/micromamba.mk
> @@ -0,0 +1,43 @@
> +################################################################################
> +#
> +# micromamba
> +#
> +################################################################################
> +
> +MICROMAMBA_VERSION = 1.0.0
> +MICROMAMBA_SITE = $(call github,mamba-org,mamba,micromamba-$(MICROMAMBA_VERSION))
> +MICROMAMBA_LICENSE = BSD-3-Clause
> +MICROMAMBA_LICENSE_FILES = LICENSE
> +MICROMAMBA_DEPENDENCIES = \
> +	$(BR2_PYTHON3_HOST_DEPENDENCY) \
> +	cli11 \
> +	fmt \
> +	json-for-modern-cpp \
> +	libarchive \
> +	libcurl \
> +	libsolv \
> +	openssl \
> +	reproc \
> +	spdlog \
> +	termcolor \
> +	tl-expected \
> +	yaml-cpp
> +
> +MICROMAMBA_CONF_OPTS = -DBUILD_LIBMAMBA=ON -DBUILD_MICROMAMBA=ON
> +
> +# See libmamba/include/mamba/core/context.hpp
> +ifeq ($(BR2_ARM_CPU_ARMV6),y)
> +MICROMAMBA_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -D___ARM_ARCH_6__"
> +else ifeq ($(BR2_ARM_CPU_ARMV7A),y)
> +MICROMAMBA_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -D__ARM_ARCH_7__"
> +endif

This is really a bug in the mamba code, because these defines are
normally provided by the compiler. For example, an ARMv7 compiler
defines:

#define __ARM_ARCH 7
#define __ARM_ARCH_7A__ 1

So mamba could use that instead.

But one could even wonder why it really matters to define a different
MAMBA_PLATFORM name:

#ifdef ___ARM_ARCH_6__
        static const char MAMBA_PLATFORM[] = "linux-armv6l";
#elif __ARM_ARCH_7__
        static const char MAMBA_PLATFORM[] = "linux-armv7l";
#else

It is really crazy that this is the *only* thing that causes mamba to
have an architecture dependency. Why is that even needed in the first
place?

> +
> +ifeq ($(BR2_STATIC_LIBS),y)
> +MICROMAMBA_CONF_OPTS += -DBUILD_STATIC=ON -DMICROMAMBA_LINKAGE=FULL_STATIC
> +else ifeq ($(BR2_SHARED_STATIC_LIBS),y)
> +MICROMAMBA_CONF_OPTS += -DBUILD_SHARED=ON -DBUILD_STATIC=ON
> +else # BR2_SHARED_LIBS
> +MICROMAMBA_CONF_OPTS += -DBUILD_SHARED=ON
> +endif

Please pass BUILD_STATIC=OFF and BUILD_SHARED=OFF explicitly in the
appropriate cases.

Thanks a lot!

Thomas Petazzoni
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2023-02-07 16:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 12:34 [Buildroot] [PATCH/next 1/6] package/cli11: new package TIAN Yuanhao
2022-11-28 12:34 ` [Buildroot] [PATCH/next 2/6] package/termcolor: " TIAN Yuanhao
2023-02-07 16:04   ` Thomas Petazzoni via buildroot
2022-11-28 12:34 ` [Buildroot] [PATCH/next 3/6] package/tl-expected: " TIAN Yuanhao
2023-02-07 16:04   ` Thomas Petazzoni via buildroot
2022-11-28 12:34 ` [Buildroot] [PATCH/next 4/6] package/libsolv: " TIAN Yuanhao
2023-02-07 16:14   ` Thomas Petazzoni via buildroot
2022-11-28 12:34 ` [Buildroot] [PATCH/next 5/6] package/reproc: " TIAN Yuanhao
2023-02-07 16:14   ` Thomas Petazzoni via buildroot
2022-11-28 12:34 ` [Buildroot] [PATCH/next 6/6] package/micromamba: " TIAN Yuanhao
2023-02-07 16:26   ` Thomas Petazzoni via buildroot [this message]
2023-02-07 16:03 ` [Buildroot] [PATCH/next 1/6] package/cli11: " Thomas Petazzoni via buildroot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230207172615.48ac9871@windsurf \
    --to=buildroot@buildroot.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tianyuanhao3@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.