serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Ports/alpine: Force re-generation of all autotools files after patch

Open tajmorton opened this issue 2 years ago • 6 comments

Ports/alpine: Force re-generation of all autotools files after patch

Required to ensure that all autogenerated files are compatible with the verions of autotools installed on the build system. Resolves SerenityOS/serenity#19891.

Verification:

  • Rebuild alpine package from scratch (cd Ports/Alpine; ./package.sh) and ensured Alpine launched successfully.
  • Equivalent patch tested by @BenWiederhake in #19891 on a system where the build previously failed (due to mismatch between libtool in the generated files and local build system).

tajmorton avatar Jul 09 '23 16:07 tajmorton

Realistically, we should not touch this at all. The tarball that we use arrives preconfigured, so we should patch configure to include SerenityOS support and leave the rest be. Using any of the auto* tools from the host has the opportunity to introduce entirely new kinds of compatibility problems (#14060).

timschumi avatar Jul 09 '23 16:07 timschumi

@timschumi is the suggestion to manually run autoconf after applying the autoconf/automake patches, and then commit the results of running autoconf? I'm game, but this is definitely a big patch, and a larger maintenance burden for future upgrades

Are there any other neat patterns I'm missing that avoid having to commit all the auto-generated files, or to make it easier to regenerate this patch in the future?

 git diff --stat
 Makefile.in                   |    44 +-
 aclocal.m4                    |   224 ++-
 alpine/Makefile.in            |   258 +++-
 alpine/osdep/Makefile.in      |    98 +-
 configure                     | 10801 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------
 include/config.h.in           |    15 +-
 m4/Makefile.in                |    16 +-
 pico/Makefile.in              |   166 ++-
 pico/osdep/Makefile.in        |   124 +-
 pith/Makefile.in              |   391 +++--
 pith/charconv/Makefile.in     |    57 +-
 pith/osdep/Makefile.in        |   147 +-
 web/src/Makefile.in           |    16 +-
 web/src/alpined.d/Makefile.in |   195 ++-
 web/src/pubcookie/Makefile.in |    84 +-
 15 files changed, 7145 insertions(+), 5491 deletions(-)

tajmorton avatar Jul 09 '23 16:07 tajmorton

The switch from configure.ac that the patch is modifying ends up in the configure script unmodified, so you just need to find the correct location in there, add those six lines manually, and generate a patch from that.

timschumi avatar Jul 09 '23 16:07 timschumi

Thank you @timschumi, that seems obvious in retrospect. I switched to this approach in ce81553d0b60620b54f4018d8593de99d5e25720.

@BenWiederhake any chance you could try again to ensure alpine still builds for you with the patched build files?

tajmorton avatar Jul 09 '23 17:07 tajmorton

Just rebuilt it from scratch, and yes, ce81553 builds and runs :D

BenWiederhake avatar Jul 09 '23 17:07 BenWiederhake

Looking pretty good, the last nitpick from my side would be to just remove the changes to configure.ac and Makefile.am entirely and replace their patch chunks with the freshly generated ones.

timschumi avatar Jul 09 '23 17:07 timschumi

Looking pretty good, the last nitpick from my side would be to just remove the changes to configure.ac and Makefile.am entirely and replace their patch chunks with the freshly generated ones.

I thought about doing that, but was concerned about maintainability for future Alpine releases. Primarily, any future merge conflicts would need to be resolved by hand (ie manually updating Makefile.in and configure). If we keep the input files around, it's possible to just rerun to re-generate the files, selectively add the changes in their new locations, and update the existing patch.

No strong feelings either way on this, I'm happy to drop the input file patches to keep things clean within the serenity repo.

tajmorton avatar Jul 09 '23 18:07 tajmorton

Primarily, any future merge conflicts would need to be resolved by hand (ie manually updating Makefile.in and configure). If we keep the input files around, it's possible to just rerun to re-generate the files, selectively add the changes in their new locations, and update the existing patch.

Keeping the input file patches around probably won't make future upgrades any more straightforward than they already are. Worst case, if there really is a conflict, you'll now have to resolve the exact same conflict twice (since the particular code that we are changing isn't very generator-heavy).

In any case, if we ever need the input file patches unexpectedly again, we can just dig them up from the Git history.

Regenerating the files and looking at the changes between generated files probably isn't very efficient either, as you have seen that quickly turns into a 10000+ line diff.

timschumi avatar Jul 09 '23 18:07 timschumi

Thanks for the guidance. Remove makefile.in and configure.ac changes in 253c4a0067688384f8e98de3b4e2550dafb9dc98.

tajmorton avatar Jul 09 '23 19:07 tajmorton