Re: [ptxdist] [PATCH] zstd: new package

[Roland Hieber <rhi@pengutronix.de>]

Hi Florian,

unfortunately your mailer seems to apply word wrapping to the patch, so
it didn't apply for me. The git-format-patch manpage [1] has some tips how to
prevent that.

[1]: https://git-scm.com/docs/git-format-patch#_thunderbird

Nevertheless some feedback below:

On Fri, Dec 27, 2019 at 06:13:19PM +0100, Florian Faber wrote:
> 
> Add zstd-1.4.4 to the repository.
> 
> Signed-off-by: Florian Faber <faber@faberman.de>
> ---
> diff -upNr a/zstd.in rules/zstd.in
> --- a/zstd.in   1970-01-01 01:00:00.000000000 +0100
> +++ b/zstd.in   2019-12-18 16:56:31.000000000 +0100
> @@ -0,0 +1,42 @@
> +## SECTION=shell_and_console
> +
> +menuconfig ZSTD
> +       bool
> +       prompt "zstd"

We'd like all "-->" submenu markers to line up in the graphical menu, so
please add an appropriate amount of whitespace to the end of the string.

Also please indent the lines with one tab here instead of spaces.

> +       select HOST_CMAKE
> +       help
> +         zstd is a fast lossless compression algorithm and data
> +         compression tool, with command line syntax similar to
> +         gzip and xz. It is based on the LZ77 family, with further
> +         FSE & huff0 entropy stages. zstd offers highly configurable
> +         compression speed, with fast modes at > 200 MB/s per code,
> +         and strong modes nearing lzma compression ratios. It also
> +         features a very fast decoder, with speeds > 500 MB/s per core.
> +
> +if ZSTD
> +
> +config ZSTD_LIBZSTD
> +       tristate

Hmm, sub-options usually are not tristate, but I guess it doesn't hurt
either.

> +       prompt "install zstd library"
> +
> +config ZSTD_ZSTD
> +       tristate
> +       select ZSTD_LIBZSTD
> +       prompt "install zstd tool"
> +
> +config ZSTD_ZSTDCAT
> +       tristate
> +       select ZSTD_LIBZSTD
> +       prompt "install zstdcat"

zstdcat seems to be a link to zstd, so you have to select
ZSTD_ZSTD here too.

> +
> +config ZSTD_ZSTDGREP
> +       tristate
> +       select ZSTD_LIBZSTD
> +       prompt "install zstdgrep"
> +
> +config ZSTD_ZSTDLESS
> +       tristate
> +       select ZSTD_LIBZSTD
> +       prompt "install zstdless"
> +
> +endif
> diff -upNr a/zstd.make rules/zstd.make
> --- a/zstd.make 1970-01-01 01:00:00.000000000 +0100
> +++ b/zstd.make 2019-12-18 16:52:12.000000000 +0100
> @@ -0,0 +1,69 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2019 by Florian Faber <faber@faberman.de>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_ZSTD) += zstd
> +
> +#
> +# Paths and names
> +#
> +ZSTD_VERSION   := 1.4.4
> +ZSTD_MD5       := 532aa7b3a873e144bbbedd9c0ea87694
> +ZSTD           := zstd-$(ZSTD_VERSION)
> +ZSTD_SUFFIX    := tar.gz
> +ZSTD_URL       :=
> https://github.com/facebook/zstd/archive/v$(ZSTD_VERSION).$(ZSTD_SUFFIX)
> +ZSTD_SOURCE    := $(SRCDIR)/$(ZSTD).$(ZSTD_SUFFIX)
> +ZSTD_DIR       := $(BUILDDIR)/$(ZSTD)
> +ZSTD_SUBDIR    := build/cmake
> +ZSTD_LICENSE   := BSD
> +
> +#
> ----------------------------------------------------------------------------
> +# Prepare
> +#
> ----------------------------------------------------------------------------

Mangled line, keep these separators on one line with the comment marker.

> +
> +ZSTD_CONF_TOOL := cmake
> +ZSTD_BUILD_DIR  := $(ZSTD_DIR)-build
> +ZSTD_CONF_OPT  := \
> +       $(CROSS_CMAKE_USR) \
> +       -B$(ZSTD_BUILD_DIR)
> +
> +#
> ----------------------------------------------------------------------------
> +# Target-Install
> +#
> ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/zstd.targetinstall:
> +       @$(call targetinfo)

Make sure to indent with tabs here, not spaces, or make will error out
because of syntax errors.

> +
> +       @$(call install_init, zstd)
> +       @$(call install_fixup, zstd, PRIORITY, optional)
> +       @$(call install_fixup, zstd, SECTION, base)
> +       @$(call install_fixup, zstd, AUTHOR, "Florian Faber
> <faber@faberman.de>")

Mangled line.

> +       @$(call install_fixup, zstd, DESCRIPTION, missing)
> +
> +ifdef PTXCONF_ZSTD_LIBZSTD
> +       $(call install_lib, zstd, 0, 0, 0644, libzstd)
> +endif
> +ifdef PTXCONF_ZSTD_ZSTD
> +       $(call install_copy, zstd, 0, 0, 0755, -, /usr/bin/zstd)
> +endif
> +ifdef PTXCONF_ZSTD_ZSTDCAT
> +       $(call install_copy, zstd, 0, 0, 0755, -, /usr/bin/zstdcat)

This generates an error:

  ptxdist: error: file 'distrokit-next/platform-v7a/packages/zstd-1.4.4/usr/bin/zstdcat' is a link
   using 'distrokit-next/platform-v7a/packages/zstd-1.4.4/usr/bin/zstd' instead

Use $(call install_link, ...) instead.

  - Roland

> +endif
> +ifdef PTXCONF_ZSTD_ZSTDGREP
> +       $(call install_copy, zstd, 0, 0, 0755, -, /usr/bin/zstdgrep)
> +endif
> +ifdef PTXCONF_ZSTD_ZSTDLESS
> +       $(call install_copy, zstd, 0, 0, 0755, -, /usr/bin/zstdless)
> +endif
> +
> +       @$(call install_finish, zstd)
> +
> +       @$(call touch)
> -- 
> Machines can do the work, so people have time to think.
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de