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.

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

Package dnst as DEB & RPM packages, and Docker images. #22

Draft
wants to merge 70 commits into
base: main
Choose a base branch
from

Conversation

ximon18
Copy link
Member

@ximon18 ximon18 commented Nov 13, 2024

This PR creates DEB and RPM packages for installing dnst plus ldns-xxx symbolic links. It also creates a dnst Docker image (it does not attempt to support ldns-xxx commands).

Ideally the DEB and RPM packages would either hard conflict with an existing ldns(-)utils installed package (the naming differs slightly between DEB and RPM at least in Ubuntu and Fedora repositories) or would obsolete/replace an existing ldns(-)utils installed package. For DEBs this seems to be doable, but for RPMs the support for handling upgrades is less "smart" and requires %posttrans support which Ploutos lacks (as the version of cargo-generate-rpm that it uses lacks the support, though newer (compatible with Ploutos?) cargo-generate-rpm does support it).

Relevant background info:

Example output: https://github.com/NLnetLabs/dnst/actions/runs/11833713033

Note that man pages are not included properly yet because I am interested in using Ploutos to help with that.

…g is not specified which will be fixed by PR #428.
… more human readable error message on test failure.
@ximon18 ximon18 changed the title Initial packaging attempt. Package dnst for DEB, RPM and Docker audiences. Nov 14, 2024
@ximon18 ximon18 changed the title Package dnst for DEB, RPM and Docker audiences. Package dnst as DEB & RPM packages, and as a Docker image. Nov 14, 2024
@ximon18 ximon18 changed the title Package dnst as DEB & RPM packages, and as a Docker image. Package dnst as DEB & RPM packages, and Docker images. Nov 14, 2024
@pemensik
Copy link

Comment about licenses. Fedora has switched to SPDX licenses only. license tag should contain SPDX expression even for RPM. afaik. We have rust2rpm tool for autogenerating rpm spec file for rust projects. Those are a way to get official package into Fedora, at least. But requires each dependency properly packaged in distribution in compatible versions. Which blocks for example review of dnsi tool.

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

Hi @pemensik,

For our Rust projects we provide packages using our packages.nlnetlabs.nl repository built using our Ploutos tool.

We do not at present plan to provide packages in the form required by particular distributions.

If others wish to take up packaging of our tools for inclusion in official distribution repositories we of course welcome such efforts.

Ximon

@pemensik
Copy link

There is no need for you to prepare any packages. It is just important that package contain all required files for packagers, like me, to prepare a package. And eventually maintain it in official distribution repository.

That is not in conflict with your prepared packages.

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

If we can adjust the fields to be compatible with external needs while not breaking our existing processes we will be happy to do so.

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

By the way, https://github.com/NLnetLabs/routinator Cargo.toml has the same license field as this project (it was copied from there) so presumably that project is also incompatible with rust2rpm? I ask because https://rhel.pkgs.org/9/epel-x86_64/routinator-0.14.0-3.el9.x86_64.rpm.html exists.

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

But requires each dependency properly packaged in distribution in compatible versions. Which blocks for example review of dnsi tool.

If I understand correctly you are saying that each Rust dependency needs its own package in Fedora. I have no opinion about that approach to packaging and appreciate that distribution maintainers have their reasons for choosing this approach.

What it is not clear to me (and my apologies, I have not read up on the Fedora Rust packaging requirements) is whether something is expected from us to get those dependencies packaged.

Also, did you mean to say dns_i which is a different project than dns_t? (the names are not very different so it could be a typo)

@pemensik
Copy link

If you are interested, here are Rust guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/
No, I am aware dnsi differs from dnst.

dnst has much lighter dependencies, in fact since we have rust-domain in Fedora already, it might become normal package if review is done.

You can check dnsi dependencies at my copr: https://copr.fedorainfracloud.org/coprs/pemensik/rust/

Yes, preparing truly shared packages is not a small task, especially when some package conflicts with other dependencies sometime.

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

So, looking further down Cargo.toml we do set the license to a Fedora compatible BSD value for use by cargo-generate-rpm.

Looking at https://doc.rust-lang.org/cargo/reference/manifest.html#the-license-and-license-file-fields it requires an SPDX license name, and given that we successfully publish Routinator to crates.io with the same license field value that dnst has in this PR, I'm confused as to what the problem is that you are asking us to fix?

@ximon18
Copy link
Member Author

ximon18 commented Nov 15, 2024

Another thought on this: if the current license field value is valid per Cargo manifest format and crates.io rules, changing it to match one external tool such as rust2rpm doesn't seem right as it might then become incompatible with some theoretical rust2deb tool that has different requirements.

Perhaps rust2rpm should look first for a package.metadata.rust2rpm.license field instead?

# complain with "invalid-license".
# See: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing
license = "BSD"

Choose a reason for hiding this comment

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

Here, line 53 of Cargo.toml, contains BSD. That is not SPDX license. recent rpmlint should not complain about BSD-3-Clause anymore. It should complain about BSD now. Just remove extra license for rpm?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can look at changing that, but why would a field in an external tool specific table cause a problem for rust2rpm, it should ignore this field?

Also, we build packages for multiple versions of different distros and the rpmlint version used is the one present in that distro version, it's not a fixed or latest rpmlint version. So changing this to a value compatible with recent Fedora might make it incompatible with older CentOS or such. We can work around that using per variant configs I think if needed. We probably haven't hit this issue that you describe because we haven't yet had time to update Ploutos to test against newer O/S versions.

Choose a reason for hiding this comment

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

# "BSD" alone is the 3-clause license. Inheriting "license" from above causes rpmlint to
# complain with "invalid-license".
# See: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing

Choose a reason for hiding this comment

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

That has changed and is now invalid. SPDX should be there as well. Use recent enough rpmlint. https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_field

@ximon18 ximon18 changed the base branch from use-domain-nsec3-support to main November 22, 2024 22:53
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.

2 participants