Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Abilio Marques <abiliojr@gmail.com>
Cc: Chris Packham <judge.packham@gmail.com>, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v2 1/1] package/micropython: make use of recent micropython-lib
Date: Mon, 29 Jan 2024 21:50:20 +0100	[thread overview]
Message-ID: <ZbgPjOux3Jjl2KTj@landeda> (raw)
In-Reply-To: <20240129022913.3367510-1-abiliojr@gmail.com>

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

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

      parent reply	other threads:[~2024-01-29 20:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-29  2:29 [Buildroot] [PATCH v2 1/1] package/micropython: make use of recent micropython-lib Abilio Marques
2024-01-29 18:49 ` Yann E. MORIN
2024-01-29 20:50 ` Yann E. MORIN [this message]

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=ZbgPjOux3Jjl2KTj@landeda \
    --to=yann.morin.1998@free.fr \
    --cc=abiliojr@gmail.com \
    --cc=buildroot@buildroot.org \
    --cc=judge.packham@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox