Skip to content
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.

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

Various CI fixes #23

Merged
merged 34 commits into from
Sep 18, 2024
Merged

Conversation

raneamri
Copy link
Contributor

Changes to merge include:

  • CXX20 & CXX17 in matrix
  • Small readability changes in Windows build
  • Boost1.85.0 manual installation, as per QuantLib's CI, for Linux builds

@CLAassistant
Copy link

CLAassistant commented Aug 11, 2024

CLA assistant check
All committers have signed the CLA.

@@ -56,8 +59,19 @@ jobs:
path: QuantLib-Risks-Cpp
- name: Setup
run: |
sudo apt update
sudo apt install -y libboost-dev ccache ninja-build
sudo apt-get update \
Copy link
Contributor

@xcelerit-team xcelerit-team Sep 10, 2024

Choose a reason for hiding this comment

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

I think we can avoid all of this if we simply use the same build container that QuantLib uses directly. Using the container argument - as it's done here with the rolling tag. That way we follow the QuantLib pre-requisites and we don't need to install all these dependencies every time.

&& cd ${{ matrix.boost_dir }} \
&& ./bootstrap.sh \
&& sudo ./b2 --without-python --prefix=/usr -j 4 link=shared runtime-link=shared install \
&& cd .. && sudo rm -rf ${{ matrix.boost_dir }} && sudo ldconfig
Copy link
Contributor

Choose a reason for hiding this comment

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

and the same thing here - use the container image provided by QuantLib.

@xcelerit-team
Copy link
Contributor

Can add an apt-get install -y ninja-build to the Linux builds? That should fix it.

@raneamri
Copy link
Contributor Author

image

I solved the prior issue but now I'm getting a path issue. Any idea why this is happening?

Copy link
Contributor

@xcelerit-team xcelerit-team left a comment

Choose a reason for hiding this comment

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

About the path issue - the problem is the ${{ github.workspace }} doesnt resolve to the right path within the container. It point to the dir outside the container, which is not working. This is a known issue with containers - perhaps you can use something like $(pwd)/../xad etc to point to the right directory where xad is?

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@raneamri
Copy link
Contributor Author

This is ready to merge. The windows cxx20 failure is not part of this pull request.

@xcelerit-team
Copy link
Contributor

This is ready to merge. The windows cxx20 failure is not part of this pull request.

Then please disable that workflow (comment it or put an if false). Can't merge with failing CI/CD.

.github/workflows/ci.yaml Show resolved Hide resolved
@auto-differentiation-dev auto-differentiation-dev merged commit 69aa4ad into auto-differentiation:main Sep 18, 2024
12 checks passed
@raneamri raneamri deleted the fix/ci branch September 27, 2024 11:06
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.

4 participants