Skip to content

github CI workflow for module on linux #4

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

Closed
wants to merge 25 commits into from
Closed

github CI workflow for module on linux #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2cc1b9b
github CI workflow for module on linux
icing Jan 15, 2025
0d5f482
adjust workflow job names
icing Jan 15, 2025
510731b
fix workflow var use
icing Jan 15, 2025
0c51d4b
try setting the crypto provider to get rustls to build
icing Jan 16, 2025
76926a1
try rust install and crypro provider like rustls-ffi project does
icing Jan 16, 2025
251e07a
add tqdm to python requirements
icing Jan 16, 2025
77d1f4d
add aws-lc-rs crypt provider into build matrix
icing Jan 16, 2025
02168dd
more matrixification
icing Jan 16, 2025
042971b
fixed moved rustls-version
icing Jan 16, 2025
b74c115
fix rustls-version branch name
icing Jan 16, 2025
9203e82
try building with cmake for rustls-ffi main branch
icing Jan 16, 2025
b8b529c
remove duplicate clone step
icing Jan 16, 2025
fb55259
try to fix cmake build steps
icing Jan 16, 2025
6d97fb2
install cargo-c for cmake build
icing Jan 16, 2025
0326cef
fix cmake build type
icing Jan 16, 2025
ceb4519
try to find out if cmake installs in the right place
icing Jan 16, 2025
6bbdf8c
hmm, cmake is weird
icing Jan 16, 2025
e25ae4d
using verbose and explicit prefix in cmake install
icing Jan 16, 2025
c581b47
try cmake install another way (did i mention cmake is weird?)
icing Jan 16, 2025
39aca9b
the ubuntu-lastest cmake seems old and does not understand new args
icing Jan 16, 2025
9d764d6
well, the -v at the beginning seems confusing...???
icing Jan 16, 2025
b0e0267
seems as if rustls-ffi cmake does not support an install step...
icing Jan 16, 2025
501489f
place rustls-ffi checkout into $HOME
icing Jan 16, 2025
7515ab3
add ld runtime search paths to LDFLAGS
icing Jan 16, 2025
da8ce6c
use the latest release for download of cargo-c
icing Jan 16, 2025
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
129 changes: 129 additions & 0 deletions .github/workflows/linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# Copyright 2025 Stefan Eissing (https://dev-icing.de)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

name: Linux

'on':
push:
branches:
- master
- '*/ci'
paths-ignore:
- '**/*.md'
pull_request:
branches:
- master
paths-ignore:
- '**/*.md'

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
cancel-in-progress: true

permissions: {}

env:
MARGS: "-j5"
CFLAGS: "-g"

jobs:
linux:
name: ${{ matrix.build.name }} (rustls-ffi ${{matrix.rustls-version}} ${{ matrix.crypto }} ${{matrix.rust}})
runs-on: ubuntu-latest
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
rust:
- stable
- nightly
crypto:
- ring
# aws-lc-sys v0.21.1 is not building due to compiler warnings
# - aws-lc-rs
rustls-version:
- v0.14.1
- main
build:
- name: mod_tls
install_packages:

steps:
- name: 'install prereqs'
run: |
sudo apt-get update -y
sudo apt-get install -y --no-install-suggests --no-install-recommends \
libtool autoconf automake pkgconf cmake apache2 apache2-dev openssl \
curl nghttp2-client libssl-dev \
${{ matrix.build.install_packages }}
python3 -m venv $HOME/venv

- uses: actions/checkout@v4

- name: Install ${{ matrix.rust }} toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}

- name: 'checkout rustls-ffi'
run: |
cd $HOME/
git clone --quiet --depth=1 -b ${{ matrix.rustls-version }} --recursive https://github.com/rustls/rustls-ffi.git

- name: 'build rustls-ffi (Makefile)'
if: matrix.rustls-version != 'main'
run: |
cd $HOME/rustls-ffi
make DESTDIR=$HOME/rustls-ffi/build/rust CRYPTO_PROVIDER=${{ matrix.crypto }} install

- name: Install cargo-c
if: matrix.rustls-version == 'main'
env:
# Version picked for MSRV compat.
LINK: https://github.com/lu-zero/cargo-c/releases/latest/download/
CARGO_C_FILE: cargo-c-x86_64-unknown-linux-musl.tar.gz
run: |
curl -L $LINK/$CARGO_C_FILE | tar xz -C ~/.cargo/bin

- name: 'build rustls-ffi (cmake)'
if: matrix.rustls-version == 'main'
run: |
cd $HOME/rustls-ffi
cmake \
-DCRYPTO_PROVIDER=${{matrix.crypto}} \
-DDYN_LINK=on \
-DCMAKE_BUILD_TYPE=Release \
-S librustls -B build
cmake --build build --config "Release"
Comment on lines +100 to +109
Copy link
Contributor

@cpu cpu Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Using cmake here is making your life harder for no reason. It's for building the client.c and server.c examples and only coincidentally builds the library as a prereq.

I would recommend using cargo-c directly like the README describes:

cargo capi install --release --prefix <wherever> --features ${{matrix.features}}.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?

Btw. it took me some time to find out that the generate cmake accepts a -DCMAKE_INSTALL_PREFIX=path, performs a --install without complain, but does not install anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?

I mentioned in a DM, but we plan to publish binary artifacts for common platforms for the next release and that will remove the need for cargo c. You can download a tarball with the .so/.a/.pc all ready to go.

We had a Makefile before but it:

  • Was brittle and hard to maintain - especially for things like mutually exclusive features
  • Didn't produce a .pc file for pkg-config
  • Didn't produce a .so for dynamic linking
  • Didn't support Windows

Cargo-c fixes all of the above and it's being used by a number of similar Rust projects producing native libraries. To me it seems like the best solution short of cargo itself building in support for producing these types of build artifacts (which doesn't seem likely to happen anytime soon unfortunately).

Btw. it took me some time to find out that the generate cmake accepts a -DCMAKE_INSTALL_PREFIX=path, performs a --install without complain, but does not install anything.

Did you find the rustls-ffi README made it seem like this should work? I thought it was fairly clear about the role of cmake but I'm open to suggestions to make it clearer!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Did you find the rustls-ffi README made it seem like this should work? I thought it was fairly clear about the role of cmake but I'm open to suggestions to make it clearer!

Cannot say. Hacker as I am, I just expected a project with a cmake to support the standard features. I could have live with an error. Don't know enough about cmake (and prefer not to), to suggest how to address that. Nothing major.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?

Thinking on this more it does seem reasonable to leave behind a Makefile that does the right cargo-c invocations under the hood.

My objections were about implementing the build/install logic in the Makefile itself and aren't relevant to using the Makefile as a compat shim for invoking cargo-c in a way more familiar to Makefile users.

Thanks for the suggestion, I will handle this in rustls/rustls-ffi#525

Copy link
Contributor

@cpu cpu Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Don't know enough about cmake (and prefer not to), to suggest how to address that.

😆 Tell me about it. I learned cmake recently just for rustls-ffi and it's a mess.

