dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
363 stars 66 forks source link

Clean up Autotools #103

Closed SoapGentoo closed 3 years ago

SoapGentoo commented 3 years ago

@dbry modernizing and simplifying the autotools part. There should be no functional changes.

dbry commented 3 years ago

Thanks so much for doing this! I am just out the door for the weekend, but I will try this all out first thing next week.

dbry commented 3 years ago

Travis and I are both having trouble with AM_ICONV. I updated Travis to focal (the latest) to get to Autotools 1.16, but still got failure. You can see them here. The Mac and Focal versions fail slightly differently, but they're both related to AM_ICONV.

SoapGentoo commented 3 years ago

can you try installing gettext too?

dbry commented 3 years ago
gettext (GNU gettext-runtime) 0.19.8.1
Copyright (C) 1995-1997, 2000-2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Written by Ulrich Drepper.
SoapGentoo commented 3 years ago

Where are you defining the dependencies you need to pull in for Homebrew in Travis?

dbry commented 3 years ago

As far as I know, everything is in this file and most of the Homebrew stuff is to build the man pages.

dbry commented 3 years ago

This worked for me to build locally:

touch config.rpath
./autogen.sh
dbry commented 3 years ago

Okay, I got the Travis Mac build fixed by upgrading from xcode11 to xcode12. And my touch fix for Ubuntu worked on Travis too, but that's probably not the right fix, correct?

SoapGentoo commented 3 years ago

For the touch fix, try the following:

cp /usr/share/gettext/config.rpath .
dbry commented 3 years ago

I guess that should go in autogen.sh since that's where everything needed to prep the environment should go, right? I don't imagine that the correct why to handle this is to put a recent copy of config.rpath in the repo, right? I made an attempt here and Travis is happy at last.

SoapGentoo commented 3 years ago

Good question. config.rpath is a gettext file, so generally gets imported if you enable the whole gettext machinery (which you aren't). You can import it into your repo or add it in autogen.sh, I'd probably choose the latter if given the choice, because it is less likely to rot away in a repo and only gets copied into the root source dir when actually needed. Want me to add it in this PR?

dbry commented 3 years ago

I created a test / scratch branch https://github.com/dbry/WavPack/compare/autotools-testing on top of your changes for triggering Travis runs. Not sure if you noticed, but a Travis run is triggered every PR or push to any branch, and you can see the latest run at https://travis-ci.org/github/dbry/WavPack.

I have pushed 6 times to that branch to get Travis happy again, including the autogen.sh change, so if you want to squash those changes and bring them into your PR, I can merge the whole thing together.

SoapGentoo commented 3 years ago

@dbry all squashed into one commit

dbry commented 3 years ago

I have tested this further and just about everything works just as expected. Thanks again for this!

I did have one question though. The required automake version is 1.16. Is that really necessary, or did you just set that by default? The reason I ask is that 1.16 is fairly new and one of my development computers is on Linux Mint 19.3 (barely a year old) and I really have no interest in upgrading it because it works fine and I don't want to break it. I rolled back the minimum requirement to 1.15 and everything worked great, so I'll use that for now, but I have no idea if that's actually acceptable.

SoapGentoo commented 3 years ago

@dbry I've pulled it down to 1.15. In general, because Automake is something that gets bootstrapped into the tarball, I consider requiring the latest version slightly less of a hassle than say, the latest GCC or Clang. That said, there's no feature in this PR (or one to come) that depends on 1.16, hence I've relaxed the lower bound to 1.15.

dbry commented 3 years ago

Great, thanks!