All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baruch Siach <baruch@tkos.co.il>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/1] encfs: new package
Date: Wed, 29 Aug 2018 09:13:58 +0300	[thread overview]
Message-ID: <87efehya1l.fsf@tkos.co.il> (raw)
In-Reply-To: <1535517151-26179-1-git-send-email-yairba@tkos.co.il>

Hi Yair,

Thanks for your contribution. A few comments below.

Yair Ben Avraham writes:
> EncFS: an Encrypted Filesystem for FUSE.
> Signed-off-by: Yair Ben Avraham <yairba@tkos.co.il>

You should add an empty line before the sign-off line.

> ---
>  package/Config.in                                  |  1 +
>  ...Remove-make-j2-unittests-test-integration.patch | 33 +++++++++++++++++++++
>  package/encfs/Config.in                            | 24 +++++++++++++++
>  package/encfs/encfs.hash                           |  7 +++++
>  package/encfs/encfs.mk                             | 34 ++++++++++++++++++++++
>  5 files changed, 99 insertions(+)
>  create mode 100644 package/encfs/0001-Remove-make-j2-unittests-test-integration.patch
>  create mode 100644 package/encfs/Config.in
>  create mode 100644 package/encfs/encfs.hash
>  create mode 100644 package/encfs/encfs.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index f5a1749..768d182 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -181,6 +181,7 @@ menu "Filesystem and flash utilities"
>  	source "package/e2fsprogs/Config.in"
>  	source "package/e2tools/Config.in"
>  	source "package/ecryptfs-utils/Config.in"
> +	source "package/encfs/Config.in"
>  	source "package/exfat/Config.in"
>  	source "package/exfat-utils/Config.in"
>  	source "package/f2fs-tools/Config.in"
> diff --git a/package/encfs/0001-Remove-make-j2-unittests-test-integration.patch b/package/encfs/0001-Remove-make-j2-unittests-test-integration.patch
> new file mode 100644
> index 0000000..c6730d8
> --- /dev/null
> +++ b/package/encfs/0001-Remove-make-j2-unittests-test-integration.patch
> @@ -0,0 +1,33 @@
> +From b0b6cb0bfd61fd947d3d6e69708688b753849102 Mon Sep 17 00:00:00 2001
> +From: Yair Ben Avraham <yairba@tkos.co.il>
> +Date: Tue, 28 Aug 2018 21:30:56 +0300
> +Subject: [PATCH 1/1] Remove make {-j2, unittests, test, integration}
> +
> +Signed-off-by: Yair Ben Avraham <yairba@tkos.co.il>
> +---
> + build.sh | 11 +----------
> + 1 file changed, 1 insertion(+), 10 deletions(-)
> +
> +diff --git a/build.sh b/build.sh
> +index b10107c..74925a3 100755
> +--- a/build.sh
> ++++ b/build.sh
> +@@ -22,14 +22,5 @@ fi
> +
> + cd build
> + ${CMAKE} .. ${CFG}
> +-make -j2
> +-make unittests
> +-make test
> +-if [[ "$INTEGRATION" == "true" ]]; then
> +-  make integration
> +-fi
> +-
> + cd ..
> +-
> +-echo
> +-echo 'Everything looks good, you can install via "make install -C build".'
> ++make
> +--
> +2.7.4
> +
> diff --git a/package/encfs/Config.in b/package/encfs/Config.in
> new file mode 100644
> index 0000000..a56ef48
> --- /dev/null
> +++ b/package/encfs/Config.in
> @@ -0,0 +1,24 @@
> +config BR2_PACKAGE_ENCFS
> +	bool "encfs"
> +	depends on BR2_USE_WCHAR
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on BR2_USE_MMU
> +	depends on !BR2_STATIC_LIBS
> +	depends on BR2_PACKAGE_HOST_CMAKE

Should not be needed. The cmake package infrastructure automatically
builds host-cmake if there is no suitable host installed cmake.

> +	depends on BR2_TOOLCHAIN_BUILDROOT_CXX

This is only enabled for the internal toolchain. Use the
BR2_INSTALL_LIBSTDCPP dependency instead to cover both internal and
external toolchains.

> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_LIBFUSE
> +	select BR2_PACKAGE_LIBGLIB2

You do not add libglib2 to the package _DEPENDENCIES. Is that intended?
If so, please explain in a comment.

> +	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE

Same here.

> +	help
> +	  EncFS is an Encrypted Filesystem for FUSE. EncFS provides an encrypted
> +	  filesystem in user-space. It runs in userspace, using the FUSE library
> +	  for the filesystem interface.

The first sentence looks redundant to me.

> +
> +	  https://github.com/vgough/encfs
> +
> +comment "encfs needs a toolchain w/ wchar, threads, dynamic library"

You need to mention C++ as well...

> +	depends on BR2_USE_MMU
> +	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS

... and add !BR2_INSTALL_LIBSTDCPP here.

> +	depends on BR2_PACKAGE_HOST_CMAKE
> +	depends on BR2_TOOLCHAIN_BUILDROOT_CXX

These dependencies should not be needed. They effectively hide the
comment that we want to show.

> diff --git a/package/encfs/encfs.hash b/package/encfs/encfs.hash
> new file mode 100644
> index 0000000..73aa4c3
> --- /dev/null
> +++ b/package/encfs/encfs.hash
> @@ -0,0 +1,7 @@
> +# Locally calculated after checking pgp signature
> +sha256	3cc9d8fb3592319d7c50ae968e7e2a7c4b4463e1e5421c6c6d5ef9a1ac27a2a2	encfs-1.9.5.zip

Which signature have you checked? I could not find a signature for the
zip archive.

If you use the tarball (see below), please add a reference to the
signature URL in the comment.

> +# Hash for license files:
> +sha256	fdba14f0ecddd3e6d31a76177045d99df90070e6377967fdbfbc616166e0dcce	COPYING
> +sha256	8ceb4b9ee5adedde47b31e975c1d90c73ad27b6b165a1dcd80c7c545eb65b903	COPYING.GPL
> +sha256	da7eabb7bafdf7d3ae5e9f223aa5bdc1eece45ac569dc21b3b037520b4464768	COPYING.LGPL
> diff --git a/package/encfs/encfs.mk b/package/encfs/encfs.mk
> new file mode 100644
> index 0000000..8147188
> --- /dev/null
> +++ b/package/encfs/encfs.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# encfs
> +#
> +################################################################################
> +
> +ENCFS_VERSION = 1.9.5
> +ENCFS_SOURCE = encfs-$(ENCFS_VERSION).zip
> +ENCFS_SITE = $(call github,vgough,encfs,v$(ENCFS_VERSION))

Upstream provides a proper tarball:

  https://github.com/vgough/encfs/releases/download/v1.9.5/encfs-1.9.5.tar.gz

Please use that instead of the github helper.

> +ENCFS_LICENSE = LGPL (libencfs), GPL, MIT (easylogging++), Zlib (tinyxml2)

Please use the Buildroot tinyxml2 package. You can set the
USE_INTERNAL_TINYXML cmake variable to OFF.

> +ENCFS_LICENSE_FILES = COPYING COPYING.GPL COPYING.LGPL
> +ENCFS_DEPENDENCIES = \
> +	libfuse openssl
> +
> +define ENCFS_EXTRACT_CMDS
> +        $(UNZIP) -d $(BUILD_DIR) $(ENCFS_DL_DIR)/$(ENCFS_SOURCE)
> +endef

Not needed if you use the tarball.

> +
> +define ENCFS_BUILD_CMDS
> +        (cd $(@D) && $(TARGET_MAKE_ENV) ./build.sh)
> +endef
> +
> +define ENCFS_INSTALL_CMDS
> +        (cd $(@D) && make install)
> +endef

Why cant use use the default cmake build and install rules? Please
explain in a comment and in the commit log. In case these special rules
are not needed, you can drop the build.sh patch as well.

> +ENCFS_CONF_OPTS += \
> +        -DFUSE_INCLUDE_DIR=$(BUILD_DIR)/libfuse* \

You should point to the Buildroot staging directory.

> +        -DCMAKE_CROSSCOMPILING=1 \

Why is this needed?

> +        -DRUNTIME_OUTPUT_DIRECTORY=$(BUILD_DIR) \
> +        -DCMAKE_RUNTIME_OUTPUT_DIRECTORY=$(BUILD_DIR)/usr/bin \

These are only used for the bundled libxml2. If you use the Buildroot
package, this would not be needed.

> +        -DBUILD_UNIT_TESTS=0
> +
> +$(eval $(cmake-package))

baruch

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

  reply	other threads:[~2018-08-29  6:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29  4:32 [Buildroot] [PATCH 1/1] encfs: new package Yair Ben Avraham
2018-08-29  6:13 ` Baruch Siach [this message]
2018-08-29  8:47   ` Yair Ben Avraham
2018-08-29  9:01     ` Baruch Siach

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=87efehya1l.fsf@tkos.co.il \
    --to=baruch@tkos.co.il \
    --cc=buildroot@busybox.net \
    /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.