I'll put some thought into how we can emit an error (or just support installing that way if it's not too tricky). (Edit: rustls/rustls-ffi#526)


- name: 'install test prereqs'
run: |
[ -x "$HOME/venv/bin/activate" ] && source $HOME/venv/bin/activate
python3 -m pip install -r test/requirements.txt

- name: 'configure'
run: |
autoreconf -fi
./configure --enable-werror --with-rustls=$HOME/rustls-ffi/build/rust

- name: 'build'
run: make V=1

- name: pytest
env:
PYTEST_ADDOPTS: "--color=yes"
run: |
[ -x "$HOME/venv/bin/activate" ] && source $HOME/venv/bin/activate
pytest -v
48 changes: 34 additions & 14 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,24 +181,43 @@ CPPFLAGS="-I$($APXS -q includedir) -I$($APXS -q APR_INCLUDEDIR) $($APXS -q EXTRA
HTTPD_VERSION="$($APXS -q HTTPD_VERSION)"
AC_SUBST(HTTPD_VERSION)

APACHECTL="$sbindir/apachectl"
if test ! -x "$APACHECTL"; then
# rogue distros rename things! =)
APACHECTL="$sbindir/apache2ctl"
HTTPD="$sbindir/httpd"
if test -x "$HTTPD"; then
: # all fine
else
HTTPD="$sbindir/apache2"
if test -x "$HTTPD"; then
: # all fine
else
HTTPD=""
AC_PATH_PROG([HTTPD], [httpd])
if test -x "$HTTPD"; then
: # ok
else
HTTPD=""
AC_PATH_PROG([HTTPD], [apache2])
if test -x "$HTTPD"; then
: # ok
else
AC_MSG_ERROR([httpd/apache2 not in PATH])
fi
fi
fi
fi
AC_SUBST(APACHECTL)

if test -x "$APACHECTL"; then
DSO_MODULES="$($APACHECTL -t -D DUMP_MODULES | fgrep '(shared)'| sed 's/_module.*//g'|tr -d \\n)"
if test -x "$HTTPD"; then
DSO_MODULES="$($HTTPD -t -D DUMP_MODULES | fgrep '(shared)'| sed 's/_module.*//g'|tr -d \\n)"
AC_SUBST(DSO_MODULES)
STATIC_MODULES="$($APACHECTL -t -D DUMP_MODULES | fgrep '(static)'| sed 's/_module.*//g'|tr -d \\n)"
STATIC_MODULES="$($HTTPD -t -D DUMP_MODULES | fgrep '(static)'| sed 's/_module.*//g'|tr -d \\n)"
AC_SUBST(STATIC_MODULES)
MPM_MODULES="mpm_event mpm_worker"
AC_SUBST(MPM_MODULES)
else
AC_MSG_WARN("apachectl not found in '$BINDIR', test suite will not work!")
APACHECTL=""
AC_MSG_WARN("httpd/apache2 not found, test suite will not work!")
HTTPD=""
fi
AC_SUBST(HTTPD)

AC_SUBST(LOAD_LOG_CONFIG)
AC_SUBST(LOAD_LOGIO)
AC_SUBST(LOAD_UNIXD)
Expand All @@ -208,18 +227,18 @@ AC_SUBST(LOAD_WATCHDOG)
export BUILD_SUBDIRS="src"

if test x"$request_rustls" != "xcheck"; then
LDFLAGS="$LDFLAGS -L$request_rustls/lib";
LDFLAGS="$LDFLAGS -L$request_rustls/lib -Wl,-rpath,$request_rustls/lib";
CFLAGS="$CFLAGS -I$request_rustls/include";
CPPFLAGS="$CPPFLAGS -I$request_rustls/include";
fi

# Need some additional things for rustls linkage. This seems platform specific.
if test $(uname) = "Darwin"; then
CRUSTLS_LDFLAGS="-Wl,-dead_strip -framework Security -framework Foundation"
RUSTLS_LDFLAGS="-Wl,-dead_strip -framework Security -framework Foundation"
else
CRUSTLS_LDFLAGS="-Wl,--gc-sections -lpthread -ldl"
RUSTLS_LDFLAGS="-Wl,--gc-sections -lpthread -ldl"
fi
LDFLAGS="$LDFLAGS $CRUSTLS_LDFLAGS"
LDFLAGS="$LDFLAGS $RUSTLS_LDFLAGS"

# verify that we can link rustls now
# commented: problem running on debian
Expand Down Expand Up @@ -296,6 +315,7 @@ AC_MSG_NOTICE([summary of build options:
Install prefix: ${prefix}
APXS: ${APXS}
HTTPD-VERSION: ${HTTPD_VERSION}
HTTPD: ${HTTPD}
C compiler: ${CC} ${COMPILER_VERSION}
CFLAGS: ${CFLAGS}
WARNCFLAGS: ${WERROR_CFLAGS}
Expand Down
3 changes: 1 addition & 2 deletions test/modules/tls/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
from .env import TlsTestEnv


def pytest_report_header(config, startdir):
def pytest_report_header(config):
_x = config
_x = startdir
env = TlsTestEnv()
return "mod_tls [apache: {aversion}({prefix})]".format(
prefix=env.prefix,
Expand Down
10 changes: 1 addition & 9 deletions test/modules/tls/env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,8 @@ def __init__(self, env: 'HttpdTestEnv'):
super().__init__(env=env)
self.add_source_dir(os.path.dirname(inspect.getfile(TlsTestSetup)))
self.add_modules(["http2", "cgid", "watchdog", "proxy_http2", "ssl"])
self.add_local_module("tls", "src/.libs/mod_tls.so")

def make(self):
super().make()
self._add_mod_tls()

def _add_mod_tls(self):
modules_conf = os.path.join(self.env.server_dir, 'conf/modules.conf')
with open(modules_conf, 'a') as fd:
# load our test module which is not installed
fd.write(f"LoadModule tls_module \"{self.env.src_dir}/.libs/mod_tls.so\"\n")

class TlsCipher:

Expand Down
Loading
Loading