Rust NSS: Update packaging

[denisonbarbosa]

Updating packaging for the aad-auth project.

DEENG-570

[GabrielNagy]

Nice work, only had a few comments:

I tested the branch locally and it built fine for me, with a few lintian errors that made me think:

Now running lintian aad-auth_0.3_amd64.changes ...
E: libnss-aad: lacks-versioned-link-to-shared-library usr/lib/x86_64-linux-gnu/libnss_aad.so [usr/lib/x86_64-linux-gnu/libnss_aad.so.2]
W: aad-auth source: dependency-is-not-multi-archified libnss-aad depends on aad-common (multi-arch: no)
W: libnss-aad: shared-library-lacks-version usr/lib/x86_64-linux-gnu/libnss_aad.so.2 libnss_aad.so
W: aad-auth source: unknown-field Vendored-Sources-Rust

I think we can get rid of some of them by compiling the NSS library with the proper soname (libnss_aad.so.2 instead of libnss_aad.so), like we did for Go.

For the dependency-is-not-multi-archified one we may need to set Multi-Arch: same on aad-common as well. Can you remind me why we need the option on debian/control in the first place?

[GabrielNagy]

Just noting here that the lib name changed from libnss_aad.so.2 to libnss_aad.so in case it was not intentional

[denisonbarbosa]

This is likely due to the cargo build problem that I've mentioned in the general comment. I'm not sure how to evaluate this diff, tbh. Maybe you or @didrocks can provide me some insight whether this is a big problem or not?

[GabrielNagy]

Mmm okay then. IMO it's fine to keep it as is - but it'd be nice to suppress the warnings by adding them to the lintian-overrides file, adding the context you wrote in a PR as comments. There should be existing lintian-overrides files you can take inspiration from.

[didrocks]

actually, this is possible (rust-lang/cargo#5045 (comment)), but only with cargo from lunar, not kinetic or jammy. Still worth keeping it in the override file IMHO explicitely. The fact that is not important is not well described in the current override file IMHO.

[denisonbarbosa]

Yeah, it's possible using cargo rustc (not cargo build), but I'm not sure this works with the dh-cargo wrapper (that was my concern).

[didrocks]

it seems it does it, but then, we are back to what we do when we backport this to jammy as we won’t have a relevant version of Cargo for now. So, let’s do with a better lintian override comments, explaining exactly why we can’t (and don’t) need a soname anyway, with the link to the bug report.

[denisonbarbosa]

Nice work, only had a few comments:

I tested the branch locally and it built fine for me, with a few lintian errors that made me think:

Now running lintian aad-auth_0.3_amd64.changes ...
E: libnss-aad: lacks-versioned-link-to-shared-library usr/lib/x86_64-linux-gnu/libnss_aad.so [usr/lib/x86_64-linux-gnu/libnss_aad.so.2]
W: aad-auth source: dependency-is-not-multi-archified libnss-aad depends on aad-common (multi-arch: no)
W: libnss-aad: shared-library-lacks-version usr/lib/x86_64-linux-gnu/libnss_aad.so.2 libnss_aad.so
W: aad-auth source: unknown-field Vendored-Sources-Rust

I think we can get rid of some of them by compiling the NSS library with the proper soname (libnss_aad.so.2 instead of libnss_aad.so), like we did for Go.

For the dependency-is-not-multi-archified one we may need to set Multi-Arch: same on aad-common as well. Can you remind me why we need the option on debian/control in the first place?

Hey @GabrielNagy, thanks for the review. It is annoying to have this amount of warnings, I agree (I didn't see them, but it's probably because of a version diff since I'm still on Jammy). We have a few problems though:

1. Cargo does not allow you to change the output name by default (which is very annoying) and we must use cargo because of the dh-cargo wrapper, which sets the proper flags and stuff, that's why on the debian/rules, during the dh_auto_install step, I manually rename the file before actually installing it.
2. About the Multi-Arch: same comment, I've added it because of the Debian policy for Rust packaging which states:

In order to support cross-compiling, each library binary package - the main package plus the feature packages - should use Architecture: any and Multi-Arch: same. (This is not ideal; it is due to a deficiency in the current Debian cross-compiling infrastructure and the Multi-Arch specification.) This applies even if the library crate only runs on certain target architectures or operating systems. Note that if running a test build in dh_auto_test, this might need to be disabled based on the target/foreign architecture (DEB_HOST_ARCH) and unlikely but possibly also the host/native architecture (DEB_BUILD_ARCH).

[GabrielNagy]

1. Cargo does not allow you to change the output name by default (which is very annoying) and we must use cargo because of the dh-cargo wrapper, which sets the proper flags and stuff, that's why on the debian/rules, during the dh_auto_install step, I manually rename the file before actually installing it.

Got it. Let's suppress the warning by adding it to lintian-overrides with a comment.

2. On the Multi-Arch: same comment, I've added it because of the Debian policy for Rust packaging which states:

In order to support cross-compiling, each library binary package - the main package plus the feature packages - should use Architecture: any and Multi-Arch: same. (This is not ideal; it is due to a deficiency in the current Debian cross-compiling infrastructure and the Multi-Arch specification.) This applies even if the library crate only runs on certain target architectures or operating systems. Note that if running a test build in dh_auto_test, this might need to be disabled based on the target/foreign architecture (DEB_HOST_ARCH) and unlikely but possibly also the host/native architecture (DEB_BUILD_ARCH).

👍 - we should add Multi-Arch: same to aad-common as well then which should get rid of the warning in theory.

[didrocks]

Some parts to adjust and look at.

[didrocks]

it seems it does it, but then, we are back to what we do when we backport this to jammy as we won’t have a relevant version of Cargo for now. So, let’s do with a better lintian override comments, explaining exactly why we can’t (and don’t) need a soname anyway, with the link to the bug report.

[didrocks]

Good to me now., Trusting that you did some package build tests as well as autopkgtests :)