diff mbox series

[v2,1/1] package/micropython: make use of recent micropython-lib

Message ID 20240129022913.3367510-1-abiliojr@gmail.com
State Accepted
Headers show
Series [v2,1/1] package/micropython: make use of recent micropython-lib | expand

Commit Message

Abilio Marques Jan. 29, 2024, 2:29 a.m. UTC
Until now, micropython-lib was a package that installed v1.9.3,
which is more than 6 years old. This was acceptable since micropython
never made any other official release of the library until v1.20.

Meanwhile, the libraries underwent a reorganization, and they are now
available in a directory structure that cannot be copied directly into
the target. This is my theory on why v1.9.3 is still present in the
current day buildroot (which comes with micropython v1.22).

This commit introduces an auxiliary script to collect those libraries
and reorder them into a structure that can then be copied into
/usr/lib/micropython. The script utilizes a module from the tools
directory of the micropython repo.

As part of the changes made by the micropython project, the libraries
are now released together with the interpreter. They are cloned as a
submodule into the lib/micropython-lib directory.

These are the 2 main reasons why the old buildroot micropython-lib
package is being removed.

With this commit, micropython-lib is installed (optionally) as part
of micropython. The original config variable name was retained as it
fits with the micropython package 'namespace", and thus this is
backward compatible and no legacy handling is needed.

I tried to keep it simple. It makes use of existing micropython tools
(used to process manifests) to discover the list of packages available
in micropython-lib. My hope is that by relying on them, any future
changes in directory structure will be covered by the official
"manifestfile.py" tool.

This commit also ensures that the libraries in micropython-lib will
be updated together with newer versions of micropython.

Signed-off-by: Abilio Marques <abiliojr@gmail.com>
---
 DEVELOPERS                                    |  1 -
 package/Config.in                             |  1 -
 package/micropython-lib/Config.in             |  8 --
 package/micropython-lib/micropython-lib.hash  |  3 -
 package/micropython-lib/micropython-lib.mk    | 18 -----
 package/micropython/Config.in                 |  8 ++
 .../micropython/collect_micropython_lib.py    | 79 +++++++++++++++++++
 package/micropython/micropython.mk            | 17 ++++
 8 files changed, 104 insertions(+), 31 deletions(-)
 delete mode 100644 package/micropython-lib/Config.in
 delete mode 100644 package/micropython-lib/micropython-lib.hash
 delete mode 100644 package/micropython-lib/micropython-lib.mk
 create mode 100755 package/micropython/collect_micropython_lib.py

Comments

Yann E. MORIN Jan. 29, 2024, 6:49 p.m. UTC | #1
Abilio, All,

Thanks for this new iteration!

On 2024-01-28 18:29 -0800, Abilio Marques spake thusly:
> Until now, micropython-lib was a package that installed v1.9.3,
> which is more than 6 years old. This was acceptable since micropython
> never made any other official release of the library until v1.20.
> 
> Meanwhile, the libraries underwent a reorganization, and they are now
> available in a directory structure that cannot be copied directly into
> the target. This is my theory on why v1.9.3 is still present in the

We try to avoid singular first-person messages in the commit log, and
prefer the more neutral plural first-person "we", or the passive tense.
Here, I'd rephrase it as;

    This might explain why v1.9.3 is still...

> current day buildroot (which comes with micropython v1.22).
> 
> This commit introduces an auxiliary script to collect those libraries
> and reorder them into a structure that can then be copied into
> /usr/lib/micropython. The script utilizes a module from the tools
> directory of the micropython repo.
> 
> As part of the changes made by the micropython project, the libraries
> are now released together with the interpreter. They are cloned as a
> submodule into the lib/micropython-lib directory.
> 
> These are the 2 main reasons why the old buildroot micropython-lib
> package is being removed.
> 
> With this commit, micropython-lib is installed (optionally) as part
> of micropython. The original config variable name was retained as it
> fits with the micropython package 'namespace", and thus this is
> backward compatible and no legacy handling is needed.
> 
> I tried to keep it simple. It makes use of existing micropython tools
> (used to process manifests) to discover the list of packages available
> in micropython-lib. My hope is that by relying on them, any future
> changes in directory structure will be covered by the official
> "manifestfile.py" tool.

Ditto, no "I" or "my". No need to resend just ofe of the phrasing, it
can be fixed when applying.

One thing that again got me down the host-micropython rabbit-hole, is
that we don't need host-micropython at all, because the manifestfile
module is actual CPython code, not micropython code, so we really, like
really-really, want to write that script for CPython.

[--SNIP--]
> diff --git a/package/micropython/micropython.mk b/package/micropython/micropython.mk
> index 37c148da94..5a0c5616a5 100644
> --- a/package/micropython/micropython.mk
> +++ b/package/micropython/micropython.mk
> @@ -57,4 +57,21 @@ define MICROPYTHON_INSTALL_TARGET_CMDS
>  		install
>  endef
>  
> +ifeq ($(BR2_PACKAGE_MICROPYTHON_LIB),y)
> +define MICROPYTHON_COLLECT_LIBS
> +	$(EXTRA_ENV) PYTHONPATH=$(@D)/tools:/$PYTHONPATH \

There is no need to complement PYTHON path with $PYTHONPATH. Indeed,
PYTHONPATH only *augments* the search path, it does not replace the
standard path, so we should just need:

    $(EXTRA_ENV) PYTHONPATH=$(@D)/tools \
    package/micropython/collect_micropython_lib.py \
        $(@D) $(@D)/.built_pylib

No need to resend, I'll tinker with it tonight.

Thanks!

Regards,
Yann E. MORIN.
Yann E. MORIN Jan. 29, 2024, 8:50 p.m. UTC | #2
Abilio, All,

On 2024-01-28 18:29 -0800, Abilio Marques spake thusly:
> Until now, micropython-lib was a package that installed v1.9.3,
> which is more than 6 years old. This was acceptable since micropython
> never made any other official release of the library until v1.20.
> 
> Meanwhile, the libraries underwent a reorganization, and they are now
> available in a directory structure that cannot be copied directly into
> the target. This is my theory on why v1.9.3 is still present in the
> current day buildroot (which comes with micropython v1.22).
> 
> This commit introduces an auxiliary script to collect those libraries
> and reorder them into a structure that can then be copied into
> /usr/lib/micropython. The script utilizes a module from the tools
> directory of the micropython repo.
> 
> As part of the changes made by the micropython project, the libraries
> are now released together with the interpreter. They are cloned as a
> submodule into the lib/micropython-lib directory.
> 
> These are the 2 main reasons why the old buildroot micropython-lib
> package is being removed.
> 
> With this commit, micropython-lib is installed (optionally) as part
> of micropython. The original config variable name was retained as it
> fits with the micropython package 'namespace", and thus this is
> backward compatible and no legacy handling is needed.
> 
> I tried to keep it simple. It makes use of existing micropython tools
> (used to process manifests) to discover the list of packages available
> in micropython-lib. My hope is that by relying on them, any future
> changes in directory structure will be covered by the official
> "manifestfile.py" tool.
> 
> This commit also ensures that the libraries in micropython-lib will
> be updated together with newer versions of micropython.
> 
> Signed-off-by: Abilio Marques <abiliojr@gmail.com>

I have slightly reworded and somewhat erorganmised th ecommit log in a
more logical sequence, but it was already very good to begin with!

[--SNIP--]
> diff --git a/package/micropython/Config.in b/package/micropython/Config.in
> index 30161c8b70..966e4b958e 100644
> --- a/package/micropython/Config.in
> +++ b/package/micropython/Config.in
> @@ -9,5 +9,13 @@ config BR2_PACKAGE_MICROPYTHON
>  
>  	  http://micropython.org
>  
> +config BR2_PACKAGE_MICROPYTHON_LIB
> +	bool "micropython-lib"
> +	default y

There is no reason to 'default y' here, as the previously separate
micropython-lib did not. The new ones are no more required than the old
ones, and we really need default y' to hae a good rationale. If you
think there is one, then do not hesitate to send a follow up patch that
explains why switching to 'default y' is needed.

> +	depends on BR2_PACKAGE_MICROPYTHON

Even for a single sub-options, we prefer an enclosing if-block, so I
converted the dependency to that.

[--SNIP--]
> diff --git a/package/micropython/micropython.mk b/package/micropython/micropython.mk
> index 37c148da94..5a0c5616a5 100644
> --- a/package/micropython/micropython.mk
> +++ b/package/micropython/micropython.mk
> @@ -57,4 +57,21 @@ define MICROPYTHON_INSTALL_TARGET_CMDS
>  		install
>  endef
>  
> +ifeq ($(BR2_PACKAGE_MICROPYTHON_LIB),y)
> +define MICROPYTHON_COLLECT_LIBS
> +	$(EXTRA_ENV) PYTHONPATH=$(@D)/tools:/$PYTHONPATH \

