Skip to content

Add archlinux packaging to nfpm backend#21299

Merged
cognifloyd merged 218 commits intomainfrom
cognifloyd/nfpm-archlinux
Aug 14, 2024
Merged

Add archlinux packaging to nfpm backend#21299
cognifloyd merged 218 commits intomainfrom
cognifloyd/nfpm-archlinux

Conversation

@cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Aug 13, 2024

This adds archlinux packaging to the nfpm backend introduced in #19308.

New target (and related fields):

  • nfpm_archlinux_package

Also, add type hints for some of the field.alias properties
because mypy was getting confused.
This is needed for the field_set calculations
A common field that all nfpm packages will share,
making it easier to build a common FieldSet base.
Almost all fields are "required" in that, if present, they are
used in the generation of the package.

Users do not have to supply them, but the fields have to exist
on the Nfpm*Package targets.
@cognifloyd cognifloyd force-pushed the cognifloyd/nfpm-archlinux branch from 6d02dbb to 7e9c57f Compare August 14, 2024 04:42
@cognifloyd cognifloyd requested review from a team and tdyas August 14, 2024 04:46
@cognifloyd cognifloyd marked this pull request as ready for review August 14, 2024 04:46
core_fields = (
*COMMON_NFPM_PACKAGE_FIELDS,
*ARCHLINUX_FIELDS,
NfpmArchlinuxScriptsField,
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that the scripts fields are not part of the PACKAGER_FIELDS constants. For my own education, why are they not included there?

Copy link
Member Author

Choose a reason for hiding this comment

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

The packager field constants are used to populate the field sets required_fields. Then, the field set's nfpm_config method loops over required_fields to add each field's config. But, the logic for scripts fields is not the same as the logic for the other fields. So, excluding the scripts fields from required_fields lets me used different logic for them.

Admittedly, this is an unorthodox use of field sets, but it was the simplest way I came up with for dealing with the vast number of fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth an explanatory comment somewhere? (And one-line comment before the line with the scripts field in these target definitions referring to the explanatory comment.)

@cognifloyd cognifloyd merged commit 219cb02 into main Aug 14, 2024
@cognifloyd cognifloyd deleted the cognifloyd/nfpm-archlinux branch August 14, 2024 05:20
cognifloyd added a commit that referenced this pull request Aug 16, 2024
)

This moves the scripts fields into the `<packager>_FIELDS` constants to
satisfy this review suggestions:
#21299 (comment)
#21310 (comment)

It also adds `param.pytest(..., id="...")` to name tests in a sane way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants