feat(build-debian): Use docker to manually run build and prepare steps and add tests #28

3v1n0 · 2024-02-26T05:46:17Z

As underlined in ubuntu/authd#130 (comment) using jtdor/build-deb-action for building debian packages has some problems that are yet not clear, and it also does not seem to properly support building non-native packages.

As per this, given that the code needed for handling the build inside a simpler docker container is quite small, I've moved the build to manual handling which allows us some more control over the steps at the cost of being a bit more verbose.

As bonus:

We now only build as user (not the sources, since there's no much point)
All the network traffic is totally blocked for the builder user (not just HTTPS)
DEB_BUILD_OPTIONS can be customized for special builds (e.g. nocheck)
Version uses actual distro version

I've added some commits from #22, but not included the lintian build yet (that I've ready here) not to make this PR harder to check, but also because I think those could instead be part of another action to allow more parallelization when used.

By doing this change, not only both test cases pass, but also authd is correctly built: ubuntu/authd#130 (e.g. https://github.com/ubuntu/authd/actions/runs/8044158864)

UDENG-2439

Fix warnings related to: - Version number - File too long - No contributor name - Extra whitespace around name

We don't fully support source builts for non-native packages yet though, so ignore the error for now

Artifacts could be exposed by various other composed jobs so make this clearer for binary packages too

This allows us to have more control on the volumes we mount in docker and on the location of the source path. And permits us to properly mount the parent folder of the source that we need to access to read the .orig files for non-native packages. It comes as a little extra cost, but also it give us way more control on the builder instance.

In general when building sources we should not need anything else a part what is in build-depends, but for some packages we may need some extra packages such as ca-certificates or git. So, expose these values as extra-source-build-deps input. This option should actually be unset by default, but we don't yet not to break existing packages, but we should change it ASAP so that it depends on each repo needs.

It may be useful for debugging purposes, so upload it once it's ready

For some reason using jtdor/build-deb-action@v1 does not work well with some rust builds we have, so re-implement it using some manual labor. It's not really much work and it will allow us more control. Other than, working authd builds.

When building debian packages we should not have high privileges, so let's just use an unprivileged user to perform the binary build. It's not too needed for source builds, since those are not really doing tests or anything where having root privileges may lead to different behavior

Ensure that internet access is completely disabled for builder user when creating the binary package. This is something that we somewhat had implicitly for HTTPS only since ca-certificates were not installed. But let's ensure this in any case.

It allows to perform special builds (such as nocheck ones), so expose it in case, for example, one may want to build without running tests.

In this way we can use the proper distro version number

In case a git repository is setup, then use version number that includes the revision list and use actual author name

It can provide some quick information about what's in the packages

…tputs

didrocks

I have to stop the review for now here, already posting what I found.

This is clearly post FF material…

didrocks · 2024-02-26T08:02:59Z

gh-actions/common/build-debian/action.yml

        echo "::endgroup::"

+    - name: Prepare source package
+      uses: kohlerdominik/[email protected]


Have you monitored how trustworthy, monitored and updated this action is?

So, not really very updated but still more recent than the more popular https://github.com/addnab/docker-run-action that also does not provide many inputs, and so it requires us to use more options which are still fine but more prone to error and less clean IMHO.

So, I had reviewed the code and it's quite simple and fine from my POV.

In case we can also fork it to something even simpler in bash that does the same, but it seems an overkill to me.

didrocks · 2024-02-26T08:06:17Z

gh-actions/common/build-debian/action.yml

+    required: false
+    # FIXME: this should default to '', but we don't want to break job depending on us for now
+    default: 'ca-certificates git'
+  DEB_BUILD_OPTIONS:


I don’t think we should provide those options. This can end up with "I add this build-dep here or this DEB_BUILD_OPTIONS in my ci and forget to really fix the packages). Let’s ensure we have a real need (I don’t think the package should be run with nocheck as you illustrated for those before adding them.

I have removed DEB_BUILD_OPTIONS, but IMHO it can be quite convenient for testing nocheck builds too, which are somewhat supported by the ecosystem (other than useful to do simpler test-builds).

Regarding extra-source-build-deps I think we still need it as explained in the commit, and the FIXME, IMHO is very wrong always installing ca-certificates and git even on source builds.

So, if you want I can make this more strict by not allowing people to customize what install (I need a good variable name though :)), but my point here is that all the projects that really need this should define it, not this low-level action.

I think our goal is really to add options when we have a first consumer requesting for it and revisting the rationale.

btw, my comment was (see my first comment) for both options.

Yes, I noticed. But I'd like to have a different proposal for extra-source-build-deps 😄

As really I don't think we should not default to having it populated, unless explicitly required.

didrocks · 2024-02-26T08:08:55Z

gh-actions/common/build-debian/action.yml

@@ -46,69 +55,194 @@ runs:
      run: |
        set -eu


While you are changing it, please set -eu after the echo so that we have a readable step title.

Yeah, I was thinking about that... I followed the style of the file but indeed had to be addressed.

didrocks · 2024-02-26T08:10:34Z

gh-actions/common/build-debian/action.yml

+      with:
+        image: ${{ inputs.docker-image }}
+        environment: |
+          TERM=dumb


Can you expand why this is needed?

Well dpkg build may warn about TERM not being set, and having this in theory should make tools aware of the environment we are in.

It's not strictly needed but a nice to have one since we're so low level, but let me know if you prefer avoid it.

check if there is a warning. If there is a warning, let’s set it, but here, on GitHub action, I haven’t seen it IIRC, so if no warning, let’s not predict the future :)

eh, I'm sure I saw it while building... IIRC was on go packages, but I need to check this again as I don't see it in the test job.

didrocks · 2024-02-26T08:10:47Z

gh-actions/common/build-debian/action.yml

+        workdir: ${{ github.workspace }}/${{ inputs.source-dir }}
+        shell: bash
+        run: |
+          set -eu


ditto on set -eu

didrocks · 2024-02-26T08:12:20Z

gh-actions/common/build-debian/action.yml

+          apt install devscripts lsb-release
+          echo "::endgroup::"
+
+          dch --local "~$(lsb_release -r -s)+${{ env.VERSION_REF }}" \


This doesn’t ensure we are always bumping the version up as we publish this artefact. as the second part is based on the sha. I suggest we prefix it with the date in us format.

Yeah, ideally to have this automatically we need to use some higher depth when cloning, that isn't always expected.

Also I didn't change that, but I feel we should use + as first instead of ~ because we are building on top of a release normally, no?

True, but what is the changelog is set to the next version, but UNRELEASED?

Yeah, changed both...

…package

So that github groups things better

…package

…source package

This reverts commit 3f55310.

3v1n0 · 2024-02-26T13:27:44Z

This is clearly post FF material…

Yeah, in theory it is, but I liked the idea to have ubuntu/authd#223 in before and so being tested too.

didrocks · 2024-02-26T13:57:13Z

I propose we focus on the FF itself, remove the deb building part, polish the packaging, release and then, we can go over that one.

…package

Use a +git syntax and there we use incremental version values. Finally add the distro version as a minor factor, so that builds on the same code for different distros only rely on that

… package

… packages

…re the source package

3v1n0 requested review from didrocks and tim-hm as code owners February 26, 2024 05:46

3v1n0 mentioned this pull request Feb 26, 2024

CI: Build debian packages and run autopackage tests ubuntu/authd#130

Open

3v1n0 force-pushed the build-deb-docker branch 2 times, most recently from 579f2a4 to 05d1c3c Compare February 26, 2024 06:01

Make debian version compliant

7d7f0bc

Fix warnings related to: - Version number - File too long - No contributor name - Extra whitespace around name

3v1n0 force-pushed the build-deb-docker branch 4 times, most recently from 4dc3e58 to 7a5ba6b Compare February 26, 2024 06:11

3v1n0 added 15 commits February 26, 2024 07:16

workflows: Add workflow to test build-deb action

0839b2c

We don't fully support source builts for non-native packages yet though, so ignore the error for now

build-debian: Clarify what artifacts are for

61ce923

Artifacts could be exposed by various other composed jobs so make this clearer for binary packages too

build-debian: Use stricter selection on dsc file based on env

8f4e33c

build-debian: Upload source package as artifact

1594fae

It may be useful for debugging purposes, so upload it once it's ready

build-debian: Add DEB_BUILD_OPTIONS input

3f55310

It allows to perform special builds (such as nocheck ones), so expose it in case, for example, one may want to build without running tests.

build-debian: Use docker container also to prepare the source package

d0ad2ae

In this way we can use the proper distro version number

build-debian: Use more featured version number if in a git repo

66d3c19

In case a git repository is setup, then use version number that includes the revision list and use actual author name

build-debian: Remove trailing whitespace

255c19b

build-debian: Show binaries contents and info after building

811901e

It can provide some quick information about what's in the packages

build-debian: Add IDs to upload steps so that can be referenced as ou…

188e5a8

…tputs

3v1n0 force-pushed the build-deb-docker branch from 7a5ba6b to 188e5a8 Compare February 26, 2024 06:16

didrocks requested changes Feb 26, 2024

View reviewed changes

3v1n0 mentioned this pull request Feb 26, 2024

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

Closed

3v1n0 force-pushed the build-deb-docker branch from 2d1772f to b574a4e Compare February 26, 2024 12:49

fixup! workflows: Add workflow to test build-deb action

2a1e900

3v1n0 added 11 commits February 26, 2024 13:58

fixup! build-debian: Use docker container also to prepare the source …

be2dfc8

…package

fixup! build-debian: Use docker container also to prepare the source …

29bdeb4

…package

fixup! build-debian: Use docker container also to prepare the source …

c936bfe

…package

fixup! build-debian: Use manual docker build for source package

2310c49

fixup! build-debian: Use manual docker run to build the debian packages

7df822a

build-debian: Ensure we show group message as first thing

c0c0974

So that github groups things better

fixup! build-debian: Use manual docker build for source package

a2dadfa

fixup! build-debian: Use manual docker run to build the debian packages

fec8936

fixup! build-debian: Use docker container also to prepare the source …

b586fcb

…package

fixup! build-debian: Use more featured version number if in a git repo

e1329bd

fixup! fixup! build-debian: Use docker container also to prepare the …

0a2059d

…source package

3v1n0 force-pushed the build-deb-docker branch from b574a4e to 0a2059d Compare February 26, 2024 13:22

Revert "build-debian: Add DEB_BUILD_OPTIONS input"

2495250

This reverts commit 3f55310.

3v1n0 added 4 commits February 26, 2024 15:43

fixup! build-debian: Use docker container also to prepare the source …

4faa610

…package

fixup! build-debian: Use more featured version number if in a git repo

03cdf88

fixup! fixup! build-debian: Use manual docker build for source package

d6d6f31

build-debian: Include distribution in changelog release

196404e

3v1n0 force-pushed the build-deb-docker branch from 10a0fa4 to d35f929 Compare February 26, 2024 15:03

3v1n0 mentioned this pull request Feb 26, 2024

ci(go): Cleanup some job steps #26

Open

3v1n0 force-pushed the build-deb-docker branch from d35f929 to 524fee0 Compare February 26, 2024 15:08

debian-build: Use version number so that it can override last release

6f061b8

Use a +git syntax and there we use incremental version values. Finally add the distro version as a minor factor, so that builds on the same code for different distros only rely on that

3v1n0 force-pushed the build-deb-docker branch from 524fee0 to 6f061b8 Compare February 26, 2024 15:10

3v1n0 mentioned this pull request Feb 26, 2024

Update to use Node.js 20 kohlerdominik/docker-run-action#24

Closed

3v1n0 added 3 commits February 27, 2024 10:11

fixup! fixup! fixup! build-debian: Use manual docker build for source…

e2eda3b

… package

fixup! fixup! build-debian: Use manual docker run to build the debian…

6b9c6e1

… packages

fixup! fixup! fixup! build-debian: Use docker container also to prepa…

ea88959

…re the source package

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(build-debian): Use docker to manually run build and prepare steps and add tests #28

feat(build-debian): Use docker to manually run build and prepare steps and add tests #28

3v1n0 commented Feb 26, 2024 •

edited

Loading

didrocks left a comment

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024 •

edited

Loading

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

didrocks Feb 26, 2024

3v1n0 Feb 26, 2024

3v1n0 commented Feb 26, 2024

didrocks commented Feb 26, 2024

feat(build-debian): Use docker to manually run build and prepare steps and add tests #28

Are you sure you want to change the base?

feat(build-debian): Use docker to manually run build and prepare steps and add tests #28

Conversation

3v1n0 commented Feb 26, 2024 • edited Loading

didrocks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3v1n0 Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3v1n0 commented Feb 26, 2024

didrocks commented Feb 26, 2024

3v1n0 commented Feb 26, 2024 •

edited

Loading

3v1n0 Feb 26, 2024 •

edited

Loading