Re: [Buildroot] [PATCH v9 4/7] package/flutter-engine: new package

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Adam, All,

This is really a good job you did on such a tough package!

We're entering the realm of the details, so it is a good sign that we're
close to a merge!

On 2023-09-19 14:42 -0600, Adam Duskett spake thusly:
[--SNIP--]
> diff --git a/package/flutter-engine/dot-gclient b/package/flutter-engine/dot-gclient
> new file mode 100644
> index 0000000000..4e5787419b
> --- /dev/null
> +++ b/package/flutter-engine/dot-gclient
> @@ -0,0 +1,12 @@
> +# This file is taken from the output of meta-flutter/lib/gn.py

This comment is a bit misleading: it hints that this file is generated
(it is!) from some metadata. So my question was goign to be: why can't
we carry that metadata, the script, and generate the file ourselves?

But in fact, there is no interesting metadata needed to generate that
file; the only thing that is not hard-coded in the generating script
(besides the name, version and URI) is the set of download dependencies,
all set to False in the bitbake recipe.

So indeed it is not interesting to generate that file. And thus the
comment is indeed a bit misleading. I'll try to tweak it when
applying...

> +solutions = [{
> +    "managed": False,
> +    "name": "src/flutter",
> +    "url": "https://github.com/flutter/engine.git@!FLUTTER_VERSION!",
> +    "custom_vars": {
> +        "download_android_deps": False,
> +        "download_windows_deps": False,
> +        "download_linux_deps": False
> +    },
> +}]
> diff --git a/package/flutter-engine/flutter-engine.mk b/package/flutter-engine/flutter-engine.mk
> new file mode 100644
> index 0000000000..76c6610fcd
> --- /dev/null
> +++ b/package/flutter-engine/flutter-engine.mk
> @@ -0,0 +1,234 @@
> +################################################################################
> +#
> +# flutter-engine
> +#
> +################################################################################
[--SNIP--]
> +# Generate a tarball if one does not already exist.
> +define FLUTTER_ENGINE_GENERATE_TARBALL
> +	PATH=$(BR_PATH):$(HOST_DIR)/share/depot_tools \

So here, two things:
  - you put $(HOST_DIR)/share/depot_tools in the PATH but .. -> [0]
  - you should probably set it in the front:
        PATH="$(HOST_DIR)/share/depot_tools:$(BR_PATH)"

> +	PYTHONPATH=$(HOST_DIR)/lib/python$(PYTHON3_VERSION_MAJOR) \
> +	$(FLUTTER_ENGINE_PKGDIR)/gen-tarball \
> +		--dot-gclient $(FLUTTER_ENGINE_PKGDIR)/dot-gclient \
> +		--jobs $(PARALLEL_JOBS) \
> +		--scratch-dir $(@D)/dl-tmp \
> +		--tarball-dl-path $(FLUTTER_ENGINE_TARBALL_PATH) \
> +		--version $(FLUTTER_ENGINE_VERSION)
> +endef
> +FLUTTER_ENGINE_POST_DOWNLOAD_HOOKS += FLUTTER_ENGINE_GENERATE_TARBALL
> +
> +define FLUTTER_ENGINE_EXTRACT_CMDS
> +	gzip -d -c $(FLUTTER_ENGINE_TARBALL_PATH) | tar --strip-components 1 -C $(@D) -xf -

We usually use suitable-extractor to decompress, and use $(TAR) and
$(TAR_OPTIONS) to extract:

    $(call suitable-extractor,$(not dir $(FLUTTER_ENGINE_TARBALL_PATH))) $(FLUTTER_ENGINE_TARBALL_PATH) \
    |$(TAR) --strip-components 1 -C $(@D) $(TAR_OPTIONS) -

[--SNIP--]
> diff --git a/package/flutter-engine/gen-tarball b/package/flutter-engine/gen-tarball
> new file mode 100755
> index 0000000000..e040beac75
> --- /dev/null
> +++ b/package/flutter-engine/gen-tarball
> @@ -0,0 +1,111 @@
[--SNIP--]
> +run_gclient() {
> +  local gclient="${HOST_DIR}"/share/depot_tools/gclient.py

[0] here you don't use PATH to locate gclient.py.

> +  if [[ ! -e "${gclient}" ]]; then
> +    message "${gclient} does not exist. Is host-depot-tools built and installed?"

That should not happen because host-depot-tools is a dependency of
flutter-engine.

> +    exit 1
> +  fi
> +  message "Downloading"
> +  "${gclient}" \

Assuming $(HOST_DIR)/share/depot_tools first int he PATH, can we just
run:

    gclient.py \
        sync \
        [...]

> +    sync \
> +    --delete_unversioned_trees \
> +    --no-history \
> +    --reset \
> +    --shallow \
> +    -j"${JOBS}"
> +}
> +
> +gen_tarball() {
> +  message "Generating tarball"
> +  mkdir -p "${DL_DIR}"
> +  # We would use the mk_tar_gz helper method from the support/download/helpers
> +  # script here. However, due to unknown reasons other than what I can
> +  # articulate as "Google," this is not possible. Even though creating a
> +  # tarball using the mk_tar_gz command results in a tarball that extracts and
> +  # builds flutter-engine without warnings or errors, the gen_snapshot host
> +  # utility, which turns dart applications into usable .so files for the
> +  # flutter engine, results in the error:
> +  # "Can't load kernel binary: Invalid SDK hash." As we are not interested in
> +  # creating a reproducible tarball, we need to use tar -C instead.

As discussed on IRC, I think the real reason was that mk_tar_gz only
archives non-directories, i.e. files, symlinks, etc... so empty
directories are missing, and the flutter-engine buildsystem requires
them.

So, based on your replies, I may be able to ammend the commit when
applying and save you a respin. So, be careful in your replies! ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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