Re: dpkg: scripts/mk: add fragment to preen env
Hi!
On Thu, 2023-01-05 at 08:48:59 +0300, Konstantin Demin wrote:
> I'd like to contribute a Makefile fragment to ease env "preen".
Thanks for the patch!
> diff --git a/scripts/mk/env-default.mk b/scripts/mk/env-default.mk
> new file mode 100644
> index 00000000..54f4af30
> --- /dev/null
> +++ b/scripts/mk/env-default.mk
> @@ -0,0 +1,12 @@
> +define dpkg_flush_vars=
> +$(foreach i,$(1),$(eval unexport $(i)))
> +$(foreach i,$(1),$(eval override undefine $(i)))
> +endef
> +
> +$(call dpkg_flush_vars, LANGUAGE LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE )
> +$(call dpkg_flush_vars, LC_MONETARY LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS )
> +$(call dpkg_flush_vars, LC_TELEPHONE LC_MEASUREMENT LC_IDENTIFICATION )
> +$(call dpkg_flush_vars, POSIXLY_CORRECT )
> +
> +export LC_ALL :=C.UTF-8
> +export LANG :=C.UTF-8
Related to this there was #843776, which matches my thinking on why we
should not be globally resetting locale variables.
At the time I ended up adding support for that bug report as a vendor
setting, to be enabled explicitly via dpkg-buildpackage option. But I
guess a make fragment file works too.
So I started playing with this, and I think we should at most set
LC_COLLATE, and perhaps remove POSIXLY_CORRECT, but given that this
one has the potential of making packages regress on portability pushed
upstream, I'm not sure it should be globally set?
In any case what I've for now got is attached. (This would be
targeted for dpkg 1.22.x once that opens up.)
Thanks,
Guillem
From 9fe659d104495c7de98e4aaf164ff168e69c77d1 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Sun, 29 Jan 2023 16:27:34 +0100
Subject: [PATCH] scripts/mk: Add a new buildenv Makefile fragment file
Prompted-by: Konstantin Demin <rockdrilla@gmail.com>
---
scripts/Dpkg/Vendor/Debian.pm | 2 ++
scripts/Makefile.am | 3 +++
scripts/mk/Makefile.am | 6 +++++
scripts/mk/buildenv.mk | 29 ++++++++++++++++++++++++
scripts/t/bin/test-eq-envvar | 12 ++++++++++
scripts/t/bin/test-no-envvar | 11 +++++++++
scripts/t/mk.t | 42 ++++++++++++++++++++++++++++++++++-
scripts/t/mk/buildenv.mk | 20 +++++++++++++++++
8 files changed, 124 insertions(+), 1 deletion(-)
create mode 100644 scripts/mk/buildenv.mk
create mode 100755 scripts/t/bin/test-eq-envvar
create mode 100755 scripts/t/bin/test-no-envvar
create mode 100644 scripts/t/mk/buildenv.mk
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index 06aa49ad6..574bf4479 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -87,6 +87,8 @@ sub run_hook {
umask 0022;
# Reset locale to a sane default.
$ENV{LC_COLLATE} = 'C.UTF-8';
+ # Remove functionality affecting variables.
+ delete $ENV{POSIXLY_CORRECT};
} elsif ($hook eq 'backport-version-regex') {
return qr/~(bpo|deb)/;
} else {
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index 676646189..33e3a7ae6 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -346,6 +346,8 @@ test_data = \
t/Dpkg_Source_Package/package_1.0.orig.tar.sig \
t/Dpkg_Substvars/substvars1 \
t/Dpkg_Substvars/substvars2 \
+ t/bin/test-eq-envvar \
+ t/bin/test-no-envvar \
t/dpkg_buildpackage/dpkgdb/status \
t/dpkg_buildpackage/test-source_0.dsc \
t/dpkg_buildpackage/test-source_0_all.changes \
@@ -374,6 +376,7 @@ test_data = \
t/merge_changelogs/ch-unreleased-merged-basic \
t/merge_changelogs/ch-unreleased-old \
t/mk/architecture.mk \
+ t/mk/buildenv.mk \
t/mk/buildflags.mk \
t/mk/buildopts.mk \
t/mk/buildtools.mk \
diff --git a/scripts/mk/Makefile.am b/scripts/mk/Makefile.am
index a82e409d6..d9777114e 100644
--- a/scripts/mk/Makefile.am
+++ b/scripts/mk/Makefile.am
@@ -2,6 +2,7 @@
dist_pkgdata_DATA = \
architecture.mk \
+ buildenv.mk \
buildflags.mk \
buildopts.mk \
buildtools.mk \
@@ -21,6 +22,11 @@ install-data-hook:
mv $(DESTDIR)$(pkgdatadir)/default.mk.new \
$(DESTDIR)$(pkgdatadir)/default.mk
+ $(do_make_subst) <$(DESTDIR)$(pkgdatadir)/buildenv.mk \
+ >$(DESTDIR)$(pkgdatadir)/buildenv.mk.new
+ mv $(DESTDIR)$(pkgdatadir)/buildenv.mk.new \
+ $(DESTDIR)$(pkgdatadir)/buildenv.mk
+
$(do_make_subst) <$(DESTDIR)$(pkgdatadir)/buildtools.mk \
>$(DESTDIR)$(pkgdatadir)/buildtools.mk.new
mv $(DESTDIR)$(pkgdatadir)/buildtools.mk.new \
diff --git a/scripts/mk/buildenv.mk b/scripts/mk/buildenv.mk
new file mode 100644
index 000000000..dc6f61428
--- /dev/null
+++ b/scripts/mk/buildenv.mk
@@ -0,0 +1,29 @@
+# This Makefile fragment (since dpkg 1.22.0) sanitizes the environment
+# variables.
+#
+# It unexports and undefines the following variables:
+#
+# POSIXLY_CORRECT
+#
+# And sets the following to C.UTF-8 on Debian and derivatives or C otherwose:
+#
+# LC_COLLATE
+#
+
+dpkg_datadir = $(srcdir)/mk
+include $(dpkg_datadir)/vendor.mk
+
+dpkg_reset_envvars := \
+ POSIXLY_CORRECT \
+ # EOL
+
+unexport $(dpkg_reset_envvars)
+$(foreach v,$(dpkg_reset_envvars),\
+ $(eval override undefine $(v)))
+
+ifeq ($(call dpkg_vendor_derives_from_v1,debian),yes)
+LC_COLLATE = C.UTF-8
+else
+LC_COLLATE = C
+endif
+export LC_COLLATE
diff --git a/scripts/t/bin/test-eq-envvar b/scripts/t/bin/test-eq-envvar
new file mode 100755
index 000000000..48b18d943
--- /dev/null
+++ b/scripts/t/bin/test-eq-envvar
@@ -0,0 +1,12 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+die "error: missing variable name argument\n" if @ARGV < 1;
+
+my $var = shift;
+
+die "error: envvar $var does not exist\n" unless exists $ENV{$var};
+die "error: envvar $var != TEST_$var\n" unless $ENV{$var} eq $ENV{"TEST_$var"};
+exit 0;
diff --git a/scripts/t/bin/test-no-envvar b/scripts/t/bin/test-no-envvar
new file mode 100755
index 000000000..af88a5384
--- /dev/null
+++ b/scripts/t/bin/test-no-envvar
@@ -0,0 +1,11 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+die "error: missing variable name argument\n" if @ARGV < 1;
+
+my $var = shift;
+
+die "error: environment variable $var=$ENV{$var} exists \n" if exists $ENV{$var};
+exit 0;
diff --git a/scripts/t/mk.t b/scripts/t/mk.t
index 9d668050b..17143cbb3 100644
--- a/scripts/t/mk.t
+++ b/scripts/t/mk.t
@@ -16,7 +16,7 @@
use strict;
use warnings;
-use Test::More tests => 11;
+use Test::More tests => 13;
use Test::Dpkg qw(:paths);
use File::Spec::Functions qw(rel2abs);
@@ -87,6 +87,46 @@ while (my ($k, $v) = each %arch) {
}
test_makefile('architecture.mk', 'with envvars');
+{
+ local %ENV = %ENV;
+
+ my @locale_envvars = qw(
+ LC_ADDRESS
+ LC_COLLATE
+ LC_CTYPE
+ LC_IDENTIFICATION
+ LC_MEASUREMENT
+ LC_MESSAGES
+ LC_MONETARY
+ LC_NAME
+ LC_NUMERIC
+ LC_PAPER
+ LC_TELEPHONE
+ LC_TIME
+ );
+ my @test_envvars = (qw(POSIXLY_CORRECT LANGUAGE), @locale_envvars);
+
+ $ENV{POSIXLY_CORRECT} = 1;
+ $ENV{LANGUAGE} = 'ca:de';
+ $ENV{$_} = 'fr.UTF-8' foreach @locale_envvars;
+
+ $ENV{"TEST_$_"} = $ENV{$_} foreach @test_envvars;
+ $ENV{TEST_LC_COLLATE} = 'C.UTF-8';
+
+ $ENV{DEB_VENDOR} = 'Debian';
+
+ test_makefile('buildenv.mk', 'with envvars');
+
+ delete $ENV{POSIXLY_CORRECT};
+ $ENV{LANGUAGE} = '';
+ $ENV{$_} = '' foreach @locale_envvars;
+
+ $ENV{"TEST_$_"} = $ENV{$_} foreach @test_envvars;
+ $ENV{TEST_LC_COLLATE} = 'C.UTF-8';
+
+ test_makefile('buildenv.mk', 'without envvars');
+}
+
$ENV{DEB_BUILD_OPTIONS} = 'parallel=16';
$ENV{TEST_DEB_BUILD_OPTION_PARALLEL} = '16';
test_makefile('buildopts.mk');
diff --git a/scripts/t/mk/buildenv.mk b/scripts/t/mk/buildenv.mk
new file mode 100644
index 000000000..1b738457c
--- /dev/null
+++ b/scripts/t/mk/buildenv.mk
@@ -0,0 +1,20 @@
+include $(srcdir)/mk/buildenv.mk
+
+TN = $(srcdir)/t/bin/test-no-envvar
+TE = $(srcdir)/t/bin/test-eq-envvar
+
+test:
+ $(TE) LANGUAGE
+ $(TE) LC_ADDRESS
+ $(TE) LC_COLLATE
+ $(TE) LC_CTYPE
+ $(TE) LC_IDENTIFICATION
+ $(TE) LC_MEASUREMENT
+ $(TE) LC_MESSAGES
+ $(TE) LC_MONETARY
+ $(TE) LC_NAME
+ $(TE) LC_NUMERIC
+ $(TE) LC_PAPER
+ $(TE) LC_TELEPHONE
+ $(TE) LC_TIME
+ $(TN) POSIXLY_CORRECT
--
2.39.1
Reply to: