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

New Packages: sequoia cli utilities #32480

Merged
merged 3 commits into from
Sep 21, 2022
Merged

Conversation

jcgruenhage
Copy link
Contributor

@jcgruenhage jcgruenhage commented Aug 13, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

TODO

  • sq
  • sqop
  • sqv
  • add man pages
  • figure out cross compiling (was broken due to nettle-dev being required on the host as well as the target)
  • add shell completions will be done after https://gitlab.com/sequoia-pgp/sequoia/-/issues/798 is fixed.
  • tell upstream about adding git tags for openpgp-card-tools (upstream issue)
  • figure out building for armv7l (some lib stuff missing, see log) marked as nocross for armv7l for now, considering neither we nor upstream really know why it's failing to cross compile
  • figure out building for x86_64-musl (some deprecated musl specific rust code, see log)

@jcgruenhage
Copy link
Contributor Author

So, I've done some digging, and I'd like to just outright disable armv7l and musl targets for now.

In the case of musl, this is part of time64, the migration to using 64 bit integers for time on 32 bit systems. Rust added a deprecation warning for that, and this warning is currently breaking the build here. There's stuff happening in that area, but it's a bigger problem that's not really related to sequoia directly, so not really something that can be addressed in this PR.

As for armv7l: A crypto library used by sequoia, nettle, for some reason fails to have it's rust bindings compiled for armv7l, because cross-armv7l-linux-gnueabihf-libc doesn't provide gnu/stubs-soft.h, which is referenced by gnu/stubs.h, based on some ifdef magic. I don't get where this is going wrong, but I don't understand enough of the arkane ifdef and crosscompiling magic to dig through this. If anyone else wants to, I'd be more than happy, but I won't invest any more time in trying to figure out why stubs-soft.h would even be considered required on a hf target.

@ericonr
Copy link
Member

ericonr commented May 15, 2022

In the case of musl, this is part of time64, the migration to using 64 bit integers for time on 32 bit systems. Rust added a deprecation warning for that, and this warning is currently breaking the build here. There's stuff happening in that area, but it's a bigger problem that's not really related to sequoia directly, so not really something that can be addressed in this PR.

That's on Rust, they shouldn't have deprecated the type on 64-bit musl. And more importantly, the software should still be buildable with warnings like that....

As for armv7l: A crypto library used by sequoia, nettle, for some reason fails to have it's rust bindings compiled for armv7l, because cross-armv7l-linux-gnueabihf-libc doesn't provide gnu/stubs-soft.h, which is referenced by gnu/stubs.h, based on some ifdef magic. I don't get where this is going wrong, but I don't understand enough of the arkane ifdef and crosscompiling magic to dig through this. If anyone else wants to, I'd be more than happy, but I won't invest any more time in trying to figure out why stubs-soft.h would even be considered required on a hf target.

That sounds like bindgen not receiving all the flags it should have and looking at the wrong headers/missing some target definitions (probably the latter). We might have a fix for this by using a patched bindgen, iirc.

@classabbyamp
Copy link
Member

has this branch been rebased recently (after mid-March)? I think i was getting some similar errors with bindgen + cross and solved it by adding this to the rust build helper:

# Crates that use bindgen via build.rs are not cross-aware unless these are set
export BINDGEN_EXTRA_CLANG_ARGS="--sysroot=${XBPS_CROSS_BASE} -I${XBPS_CROSS_BASE}/usr/include"

@jcgruenhage
Copy link
Contributor Author

has this branch been rebased recently (after mid-March)? I think i was getting some similar errors with bindgen + cross and solved it by adding this to the rust build helper:

Yes, it has been rebased since then. I just rebased it again to incorporate some package updates and to drop the license files for openpgp-card-tools (as that now has git tags 🎉), but the bindgen problems persist.

That sounds like bindgen not receiving all the flags it should have and looking at the wrong headers/missing some target definitions (probably the latter). We might have a fix for this by using a patched bindgen, iirc.

@ericonr do you happen to have a link to a different package that had these problems and how we worked around it over there?

That's on Rust, they shouldn't have deprecated the type on 64-bit musl. And more importantly, the software should still be buildable with warnings like that....

I fully agree. Something somewhere in the build process is setting it to deny building with warnings, I'll do some digging where that happens.

@jcgruenhage
Copy link
Contributor Author

I've looked for quite a while now, and I just can't find it. The openpgp crate which fails doesn't have deny(warnings) set, I can't find anything adding -D warnings to RUSTFLAGS either. From the error message in the build, it seems to be the latter, but I can't find where if at all it happens.

@jcgruenhage jcgruenhage force-pushed the sequoia branch 3 times, most recently from cc21fa2 to 2132dcc Compare June 14, 2022 14:10
@jcgruenhage
Copy link
Contributor Author

After asking upstream, I found the -Dwarnings flag, and with that removed and a conflict added for squirrel, it's now nearly working correctly. Crosscompiling is still not quite working in all cases, but aside of that it's getting there.

srcpkgs/sequoia-sop/template Outdated Show resolved Hide resolved
srcpkgs/sequoia-sq/template Outdated Show resolved Hide resolved
@jcgruenhage
Copy link
Contributor Author

TL;DW of the problem here: sq depends on nettle-sys for both host (in build.rs) as well as target (for the application itself). Getting bindgen to work correctly for both host and target in our cross compilation setup is super tricky, and I've sunken many, many hours into it without getting anywhere so far. For most architectures it works anyway, but on armv*l it breaks due to some import trying to get in /usr/include/gnu/stubs-soft.h even though they are hard-float architectures, meaning that the file just doesn't exist.

I've tried patching out the requirement for nettle-sys in sq's build.rs script, but that ended up being a way bigger challenge than anticipated, and I'd prefer to just mark this as nocross for the affected architectures for now. If someone could validate that it works on those architectures at all, I'd be very happy about it as I currently don't have any armv7l device anymore to test on, but I suspect that it just works on those.

@jcgruenhage
Copy link
Contributor Author

(Last but not least: The probability that there is even a single user on older arm architectures that wants to use sq on Void is very slim, so.. yeah. I don't have any more time to sink into this)

conflicts="squirrel"

case "$XBPS_TARGET_MACHINE" in
armv*l) nocross="Requires C libs included in build.rs, which is currently broken in xbps-src. These failures only manifest on a hf archs right now";;
Copy link
Member

Choose a reason for hiding this comment

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

We talked on IRC about fixing this by updating the bindgen crate, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I tried it with EXTRA_CLANG_ARGS_<TARGET> after updating bindgen to 0.59, which I had hoped would help, but without success. The problems persisted

Copy link
Member

Choose a reason for hiding this comment

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

I tested a native build on armv7l and it seems to work, so nocross seems like the right choice here.

@ahesford ahesford merged commit 935f707 into void-linux:master Sep 21, 2022
@jcgruenhage jcgruenhage deleted the sequoia branch September 21, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants