* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
@ 2019-03-15 2:34 Vadim Kochan
2019-03-26 19:28 ` Yann E. MORIN
0 siblings, 1 reply; 6+ messages in thread
From: Vadim Kochan @ 2019-03-15 2:34 UTC (permalink / raw)
To: buildroot
Add custom ecryptfs-common script which provides gettext wrapper as
function, script checks if there is gettext tool, if no - the "printf"
will be used instead. Each script which uses gettext is patched with
including this ecryptfs-common script.
gettext package is now selected automatically only if NLS is enabled.
Also replaced AM_GLIB_GNU_GETTEXT by AM_GNU_GETTEXT because original
macro is removed during autoreconf. Actually AM_GLIB_GNU_GETTEXT is
marked as deprecated in:
https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
so it is ok to use the gettext's one.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
v2:
1) Move ecryptfs-common from package/ecryptfs-utils to *.patch and
install it from src/utils/Makefile.am
2) Use solution from Yann E. Morin for gettext-wrapper
3) Replace AM_GLIB_GNU_GETTEXT by AM_GNU_GETTEXT
4) Use @prefix@ and @PACKAGE@ variables to properly include the
ecryptfs-common script in utils which uses gettext, it requires
to add these utils scripts into configure.ac AC_CONFIG_FILES for
variable replacement.
...tils-Use-printf-if-gettext-does-not-exist.patch | 217 +++++++++++++++++++++
package/ecryptfs-utils/Config.in | 2 +-
package/ecryptfs-utils/ecryptfs-utils.mk | 5 +
3 files changed, 223 insertions(+), 1 deletion(-)
create mode 100644 package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
diff --git a/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
new file mode 100644
index 0000000000..7fe8756323
--- /dev/null
+++ b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
@@ -0,0 +1,217 @@
+From e2a69d509653ff189dd8dd7ed30ff568a0183f1a 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 "printf" if gettext does not exist
+
+There might be a case when gettext does not exist on the system,
+so use just "printf" 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.
+
+AM_GLIB_GNU_GETTEXT is replaced by AM_GNU_GETTEXT, to get rid of
+glib2 dependency just only for this macro. Actually this macro is marked
+as deprecated in:
+
+ https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
+
+Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
+---
+ configure.ac | 12 ++++++++++--
+ src/utils/Makefile.am | 4 +++-
+ src/utils/ecryptfs-common | 30 ++++++++++++++++++++++++++++++
+ 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 ++
+ 10 files changed, 58 insertions(+), 3 deletions(-)
+ create mode 100755 src/utils/ecryptfs-common
+
+diff --git a/configure.ac b/configure.ac
+index bbc6ffc..eec1ebc 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -13,7 +13,7 @@ AC_PREREQ(2.59)
+ AC_INIT([ecryptfs-utils],[111])
+ AC_CANONICAL_HOST
+ AC_CANONICAL_TARGET
+-AM_INIT_AUTOMAKE([${PACKAGE_NAME}], [${PACKAGE_VERSION}])
++AM_INIT_AUTOMAKE([foreign])
+ AC_CONFIG_SRCDIR([src/libecryptfs])
+ AC_CONFIG_HEADERS([config.h])
+ AC_SUBST(AM_CPPFLAGS, '-include $(top_builddir)/config.h')
+@@ -369,7 +369,8 @@ AC_SUBST(GETTEXT_PACKAGE)
+ AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE,"$GETTEXT_PACKAGE",
+ [the gettext translation domain])
+
+-AM_GLIB_GNU_GETTEXT
++AM_GNU_GETTEXT_VERSION([0.19])
++AM_GNU_GETTEXT([external])
+
+ IT_PROG_INTLTOOL([0.41.0])
+
+@@ -439,6 +440,13 @@ AC_CONFIG_FILES([
+ tests/kernel/Makefile
+ tests/userspace/Makefile
+ po/Makefile.in
++ src/utils/ecryptfs-migrate-home:src/utils/ecryptfs-migrate-home
++ src/utils/ecryptfs-mount-private:src/utils/ecryptfs-mount-private
++ src/utils/ecryptfs-rewrite-file:src/utils/ecryptfs-rewrite-file
++ src/utils/ecryptfs-setup-private:src/utils/ecryptfs-setup-private
++ src/utils/ecryptfs-setup-swap:src/utils/ecryptfs-setup-swap
++ src/utils/ecryptfs-umount-private:src/utils/ecryptfs-umount-private
++ src/utils/ecryptfs-verify:src/utils/ecryptfs-verify
+ ])
+ AC_OUTPUT
+
+diff --git a/src/utils/Makefile.am b/src/utils/Makefile.am
+index 83cf851..54c3bdc 100644
+--- a/src/utils/Makefile.am
++++ b/src/utils/Makefile.am
+@@ -21,9 +21,11 @@ bin_SCRIPTS = ecryptfs-setup-private \
+ ecryptfs-find \
+ ecryptfs-verify
+ bin2dir = $(bindir)
+-
+ noinst_PROGRAMS=test
+
++commonlibdir=$(libdir)/ecryptfs-utils
++commonlib_SCRIPTS=ecryptfs-common
++
+ if ENABLE_TESTS
+ TESTS=test
+ endif
+diff --git a/src/utils/ecryptfs-common b/src/utils/ecryptfs-common
+new file mode 100755
+index 0000000..3b18568
+--- /dev/null
++++ b/src/utils/ecryptfs-common
+@@ -0,0 +1,30 @@
++# Thanks to "Yann E. MORIN" <yann.morin.1998@free.fr>
++# for this gettext replacement function
++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
+diff --git a/src/utils/ecryptfs-migrate-home b/src/utils/ecryptfs-migrate-home
+index b810146..1233910 100755
+--- a/src/utils/ecryptfs-migrate-home
++++ b/src/utils/ecryptfs-migrate-home
+@@ -24,6 +24,8 @@
+
+ set -e
+
++. @prefix@/lib/@PACKAGE@/ecryptfs-common
++
+ PRIVATE_DIR="Private"
+
+ usage() {
+diff --git a/src/utils/ecryptfs-mount-private b/src/utils/ecryptfs-mount-private
+index c32708f..218d0a2 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
+
++. @prefix@/lib/@PACKAGE@/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..6b00370 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/>.
+
++. @prefix@/lib/@PACKAGE@/ecryptfs-common
++
+ TEXTDOMAIN="ecryptfs-utils"
+
+ error() {
+diff --git a/src/utils/ecryptfs-setup-private b/src/utils/ecryptfs-setup-private
+index e90d1d0..50073d4 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
++
++. @prefix@/lib/@PACKAGE@/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..f122674 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
+
++. @prefix@/lib/@PACKAGE@/ecryptfs-common
++
+ TEXTDOMAIN="ecryptfs-utils"
+
+ error() {
+diff --git a/src/utils/ecryptfs-umount-private b/src/utils/ecryptfs-umount-private
+index 27edeaa..5645f4d 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>
+
++. @prefix@/lib/@PACKAGE@/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..f102d2b 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/>.
+
++. @prefix@/lib/@PACKAGE@/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-utils.mk b/package/ecryptfs-utils/ecryptfs-utils.mk
index eb3194b6d0..1b49397137 100644
--- a/package/ecryptfs-utils/ecryptfs-utils.mk
+++ b/package/ecryptfs-utils/ecryptfs-utils.mk
@@ -25,4 +25,9 @@ else
ECRYPTFS_UTILS_CONF_OPTS += --disable-openssl
endif
+define ECRYPTFS_UTILS_RUN_AUTOGEN
+ cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
+endef
+ECRYPTFS_UTILS_PRE_CONFIGURE_HOOKS += ECRYPTFS_UTILS_RUN_AUTOGEN
+
$(eval $(autotools-package))
--
2.14.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
2019-03-15 2:34 [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext Vadim Kochan
@ 2019-03-26 19:28 ` Yann E. MORIN
2019-03-26 20:49 ` Arnout Vandecappelle
2019-03-26 21:09 ` Vadim Kochan
0 siblings, 2 replies; 6+ messages in thread
From: Yann E. MORIN @ 2019-03-26 19:28 UTC (permalink / raw)
To: buildroot
Vadim, All,
On 2019-03-15 04:34 +0200, Vadim Kochan spake thusly:
> Add custom ecryptfs-common script which provides gettext wrapper as
> function, script checks if there is gettext tool, if no - the "printf"
> will be used instead. Each script which uses gettext is patched with
> including this ecryptfs-common script.
>
> gettext package is now selected automatically only if NLS is enabled.
>
> Also replaced AM_GLIB_GNU_GETTEXT by AM_GNU_GETTEXT because original
> macro is removed during autoreconf. Actually AM_GLIB_GNU_GETTEXT is
> marked as deprecated in:
>
> https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
>
> so it is ok to use the gettext's one.
I know you've pourred quite some effort into this solution, but I am
still not entirely convinced.
So, my position on how we should handle this in Buildroot is as follows:
1. When the real 'gettext' utility is not available (for whatever
reason), then we should install our own, fake gettext, like the one
I previously suggested.
2. If you are really interested, push a change upstream that makes
ecryptfs-utils not require gettext, but with a solution that does
not use a wrapper. see below for more on that.
3. If/when upstream supports running without a gettext utility, then
we can either bump the version in Buildroot, or backport the
patches.
Now, for (1), what package should be responsible for providing the fake
gettext is not clear-cut. gettext-tiny would look like a good candidate,
though.
For (2), see below...
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
[--SNIP--]
> diff --git a/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
> new file mode 100644
> index 0000000000..7fe8756323
> --- /dev/null
> +++ b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
> @@ -0,0 +1,217 @@
> +From e2a69d509653ff189dd8dd7ed30ff568a0183f1a 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 "printf" if gettext does not exist
You should definitely send this upstream.
> +There might be a case when gettext does not exist on the system,
> +so use just "printf" 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.
I think upstream will want a solution that does not require a fake
wrapper that really mimicks the reall gettext entirely, this their use
of it is entirely confined to their scripts, and they always call it
with just the message to display.
In this case, the wrapper-function would probably be just as easy as:
# This is in ${libdir}/ecryptfs-utils/sh-functions
if ! which gettext >/dev/null 2>&1; then
gettext() {
if [ -n "${GETTEXT}" ]; then
"${GETTEXT}" "${@}"
else
printf "%s" "${*}"
fi || return $? # Trick for 'set -e' to fail at call-site, not here
}
fi
and be done with it.
> +AM_GLIB_GNU_GETTEXT is replaced by AM_GNU_GETTEXT, to get rid of
> +glib2 dependency just only for this macro. Actually this macro is marked
> +as deprecated in:
> +
> + https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
So, when you send this upstream, you should probably split it up in
different patches:
1. fix for the call to AM_INIT_AUTOMAKE()
2. change from AM_GLIB_GNU_GETTEXT() to AM_GNU_GETTEXT()
3. Convert of scripts to use the gettext wrapper-function
> +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> +---
> + configure.ac | 12 ++++++++++--
> + src/utils/Makefile.am | 4 +++-
> + src/utils/ecryptfs-common | 30 ++++++++++++++++++++++++++++++
> + 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 ++
> + 10 files changed, 58 insertions(+), 3 deletions(-)
> + create mode 100755 src/utils/ecryptfs-common
> +
> +diff --git a/configure.ac b/configure.ac
> +index bbc6ffc..eec1ebc 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -13,7 +13,7 @@ AC_PREREQ(2.59)
> + AC_INIT([ecryptfs-utils],[111])
> + AC_CANONICAL_HOST
> + AC_CANONICAL_TARGET
> +-AM_INIT_AUTOMAKE([${PACKAGE_NAME}], [${PACKAGE_VERSION}])
> ++AM_INIT_AUTOMAKE([foreign])
> + AC_CONFIG_SRCDIR([src/libecryptfs])
> + AC_CONFIG_HEADERS([config.h])
> + AC_SUBST(AM_CPPFLAGS, '-include $(top_builddir)/config.h')
> +@@ -369,7 +369,8 @@ AC_SUBST(GETTEXT_PACKAGE)
> + AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE,"$GETTEXT_PACKAGE",
> + [the gettext translation domain])
> +
> +-AM_GLIB_GNU_GETTEXT
> ++AM_GNU_GETTEXT_VERSION([0.19])
> ++AM_GNU_GETTEXT([external])
> +
> + IT_PROG_INTLTOOL([0.41.0])
> +
> +@@ -439,6 +440,13 @@ AC_CONFIG_FILES([
> + tests/kernel/Makefile
> + tests/userspace/Makefile
> + po/Makefile.in
> ++ src/utils/ecryptfs-migrate-home:src/utils/ecryptfs-migrate-home
> ++ src/utils/ecryptfs-mount-private:src/utils/ecryptfs-mount-private
> ++ src/utils/ecryptfs-rewrite-file:src/utils/ecryptfs-rewrite-file
> ++ src/utils/ecryptfs-setup-private:src/utils/ecryptfs-setup-private
> ++ src/utils/ecryptfs-setup-swap:src/utils/ecryptfs-setup-swap
> ++ src/utils/ecryptfs-umount-private:src/utils/ecryptfs-umount-private
> ++ src/utils/ecryptfs-verify:src/utils/ecryptfs-verify
Since those will now undergo pattern substitution, they should be
renamed with a .in extension.
Also, it is usualy bad practice to generate such scripts from configure.
Instead, they should be generated in Makefile.am as new targets. Yes,
this is a bit more involved, but is much cleaner: when you change such a
script, you just have to run 'make' again, rather than re-run configure.
> ++commonlibdir=$(libdir)/ecryptfs-utils
> ++commonlib_SCRIPTS=ecryptfs-common
I think 'sh-functions' is more appropriate.
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
2019-03-26 19:28 ` Yann E. MORIN
@ 2019-03-26 20:49 ` Arnout Vandecappelle
2019-03-26 21:45 ` Yann E. MORIN
2019-03-26 21:09 ` Vadim Kochan
1 sibling, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2019-03-26 20:49 UTC (permalink / raw)
To: buildroot
On 26/03/2019 20:28, Yann E. MORIN wrote:
> Also, it is usualy bad practice to generate such scripts from configure.
> Instead, they should be generated in Makefile.am as new targets. Yes,
> this is a bit more involved, but is much cleaner: when you change such a
> script, you just have to run 'make' again, rather than re-run configure.
Is that really so? I thought automake would generate a rule that just re-runs
config.status when such a file changes. Haven't tested though.
Regards,
Arnout
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
2019-03-26 20:49 ` Arnout Vandecappelle
@ 2019-03-26 21:45 ` Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2019-03-26 21:45 UTC (permalink / raw)
To: buildroot
Arnout, All,
On 2019-03-26 21:49 +0100, Arnout Vandecappelle spake thusly:
> On 26/03/2019 20:28, Yann E. MORIN wrote:
> > Also, it is usualy bad practice to generate such scripts from configure.
> > Instead, they should be generated in Makefile.am as new targets. Yes,
> > this is a bit more involved, but is much cleaner: when you change such a
> > script, you just have to run 'make' again, rather than re-run configure.
>
> Is that really so? I thought automake would generate a rule that just re-runs
> config.status when such a file changes. Haven't tested though.
Hmm.. Maybe. It's been a while I wrote my last autotool package.
In any case, I still remember that it's good practice to only generate
from ./configure things that are needed to do the actual build, and that
anything not needed to run 'make' should be handled in Makefile.am.
(pushing the comparison to the limits: if they were C files, you'd
compile them from Makefile.am, not configure).
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
2019-03-26 19:28 ` Yann E. MORIN
2019-03-26 20:49 ` Arnout Vandecappelle
@ 2019-03-26 21:09 ` Vadim Kochan
2019-03-26 22:01 ` Yann E. MORIN
1 sibling, 1 reply; 6+ messages in thread
From: Vadim Kochan @ 2019-03-26 21:09 UTC (permalink / raw)
To: buildroot
Hi Yann,
On Tue, Mar 26, 2019 at 08:28:19PM +0100, Yann E. MORIN wrote:
> Vadim, All,
>
> On 2019-03-15 04:34 +0200, Vadim Kochan spake thusly:
> > Add custom ecryptfs-common script which provides gettext wrapper as
> > function, script checks if there is gettext tool, if no - the "printf"
> > will be used instead. Each script which uses gettext is patched with
> > including this ecryptfs-common script.
> >
> > gettext package is now selected automatically only if NLS is enabled.
> >
> > Also replaced AM_GLIB_GNU_GETTEXT by AM_GNU_GETTEXT because original
> > macro is removed during autoreconf. Actually AM_GLIB_GNU_GETTEXT is
> > marked as deprecated in:
> >
> > https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
> >
> > so it is ok to use the gettext's one.
>
> I know you've pourred quite some effort into this solution, but I am
> still not entirely convinced.
>
> So, my position on how we should handle this in Buildroot is as follows:
>
> 1. When the real 'gettext' utility is not available (for whatever
> reason), then we should install our own, fake gettext, like the one
> I previously suggested.
>
> 2. If you are really interested, push a change upstream that makes
> ecryptfs-utils not require gettext, but with a solution that does
> not use a wrapper. see below for more on that.
>
> 3. If/when upstream supports running without a gettext utility, then
> we can either bump the version in Buildroot, or backport the
> patches.
>
> Now, for (1), what package should be responsible for providing the fake
> gettext is not clear-cut. gettext-tiny would look like a good candidate,
> though.
>
> For (2), see below...
>
> > Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> [--SNIP--]
> > diff --git a/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
> > new file mode 100644
> > index 0000000000..7fe8756323
> > --- /dev/null
> > +++ b/package/ecryptfs-utils/0003-utils-Use-printf-if-gettext-does-not-exist.patch
> > @@ -0,0 +1,217 @@
> > +From e2a69d509653ff189dd8dd7ed30ff568a0183f1a 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 "printf" if gettext does not exist
>
> You should definitely send this upstream.
>
> > +There might be a case when gettext does not exist on the system,
> > +so use just "printf" 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.
>
> I think upstream will want a solution that does not require a fake
> wrapper that really mimicks the reall gettext entirely, this their use
> of it is entirely confined to their scripts, and they always call it
> with just the message to display.
>
> In this case, the wrapper-function would probably be just as easy as:
>
> # This is in ${libdir}/ecryptfs-utils/sh-functions
> if ! which gettext >/dev/null 2>&1; then
> gettext() {
> if [ -n "${GETTEXT}" ]; then
> "${GETTEXT}" "${@}"
> else
> printf "%s" "${*}"
> fi || return $? # Trick for 'set -e' to fail at call-site, not here
> }
> fi
>
> and be done with it.
>
> > +AM_GLIB_GNU_GETTEXT is replaced by AM_GNU_GETTEXT, to get rid of
> > +glib2 dependency just only for this macro. Actually this macro is marked
> > +as deprecated in:
> > +
> > + https://gitlab.gnome.org/GNOME/glib/blob/master/m4macros/glib-gettext.m4#L437
>
> So, when you send this upstream, you should probably split it up in
> different patches:
>
> 1. fix for the call to AM_INIT_AUTOMAKE()
> 2. change from AM_GLIB_GNU_GETTEXT() to AM_GNU_GETTEXT()
> 3. Convert of scripts to use the gettext wrapper-function
>
> > +Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> > +---
> > + configure.ac | 12 ++++++++++--
> > + src/utils/Makefile.am | 4 +++-
> > + src/utils/ecryptfs-common | 30 ++++++++++++++++++++++++++++++
> > + 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 ++
> > + 10 files changed, 58 insertions(+), 3 deletions(-)
> > + create mode 100755 src/utils/ecryptfs-common
> > +
> > +diff --git a/configure.ac b/configure.ac
> > +index bbc6ffc..eec1ebc 100644
> > +--- a/configure.ac
> > ++++ b/configure.ac
> > +@@ -13,7 +13,7 @@ AC_PREREQ(2.59)
> > + AC_INIT([ecryptfs-utils],[111])
> > + AC_CANONICAL_HOST
> > + AC_CANONICAL_TARGET
> > +-AM_INIT_AUTOMAKE([${PACKAGE_NAME}], [${PACKAGE_VERSION}])
> > ++AM_INIT_AUTOMAKE([foreign])
> > + AC_CONFIG_SRCDIR([src/libecryptfs])
> > + AC_CONFIG_HEADERS([config.h])
> > + AC_SUBST(AM_CPPFLAGS, '-include $(top_builddir)/config.h')
> > +@@ -369,7 +369,8 @@ AC_SUBST(GETTEXT_PACKAGE)
> > + AC_DEFINE_UNQUOTED(GETTEXT_PACKAGE,"$GETTEXT_PACKAGE",
> > + [the gettext translation domain])
> > +
> > +-AM_GLIB_GNU_GETTEXT
> > ++AM_GNU_GETTEXT_VERSION([0.19])
> > ++AM_GNU_GETTEXT([external])
> > +
> > + IT_PROG_INTLTOOL([0.41.0])
> > +
> > +@@ -439,6 +440,13 @@ AC_CONFIG_FILES([
> > + tests/kernel/Makefile
> > + tests/userspace/Makefile
> > + po/Makefile.in
> > ++ src/utils/ecryptfs-migrate-home:src/utils/ecryptfs-migrate-home
> > ++ src/utils/ecryptfs-mount-private:src/utils/ecryptfs-mount-private
> > ++ src/utils/ecryptfs-rewrite-file:src/utils/ecryptfs-rewrite-file
> > ++ src/utils/ecryptfs-setup-private:src/utils/ecryptfs-setup-private
> > ++ src/utils/ecryptfs-setup-swap:src/utils/ecryptfs-setup-swap
> > ++ src/utils/ecryptfs-umount-private:src/utils/ecryptfs-umount-private
> > ++ src/utils/ecryptfs-verify:src/utils/ecryptfs-verify
>
> Since those will now undergo pattern substitution, they should be
> renamed with a .in extension.
>
> Also, it is usualy bad practice to generate such scripts from configure.
> Instead, they should be generated in Makefile.am as new targets. Yes,
You meant - replace inclusion of this ecryptfs-utils from Makefile.am ?
> this is a bit more involved, but is much cleaner: when you change such a
> script, you just have to run 'make' again, rather than re-run configure.
>
> > ++commonlibdir=$(libdir)/ecryptfs-utils
> > ++commonlib_SCRIPTS=ecryptfs-common
>
> I think 'sh-functions' is more appropriate.
>
So, in case of (2) then we should wait till ecryptfs's community
apply the solution. In case of gettext-tiny ... as I remember Thomas did
not like the idea with using gettext-tiny for target :), ehhh ... but I
totally AGREE with you because your suggestions are more clear and well
defined, what I tried - is to provide solution which touches not so much
ectyptfs's sources but provides solution w/o gettext to finally close
one gap with gettext-tiny solution :)
Back to gettext-tiny, may be it is OK to start with providing
gettext-tiny (as target's gettext provider) just only with this gettext
wrapper (which you suggested previously), and after take gettext-tiny
patches (which I sent and Thomas adapted in his repo) and merge with
solution which provides just gettext util.
Thanks Yann with your suggestions.
Regards,
Vadim Kochan
Regards,
Vadim Kochan
^ permalink raw reply [flat|nested] 6+ messages in thread* [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext
2019-03-26 21:09 ` Vadim Kochan
@ 2019-03-26 22:01 ` Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2019-03-26 22:01 UTC (permalink / raw)
To: buildroot
Vadim, All,
On 2019-03-26 23:09 +0200, Vadim Kochan spake thusly:
> On Tue, Mar 26, 2019 at 08:28:19PM +0100, Yann E. MORIN wrote:
> > On 2019-03-15 04:34 +0200, Vadim Kochan spake thusly:
> > > Add custom ecryptfs-common script which provides gettext wrapper as
[--SNIP--]
> > I know you've pourred quite some effort into this solution, but I am
> > still not entirely convinced.
> >
> > So, my position on how we should handle this in Buildroot is as follows:
> >
> > 1. When the real 'gettext' utility is not available (for whatever
> > reason), then we should install our own, fake gettext, like the one
> > I previously suggested.
> >
> > 2. If you are really interested, push a change upstream that makes
> > ecryptfs-utils not require gettext, but with a solution that does
> > not use a wrapper. see below for more on that.
> >
> > 3. If/when upstream supports running without a gettext utility, then
> > we can either bump the version in Buildroot, or backport the
> > patches.
[--SNIP--]
> > > ++ src/utils/ecryptfs-migrate-home:src/utils/ecryptfs-migrate-home
> > > ++ src/utils/ecryptfs-mount-private:src/utils/ecryptfs-mount-private
> > > ++ src/utils/ecryptfs-rewrite-file:src/utils/ecryptfs-rewrite-file
> > > ++ src/utils/ecryptfs-setup-private:src/utils/ecryptfs-setup-private
> > > ++ src/utils/ecryptfs-setup-swap:src/utils/ecryptfs-setup-swap
> > > ++ src/utils/ecryptfs-umount-private:src/utils/ecryptfs-umount-private
> > > ++ src/utils/ecryptfs-verify:src/utils/ecryptfs-verify
> >
> > Since those will now undergo pattern substitution, they should be
> > renamed with a .in extension.
> >
> > Also, it is usualy bad practice to generate such scripts from configure.
> > Instead, they should be generated in Makefile.am as new targets. Yes,
>
> You meant - replace inclusion of this ecryptfs-utils from Makefile.am ?
Yes. That would IMHO be the best. But if upstream is willing to take a
patch that generates them from configure, I won't be picky! ;-)
So, you could keep them generated by configure and send the patch
upstream, and see what they say about it.
> > this is a bit more involved, but is much cleaner: when you change such a
> > script, you just have to run 'make' again, rather than re-run configure.
> >
> > > ++commonlibdir=$(libdir)/ecryptfs-utils
> > > ++commonlib_SCRIPTS=ecryptfs-common
> >
> > I think 'sh-functions' is more appropriate.
> >
>
> So, in case of (2) then we should wait till ecryptfs's community
> apply the solution.
Before we can use it in Buildroot? Yes.
> In case of gettext-tiny ... as I remember Thomas did
> not like the idea with using gettext-tiny for target :),
We discussed this live a few days ago, and I think I could somehow
mostly convince him. I think. Probably. ;-)
He also inquired on IRC a few hours ago about my opinion, and after I
exposed it, he asked that I did the reply to you. So again, I think he
mostly aligns with the idea now. Hopefully! ;-)
> ehhh ... but I
> totally AGREE with you because your suggestions are more clear and well
> defined, what I tried - is to provide solution which touches not so much
> ectyptfs's sources but provides solution w/o gettext to finally close
> one gap with gettext-tiny solution :)
Yes, this is an "easier" first step, at least.
> Back to gettext-tiny, may be it is OK to start with providing
> gettext-tiny (as target's gettext provider) just only with this gettext
> wrapper (which you suggested previously), and after take gettext-tiny
> patches (which I sent and Thomas adapted in his repo) and merge with
> solution which provides just gettext util.
Sorry, I am not sure I understood the above.
When we introduce gettext-tiny, it should also install this fake
gettext. I.e. gettext-tiny is as much as possible a drop-in replacement
for the real gettext (the package) and thus shall provide gettext (the
program).
> Thanks Yann with your suggestions.
My pleasure. And thank you for staying afloat with the back-and-forth
mind-switching there's been on this series. ;-)
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-26 22:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-15 2:34 [Buildroot] [PATCH v2 1/1] package/ecryptfs-utils: Add support without gettext Vadim Kochan
2019-03-26 19:28 ` Yann E. MORIN
2019-03-26 20:49 ` Arnout Vandecappelle
2019-03-26 21:45 ` Yann E. MORIN
2019-03-26 21:09 ` Vadim Kochan
2019-03-26 22:01 ` Yann E. MORIN
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox