feat(build): add autoreconf dependency check #1584

pull epiccurious wants to merge 2 commits into bitcoin-core:master from epiccurious:add-autoreconf-dependency-check changing 1 files +3 −1
  1. epiccurious commented at 7:38 pm on August 13, 2024: contributor

    Add check for autoreconf dependency to autogen.sh and refactor -if to long forms of optional arguments.

    Impact

    • Add a functional change to improves error handling to provide a more helpful message if autoreconf is not found.
    • Add a non-functional refactor to change the autoreconf optional arguments from -if to –install –force.
    • These changes align with the format of autogen.sh on the bitcoin repo

    Background

    10 years ago: Created the entire file with no other updates.

  2. feat(build): add autoreconf dependency check de3c8ac2ad
  3. refactor: use `--install --force` instead of `-if` fa50c90ae4
  4. hebasto commented at 7:44 pm on August 13, 2024: member
    • Add a functional change to improves error handling to provide a more helpful message if autoreconf is not found.

    On my system, the current error message is already quite descriptive:

    0$ ./autogen.sh 
    1./autogen.sh: 3: autoreconf: not found
    
  5. epiccurious commented at 1:24 pm on August 14, 2024: contributor

    the current error message is already quite descriptive

    I agree, but the error message could be made even more helpful for those less familiar with the build process.

    Since autoreconf is a part of the autoconf package, directing users to install autoconf (rather than just indicating that autoreconf is missing) makes it clearer which package to install.

    autoconf is named consistently across popular Unix-based package managers, so this saves a step for users who try to install autoreconf:

    0apt-get install autoconf
    1dnf install autoconf
    2pacman -S autoconf
    3install autoconf
    4zypper install autoconf
    5apk add autoconf
    6nix-env -iA nixpkgs.autoconf
    7emerge dev-util/autoconf
    
  6. real-or-random commented at 1:37 pm on September 25, 2024: contributor
    I don’t think we need this, so I don’t think we should spend time reviewing this. But I’m not against merging it.
  7. real-or-random added the label build on Sep 25, 2024
  8. hebasto commented at 5:19 pm on September 25, 2024: member

    the current error message is already quite descriptive

    I agree, but the error message could be made even more helpful for those less familiar with the build process.

    Since autoreconf is a part of the autoconf package, directing users to install autoconf (rather than just indicating that autoreconf is missing) makes it clearer which package to install.

    autoconf is named consistently across popular Unix-based package managers, so this saves a step for users who try to install autoreconf:

    0apt-get install autoconf
    1dnf install autoconf
    2pacman -S autoconf
    3install autoconf
    4zypper install autoconf
    5apk add autoconf
    6nix-env -iA nixpkgs.autoconf
    7emerge dev-util/autoconf
    

    The suggested approach is incomplete. Wouldn’t it be better to document build prerequisites, which also include automake, libtool, etc?

  9. real-or-random commented at 8:23 pm on September 25, 2024: contributor

    The suggested approach is incomplete. Wouldn’t it be better to document build prerequisites, which also include automake, libtool, etc?

    Yes, I think so, and this should probably simply go to the README then.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-02 14:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me