And I have indeed dropped the latter part of PYTHONPATH, because it is
not useful and is even incorrect.

> +		package/micropython/collect_micropython_lib.py \
> +		$(@D) $(@D)/.built_pylib
> +endef
> +
> +define MICROPYTHON_INSTALL_LIBS
> +	$(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/lib/micropython
> +	cp -a $(@D)/.built_pylib/* $(TARGET_DIR)/usr/lib/micropython
> +endef
> +
> +MICROPYTHON_POST_BUILD_HOOKS += MICROPYTHON_COLLECT_LIBS
> +MICROPYTHON_POST_INSTALL_TARGET_HOOKS += MICROPYTHON_INSTALL_LIBS
> +endif
> +
> +

    $ ./utils/docker-run make check-package
    package/micropython/micropython.mk:76: consecutive empty lines

So I've fixed that too. So here are the few minoor changes I made:

  - use if-block in Config.in
  - simplify PYTHONPATH
  - fix check-package
  - reword and reorder parts of the commit log

Applied to master, thanks.

Regards,
Yann E. MORIN.

>  $(eval $(generic-package))
> -- 
> 2.43.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/DEVELOPERS b/DEVELOPERS
index 9528837dd0..cc9bc5b5cc 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -572,7 +572,6 @@  F:	package/coremark/
 F:	package/coremark-pro/
 F:	package/gstreamer1/gst1-shark/
 F:	package/micropython/
-F:	package/micropython-lib/
 F:	package/syslog-ng/
 
 N:	Christian Kellermann <christian.kellermann@solectrix.de>
diff --git a/package/Config.in b/package/Config.in
index 5b8b15fa54..426bd7d090 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -798,7 +798,6 @@  menu "Lua libraries/modules"
 endmenu
 endif
 	source "package/micropython/Config.in"
-	source "package/micropython-lib/Config.in"
 	source "package/moarvm/Config.in"
 	source "package/mono/Config.in"
 if BR2_PACKAGE_MONO
diff --git a/package/micropython-lib/Config.in b/package/micropython-lib/Config.in
deleted file mode 100644
index 76557b220b..0000000000
--- a/package/micropython-lib/Config.in
+++ /dev/null
@@ -1,8 +0,0 @@ 
-config BR2_PACKAGE_MICROPYTHON_LIB
-	bool "micropython-lib"
-	depends on BR2_PACKAGE_MICROPYTHON
-	select BR2_PACKAGE_PCRE # runtime
-	help
-	  Core Python libraries ported to MicroPython.
-
-	  http://micropython.org
diff --git a/package/micropython-lib/micropython-lib.hash b/package/micropython-lib/micropython-lib.hash
deleted file mode 100644
index cbdda23844..0000000000
--- a/package/micropython-lib/micropython-lib.hash
+++ /dev/null
@@ -1,3 +0,0 @@ 
-# Locally computed
-sha256  66e15380eb109613263beb6825b8eecb9191088270c1a59e8c7d922dd57183c7  micropython-lib-1.9.3.tar.gz
-sha256  baed4196a4310c576c2010f0a49f987a49e63856df7cd45af11cb3571df4bf74  LICENSE
diff --git a/package/micropython-lib/micropython-lib.mk b/package/micropython-lib/micropython-lib.mk
deleted file mode 100644
index 78ac0d3b35..0000000000
--- a/package/micropython-lib/micropython-lib.mk
+++ /dev/null
@@ -1,18 +0,0 @@ 
-################################################################################
-#
-# micropython-lib
-#
-################################################################################
-
-MICROPYTHON_LIB_VERSION = 1.9.3
-MICROPYTHON_LIB_SITE = $(call github,micropython,micropython-lib,v$(MICROPYTHON_LIB_VERSION))
-MICROPYTHON_LIB_LICENSE = Python-2.0 (some modules), MIT (everything else)
-MICROPYTHON_LIB_LICENSE_FILES = LICENSE
-
-define MICROPYTHON_LIB_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
-		PREFIX=$(TARGET_DIR)/usr/lib/micropython \
-		install
-endef
-
-$(eval $(generic-package))
diff --git a/package/micropython/Config.in b/package/micropython/Config.in
index 30161c8b70..966e4b958e 100644
--- a/package/micropython/Config.in
+++ b/package/micropython/Config.in
@@ -9,5 +9,13 @@  config BR2_PACKAGE_MICROPYTHON
 
 	  http://micropython.org
 
+config BR2_PACKAGE_MICROPYTHON_LIB
+	bool "micropython-lib"
+	default y
+	depends on BR2_PACKAGE_MICROPYTHON
+	select BR2_PACKAGE_PCRE # runtime
+	help
+	  Core Python libraries ported to MicroPython.
+
 comment "micropython needs a toolchain w/ threads, dynamic library"
 	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
diff --git a/package/micropython/collect_micropython_lib.py b/package/micropython/collect_micropython_lib.py
new file mode 100755
index 0000000000..8dce78337a
--- /dev/null
+++ b/package/micropython/collect_micropython_lib.py
@@ -0,0 +1,79 @@ 
+#!/usr/bin/env python3
+
+"""
+Sometime after v1.9.3, micropython-lib's directory structure was cleaned up and
+enhanced with manifest files.
+
+Though cleaner, it means it cannot be directly copied into the target.
+This script is during build process to perform that conversion.
+
+It makes use of manifestfile.py, which is normally located in micropython's
+tool directory.
+
+It also depends on the micropython-lib that is cloned in lib/micropython-lib
+during build.
+"""
+
+import argparse
+import manifestfile
+import os
+import shutil
+
+
+def get_library_name_type(manifest_path: str) -> tuple[str, bool]:
+    split = manifest_path.split("/")
+    return (split[-2], "unix-ffi" in split)  # -1: "manifest.py", -2: library name
+
+
+def get_all_libraries(mpy_lib_dir: str):
+    # reuse manifestfile module capabilities to scan the micropython-lib directory
+
+    collected_list = manifestfile.ManifestFile(
+        manifestfile.MODE_FREEZE, {"MPY_LIB_DIR": mpy_lib_dir}
+    )
+    collected_list.freeze(mpy_lib_dir)
+
+    for file in collected_list.files():
+        if file.target_path.endswith("manifest.py"):
+            yield get_library_name_type(file.full_path)
+
+
+def copy_file(src: str, target: str, destination: str):
+    s = target.split("/")
+    s.pop()
+    d = f"{destination}/{'/'.join(s)}"
+    os.makedirs(d, exist_ok=True)
+    shutil.copy(src, f"{destination}/{target}")
+
+
+def copy_libraries(manifest, destination: str):
+    for f in manifest.files():
+        copy_file(f.full_path, f.target_path, destination)
+
+
+def process_cmdline_args():
+    parser = argparse.ArgumentParser(
+        description="Prepare micropython-lib to be installed"
+    )
+    parser.add_argument("micropython", help="Path to micropython source directory")
+    parser.add_argument("destination", help="Destination directory")
+
+    args = parser.parse_args()
+    return os.path.abspath(args.micropython), os.path.abspath(args.destination)
+
+
+def main():
+    micropython_dir, destination_dir = process_cmdline_args()
+    mpy_lib_dir = f"{micropython_dir}/lib/micropython-lib"
+
+    manifest = manifestfile.ManifestFile(
+        manifestfile.MODE_FREEZE, {"MPY_LIB_DIR": mpy_lib_dir}
+    )
+
+    for library, is_ffi in get_all_libraries(mpy_lib_dir):
+        manifest.require(library, unix_ffi=is_ffi)
+
+    copy_libraries(manifest, destination_dir)
+
+
+main()
diff --git a/package/micropython/micropython.mk b/package/micropython/micropython.mk
index 37c148da94..5a0c5616a5 100644
--- a/package/micropython/micropython.mk
+++ b/package/micropython/micropython.mk
@@ -57,4 +57,21 @@  define MICROPYTHON_INSTALL_TARGET_CMDS
 		install
 endef
 
+ifeq ($(BR2_PACKAGE_MICROPYTHON_LIB),y)
+define MICROPYTHON_COLLECT_LIBS
+	$(EXTRA_ENV) PYTHONPATH=$(@D)/tools:/$PYTHONPATH \
+		package/micropython/collect_micropython_lib.py \
+		$(@D) $(@D)/.built_pylib
+endef
+
+define MICROPYTHON_INSTALL_LIBS
+	$(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/lib/micropython
+	cp -a $(@D)/.built_pylib/* $(TARGET_DIR)/usr/lib/micropython
+endef
+
+MICROPYTHON_POST_BUILD_HOOKS += MICROPYTHON_COLLECT_LIBS
+MICROPYTHON_POST_INSTALL_TARGET_HOOKS += MICROPYTHON_INSTALL_LIBS
+endif
+
+
 $(eval $(generic-package))