Skip to content

Build script to build liboqs#2417

Open
charishma1407 wants to merge 45 commits into
open-quantum-safe:mainfrom
charishma1407:charishma_build_script
Open

Build script to build liboqs#2417
charishma1407 wants to merge 45 commits into
open-quantum-safe:mainfrom
charishma1407:charishma_build_script

Conversation

@charishma1407
Copy link
Copy Markdown

@charishma1407 charishma1407 commented May 5, 2026

Purpose

This PR adds a build script (build_liboqs.sh) to simplify the compilation process for liboqs. The script automates the CMake configuration and build steps, making it easier for developers to build the library with common options.

Changes

  • Added build_liboqs.sh script to automate the build process such as installing the dependencies and building.
  • Script handles CMake configuration and compilation
  • Provides a convenient wrapper for common build operations

Testing

The script has been tested locally and successfully builds the liboqs library.

Checklist

  1. Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)?
    No - This PR only adds a build convenience script and does not modify any cryptographic algorithms.
  2. Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API?
    No - This PR does not modify the algorithm list or any APIs. It only adds a build helper script.

AI Assistance

This contribution was created with the use of generative AI for builld-script-test.yml workflow

Copy link
Copy Markdown
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @charishma1407 ! I like it as it makes life easier for users -- and is even more complete than the similar build script we have in the even more user-focused oqs-provider project.

That said, some questions:

  • Who is the target audience for this? liboqs is a library that not too many users would (arguably, should :) build for themselves: One could argue that not having a build script excludes people from building the project that may have a propensity to "shoot themselves into the foot" having/wrongly using such script.
  • Why no Windows support?
  • Why no CI (testing)?
  • Who would maintain this going forward (e.g., when new config options or algs get added/changed)?

Copy link
Copy Markdown
Member

@bhess bhess left a comment

Choose a reason for hiding this comment

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

Hi @charishma1407, thanks for the PR, I agree this is a useful convenience for users. To echo and build on @baentsch's last points: my main concern is long‑term maintainability. The script hard‑codes build options and algorithms but isn't exercised in CI, so as new options or algorithms are added it could easily drift out of sync. Adding CI coverage for the script, or otherwise deriving its options from a single source of truth, would help reduce that risk. On the other hand, this might add quite some complexity that counters the goal to simplify the build.

@baentsch
Copy link
Copy Markdown
Member

baentsch commented May 5, 2026

Thanks @charishma1407 for adding thorough CI testing! This addresses those parts of the questions of @bhess and myself -- but it has one drawback: It requires an awful lot of compute cycles covering many --but surely not all-- current option-configs; also it will conceptually never be able to address the maintenance question (new config options added in the future).

So allow me to make a "wild" proposal: Would it be conceivable that a) at minimum this PR adds a feature to the script to issue a build-time warning if it discovers that any config file it depends upon and that are checked in (e.g., CONFIGURE.md, CMakeLists.txt) is more current than the build script? Even better (but I don't know whether it's possible) would be b) that such "build-script-out-of-date" check (in CI) triggers a Copilot "agent" creating an "AI-written" update to the build script taking the latest config options into account and ideally, even generating a PR for that?

@charishma1407
Copy link
Copy Markdown
Author

charishma1407 commented May 5, 2026

Hi @baentsch and @bhess I made changes according to the comments

  1. Created the workflow .yml file to run a CI test, and reduced it from 13 to 4 jobs to prevent the cycle.
  2. Updated the build script with the stale code check in it.
  3. As integrating it to copilot, I am not sure that copilot has the feature of creating the PR on its own.
  4. Build script can make dynamically but one problem with that is it make the script more complex and also it looses it's simplicity.

@charishma1407 charishma1407 requested review from baentsch and bhess May 5, 2026 11:40
@charishma1407 charishma1407 force-pushed the charishma_build_script branch from 8a95d63 to bc367cc Compare May 6, 2026 07:55
Comment thread build_liboqs.sh Outdated
@charishma1407 charishma1407 requested a review from baentsch May 7, 2026 04:07
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 82.269% (-0.001%) from 82.27% — charishma1407:charishma_build_script into open-quantum-safe:main

Comment thread .github/workflows/build-script-test.yml Outdated
Comment thread .github/workflows/build-script-test.yml
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
Copy link
Copy Markdown
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Basically LGTM, see single comments (Yes, I should have grouped them into one coherent review :-()

@charishma1407 charishma1407 force-pushed the charishma_build_script branch from 6cd9dfb to dc886ac Compare May 12, 2026 10:19
@charishma1407 charishma1407 requested a review from xuganyu96 as a code owner May 12, 2026 10:19
@charishma1407 charishma1407 requested a review from baentsch May 12, 2026 10:27
Comment thread README.md Outdated
Comment thread .github/workflows/build-script-test.yml Outdated
Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
echo ""

echo "Entering Nix development environment..."
nix develop
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.

Have you tested the nix version? Claude says the following (but I don't use nix so I have no way of verifying whether this is a legitimate complaint or not):

nix develop spawns an interactive subshell and then falls through to cmake/ninja in the parent shell outside the nix env — the NixOS path almost certainly doesn't work as intended. Either exec nix develop -c ./build_liboqs.sh --inside-nix style, or document that users should nix develop themselves first.

Comment thread build_liboqs.sh Outdated
Comment thread build_liboqs.sh Outdated
Comment thread README.md
Comment thread .github/workflows/build-script-test.yml Outdated
@dstebila
Copy link
Copy Markdown
Member

* Who would maintain this going forward (e.g., when new config options or algs get added/changed)?

I'm also worried about maintainability going forward -- not that the script is unmaintainable, just that we will forget to update it. I think the staleness check is a good step. But probably we need some documentation reminding us? Maybe in the PR template? Or somewhere else?

@charishma1407 charishma1407 force-pushed the charishma_build_script branch from 96aaa02 to f94eca7 Compare May 13, 2026 04:57
@charishma1407 charishma1407 requested a review from dstebila May 13, 2026 09:19
P Charishma Kumari and others added 26 commits May 19, 2026 10:40
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Mac.lan>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
- Tests script on Ubuntu and macOS with multiple configurations
- Validates drift detection between script and CMake options
- Includes syntax and quality checks
- Addresses maintainer feedback about keeping script in sync

Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
- Reduced test matrix from 13 to 4 essential configurations
- 69% reduction in test jobs, ~70% reduction in compute time
- Focuses on: default, minimal, shared+no-openssl, and cross-platform
- Staleness detection provides early warning for drift
- Addresses maintainer concern about compute cost

Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
…ed on CMake options

Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@mac.j9d-in.ibm.com>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
@charishma1407 charishma1407 force-pushed the charishma_build_script branch from f94eca7 to 16e2b28 Compare May 19, 2026 16:06
@charishma1407
Copy link
Copy Markdown
Author

Hi @xuganyu96
Thank you for the detailed feedback on the script. I've addressed all three concerns:

  1. Added an option only to build not installing the dependencies.
  2. If a user selects for default build it will be installing the default dependencies and build with default settings.
  3. Created requirements.txt as you suggested
  4. Updated the regex for more accurate.
  5. In the script there is another option where user can directly give -D and build option apart from the given options.
  6. Script help text focuses on common use cases

This is script is not intended to remove the dependency on manual installation or CMAKE usage. As documented user can also go with the manual installation steps also.

Signed-off-by: P Charishma Kumari <pcharishmakumari@Ps-MacBook-Pro.local>
@charishma1407 charishma1407 requested a review from xuganyu96 May 19, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants