update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. #427

zenmonkeykstop · 2023-03-14T14:58:25Z

Fixes #426

Removes option to build using PKG_VERSION. Build options are now PKG_PATH and PKG_GITREF:

when invoked without either PKG_PATH or GITREF set, will error out (previously built last known release)
when invoked with PKG_PATH, will use the source tree or tarball at $PKG_PATH to build package (no checkout, no tag verification)
when invoked with PKG_GITREF, will check out corresponding gitref attempt release key verification verification if PKG_GITREF looks like a release tag (catching sneaky branches!) and, if all is well, build package

In all cases the package version will be derived from the latest version in the source tree's debian/changelog If a release tag PKG_GITREF is specified and doesn't match the changelog version, the build will not proceed.

Testing:

test building an old prod tag X.Y.Z with PKG_GITREF=X.Y.Z scripts/build-debianpackage
test building a non-prod tag or branch with PKG_GITREF=<ref here> scripts/build-debianpackage
test building an existing source tree with PKG_PATH=../securedrop-foo scripts/build-debianpackage

zenmonkeykstop · 2023-03-14T15:02:12Z

scripts/build-debianpackage

+# Grab package version from changelog
+CHANGELOG_VERSION=`dpkg-parsechangelog --show-field Version`
+
+# If PKG_VERSION was set, check that it matches the version in debian/changelog


This is why I had this PR in draft - this check breaks CI and will probably break nightlies, as the versions won't match. could keep it and apply only for tags matching prod format regex...

legoktm · 2023-03-14T21:06:30Z

Nothing wrong with your code from a quick skim, but I'm not really sure we need to keep PKG_VERSION around? It currently does only two things 1) verify prod signed tag 2) print out what version we're using.

No. 2 is duplicative of dpkg-buildpackage's own message ("dpkg-buildpackage: info: source package securedrop-client
" & "dpkg-buildpackage: info: source version 0.9.0-rc2+bullseye").

So that just leaves No. 1, verifying the prod signed tag. And I feel like there's a better way to do that... for example, we could run git tag --points-at HEAD and see whether it prints a \d+?\.\d+?\.\d+? matching string and if so, verify against the prod key.

zenmonkeykstop · 2023-03-15T15:23:37Z

I'd go so far as to say (2) is actually misleading (hence the PR) - the package version is determined by the changelog, not by the tagged version that gets checked out. IMO we still have 3 basic cases to cover:

like, build whatever source tree we get pointed at (wanna build a branch? nightlies? wanna build an rc? this is for you)
check out an arbitrary ref and build it (wanna build as above without the hassle of prepping a source tree? et voila)
check out a ref we expect to have a prod-signed tag and if so, build it (ok this is a serious build, let's be duly diligent)

But maybe what this actually points to is that we should be decomposing this script, doing the source tree prep and optional verification elsewhere if required, always just providing the prepped source tree, and focusing on the debian build bits.

legoktm · 2023-03-15T20:32:45Z

I'd go so far as to say (2) is actually misleading (hence the PR) - the package version is determined by the changelog, not by the tagged version that gets checked out.

Agreed.

IMO we still have 3 basic cases to cover:

like, build whatever source tree we get pointed at (wanna build a branch? nightlies? wanna build an rc? this is for you)

check out an arbitrary ref and build it (wanna build as above without the hassle of prepping a source tree? et voila)

check out a ref we expect to have a prod-signed tag and if so, build it (ok this is a serious build, let's be duly diligent)

The third bullet is really just a special case of the second, no? In the MediaWiki release script that's basically how I treated it - accept any git ref, but if that ref is a tag, perform extra checks: https://gitlab.wikimedia.org/repos/releng/release/-/blob/f9857bd33d694aa6c492372f4111f599a3df6c87/make-release/makerelease2.py#L82

But maybe what this actually points to is that we should be decomposing this script, doing the source tree prep and optional verification elsewhere if required, always just providing the prepped source tree, and focusing on the debian build bits.

"Yes, but..." I think that would be the ideal state to end up in, but I think we right now don't have a lot of confidence/trust in our builds yet because it is convoluted that I think having just one thing to run for now is better and once the whole process is simpler we can start breaking it down a bit.

…om an existing source tree

zenmonkeykstop · 2023-03-31T16:07:54Z

OK this is probably in better shape if @legoktm or anyone else has 👀 time. PKG_VERSION dropped altogether, some hip-height guardrails added around production builds. (Note that I'm choosing to verify if the ref provided just looks like a tag, this is intentional to catch branches named using the release tag format.)

legoktm

Minor thing, otherwise looks good. I do think we have officially reached the limits of what we can safely + easily do in bash scripting and any more complexity likely needs a Python port first.

legoktm · 2023-03-31T19:36:03Z

scripts/build-debianpackage

-        setup_source_tree
-        PKG_PATH="/tmp/${PKG_NAME}/"
+        if [[ -n "${PKG_GITREF:-}" ]]; then
+            echo "PKG_PATH not set, using git reference $PKG_GITREF)..."


Unnecessary ) ?

zenmonkeykstop · 2023-03-31T20:15:01Z

Minor thing, otherwise looks good. I do think we have officially reached the limits of what we can safely + easily do in bash scripting and any more complexity likely needs a Python port first.

I'd agree with this assessment, was hard to resist the temptation.

- Documented workflow for releases from a dedicated release branch - Clarified process for applying bugfixes - Clarified steps for managing application and debian changelogs and versions - Updated commands to reflect changes in freedomofpress/securedrop-builder#427

legoktm

LGTM, test plan checks out. Docs PR is in progress, don't think we need to block on that.

zenmonkeykstop commented Mar 14, 2023

View reviewed changes

zenmonkeykstop marked this pull request as ready for review March 14, 2023 16:30

zenmonkeykstop force-pushed the 426-fix-deb-script branch from f65718f to d79137a Compare March 14, 2023 16:31

update build script to use changelog-derived version when building fr…

399c11d

…om an existing source tree

zenmonkeykstop force-pushed the 426-fix-deb-script branch 5 times, most recently from 18ccd06 to 490f618 Compare March 31, 2023 15:58

zenmonkeykstop changed the title ~~update build script to use changelog-derived version when building from an existing source tree~~ update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. Mar 31, 2023

zenmonkeykstop added the needs: docs This change has a documentation (user or developer impact) label Mar 31, 2023

zenmonkeykstop requested a review from legoktm March 31, 2023 16:26

legoktm requested changes Mar 31, 2023

View reviewed changes

legoktm mentioned this pull request Mar 31, 2023

Add SecureDrop-specific metadata to buildinfo files #434

Open

4 tasks

zenmonkeykstop force-pushed the 426-fix-deb-script branch from 490f618 to 0256947 Compare March 31, 2023 20:19

zenmonkeykstop mentioned this pull request Mar 31, 2023

Updated workstation release docs for Debian packages: freedomofpress/securedrop-dev-docs#46

Draft

3 tasks

zenmonkeykstop added 2 commits March 31, 2023 18:34

update debian build script to remove PKG_VERSION

a2e3fc7

update README to reference PKG_GITREF in build script instructions

271414b

zenmonkeykstop force-pushed the 426-fix-deb-script branch from 0256947 to 271414b Compare March 31, 2023 22:34

legoktm approved these changes Apr 4, 2023

View reviewed changes

legoktm merged commit f23b36a into main Apr 4, 2023

legoktm deleted the 426-fix-deb-script branch April 4, 2023 17:57

legoktm mentioned this pull request Apr 6, 2023

Port scripts/build-debianpackage to Python #436

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. #427

update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. #427

zenmonkeykstop commented Mar 14, 2023 •

edited by legoktm

Loading

zenmonkeykstop Mar 14, 2023

legoktm commented Mar 14, 2023 •

edited

Loading

zenmonkeykstop commented Mar 15, 2023

legoktm commented Mar 15, 2023

zenmonkeykstop commented Mar 31, 2023 •

edited

Loading

legoktm left a comment

legoktm Mar 31, 2023

zenmonkeykstop Mar 31, 2023

zenmonkeykstop commented Mar 31, 2023

legoktm left a comment

update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. #427

update build script to remove PKG_VERSION, use only PKG_PATH or PKG_GITREF modes. #427

Conversation

zenmonkeykstop commented Mar 14, 2023 • edited by legoktm Loading

Testing:

zenmonkeykstop Mar 14, 2023

Choose a reason for hiding this comment

legoktm commented Mar 14, 2023 • edited Loading

zenmonkeykstop commented Mar 15, 2023

legoktm commented Mar 15, 2023

zenmonkeykstop commented Mar 31, 2023 • edited Loading

legoktm left a comment

Choose a reason for hiding this comment

legoktm Mar 31, 2023

Choose a reason for hiding this comment

zenmonkeykstop Mar 31, 2023

Choose a reason for hiding this comment

zenmonkeykstop commented Mar 31, 2023

legoktm left a comment

Choose a reason for hiding this comment

zenmonkeykstop commented Mar 14, 2023 •

edited by legoktm

Loading

legoktm commented Mar 14, 2023 •

edited

Loading

zenmonkeykstop commented Mar 31, 2023 •

edited

Loading