diff mbox series

[1/1] package/ecryptfs-utils: Add support without gettext

Message ID 20190312222035.16834-1-vadim4j@gmail.com
State Changes Requested
Headers show
Series [1/1] package/ecryptfs-utils: Add support without gettext | expand

Commit Message

Vadym Kochan March 12, 2019, 10:20 p.m. UTC
Add custom ecryptfs-common script which provides gettext wrapper as
function which checks at runtime if there is gettext tool, if no - the
"echo" will be used instead. Each script which uses gettext is
patched with including this ecryptfs-common script.

The patch for ecryptfs-utils only inserts hard-coded (w/o using
@prefix@/lib/@PACKAGE@ variables) path to the ecryptfs-common, it is
very small and trivial, it allows to easy support new bumped version.

gettext package is now selected automatically only if NLS is enabled.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 ...cho-instead-of-gettext-if-it-does-not-exi.patch | 117 +++++++++++++++++++++
 package/ecryptfs-utils/Config.in                   |   2 +-
 package/ecryptfs-utils/ecryptfs-common             |  33 ++++++
 package/ecryptfs-utils/ecryptfs-utils.mk           |   6 ++
 4 files changed, 157 insertions(+), 1 deletion(-)
 create mode 100644 package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
 create mode 100755 package/ecryptfs-utils/ecryptfs-common

Comments

Thomas Petazzoni March 12, 2019, 10:14 p.m. UTC | #1
Hello Vadim,

On Wed, 13 Mar 2019 00:20:35 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> Add custom ecryptfs-common script which provides gettext wrapper as
> function which checks at runtime if there is gettext tool, if no - the
> "echo" will be used instead. Each script which uses gettext is
> patched with including this ecryptfs-common script.
> 
> The patch for ecryptfs-utils only inserts hard-coded (w/o using
> @prefix@/lib/@PACKAGE@ variables) path to the ecryptfs-common, it is
> very small and trivial, it allows to easy support new bumped version.
> 
> gettext package is now selected automatically only if NLS is enabled.
> 
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  ...cho-instead-of-gettext-if-it-does-not-exi.patch | 117 +++++++++++++++++++++
>  package/ecryptfs-utils/Config.in                   |   2 +-
>  package/ecryptfs-utils/ecryptfs-common             |  33 ++++++
>  package/ecryptfs-utils/ecryptfs-utils.mk           |   6 ++
>  4 files changed, 157 insertions(+), 1 deletion(-)
>  create mode 100644 package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
>  create mode 100755 package/ecryptfs-utils/ecryptfs-common

Thanks for this proposal. I would actually perfer if ecryptfs-common
was part of the ecryptfs-utils patch, and this patch be submitted
upstream.

Thanks!

Thomas
Vadym Kochan March 12, 2019, 10:34 p.m. UTC | #2
Hi Thomas,

On Tue, Mar 12, 2019 at 11:14:51PM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> On Wed, 13 Mar 2019 00:20:35 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > Add custom ecryptfs-common script which provides gettext wrapper as
> > function which checks at runtime if there is gettext tool, if no - the
> > "echo" will be used instead. Each script which uses gettext is
> > patched with including this ecryptfs-common script.
> > 
> > The patch for ecryptfs-utils only inserts hard-coded (w/o using
> > @prefix@/lib/@PACKAGE@ variables) path to the ecryptfs-common, it is
> > very small and trivial, it allows to easy support new bumped version.
> > 
> > gettext package is now selected automatically only if NLS is enabled.
> > 
> > Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> > ---
> >  ...cho-instead-of-gettext-if-it-does-not-exi.patch | 117 +++++++++++++++++++++
> >  package/ecryptfs-utils/Config.in                   |   2 +-
> >  package/ecryptfs-utils/ecryptfs-common             |  33 ++++++
> >  package/ecryptfs-utils/ecryptfs-utils.mk           |   6 ++
> >  4 files changed, 157 insertions(+), 1 deletion(-)
> >  create mode 100644 package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
> >  create mode 100755 package/ecryptfs-utils/ecryptfs-common
> 
> Thanks for this proposal. I would actually perfer if ecryptfs-common
> was part of the ecryptfs-utils patch, and this patch be submitted
> upstream.
> 
Me too, but it requires more work like converting these scripts to *.in
form to be processed by configure because in that case it is better to
use conf variables like:

    . @prefix@/lib/@PACKAGE@

and I am not sure if upstream community would prefer this), but while having
temporary easy solution (provided by this patch) I can work on official
patch in parellel.

My worries related to point that in case upstream will not accept the
better-for-official version it will make harder to maintain such patch
when new ecryptfs-utils version is bumped.

Regards,
Vadim Kochan
Yann E. MORIN March 13, 2019, 6:53 p.m. UTC | #3
Vadim, All,

On 2019-03-13 00:20 +0200, Vadim Kochan spake thusly:
> Add custom ecryptfs-common script which provides gettext wrapper as
> function which checks at runtime if there is gettext tool, if no - the
> "echo" will be used instead. Each script which uses gettext is
> patched with including this ecryptfs-common script.
> 
> The patch for ecryptfs-utils only inserts hard-coded (w/o using
> @prefix@/lib/@PACKAGE@ variables) path to the ecryptfs-common, it is
> very small and trivial, it allows to easy support new bumped version.
> 
> gettext package is now selected automatically only if NLS is enabled.
> 
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  ...cho-instead-of-gettext-if-it-does-not-exi.patch | 117 +++++++++++++++++++++
>  package/ecryptfs-utils/Config.in                   |   2 +-
>  package/ecryptfs-utils/ecryptfs-common             |  33 ++++++
>  package/ecryptfs-utils/ecryptfs-utils.mk           |   6 ++
>  4 files changed, 157 insertions(+), 1 deletion(-)
>  create mode 100644 package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
>  create mode 100755 package/ecryptfs-utils/ecryptfs-common
> 
> diff --git a/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch b/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
> new file mode 100644
> index 0000000000..1e3318ab22
> --- /dev/null
> +++ b/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
> @@ -0,0 +1,117 @@
> +From 55755006370fb12167dcadeb084b6645712197a9 Mon Sep 17 00:00:00 2001
> +From: Vadim Kochan <vadim4j@gmail.com>
> +Date: Tue, 12 Mar 2019 02:15:33 +0200
> +Subject: [PATCH] utils: Use "echo" instead of gettext if it does not exist
> +
> +There might be a case when gettext does not exist on the system,
> +so use just "echo" instead. Added gettext wrapper which imitates
> +gettext behaviour, wrapper is used only if gettext is not found
> +by "which". ecryptfs-common is included by each script which uses
> +gettext tool.
> +
> +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> +---
> + src/utils/ecryptfs-migrate-home   | 2 ++
> + src/utils/ecryptfs-mount-private  | 2 ++
> + src/utils/ecryptfs-rewrite-file   | 2 ++
> + src/utils/ecryptfs-setup-private  | 3 +++
> + src/utils/ecryptfs-setup-swap     | 2 ++
> + src/utils/ecryptfs-umount-private | 2 ++
> + src/utils/ecryptfs-verify         | 2 ++
> + 7 files changed, 15 insertions(+)
> +
> +diff --git a/src/utils/ecryptfs-migrate-home b/src/utils/ecryptfs-migrate-home
> +index b810146..4fa4d86 100755
> +--- a/src/utils/ecryptfs-migrate-home
> ++++ b/src/utils/ecryptfs-migrate-home
> +@@ -24,6 +24,8 @@
> + 
> + set -e
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + PRIVATE_DIR="Private"
> + 
> + usage() {
> +diff --git a/src/utils/ecryptfs-mount-private b/src/utils/ecryptfs-mount-private
> +index c32708f..a6df39f 100755
> +--- a/src/utils/ecryptfs-mount-private
> ++++ b/src/utils/ecryptfs-mount-private
> +@@ -12,6 +12,8 @@
> + #  * inserts the mount passphrase into the keyring
> + #  * and mounts a user's encrypted private folder
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + PRIVATE_DIR="Private"
> + WRAPPING_PASS="LOGIN"
> + PW_ATTEMPTS=3
> +diff --git a/src/utils/ecryptfs-rewrite-file b/src/utils/ecryptfs-rewrite-file
> +index c4f67f5..146e385 100755
> +--- a/src/utils/ecryptfs-rewrite-file
> ++++ b/src/utils/ecryptfs-rewrite-file
> +@@ -17,6 +17,8 @@
> + #    You should have received a copy of the GNU General Public License
> + #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + TEXTDOMAIN="ecryptfs-utils"
> + 
> + error() {
> +diff --git a/src/utils/ecryptfs-setup-private b/src/utils/ecryptfs-setup-private
> +index e90d1d0..ca08656 100755
> +--- a/src/utils/ecryptfs-setup-private
> ++++ b/src/utils/ecryptfs-setup-private
> +@@ -6,6 +6,9 @@
> + # Ported for use on Ubuntu by Dustin Kirkland <kirkland@ubuntu.com>
> + # Copyright (C) 2008 Canonical Ltd.
> + # Copyright (C) 2007-2008 International Business Machines
> ++
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + PRIVATE_DIR="Private"
> + WRAPPING_PASS="LOGIN"
> + ECRYPTFS_DIR="/home/.ecryptfs"
> +diff --git a/src/utils/ecryptfs-setup-swap b/src/utils/ecryptfs-setup-swap
> +index 41cf18a..157b0c5 100755
> +--- a/src/utils/ecryptfs-setup-swap
> ++++ b/src/utils/ecryptfs-setup-swap
> +@@ -19,6 +19,8 @@
> + # The cryptswap setup used here follows a guide published at:
> + #  * http://ubuntumagnet.com/2007/11/creating-encrypted-swap-file-ubuntu-using-cryptsetup
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + TEXTDOMAIN="ecryptfs-utils"
> + 
> + error() {
> +diff --git a/src/utils/ecryptfs-umount-private b/src/utils/ecryptfs-umount-private
> +index 27edeaa..4ac0ced 100755
> +--- a/src/utils/ecryptfs-umount-private
> ++++ b/src/utils/ecryptfs-umount-private
> +@@ -5,6 +5,8 @@
> + # Original by Michael Halcrow, IBM
> + # Extracted to a stand-alone script by Dustin Kirkland <kirkland@ubuntu.com>
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + TEXTDOMAIN="ecryptfs-utils"
> + 
> + if grep -qs "$HOME/.Private $PWD ecryptfs " /proc/mounts 2>/dev/null; then
> +diff --git a/src/utils/ecryptfs-verify b/src/utils/ecryptfs-verify
> +index b55641d..3592d83 100755
> +--- a/src/utils/ecryptfs-verify
> ++++ b/src/utils/ecryptfs-verify
> +@@ -16,6 +16,8 @@
> + #    You should have received a copy of the GNU General Public License
> + #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + 
> ++. /usr/lib/ecryptfs-utils/ecryptfs-common
> ++
> + error() {
> + 	echo `gettext "ERROR:"` "$@" 1>&2
> + 	echo `gettext "ERROR:"` "Configuration invalid" 1>&2
> +-- 
> +2.14.1
> +
> diff --git a/package/ecryptfs-utils/Config.in b/package/ecryptfs-utils/Config.in
> index 6652d33e0e..1438c1754c 100644
> --- a/package/ecryptfs-utils/Config.in
> +++ b/package/ecryptfs-utils/Config.in
> @@ -12,7 +12,7 @@ config BR2_PACKAGE_ECRYPTFS_UTILS
>  	select BR2_PACKAGE_LIBNSS
>  	# runtime dependency only, some scripts are using the
>  	# 'gettext' program to get translations
> -	select BR2_PACKAGE_GETTEXT
> +	select BR2_PACKAGE_GETTEXT if BR2_SYSTEM_ENABLE_NLS
>  	# runtime dependency only
>  	select BR2_PACKAGE_GETENT
>  	help
> diff --git a/package/ecryptfs-utils/ecryptfs-common b/package/ecryptfs-utils/ecryptfs-common
> new file mode 100755
> index 0000000000..8d4050d7f4
> --- /dev/null
> +++ b/package/ecryptfs-utils/ecryptfs-common
> @@ -0,0 +1,33 @@
> +GETTEXT_PROG=""
> +
> +# Thanks to "Yann E. MORIN" <yann.morin.1998@free.fr>
> +# for this gettext replacement function
> +gettext_echo() {
> +	while [ ${#} -ne 0 ]; do
> +		case "${1}" in
> +			(-h)              echo "no help"; exit 0;;
> +			(-V)              echo "0.0.0"; exit 0;;

Since this is a function, you probably want to return rather than exit.

Also, we usually use leading spaces in shell scripts, not TABs.

> +			(-d|--domain)     shift 2;;
> +			(-d*|--domain=*)  shift 1;;
> +			(-e|-E|-n)        shift 1;;
> +			(-s)              shift 1;;  # Ignore?
> +			(*)               break;;
> +		esac
> +	done
> +	case ${#} in
> +		(0)   ;;
> +		(1)   echo "${1}";;           # No TEXTDOMAIN?
> +		(*)   shift; echo "${@}";;    # Ignore TEXTDOMAIN?
> +	esac
> +}
> +
> +gettext() {
> +	if [ -z "${GETTEXT_PROG}" ]; then
> +		GETTEXT_PROG=$(which gettext)
> +		if [ $? != 0 ]; then

!= is a string comparison. For numbers, you will want to use -ne (or
-eq), see below.

> +			GETTEXT_PROG=gettext_echo
> +		fi
> +	fi
> +
> +	${GETTEXT_PROG} "$@"
> +}

This means that a script that calls gettext more than once will actually
try to resolve it each time. This is not optimum.

Instead, I think ecryptfs-common should basically look something like:

    if ! which gettext >/dev/null 2>&1; then
        gettext() {
            if [ -n "${GETTEXT}" ]; then
                # Weird construct so that script that are 'set -e'
                # fail at the call site of gettext and not here.
                "${GETTEXT}" "${@}" || return $?
                return 0
            fi
            while [ ${#} -ne 0 ]; do
                case "${1}" in
                  (-h)              printf "no help\n"; return 0;;
                  (-V)              printf "0.0.0\n"; return 0;;
                  (-d|--domain)     shift 2;;
                  (-d*|--domain=*)  shift 1;;
                  (-e|-E|-n)        shift 1;;
                  (-s)              shift 1;;  # Ignore?
                  (-*)              printf "invalid option '%s'\n" "${1}" >&2; return 1;;
                  (*)               break;;
                esac
            done
            case ${#} in
              (0)   printf "missing arguments\n" >&2; return 1;;
              (1)   printf "%s" "${1}";;
              (2)   shift; printf "%s" "${2}";;
              (*)   printf "too many arguments\n" >&2; return 1;;
            esac
        }
    fi

I.e. if there is no gettext program, you define a function named
gettext.

Regards,
Yann E. MORIN.

> diff --git a/package/ecryptfs-utils/ecryptfs-utils.mk b/package/ecryptfs-utils/ecryptfs-utils.mk
> index eb3194b6d0..bf630882c2 100644
> --- a/package/ecryptfs-utils/ecryptfs-utils.mk
> +++ b/package/ecryptfs-utils/ecryptfs-utils.mk
> @@ -25,4 +25,10 @@ else
>  ECRYPTFS_UTILS_CONF_OPTS += --disable-openssl
>  endif
>  
> +define ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
> +	$(INSTALL) -D -m 0755 $(ECRYPTFS_UTILS_PKGDIR)/ecryptfs-common \
> +		$(TARGET_DIR)/usr/lib/ecryptfs-utils/ecryptfs-common
> +endef
> +ECRYPTFS_UTILS_POST_INSTALL_TARGET_HOOKS += ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
> +
>  $(eval $(autotools-package))
> -- 
> 2.14.1
>
Vadym Kochan March 13, 2019, 7:15 p.m. UTC | #4
Hi Yann,

On Wed, Mar 13, 2019 at 07:53:50PM +0100, Yann E. MORIN wrote:
> Vadim, All,
> 
> On 2019-03-13 00:20 +0200, Vadim Kochan spake thusly:
> > Add custom ecryptfs-common script which provides gettext wrapper as
> > function which checks at runtime if there is gettext tool, if no - the
> > "echo" will be used instead. Each script which uses gettext is
> > patched with including this ecryptfs-common script.
> > 
> > The patch for ecryptfs-utils only inserts hard-coded (w/o using
> > @prefix@/lib/@PACKAGE@ variables) path to the ecryptfs-common, it is
> > very small and trivial, it allows to easy support new bumped version.
> > 
> > gettext package is now selected automatically only if NLS is enabled.
> > 
> > Signed-off-by: Vadim Kochan <vadim4j@gmail.com>

[cut]

> > +GETTEXT_PROG=""
> > +
> > +# Thanks to "Yann E. MORIN" <yann.morin.1998@free.fr>
> > +# for this gettext replacement function
> > +gettext_echo() {
> > +	while [ ${#} -ne 0 ]; do
> > +		case "${1}" in
> > +			(-h)              echo "no help"; exit 0;;
> > +			(-V)              echo "0.0.0"; exit 0;;
> 
> Since this is a function, you probably want to return rather than exit.
> 
> Also, we usually use leading spaces in shell scripts, not TABs.
> 
> > +			(-d|--domain)     shift 2;;
> > +			(-d*|--domain=*)  shift 1;;
> > +			(-e|-E|-n)        shift 1;;
> > +			(-s)              shift 1;;  # Ignore?
> > +			(*)               break;;
> > +		esac
> > +	done
> > +	case ${#} in
> > +		(0)   ;;
> > +		(1)   echo "${1}";;           # No TEXTDOMAIN?
> > +		(*)   shift; echo "${@}";;    # Ignore TEXTDOMAIN?
> > +	esac
> > +}
> > +
> > +gettext() {
> > +	if [ -z "${GETTEXT_PROG}" ]; then
> > +		GETTEXT_PROG=$(which gettext)
> > +		if [ $? != 0 ]; then
> 
> != is a string comparison. For numbers, you will want to use -ne (or
> -eq), see below.
> 
> > +			GETTEXT_PROG=gettext_echo
> > +		fi
> > +	fi
> > +
> > +	${GETTEXT_PROG} "$@"
> > +}
> 
> This means that a script that calls gettext more than once will actually
> try to resolve it each time. This is not optimum.

But I check the GETTEXT_PROG variable on the start, if it is not empty then just:

	${GETTEXT_PROG} "$@"

> 
> Instead, I think ecryptfs-common should basically look something like:
> 
>     if ! which gettext >/dev/null 2>&1; then
>         gettext() {
>             if [ -n "${GETTEXT}" ]; then
>                 # Weird construct so that script that are 'set -e'
>                 # fail at the call site of gettext and not here.
>                 "${GETTEXT}" "${@}" || return $?
>                 return 0
>             fi
>             while [ ${#} -ne 0 ]; do
>                 case "${1}" in
>                   (-h)              printf "no help\n"; return 0;;
>                   (-V)              printf "0.0.0\n"; return 0;;
>                   (-d|--domain)     shift 2;;
>                   (-d*|--domain=*)  shift 1;;
>                   (-e|-E|-n)        shift 1;;
>                   (-s)              shift 1;;  # Ignore?
>                   (-*)              printf "invalid option '%s'\n" "${1}" >&2; return 1;;
>                   (*)               break;;
>                 esac
>             done
>             case ${#} in
>               (0)   printf "missing arguments\n" >&2; return 1;;
>               (1)   printf "%s" "${1}";;
>               (2)   shift; printf "%s" "${2}";;
>               (*)   printf "too many arguments\n" >&2; return 1;;
>             esac
>         }
>     fi
> 
> I.e. if there is no gettext program, you define a function named
> gettext.
> 
> Regards,
> Yann E. MORIN.
> 
> > diff --git a/package/ecryptfs-utils/ecryptfs-utils.mk b/package/ecryptfs-utils/ecryptfs-utils.mk
> > index eb3194b6d0..bf630882c2 100644
> > --- a/package/ecryptfs-utils/ecryptfs-utils.mk
> > +++ b/package/ecryptfs-utils/ecryptfs-utils.mk
> > @@ -25,4 +25,10 @@ else
> >  ECRYPTFS_UTILS_CONF_OPTS += --disable-openssl
> >  endif
> >  
> > +define ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
> > +	$(INSTALL) -D -m 0755 $(ECRYPTFS_UTILS_PKGDIR)/ecryptfs-common \
> > +		$(TARGET_DIR)/usr/lib/ecryptfs-utils/ecryptfs-common
> > +endef
> > +ECRYPTFS_UTILS_POST_INSTALL_TARGET_HOOKS += ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
> > +
> >  $(eval $(autotools-package))
> > -- 
> > 2.14.1
> > 
> 

Thanks for your comments, anyway conceptually this patch is wrong. I
tried to do it in the right way - install ecryptfs-common from
ecryptfs-utils in src/utils/Makefile.am, but faced with autoreconf issue
(autoreconf is required because src/utils/Makefile.am is changed by
patch which adds installation of ecryptfs-common), but autoreconf fails
to regenerate needed files because it removes aclocal.m4 which has
definition of AM_GLIB_GNU_GETTEXT macro (which is not defined by host-gettext
nowhere in output/host) but this macro is required by configure.ac. So,
I am not sure I can provide conceptually better patch for now (but Yann's comments),
may be it is possible to regenerate only src/utils/Makefile.in (from
*.am file) instead of calling autoreconf for all *.am, *.ac files.

Regards,
Vadim Kochan
Yann E. MORIN March 13, 2019, 9:28 p.m. UTC | #5
Vadim, All,

On 2019-03-13 21:15 +0200, Vadym Kochan spake thusly:
> On Wed, Mar 13, 2019 at 07:53:50PM +0100, Yann E. MORIN wrote:
> > On 2019-03-13 00:20 +0200, Vadim Kochan spake thusly:
[--SNIP--]
> > > +gettext() {
> > > +	if [ -z "${GETTEXT_PROG}" ]; then
> > > +		GETTEXT_PROG=$(which gettext)
> > > +		if [ $? != 0 ]; then
> > 
> > != is a string comparison. For numbers, you will want to use -ne (or
> > -eq), see below.
> > 
> > > +			GETTEXT_PROG=gettext_echo
> > > +		fi
> > > +	fi
> > > +
> > > +	${GETTEXT_PROG} "$@"
> > > +}
> > 
> > This means that a script that calls gettext more than once will actually
> > try to resolve it each time. This is not optimum.
> 
> But I check the GETTEXT_PROG variable on the start, if it is not empty then just:
> 	${GETTEXT_PROG} "$@"

Hmm... Right, and the resolution is indeed done only once. My bad.

> > Instead, I think ecryptfs-common should basically look something like:
> > 
> >     if ! which gettext >/dev/null 2>&1; then
> >         gettext() {
> >             if [ -n "${GETTEXT}" ]; then
> >                 # Weird construct so that script that are 'set -e'
> >                 # fail at the call site of gettext and not here.
> >                 "${GETTEXT}" "${@}" || return $?
> >                 return 0
> >             fi
> >             while [ ${#} -ne 0 ]; do
> >                 case "${1}" in
> >                   (-h)              printf "no help\n"; return 0;;
> >                   (-V)              printf "0.0.0\n"; return 0;;
> >                   (-d|--domain)     shift 2;;
> >                   (-d*|--domain=*)  shift 1;;
> >                   (-e|-E|-n)        shift 1;;
> >                   (-s)              shift 1;;  # Ignore?
> >                   (-*)              printf "invalid option '%s'\n" "${1}" >&2; return 1;;
> >                   (*)               break;;
> >                 esac
> >             done
> >             case ${#} in
> >               (0)   printf "missing arguments\n" >&2; return 1;;
> >               (1)   printf "%s" "${1}";;
> >               (2)   shift; printf "%s" "${2}";;
> >               (*)   printf "too many arguments\n" >&2; return 1;;
> >             esac
> >         }
> >     fi
> > 
> > I.e. if there is no gettext program, you define a function named
> > gettext.

The difference then between your code and mine, is that yours is doing a
lazy resolution (i.e. resolving at first call) while mine is doing an
early resolution (i.e. resolving when parsing).

Additionally, since your code is using a two-level function call
(gettext -> gettext_echo), it's a bit more complex to handle scritps
that are 'set -e' and ensure they get the error at the call site rather
than in internal details of the emulating function.

But my suggestion would be that we just provide a gettext wrapper script
(not function) when gettext is not installed. This is much simpler
because there is thus no need to patch upstream.

> Thanks for your comments, anyway conceptually this patch is wrong. I
> tried to do it in the right way - install ecryptfs-common from
> ecryptfs-utils in src/utils/Makefile.am, but faced with autoreconf issue
> (autoreconf is required because src/utils/Makefile.am is changed by
> patch which adds installation of ecryptfs-common), but autoreconf fails
> to regenerate needed files because it removes aclocal.m4 which has
> definition of AM_GLIB_GNU_GETTEXT macro (which is not defined by host-gettext

Dang, indeed it is a big hammer to add a dependency onto host-libglib2
just for this oe macro, which by the way is deprecated, and that
upstream suggests to remove in favour of the official gettext one:

    https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L356
.to:
    https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437

That's a shame, as ecrypt-utils does noit even have a dependency on
libglib2 at all, except for this one macro. :-(

So, that could probably be a hint to fix that upstream in a more
reasonable fashion anyway?

Regards,
Yann E. MORIN.

> nowhere in output/host) but this macro is required by configure.ac. So,
> I am not sure I can provide conceptually better patch for now (but Yann's comments),
> may be it is possible to regenerate only src/utils/Makefile.in (from
> *.am file) instead of calling autoreconf for all *.am, *.ac files.
> 
> Regards,
> Vadim Kochan
Vadym Kochan March 14, 2019, 1:10 a.m. UTC | #6
Hi Yann, All

On Wed, Mar 13, 2019 at 10:28:06PM +0100, Yann E. MORIN wrote:
> Vadim, All,
> 
> On 2019-03-13 21:15 +0200, Vadym Kochan spake thusly:
> > On Wed, Mar 13, 2019 at 07:53:50PM +0100, Yann E. MORIN wrote:
> > > On 2019-03-13 00:20 +0200, Vadim Kochan spake thusly:
> [--SNIP--]
> > > > +gettext() {
> > > > +	if [ -z "${GETTEXT_PROG}" ]; then
> > > > +		GETTEXT_PROG=$(which gettext)
> > > > +		if [ $? != 0 ]; then
> > > 
> > > != is a string comparison. For numbers, you will want to use -ne (or
> > > -eq), see below.
> > > 
> > > > +			GETTEXT_PROG=gettext_echo
> > > > +		fi
> > > > +	fi
> > > > +
> > > > +	${GETTEXT_PROG} "$@"
> > > > +}
> > > 
> > > This means that a script that calls gettext more than once will actually
> > > try to resolve it each time. This is not optimum.
> > 
> > But I check the GETTEXT_PROG variable on the start, if it is not empty then just:
> > 	${GETTEXT_PROG} "$@"
> 
> Hmm... Right, and the resolution is indeed done only once. My bad.
> 
> > > Instead, I think ecryptfs-common should basically look something like:
> > > 
> > >     if ! which gettext >/dev/null 2>&1; then
> > >         gettext() {
> > >             if [ -n "${GETTEXT}" ]; then
> > >                 # Weird construct so that script that are 'set -e'
> > >                 # fail at the call site of gettext and not here.
> > >                 "${GETTEXT}" "${@}" || return $?
> > >                 return 0
> > >             fi
> > >             while [ ${#} -ne 0 ]; do
> > >                 case "${1}" in
> > >                   (-h)              printf "no help\n"; return 0;;
> > >                   (-V)              printf "0.0.0\n"; return 0;;
> > >                   (-d|--domain)     shift 2;;
> > >                   (-d*|--domain=*)  shift 1;;
> > >                   (-e|-E|-n)        shift 1;;
> > >                   (-s)              shift 1;;  # Ignore?
> > >                   (-*)              printf "invalid option '%s'\n" "${1}" >&2; return 1;;
> > >                   (*)               break;;
> > >                 esac
> > >             done
> > >             case ${#} in
> > >               (0)   printf "missing arguments\n" >&2; return 1;;
> > >               (1)   printf "%s" "${1}";;
> > >               (2)   shift; printf "%s" "${2}";;
> > >               (*)   printf "too many arguments\n" >&2; return 1;;
> > >             esac
> > >         }
> > >     fi
> > > 
> > > I.e. if there is no gettext program, you define a function named
> > > gettext.
> 
> The difference then between your code and mine, is that yours is doing a
> lazy resolution (i.e. resolving at first call) while mine is doing an
> early resolution (i.e. resolving when parsing).
> 
> Additionally, since your code is using a two-level function call
> (gettext -> gettext_echo), it's a bit more complex to handle scritps
> that are 'set -e' and ensure they get the error at the call site rather
> than in internal details of the emulating function.
> 
> But my suggestion would be that we just provide a gettext wrapper script
> (not function) when gettext is not installed. This is much simpler
> because there is thus no need to patch upstream.
> 
Right, that would be much better solution than fixing ecryptfs-utils, but
this wrapper should be installed by something like gettext-tiny (this
is still the option if the ecryptfs-utils's solution will fail) for the
target.

> > Thanks for your comments, anyway conceptually this patch is wrong. I
> > tried to do it in the right way - install ecryptfs-common from
> > ecryptfs-utils in src/utils/Makefile.am, but faced with autoreconf issue
> > (autoreconf is required because src/utils/Makefile.am is changed by
> > patch which adds installation of ecryptfs-common), but autoreconf fails
> > to regenerate needed files because it removes aclocal.m4 which has
> > definition of AM_GLIB_GNU_GETTEXT macro (which is not defined by host-gettext
> 
> Dang, indeed it is a big hammer to add a dependency onto host-libglib2
> just for this oe macro, which by the way is deprecated, and that
> upstream suggests to remove in favour of the official gettext one:
> 
>     https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L356
> .to:
>     https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
> 
> That's a shame, as ecrypt-utils does noit even have a dependency on
> libglib2 at all, except for this one macro. :-(
> 
> So, that could probably be a hint to fix that upstream in a more
> reasonable fashion anyway?
> 
I am trying to re-do the patch to use AM_GNU_GETTEXT instead of
AM_GLIB_GNU_GETTEXT.

Regards,
Vadim Kochan
diff mbox series

Patch

diff --git a/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch b/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
new file mode 100644
index 0000000000..1e3318ab22
--- /dev/null
+++ b/package/ecryptfs-utils/0003-utils-Use-echo-instead-of-gettext-if-it-does-not-exi.patch
@@ -0,0 +1,117 @@ 
+From 55755006370fb12167dcadeb084b6645712197a9 Mon Sep 17 00:00:00 2001
+From: Vadim Kochan <vadim4j@gmail.com>
+Date: Tue, 12 Mar 2019 02:15:33 +0200
+Subject: [PATCH] utils: Use "echo" instead of gettext if it does not exist
+
+There might be a case when gettext does not exist on the system,
+so use just "echo" instead. Added gettext wrapper which imitates
+gettext behaviour, wrapper is used only if gettext is not found
+by "which". ecryptfs-common is included by each script which uses
+gettext tool.
+
+Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
+---
+ src/utils/ecryptfs-migrate-home   | 2 ++
+ src/utils/ecryptfs-mount-private  | 2 ++
+ src/utils/ecryptfs-rewrite-file   | 2 ++
+ src/utils/ecryptfs-setup-private  | 3 +++
+ src/utils/ecryptfs-setup-swap     | 2 ++
+ src/utils/ecryptfs-umount-private | 2 ++
+ src/utils/ecryptfs-verify         | 2 ++
+ 7 files changed, 15 insertions(+)
+
+diff --git a/src/utils/ecryptfs-migrate-home b/src/utils/ecryptfs-migrate-home
+index b810146..4fa4d86 100755
+--- a/src/utils/ecryptfs-migrate-home
++++ b/src/utils/ecryptfs-migrate-home
+@@ -24,6 +24,8 @@
+ 
+ set -e
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ PRIVATE_DIR="Private"
+ 
+ usage() {
+diff --git a/src/utils/ecryptfs-mount-private b/src/utils/ecryptfs-mount-private
+index c32708f..a6df39f 100755
+--- a/src/utils/ecryptfs-mount-private
++++ b/src/utils/ecryptfs-mount-private
+@@ -12,6 +12,8 @@
+ #  * inserts the mount passphrase into the keyring
+ #  * and mounts a user's encrypted private folder
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ PRIVATE_DIR="Private"
+ WRAPPING_PASS="LOGIN"
+ PW_ATTEMPTS=3
+diff --git a/src/utils/ecryptfs-rewrite-file b/src/utils/ecryptfs-rewrite-file
+index c4f67f5..146e385 100755
+--- a/src/utils/ecryptfs-rewrite-file
++++ b/src/utils/ecryptfs-rewrite-file
+@@ -17,6 +17,8 @@
+ #    You should have received a copy of the GNU General Public License
+ #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ TEXTDOMAIN="ecryptfs-utils"
+ 
+ error() {
+diff --git a/src/utils/ecryptfs-setup-private b/src/utils/ecryptfs-setup-private
+index e90d1d0..ca08656 100755
+--- a/src/utils/ecryptfs-setup-private
++++ b/src/utils/ecryptfs-setup-private
+@@ -6,6 +6,9 @@
+ # Ported for use on Ubuntu by Dustin Kirkland <kirkland@ubuntu.com>
+ # Copyright (C) 2008 Canonical Ltd.
+ # Copyright (C) 2007-2008 International Business Machines
++
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ PRIVATE_DIR="Private"
+ WRAPPING_PASS="LOGIN"
+ ECRYPTFS_DIR="/home/.ecryptfs"
+diff --git a/src/utils/ecryptfs-setup-swap b/src/utils/ecryptfs-setup-swap
+index 41cf18a..157b0c5 100755
+--- a/src/utils/ecryptfs-setup-swap
++++ b/src/utils/ecryptfs-setup-swap
+@@ -19,6 +19,8 @@
+ # The cryptswap setup used here follows a guide published at:
+ #  * http://ubuntumagnet.com/2007/11/creating-encrypted-swap-file-ubuntu-using-cryptsetup
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ TEXTDOMAIN="ecryptfs-utils"
+ 
+ error() {
+diff --git a/src/utils/ecryptfs-umount-private b/src/utils/ecryptfs-umount-private
+index 27edeaa..4ac0ced 100755
+--- a/src/utils/ecryptfs-umount-private
++++ b/src/utils/ecryptfs-umount-private
+@@ -5,6 +5,8 @@
+ # Original by Michael Halcrow, IBM
+ # Extracted to a stand-alone script by Dustin Kirkland <kirkland@ubuntu.com>
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ TEXTDOMAIN="ecryptfs-utils"
+ 
+ if grep -qs "$HOME/.Private $PWD ecryptfs " /proc/mounts 2>/dev/null; then
+diff --git a/src/utils/ecryptfs-verify b/src/utils/ecryptfs-verify
+index b55641d..3592d83 100755
+--- a/src/utils/ecryptfs-verify
++++ b/src/utils/ecryptfs-verify
+@@ -16,6 +16,8 @@
+ #    You should have received a copy of the GNU General Public License
+ #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ 
++. /usr/lib/ecryptfs-utils/ecryptfs-common
++
+ error() {
+ 	echo `gettext "ERROR:"` "$@" 1>&2
+ 	echo `gettext "ERROR:"` "Configuration invalid" 1>&2
+-- 
+2.14.1
+
diff --git a/package/ecryptfs-utils/Config.in b/package/ecryptfs-utils/Config.in
index 6652d33e0e..1438c1754c 100644
--- a/package/ecryptfs-utils/Config.in
+++ b/package/ecryptfs-utils/Config.in
@@ -12,7 +12,7 @@  config BR2_PACKAGE_ECRYPTFS_UTILS
 	select BR2_PACKAGE_LIBNSS
 	# runtime dependency only, some scripts are using the
 	# 'gettext' program to get translations
-	select BR2_PACKAGE_GETTEXT
+	select BR2_PACKAGE_GETTEXT if BR2_SYSTEM_ENABLE_NLS
 	# runtime dependency only
 	select BR2_PACKAGE_GETENT
 	help
diff --git a/package/ecryptfs-utils/ecryptfs-common b/package/ecryptfs-utils/ecryptfs-common
new file mode 100755
index 0000000000..8d4050d7f4
--- /dev/null
+++ b/package/ecryptfs-utils/ecryptfs-common
@@ -0,0 +1,33 @@ 
+GETTEXT_PROG=""
+
+# Thanks to "Yann E. MORIN" <yann.morin.1998@free.fr>
+# for this gettext replacement function
+gettext_echo() {
+	while [ ${#} -ne 0 ]; do
+		case "${1}" in
+			(-h)              echo "no help"; exit 0;;
+			(-V)              echo "0.0.0"; exit 0;;
+			(-d|--domain)     shift 2;;
+			(-d*|--domain=*)  shift 1;;
+			(-e|-E|-n)        shift 1;;
+			(-s)              shift 1;;  # Ignore?
+			(*)               break;;
+		esac
+	done
+	case ${#} in
+		(0)   ;;
+		(1)   echo "${1}";;           # No TEXTDOMAIN?
+		(*)   shift; echo "${@}";;    # Ignore TEXTDOMAIN?
+	esac
+}
+
+gettext() {
+	if [ -z "${GETTEXT_PROG}" ]; then
+		GETTEXT_PROG=$(which gettext)
+		if [ $? != 0 ]; then
+			GETTEXT_PROG=gettext_echo
+		fi
+	fi
+
+	${GETTEXT_PROG} "$@"
+}
diff --git a/package/ecryptfs-utils/ecryptfs-utils.mk b/package/ecryptfs-utils/ecryptfs-utils.mk
index eb3194b6d0..bf630882c2 100644
--- a/package/ecryptfs-utils/ecryptfs-utils.mk
+++ b/package/ecryptfs-utils/ecryptfs-utils.mk
@@ -25,4 +25,10 @@  else
 ECRYPTFS_UTILS_CONF_OPTS += --disable-openssl
 endif
 
+define ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
+	$(INSTALL) -D -m 0755 $(ECRYPTFS_UTILS_PKGDIR)/ecryptfs-common \
+		$(TARGET_DIR)/usr/lib/ecryptfs-utils/ecryptfs-common
+endef
+ECRYPTFS_UTILS_POST_INSTALL_TARGET_HOOKS += ECRYPTFS_UTILS_INSTALL_COMMON_SCRIPT
+
 $(eval $(autotools-package))