*** Bug 2080744 has been marked as a duplicate of this bug. ***

Package looks good to me. There's one thing you need to consider:
The "ring" dependency (a BoringSSL fork) is not available on ppc64le and s390x.

If you need the features of this crate that depend on "ring" you will need to "ExcludeArch: ppc64le and s390x" in this package, as well, to avoid broken dependencies. Alternatively, if you don't need the features that require "ring", you could remove them from the package, and build it on all architectures supported by Rust.

Spec URL: https://pemensik.fedorapeople.org/rust-domain.spec
SRPM URL: https://pemensik.fedorapeople.org/rust-domain-0.6.1-1.fc35.src.rpm

Updated spec to provide ring dependent features on arches excluding power64 and s390x, according to comment #2. That way it would still build on powerpc, but without validation ability.

This is well-intentioned, but will not work, and will produce packages with broken dependencies.

Our tooling (mostly because of limitations in RPM and pungi) will still include noarch packages in repositories for all architectures, even if they're explicitly excluded on some of them (because ExcludeArch / ExclusiveArch headers are only included in SRPM files, apparently).

This is why I explicitly said to either remove those features for all architectures, or to drop ppc64le and s390x support for the whole package.

Adding links to related issues in ring package. A bit sad it takes long to support those architecures, even when there has been clearly some interest.

Updated spec to more recent release:

Spec URL: https://raw.githubusercontent.com/pemensik/domain/fedora/packaging/fedora/rust-domain.spec
SRPM URL: https://pemensik.fedorapeople.org/srpm/rust-domain-0.7.0-1.fc36.src.rpm

Created scratch build to test arches building: https://koji.fedoraproject.org/koji/taskinfo?taskID=92273439

Disabled building altogether on ppc64 and s390x. I guess fixing the original bug rust-ring bug #1869980 should have higher priority than deploying workarounds. I am not the correct person to do that, they are far more advanced than my current knowledge of rust.

The most recent release is 0.7.0.

Sorry for the delay in getting back to you, I've been exceptionslly busy lately :(
And yes, sadly, development of "ring" has pretty much stopped entirely ~ 6 months ago, with the developer being AWOL.

Do you know that you'll actually need support for the "interop", "tsig", or "validate" features of "domain"?
You haven't indicated what you're packaging this crate for, so I can't check whether these projects use any of these features of "domain".

If the answer is "no, these features are not used by what I'm working on", then I'd suggest to remove the features that depend on "ring" instead of excluding ppc64le and s390x.

