system/patch: update makedepends to include { 'autoconf', 'automake' }

requested review from @Sheila

approved this merge request

mentioned in commit 5da08cbb

merged

This isn't right.

If the change to checkdepends was incorrect when CVE-2018-1000156 was patched (which is the only reason I can see from the package recipe that autotools would be needed), the dependency should be unconditionally moved to makedepends and removed from checkdepends. Additionally, there should not be a reliance on ./configure to re-bootstrap itself and the package's bootstrapping script should be called explicitly.

I'm looking at d528964b when these were added to checkdepends. I'm not sure I follow how this relates to the CVE you mentioned above.

The actual build failure which prompted the change is this, with !check:

  AR       libpatch.a
make[3]: Leaving directory '/root/packages/system/patch/src/patch-2.7.6/lib'
make[2]: Leaving directory '/root/packages/system/patch/src/patch-2.7.6/lib'
Making all in src
make[2]: Entering directory '/root/packages/system/patch/src/patch-2.7.6/src'
  CC       inp.o
  CC       patch.o
  CC       pch.o
  CC       safe.o
  CC       util.o
  CC       version.o
  CC       merge.o
  CCLD     patch
make[2]: Leaving directory '/root/packages/system/patch/src/patch-2.7.6/src'
Making all in tests
make[2]: Entering directory '/root/packages/system/patch/src/patch-2.7.6/tests'
 cd .. && /bin/sh /root/packages/system/patch/src/patch-2.7.6/build-aux/missing automake-1.15 --gnu tests/Makefile
/root/packages/system/patch/src/patch-2.7.6/build-aux/missing: 81: automake-1.15: not found
WARNING: 'automake-1.15' is missing on your system.
         You should only need it if you modified 'Makefile.am' or
         'configure.ac' or m4 files included by 'configure.ac'.
         The 'automake' program is part of the GNU Automake package:
         <http://www.gnu.org/software/automake>
         It also requires GNU Autoconf, GNU m4 and Perl in order to run:
         <http://www.gnu.org/software/autoconf>
         <http://www.gnu.org/software/m4/>
         <http://www.perl.org/>
make[2]: *** [Makefile:1361: Makefile.in] Error 127
make[2]: Leaving directory '/root/packages/system/patch/src/patch-2.7.6/tests'
make[1]: *** [Makefile:1325: all-recursive] Error 1
make[1]: Leaving directory '/root/packages/system/patch/src/patch-2.7.6'
make: *** [Makefile:1225: all] Error 2
>>> ERROR: patch: build failed

If autotools is needed only for the tests, it looks like they're needed only to build them, not to run them. But because tests are being built during the build stage, it looks to me like we need to keep that from happening (assuming make check will instead generate whatever files).

The patch for the CVE I mentioned modifies tests/Makefile.am, which is why automake (correctly) deduces it needs to be run again.

Perhaps we should conditionally add --disable-check (or the equivalent for this package) to ./configure depending on the options; that would be the most "correct" solution to this problem. I'll look in to it in my dev env.

If we unconditionally include autotools in makedepends and allow tests/ to be updated, I see no harm in removing them from checkdepends since they shouldn't be needed. OF course, verify first.

Then if !check or not, the package will always be built the same. That seems preferable to looking at options. Thoughts?

To your first point: yes, if they are left unconditional in makedepends they do not belong in checkdepends.

To your second point: I'm not sure.

!check means "I do not want tests". I know that, for instance, user/flac spends a significant amount of time building its tests (not to mention running them). Building test suites may be desirable to skip on say, an armv7 that is doing a dry run, or an m68k doing an initial build-out.

Plasma is another example of this: a full 2/3rds of the build time of user/plasma-desktop is spent building autotests.

To expand on my last comment, I don't know how much we really want to support !check for anything but dry runs anyway.

Putting in effort to make !check work implies it is desired to actually use !check, and it really, really isn't. There is the argument that it is better to leave !check with the bare minimum amount of effort.

But it still feels more "correct" to me to have !check control not just execution of test suites, but building of test suites. This could be useful down the road to regression-test builds that fail in test suites (if a test fails to build, we can !check to see if it is just a test suite problem, or a wider issue that the test suite is revealing).

I was brainstorming earlier; imagine having builders that are independent of testers. We could build on powerful hardware rapidly (including the tests) and have a handful of less-powerful hardware (e.g. a G3 or three, Raspberry Pi) run tests as part of a CI job.

The successful passing of tests would be reported back, and this would trigger promotion of packages to mirrormaster for release from some staging environment.

If the trigger were on a per-package basis, I'd be interested

It would need to be, for it to be of any use. This is the idea.

... using a framework like APK Foundry

OK if someone can help with that, otherwise I'm doing things in the order and manner I see fit.

This is a bit OT for this MR

IRC is open, you and Max are always welcome to participate :)

mentioned in issue #406

resolved all threads