Skip to content

Minimum recommended toolchain - document and test it #1874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hebasto wants to merge 2 commits into
base: master
Choose a base branch
from
+58 −8
Open

Minimum recommended toolchain - document and test it #1874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
414cdb0
doc: Document the minimum recommended toolchain
hebasto Jun 17, 2026
6dd486e
ci: Add CI job to build on Debian "oldoldstable"
hebasto Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,34 @@ jobs:
- *CI_SCRIPT_IN_DOCKER
- *PRINT_LOGS

x86_64_debian_oldold:
name: "x86_64: Linux (Debian oldoldstable)"
runs-on: ubuntu-latest

env:
ASM: 'auto'
WITH_VALGRIND: 'no'
ECDH: 'yes'
RECOVERY: 'yes'
EXTRAKEYS: 'yes'
SCHNORRSIG: 'yes'
MUSIG: 'yes'
ELLSWIFT: 'yes'
CTIMETESTS: 'no'
CC: 'gcc'

steps:
- *CHECKOUT

- name: CI script
uses: ./.github/actions/run-in-docker-action
with:
dockerfile: ./ci/linux-debian-oldold.Dockerfile
scope: ${{ runner.arch }}
command: ./ci/ci.sh

- *PRINT_LOGS

valgrind_debian:
name: "Valgrind ${{ matrix.configuration.binary_arch }} (memcheck)"
runs-on: ${{ matrix.configuration.runner }}
Expand Down
21 changes: 13 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ Implementation details
* No runtime heap allocation.
* Extensive testing infrastructure.
* Structured to facilitate review and analysis.
* Intended to be portable to any system with a C89 compiler and uint64_t support.
* No use of floating types.
* Expose only higher level interfaces to minimize the API surface and improve application security. ("Be difficult to use insecurely.")
* Field operations
Expand Down Expand Up @@ -97,8 +96,15 @@ This can be done with the following steps:
Subkey fingerprint: 4BBB 845A 6F5A 65A6 9DFA EC23 4861 DBF2 6212 3605
```

Building with Autotools
-----------------------
## Building from source

@real-or-random real-or-random Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
## Building from source
## Building

Maybe it's just me, but "building from source" sounds like this is a special thing and we also ship binaries.

@real-or-random real-or-random Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my "code" suggestion: Just "Building"


The project is intended to be portable to any system with a C89/C90-compatible toolchain.
Additionally, the C standard library must provide the `<stdint.h>` header.

Toolchains older than the default one shipped in the current Debian "oldoldstable" release
are not tested and are not recommended for building the project.
Comment on lines +104 to +105

@real-or-random real-or-random Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove this sentence (and change the PR title accordingly).

  • I don't think we should recommend a mininum compiler. People should use modern compilers, but this is nothing we need to state. What compiler we test with is an internal thing, and this should not become a "user-facing" guarantee (The wording does not imply a guarantee, but if it's not guarantee, it doesn't belong in the README. We could add it to CONTRIBUTING but I don't think it's necessary. The CI check is enough documentation.)
  • That older than oldoldstable is not recommended could be interpreted to mean that oldoldstable and newer is recommend. But I wouldn't recommend oldoldstable to anyone.

@fanquake fanquake Jun 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I'm not sure there's much value (for this project) in trying to establish a minimum supported compiler version, and it seems possible that tests for this are going to add more friction than value. As long as they are low-effort, and can be deleted/changed easily, probably fine.

I'd also say that if we were going to document something, just state the particular compiler, rather than make people look up what compiler/version pair happened to ship on an old version of some Linux distro.

@real-or-random real-or-random Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also say that if we were going to document something, just state the particular compiler, rather than make people look up what compiler/version pair happened to ship on an old version of some Linux distro.

No strong opinion here but I still I tend to simply say "oldoldstable". Then we won't run into the need to bump the version regularly (e.g., because something breaks on an old compiler/docker image) and worse, use our brain cycles to make a decision whether the bump is justified without clear guidelines.


### Building with Autotools

$ ./autogen.sh # Generate a ./configure script
$ ./configure # Generate a build system
Expand All @@ -108,12 +114,11 @@ Building with Autotools

To compile optional modules (such as Schnorr signatures), you need to run `./configure` with additional flags (such as `--enable-module-schnorrsig`). Run `./configure --help` to see the full list of available flags.

Building with CMake
-------------------
### Building with CMake

To maintain a pristine source tree, CMake encourages to perform an out-of-source build by using a separate dedicated build tree.

### Building on POSIX systems
#### Building on POSIX systems

$ cmake -B build # Generate a build system in subdirectory "build"
$ cmake --build build # Run the actual build process
Expand All @@ -122,7 +127,7 @@ To maintain a pristine source tree, CMake encourages to perform an out-of-source

To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake -B build -LH` or `ccmake -B build` to see the full list of available flags.

### Cross compiling
#### Cross compiling

To alleviate issues with cross compiling, preconfigured toolchain files are available in the `cmake` directory.
For example, to cross compile for Windows:
Expand All @@ -133,7 +138,7 @@ To cross compile for Android with [NDK](https://developer.android.com/ndk/guides

$ cmake -B build -DCMAKE_TOOLCHAIN_FILE="${ANDROID_NDK_ROOT}/build/cmake/android.toolchain.cmake" -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=28

### Building on Windows
#### Building on Windows

The following example assumes Visual Studio 2022. Using clang-cl is recommended.

Expand Down
17 changes: 17 additions & 0 deletions ci/linux-debian-oldold.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
FROM debian:oldoldstable-slim

SHELL ["/bin/bash", "-c"]

WORKDIR /root

RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
git \
autoconf automake libtool make \
gcc \
python3-full && \
apt-get clean && rm -rf /var/lib/apt/lists/*

ENV VIRTUAL_ENV=/root/venv
RUN python3 -m venv $VIRTUAL_ENV
ENV PATH="$VIRTUAL_ENV/bin:$PATH"
RUN pip install lief
Comment on lines +14 to +17

@real-or-random real-or-random Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should omit python from this job to simplify maintenance. We don't want to be restricted to whatever Python version oldoldstable ships (and who knows if lief will runs on that version in the future).

@hebasto hebasto Jun 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling Python would require more changes in this PR to handle the testvectors build target, similar to 071523b.

@real-or-random real-or-random Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yeah. I was thinking we can just set SYMBOL_CHECK=no, but I forgot about the test vectors. Though I can we could also have TEST_VECTORS variable that is enabled by default. That would just change a single line in the script.

Loading