(You might also want to consider checking which version of "domain" is required by whatever you're working on - domain v0.7 might not even be compatible with it.)
PS: A few days ago, v0.7.1 was released, it would be great if you could update packaging for that version, if it's compatible with what you're working on.

I wanted to package it first, then later maybe to create my own project depending on it. I intended to use some DNSSEC functionality in it. I know no other project using that library already, I would package it as well. But if the development is on hold, I guess raising issue to domain crate upstream might make sense with a question how to solve that. Not sure they are aware the complication using only ring without alternatives makes.

I think https://github.com/NLnetLabs/domain-tools is not yet worthy separate package just for the sake of it.

It seems a bit weird I have to either disable partial functionality everywhere or disable whole architecture altogether, but cannot use something in between. Cannot just some form of conflict prevent installing the development headers on problematic architectures, but include them in source archive? So it would allow marking just affected subpackages to be non-installable on missing architectures?

But because I have not yet code using DNSSEC validation, I think cut off package is possible until we have a better option. I would prefer having fully working Intel and ARM builds and no package for ppc64le and s390x. I do not have any of those rare architectures and I guess I am not the only one.

Anyway, would update to 0.7.2 and recheck, what could be done with it.

Would it be possible if those ring dependendent subpackage devel files had just: Conflicts: rust(s390x) rust(ppc64le) ? Negated Requires might be a bit tricky, but still possible. That should allow using them on supported architectures, but would not allow anyone with those architectures even to install those packages, let alone to try to build with them. Basic parts should be possible even on ring unsupported arches.

I am just rust newbie and wanted to package not yet packaged library. Wanted to try packaging a rust project before I make my own.

Having Conflicts would maybe help in one way (make the packages not installable on those architectures), but it would not help in another way (the packages would still have broken dependencies on those architectures).

This is also not a limitation of Rust packaging, this is a limitation of RPM itself and Fedora build infrastructure:
https://pagure.io/koji/issue/1843
https://pagure.io/pungi/issue/1518

===

If you don't have a concrete use case for this crate yet, I don't think packaging something that will end up unused makes much sense.
Instead, I'd recommend to use one of the "trust-dns" libraries, instead:
https://crates.io/crates/trust-dns-resolver

They have support for DNSSEC, "DNS over SSL", and "DNS over HTTPS" (the latter is not enabled yet in Fedora), support OpenSSL as a crypto backend, and are already packaged for Fedora, because they are the "library of choice" for this functionality in most HTTP(S) libraries and web frameworks.

On the other hand, the DNSSec support in the "domain" crate is labeled as "incomplete" and "experimental", which doesn't inspire confidence ...

Spec URL: https://raw.githubusercontent.com/pemensik/domain/fedora/packaging/fedora/rust-domain.spec
SRPM URL: https://pemensik.fedorapeople.org/srpm/rust-domain-0.7.2-1.fc39.src.rpm

It seems to be quite strange that the package "compiles" just fine even on the ppc64le machine. It seems to me rust-ring-devel should be architecture specific and should not provide dependencies on architectures where it is known to not build. This way it cascades some workarounds on dependent libraries and has to be solved only by leaf applications, which actually build compilable code.

It is not just fault of domain project that rust-ring-devel does not work on some architectures. It should not require workarounds on other packages, which just consume it. Let alone farther in chain. Then the crate dependencies should ensure only people including optional features development headers would get issues.

Copr build:
https://copr.fedorainfracloud.org/coprs/build/5735340
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2080743-rust-domain/fedora-rawhide-x86_64/05735340-rust-domain/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

> It seems to be quite strange that the package "compiles" just fine even on the ppc64le machine.

Well, of course it does. The code that depends on ring is not compiled by default (that's how conditional compilation works).
It would only fail of the feature that depends on the "ring" dependency is enabled (either in this package or in any dependent package).

> It seems to me rust-ring-devel should be architecture specific and should not provide dependencies on architectures where it is known to not build.

That would be ideal, yes. But as I noted, this is not possible to do due to limitations of RPM and Fedora build infrastructure.

> This way it cascades some workarounds on dependent libraries and has to be solved only by leaf applications, which actually build compilable code.

This is the reality we have to deal with. Though I think I know how to work around this (similar to how the "rust-cpufeatures" package is built - skip compiling the code and running the test suite on certain architectures, but provide the source code on *all* architectures, even on those where it doesn't work).

> It is not just fault of domain project that rust-ring-devel does not work on some architectures. It should not require workarounds on other packages, which just consume it. Let alone farther in chain. Then the crate dependencies should ensure only people including optional features development headers would get issues.

Yes, yes, "should" this, "would" that ... if my grandma had wheels, she would be a bike ;)

1. You should regenerate the package with the latest rust2rpm (i.e. v24).

This regeneration step is also necessary for every version update (to ensure that RPM subpackages <-> cargo features are mapped correctly).
This will also drop "ExclusiveArch:  %{rust_arches}", which is no longer necessary, and has been removed from the Packaging Guidelines.

2. Where is this forgeurl coming from?
> %global forgeurl0 https://github.com/NLnetLabs/domain
> VCS:            git:%{forgeurl0}

You'd need to manually keep this maintained, as it will be dropped by any subsequent package updates.
Is there a specific reason why you added this?

===

Other than those two points, the package now looks good to me.

Spec URL: https://raw.githubusercontent.com/pemensik/domain/fedora/packaging/fedora/rust-domain.spec
SRPM URL: https://pemensik.fedorapeople.org/srpm/rust-domain-0.7.2-1.fc39.src.rpm

(In reply to Fabio Valentini from comment #15)
> 1. You should regenerate the package with the latest rust2rpm (i.e. v24).
> 
> This regeneration step is also necessary for every version update (to ensure
> that RPM subpackages <-> cargo features are mapped correctly).
> This will also drop "ExclusiveArch:  %{rust_arches}", which is no longer
> necessary, and has been removed from the Packaging Guidelines.
done, reuploaded minimal changes
> 
> 2. Where is this forgeurl coming from?
> > %global forgeurl0 https://github.com/NLnetLabs/domain
> > VCS:            git:%{forgeurl0}
> 
> You'd need to manually keep this maintained, as it will be dropped by any
> subsequent package updates.
> Is there a specific reason why you added this?
I just want to keep in spec direct link to place for reporting issues and primary source.
Those were added by hand. I will leave them there for now.
> 
> ===
> 
> Other than those two points, the package now looks good to me.

I am keeping:
# https://bugzilla.redhat.com/show_bug.cgi?id=1869980
ExcludeArch: %{power64} s390 s390x

hope that is okay. However I have tried borrowing ppc64le machine to test it, it seems te library itself prepares fine. Even test do pass on that platform, because no tests cover validation or tsig features. I know no tool already using those features anyway.

Thanks for the update, will complete the review later.

> hope that is okay. However I have tried borrowing ppc64le machine to test it, it seems te library itself prepares fine. Even test do pass on that platform, because no tests cover validation or tsig features. I know no tool already using those features anyway.

Sure. The affected code is not enabled by default, so it's not compiled and not run. The problem are broken dependencies.
However, I've worked on an automated workaround for library crates with limited architecture support:
https://bodhi.fedoraproject.org/updates/FEDORA-2023-a973e7d3eb

I will update the "ring" package and a few related ones to use this approach, so dependent packages don't necessarily all need to add ExcludeArch if the "ring" / "rustls" depdendency is not part of the default feature set.

Created issue at upstream to request alternative crypto backends: https://github.com/NLnetLabs/domain/issues/195

Created attachment 1955581 [details]
The .spec file difference from Copr build 5735340 to 5739947

Copr build:
https://copr.fedorainfracloud.org/coprs/build/5739947
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2080743-rust-domain/fedora-rawhide-x86_64/05739947-rust-domain/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Thanks for the updates, package looks good to me.

===

Package was generated with rust2rpm, simplifying the review.

- package builds and installs without errors on rawhide
- test suite is run and all unit tests pass
- latest version of the crate is packaged
- license matches upstream specification (BSD-3-Clause) and is acceptable for Fedora
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- add @rust-sig with "commit" access as package co-maintainer

- set bugzilla assignee overrides to @rust-sig (optional)

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- track package in koschei for all built branches


===


My updates for the "rust-ring" package to build it everywhere (including architectures where the code doesn't even compile) are now stable (or pending stable in bodhi), so you can remove the ExcludeArch tag in this package, and it should both compile fine (since the affected features are not compiled by default), and have no broken dependencies on any architecutre.

This package has been approved for three months but wasn't imported yet ... are you still interested?

Ah yes, I got distracted a bit. Can you please give another review+, it won't create a repo after 60 days.

Sure, done.

The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-domain

FEDORA-2023-ce1b6f7a75 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-ce1b6f7a75

FEDORA-2023-c1fd6f96c7 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-c1fd6f96c7

FEDORA-2023-ce1b6f7a75 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-ce1b6f7a75 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-ce1b6f7a75

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-c1fd6f96c7 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-c1fd6f96c7 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-c1fd6f96c7

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-c1fd6f96c7 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-c1fd6f96c7 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-c1fd6f96c7

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-ce1b6f7a75 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-ce1b6f7a75 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-ce1b6f7a75

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-ce1b6f7a75 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-ce1b6f7a75 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-ce1b6f7a75

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2023-c1fd6f96c7 has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2023-ce1b6f7a75 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.