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 2 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
16 changes: 15 additions & 1 deletion 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 @@ -79,6 +83,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 +115,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