Re: t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-10 Thread Mathieu Lirzin
Hello Karl,

Mathieu Lirzin  writes:

> Karl Berry  writes:
>
>> Other than that, please push asap! --thanks again, karl.
>
> I Will push that patch before the end of the week.

Done in commit f19ecc089b017d0f0cde1e960fb1a6a407005164.

Thanks for the review.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37



Re: t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-05 Thread Mathieu Lirzin
Hello Karl,

Karl Berry  writes:

> Hi Mathieu - I'm glad you replied. I saw in the ChangeLog that you
> created AUTOMAKE_UNINSTALLED :). The general idea was clear, but all the
> implications, not so much.
>
> Here is a patch that seem to fix the issue, I have added some clutter to
> AM_TESTS_ENVIRONMENT 
>
> Thanks! make distcheck now passes completely, all three failing tests
> are fixed, with both python2 and python3. That is great.
>
> The only trivial thing I would ask is that I would appreciate not
> breaking new ground with the UTF-8 quotes in the comment, since they are
> not really necessary (the comment is just as understandable with no
> quote characters, or '...' if you prefer). There are no UTF-8 quotes in
> the main source files now.
> +# the €˜pre-inst-env€™ wrapper script.

Sure that was not intentional.

> Other than that, please push asap! --thanks again, karl.

I Will push that patch before the end of the week.

Thanks.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37



Re: t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-05 Thread Karl Berry
Hi Mathieu - I'm glad you replied. I saw in the ChangeLog that you
created AUTOMAKE_UNINSTALLED :). The general idea was clear, but all the
implications, not so much.

Here is a patch that seem to fix the issue, I have added some clutter to
AM_TESTS_ENVIRONMENT 

Thanks! make distcheck now passes completely, all three failing tests
are fixed, with both python2 and python3. That is great.

The only trivial thing I would ask is that I would appreciate not
breaking new ground with the UTF-8 quotes in the comment, since they are
not really necessary (the comment is just as understandable with no
quote characters, or '...' if you prefer). There are no UTF-8 quotes in
the main source files now.
+# the â€˜pre-inst-envâ€™ wrapper script.

Other than that, please push asap! --thanks again, karl.



Re: t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-04 Thread Mathieu Lirzin
Hello Karl,

Karl Berry  writes:

> The aclocal-print-acdir.sh test fails at the make installcheck step of
> make distcheck (it succeeds in the normal make check, and it succeeds at
> the make check of make distcheck; only fails in the make installcheck).
>
> This is because AUTOMAKE_UNINSTALLED is (correctly) set in the
> environment at that point, which causes aclocal-1.16 --print-ac-dir
> to forcibly return the empty string, which does not match the expected
> acdir string:
>
>   + aclocal-1.16 -Werror --print-ac-dir
>
>   test "$($ACLOCAL --print-ac-dir)" = "$am_system_acdir"
>   ++ aclocal-1.16 -Werror --print-ac-dir
>   + test '' = /u/karl/gnu/src/akarl/automake-1.16a/_inst/share/aclocal
>   am_exit_trap $?
>   + am_exit_trap 1
>
> Per this bit in the aclocal-1.16 Perl script:
>
>   if (exists $ENV{"AUTOMAKE_UNINSTALLED"})
> {
>   @automake_includes = ();
>   @system_includes = ();
> }
>
> (The --print-ac-dir option simply prints the value of @system_includes.)
>
> So, if I unset AUTOMAKE_UNINSTALLED in the test, it works:
> test "$am_running_installcheck" = yes && unset AUTOMAKE_UNINSTALLED || :
>
> Since this test is intended to check exactly a value that only is set
> normally in an installation, that seems like a reasonable thing to do.
>
> But I am not sure. Jim, anyone with more experience, can you confirm/deny?

The AUTOMAKE_UNINSTALLED environment variable along side the
“pre-inst-env” wrapper script were introduced to isolate srcdir and
builddir environment from installdir while not having to patch the
source code at install time. This follows a pattern I discovered in the
Guix package and ported to Automake and Texinfo.

To fix ‘make installcheck’ I think it would make sense to remove the
usage of the “pre-inst-env” script in that context because as its name
suggest this is a "pre-inst{allation}-env{ironment}". Concretely This
means overriding LOG_COMPILER and PL_LOG_COMPILER and ensuring that the
installed perl modules are found for the tests.

Here is a patch that seem to fix the issue, I have added some clutter to
AM_TESTS_ENVIRONMENT which is not ideal but was less work than migrating
everything to a “test-env” wrapper script which would probably improve
readability.

What do you think?

It is nice to see some activity on Automake. :-)

>From 49a02b8ce3e1e2475ec51e432806c9fb8eb743e5 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin 
Date: Tue, 4 Feb 2020 15:28:00 +0100
Subject: [PATCH] =?UTF-8?q?build:=20fix=20=E2=80=98installcheck=E2=80=99?=
 =?UTF-8?q?=20target?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* t/local.mk (installcheck-testsuite): Do not use "pre-inst-env" script.
(AM_TESTS_ENVIRONMENT): Ensure that installed perl modules are found.
---
 t/local.mk | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/t/local.mk b/t/local.mk
index 7e3a61965..06ed70d0e 100644
--- a/t/local.mk
+++ b/t/local.mk
@@ -244,12 +244,22 @@ check-parallel:
 test_subdirs = %D% %D%/pm contrib/%D%
 include %D%/CheckListOfTests.am
 
-# Run the testsuite with the installed aclocal and automake.
+# Run the testsuite with the installed aclocal and automake without using
+# the ‘pre-inst-env’ wrapper script.
 installcheck-local: installcheck-testsuite
 installcheck-testsuite:
 	$(AM_V_GEN)$(MAKE) $(AM_MAKEFLAGS) check \
+	  LOG_COMPILER=$(AM_TEST_RUNNER_SHELL) \
+	  PL_LOG_COMPILER=$(PERL) \
 	  am_running_installcheck=yes
 
+# Ensure that the installed Automake perl modules are found when running 'installcheck' target
+AM_TESTS_ENVIRONMENT += \
+  if test "$${am_running_installcheck}" = yes; then \
+PERL5LIB="$(DESTDIR)$(pkgvdatadir)/$${PERL5LIB:+$(PATH_SEPARATOR)}$$PERL5LIB"; \
+  fi; \
+  export PERL5LIB;
+
 # Performance tests.
 .PHONY: perf
 perf: all
-- 
2.20.1


-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37


Re: t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-03 Thread Jim Meyering
On Mon, Feb 3, 2020 at 6:23 PM Karl Berry  wrote:
> The aclocal-print-acdir.sh test fails at the make installcheck step of
> make distcheck (it succeeds in the normal make check, and it succeeds at
> the make check of make distcheck; only fails in the make installcheck).
>
> This is because AUTOMAKE_UNINSTALLED is (correctly) set in the
> environment at that point, which causes aclocal-1.16 --print-ac-dir
> to forcibly return the empty string, which does not match the expected
> acdir string:
>
>   + aclocal-1.16 -Werror --print-ac-dir
>
>   test "$($ACLOCAL --print-ac-dir)" = "$am_system_acdir"
>   ++ aclocal-1.16 -Werror --print-ac-dir
>   + test '' = /u/karl/gnu/src/akarl/automake-1.16a/_inst/share/aclocal
>   am_exit_trap $?
>   + am_exit_trap 1
>
> Per this bit in the aclocal-1.16 Perl script:
>
>   if (exists $ENV{"AUTOMAKE_UNINSTALLED"})
> {
>   @automake_includes = ();
>   @system_includes = ();
> }
>
> (The --print-ac-dir option simply prints the value of @system_includes.)
>
> So, if I unset AUTOMAKE_UNINSTALLED in the test, it works:
> test "$am_running_installcheck" = yes && unset AUTOMAKE_UNINSTALLED || :
>
> Since this test is intended to check exactly a value that only is set
> normally in an installation, that seems like a reasonable thing to do.

That sounds eminently reasonable.

I don't recall ever knowing about this AUTOMAKE_UNINSTALLED envvar, so
you are now the sole active contributor who has plumbed those depths
:-)

> But I am not sure. Jim, anyone with more experience, can you confirm/deny?
>
> Thanks,
> Karl
>
> P.S. This also fails for me if I run make installcheck after
> building the released automake-1.16.
>
> P.P.S. There are two other tests, print-libdir and aclocal-print-acdir,
> which also fail at the make installcheck step of make distcheck, and not
> sooner. Unfortunately they are not solved by unsetting
> AUTOMAKE_UNINSTALLED, though I think the problem is generally similar.

Good!



t/aclocal-print-acdir.sh at installcheck and AUTOMAKE_UNINSTALLED

2020-02-03 Thread Karl Berry
The aclocal-print-acdir.sh test fails at the make installcheck step of
make distcheck (it succeeds in the normal make check, and it succeeds at
the make check of make distcheck; only fails in the make installcheck).

This is because AUTOMAKE_UNINSTALLED is (correctly) set in the
environment at that point, which causes aclocal-1.16 --print-ac-dir
to forcibly return the empty string, which does not match the expected
acdir string:

  + aclocal-1.16 -Werror --print-ac-dir

  test "$($ACLOCAL --print-ac-dir)" = "$am_system_acdir"
  ++ aclocal-1.16 -Werror --print-ac-dir
  + test '' = /u/karl/gnu/src/akarl/automake-1.16a/_inst/share/aclocal
  am_exit_trap $?
  + am_exit_trap 1

Per this bit in the aclocal-1.16 Perl script:

  if (exists $ENV{"AUTOMAKE_UNINSTALLED"})
{
  @automake_includes = ();
  @system_includes = ();
}

(The --print-ac-dir option simply prints the value of @system_includes.)

So, if I unset AUTOMAKE_UNINSTALLED in the test, it works:
test "$am_running_installcheck" = yes && unset AUTOMAKE_UNINSTALLED || :

Since this test is intended to check exactly a value that only is set
normally in an installation, that seems like a reasonable thing to do.

But I am not sure. Jim, anyone with more experience, can you confirm/deny?

Thanks,
Karl

P.S. This also fails for me if I run make installcheck after
building the released automake-1.16.

P.P.S. There are two other tests, print-libdir and aclocal-print-acdir,
which also fail at the make installcheck step of make distcheck, and not
sooner. Unfortunately they are not solved by unsetting
AUTOMAKE_UNINSTALLED, though I think the problem is generally similar.