Skip to content

libevent: call autoreconf directly instead of via autogen.sh#41057

Merged
alalazo merged 1 commit intospack:developfrom
blue42u:libevent-autogen-no-which
Nov 15, 2023
Merged

libevent: call autoreconf directly instead of via autogen.sh#41057
alalazo merged 1 commit intospack:developfrom
blue42u:libevent-autogen-no-which

Conversation

@blue42u
Copy link
Copy Markdown
Contributor

@blue42u blue42u commented Nov 14, 2023

Fixes #41056.

The autogen.sh script shipped by libevent uses which to determine if autoreconf is available, but which isn't installed by default on all systems (e.g. Fedora). If Spack is building with libtool @2.4.7 this causes build failures since the libtool M4 fragments in libevent are libtool 2.4.6.

This PR adds a patch to replace the use of which with command -v, which is a POSIX shell builtin and thus more portable.

EDIT: This PR fixes this by adding a build dependency on which.

EDIT: This PR fixes this by calling autoreconf directly, instead of trying to get autogen.sh to do so.

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Nov 14, 2023

Hi @blue42u! I noticed that the following package(s) don't yet have maintainers:

  • libevent

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers("blue42u")

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame libevent

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

@alalazo alalazo self-assigned this Nov 14, 2023
@blue42u blue42u force-pushed the libevent-autogen-no-which branch from 3946efb to e0c7691 Compare November 14, 2023 14:17
@blue42u blue42u changed the title libevent: replace which with command -v libevent: add depends on which Nov 14, 2023
alalazo
alalazo previously approved these changes Nov 14, 2023
@alalazo alalazo enabled auto-merge (squash) November 14, 2023 14:56
auto-merge was automatically disabled November 15, 2023 06:42

Head branch was pushed to by a user without write access

@blue42u blue42u force-pushed the libevent-autogen-no-which branch from e0c7691 to e834226 Compare November 15, 2023 06:42
@blue42u
Copy link
Copy Markdown
Contributor Author

blue42u commented Nov 15, 2023

Looking into the CI failure, apparently Spack's which fails if the found binary has a path longer than ~300 bytes... but only on ppc64le:

# # On ppc64le
# PATH=$(spack location -i which)/bin:$PATH which which
which: no which in (/home/software/spack/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeh/linux-ubuntu20.04-ppc64le/gcc-9.4.0/which-2.21-57arjijrsllc6tozz3mnhirvq47xpei6/bin:/opt/bootstrap/view/bin:/builds/spack/spack/bin:/opt/bootstrap/view/bin:/opt/bootstrap:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)

# # On x86-64
# PATH=$(spack location -i which)/bin:$PATH which which
/home/software/spack/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeholder__/__spack_path_placeh/linux-ubuntu20.04-x86_64_v3/gcc-11.4.0/which-2.21-e7g4ojtne4qs2qsjnd4fyt2agwncljdm/bin/which

Aside: Upon further investigation which has an interesting history of non-standardization, this post on the Debian mailing list gives a good summary of divergence stemming from ~2000. In the current world:

Given the wide range of which implementations (with different bugs and even different flags), I wonder if Spack's which should really be a virtual... but IMHO that sounds like serious overkill for a command that's already horribly non-portable in the first place.


So, take three: I've adjusted the recipe to just run autoreconf directly, which fixes the build errors. This is exactly what the shipped autogen.sh does when which works.

@blue42u blue42u changed the title libevent: add depends on which libevent: call autoreconf directly instead of via autogen.sh Nov 15, 2023
@alalazo alalazo merged commit de850e9 into spack:develop Nov 15, 2023
@blue42u blue42u deleted the libevent-autogen-no-which branch November 15, 2023 14:02
gabrielctn pushed a commit to gabrielctn/spack that referenced this pull request Nov 24, 2023
mtaillefumier pushed a commit to mtaillefumier/spack that referenced this pull request Dec 14, 2023
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 31, 2024
haampie added a commit to haampie/spack that referenced this pull request Feb 28, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

I did not check what that error is about exactly, but it looks like
Spack's `def autoreconf` step does way less than the `autgen.sh` script
provided in the source tarball.

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205, therefore, get rid of all this madness.
alalazo pushed a commit that referenced this pull request Feb 29, 2024
The deps were added in #40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in #41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since #41205.
mathomp4 pushed a commit to mathomp4/spack that referenced this pull request Mar 27, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205.
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
The deps were added in spack#40945 to make it work on macOS 11, because the
old configure scripts only detect macOS 10. Apparently people reported the
autoreconf script caused issues, later fixed in spack#41057. However, also
with that fix, things are incorrect, cause people now report:

```
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
```

HOWEVER, all this is unnecessary, because the underlying issue was
already fixed long ago, it's just that it regressed at some point, but
it's back in place since spack#41205.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Installation issue: libevent

2 participants