Skip to content

Conversation

adombeck
Copy link
Contributor

@adombeck adombeck commented Sep 15, 2025

When building authd, we need to add a PPA before the build dependencies are installed.

This is based on #87

@adombeck adombeck changed the title gh-actions/build-debian: Add pre-deps-commands input feat: gh-actions/build-debian: Add pre-deps-commands input Sep 15, 2025
@adombeck adombeck force-pushed the pre-deps-commands branch 3 times, most recently from 3da45a0 to b2b2289 Compare September 15, 2025 18:57
@adombeck adombeck force-pushed the pre-deps-commands branch 7 times, most recently from be9f07a to 6a5b3d8 Compare September 16, 2025 13:35
@adombeck adombeck marked this pull request as ready for review September 16, 2025 13:36
@adombeck adombeck requested a review from didrocks as a code owner September 16, 2025 13:36
@adombeck
Copy link
Contributor Author

@3v1n0
Copy link
Contributor

3v1n0 commented Sep 16, 2025

You may need to use

extra-source-build-script: |

However, in general while installing few things before the source package is built is kinda acceptable, I don't think we should allow to do anything during Build packages phase, or we'd end up polluting the build with stuff that is not part of debian/rules.

Maybe we should just switch here to use sbuild instead (in a container - I had planned it for some long time, TBH), now that it can create build roots automatically.

The only thing we could IMHO, is the support for adding extra-apt-sources in the same way we do it for autopkgtests, so ideally we should just modify /etc/apt/sources.list.d/, either manually in the script or we could do this by: run another image instance, add the repository using add-apt-repository, copy the content of /etc/source.list.d (and the keys if any) to the build image.

But I think we should not do anything that is not in debian/rules in the build phase, that was the main point of this action after all.

@adombeck
Copy link
Contributor Author

You may need to use [...]

Fixed, thanks!

However, in general while installing few things before the source package is built is kinda acceptable, I don't think we should allow to do anything during Build packages phase, or we'd end up polluting the build with stuff that is not part of debian/rules.

Not sure I understand your concern. You're saying it's ok to install a few things before the source package is built. That's exactly what we're doing here. In fact, the pre-deps-commands are executed before the already existing (renamed) pre-source-build-commands.

@adombeck adombeck requested a review from 3v1n0 September 18, 2025 11:14
@adombeck
Copy link
Contributor Author

@didrocks @3v1n0 could I get a review soonish? this is blocking ubuntu/authd#1074 which in turn is blocking the updates of some of our Go dependencies (golang.org/x/[email protected] and golang.org/x/[email protected] both require Go 1.24, while Noble only has Go 1.23).

@didrocks
Copy link
Contributor

I think I understand @3v1n0's concern and I agree that in general, we should not encode additional commands manually in our CI workflow as it might be manual steps to be done on everyone's machine to prepare the source package (while in general, we should have this in debian/rules as we did with autovendoring the dependencies).

This is why the idea to offer adding a ppa is fine, because restrictive, rather than letting shell scripts to be executed. I will let @3v1n0 confirms that this is what he means…

@adombeck adombeck marked this pull request as draft September 25, 2025 13:06
When building authd, we need to add a PPA before the build dependencies
are installed.
@adombeck adombeck changed the title feat: gh-actions/build-debian: Add pre-deps-commands input feat: gh-actions/build-debian: Add extra-apt-repositories input Sep 25, 2025
@adombeck
Copy link
Contributor Author

adombeck commented Sep 25, 2025

This is why the idea to offer adding a ppa is fine, because restrictive, rather than letting shell scripts to be executed.

I renamed the input to extra-apt-repositories, which is now passed to add-apt-repositories, so it doesn't allow arbitrary commands (the existing extra-source-build-script still allows to execute arbitrary commands though). Does that address your concern?

@adombeck
Copy link
Contributor Author

@adombeck adombeck marked this pull request as ready for review September 25, 2025 14:54
Copy link
Contributor

@didrocks didrocks left a comment

Choose a reason for hiding this comment

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

It fully does for me, thanks for doing that change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants