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.

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

Jump to bottom

feat(build-deb): Upload source package and run lintian #22

Closed
wants to merge 3 commits into from
Closed

feat(build-deb): Upload source package and run lintian #22

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5b8666
Add source package to artefact
EduardGomezEscandell Feb 6, 2024
918af21
Add lintian check to source package
EduardGomezEscandell Feb 6, 2024
9eb66d6
Make debian version compliant
EduardGomezEscandell Feb 6, 2024
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
32 changes: 28 additions & 4 deletions gh-actions/common/build-debian/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ inputs:
token:
required: false
description: If provided, used for git authentication in the source build
lintian:
required: false
description: Arguments to pass to lintian, if any. Set to false to skip the lintian chek.
default: ''
Copy link
Contributor

@3v1n0 3v1n0 Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This should be true IMHO (to make it clearer at the reader as I see that by default it runs).

Edit: I saw why it's unset, but as explained later I think we're maybe giving too much control to action users :)



# The process:
Expand Down Expand Up @@ -51,10 +55,20 @@ runs:
DEBIAN_FRONTEND=noninteractive sudo apt install -y devscripts
echo "::endgroup::"

echo "::group::Append commit SHA to local version"
echo "::group::Create local version with commit and docker container"
cd '${{ inputs.source-dir }}'
sanitized_docker=$( echo "${{ inputs.docker-image }}" | sed 's/://' )
debchange --local "+${sanitized_docker}+${{ github.sha }}" "Github build. Job id: ${{ github.run_id }}. Attempt: ${{ github.run_number }}."

# Sanitize the docker name so that it stick to debian policy
# https://www.debian.org/doc/debian-policy/ch-controlfields.html#version
sanitized_docker=$( echo "${{ inputs.docker-image }}" | sed -r 's/[^a-zA-Z0-9.+~]+/+/g' )
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally I'd use lsb_release -r -s, but it has to run inside the target docker so that we have an incremental number (while docker names can be static or even cyclic).


# Short commit to avoid "package-has-long-file-name"
commit=$(echo ${{ github.sha }} | cut -c1-8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use git ourselves here?

So we can ahve something fancier by using echo git+$(git rev-list --count HEAD).$(git rev-parse --short HEAD) (or similar), and we have also an incremental number together with the SHA.


export DEBFULLNAME="GitHub actions runner"
export DEBEMAIL="[email protected]"
Comment on lines +68 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe latest commit author?

Suggested change
export DEBFULLNAME="GitHub actions runner"
export DEBEMAIL="[email protected]"
export DEBFULLNAME="$(git log -1 --format='%an' HEAD) - GH Action"
export DEBEMAIL="$(git log -1 --format='%ae' HEAD)"

Plus an indication that is GH though, to make clear both things.

debchange --local "~${sanitized_docker}+${commit}" "Github build. Run id: ${{ github.run_id }}. Run number: ${{ github.run_number }}."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
debchange --local "~${sanitized_docker}+${commit}" "Github build. Run id: ${{ github.run_id }}. Run number: ${{ github.run_number }}."
debchange --local "~${sanitized_docker}+${commit}" \
"Github build. Run id: ${{ github.run_id }}. Run number: ${{ github.run_number }}."


echo "::endgroup::"

echo "::group::Parsing name and version"
Expand All @@ -79,6 +93,11 @@ runs:
if [ -n "${GITHUB_TOKEN}" ]; then
git config --system url."https://api:${GITHUB_TOKEN}@github.com/".insteadOf "https://github.com/"
fi
- name: Run lintian
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this should run in the same distro in which we build the package... So I'm wondering if we should fork the jtdor/build-deb-action@v1 action to support post-build-hooks or just a run-lintian option.

if: inputs.lintian != 'false'
shell: bash
working-directory: ${{ env.SOURCE_OUTPUT_DIR }}
run: lintian ${{ inputs.lintian }} -- *.changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: lintian ${{ inputs.lintian }} -- *.changes
run: lintian --pedantic --fail-on error ${{ inputs.lintian }} -- *.changes

So we still see what's not exactly clean. Plus, IMHO, it would be nice to have an option to control --fail-on, since ideally good packages should set it to warning

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I sorry ignore me on the --fail-on thingy... It's indeed possible to override the value.

Although, maybe this option is even too powerful, since IMHO any suppression should be in sources.

- name: Set up package build
shell: bash
run: |
Expand Down Expand Up @@ -106,7 +125,12 @@ runs:
artifacts-dir: ${{ env.BUILD_OUTPUT_DIR }}
source-dir: ${{ env.BUILD_INPUT_DIR }}
docker-image: ${{ inputs.docker-image }}
- name: Upload artifacts
- name: Copy source package to output dir
shell: bash
run: |
set -eu
cp -r ${{ env.SOURCE_OUTPUT_DIR }}/* ${{ env.BUILD_OUTPUT_DIR }}
Comment on lines +128 to +132
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, we should do this phase just after building the source, and ensure that this happens also if binary build fails: This can help a lot to debug situations failure situations, because one can download the sources locally and build them with sbuild (or similar tools) to fix the inconsistencies.

Also in commit message, we mostly spell it as "art_i_fact" even if both words seems to be correct.

- name: Upload artefacts
Copy link
Contributor

Choose a reason for hiding this comment

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

-> artifacts

uses: actions/upload-artifact@v4
with:
name: ${{ env.PKG_NAME }}_${{ env.PKG_VERSION }}
Expand Down