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

wsjtx, js8call: unvendor hamlib #265897

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mattmelling
Copy link
Contributor

@mattmelling mattmelling commented Nov 6, 2023

Description of changes

#265747 raised a problem using js8call with FLrig.

We determined that js8call should use the same version of hamlib (forked by the wsjtx team), since js8call is a fork of wsjtx.

This PR packages the wsjtx fork of hamlib as hamlib_4-wsjtx, and uses it to build both js8call and wsjtx.

This allows us to simplify the wsjtx package by building from git sources rather than relying on a tarball that contains a vendored hamlib.

Also add myself as a maintainer for wsjtx.

Result of nixpkgs-review run on x86_64-linux 1

5 packages built:
  • hamlib_4-wsjtx
  • js8call
  • openwebrx
  • openwebrx.dist
  • wsjtx

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@lasandell
Copy link
Contributor

Tested and looks good overall. Think you may want to add -wsjtx to the pname or version, not sure which, to match the attribute name and differentiate it from the vanilla hamlib.

Only question I would have is whether it would be better to implement hamlib/wsjtx.nix in terms of (import ./4.nix).overrideAttrs { ... } for reuse since they are so similar, but that is a bit opinionated and could introduce more dependencies and make things more brittle, so idk.

@dotemup
Copy link
Contributor

dotemup commented Nov 7, 2023

Tested building wsjtx and js8call with these changes. Both work!

The one thing that is odd is in the hamlib-wsjtx the 4.3.1 version is called for from the SF repo, the superbuild tarball for 2.6.1 seems to have 4.5.4 if I'm not mistaken.
image

Either way definitely better than before with the js8call issue when built with hamlib3.

The one odd thing is there is still a small delay which I don't remember my time with js8 on debian. I have a similar issue in wsjtx but I've had hundreds of successful contacts on FT8 regardless at the QTH and out in POTA's. It just seems to be more noticeable in js8. Could just be a timesync/chrony issue as I believe in wsjtx at least I've seen it correct itself.

Adding screenshots in an edit to show timesync issue, just using a signalstick inside for testing so no traffic displayed. But the double green lines show the delay I mention.

Current WSJTX "superbuild" (hamlib 4.5.4 ?)
Screenshot_20231106_190518

WSJTX Unvendored (hamlib 4.3.1)
Screenshot_20231106_190744

JS8Call Unvendored (hamlib 4.3.1)
Screenshot_20231106_190951

Update:

After a proper reboot, it all fails again. My nix-build -A wsjtx and js8call worked, and then after nixos-rebuild switch against the git nixpkgs with these changes it also worked but now a full reboot I can no longer preform actions in either wsjtx or js8call. My guess is build vs. runtime... somehow?

@mattmelling
Copy link
Contributor Author

@lasandell Thanks for the feedback :)

Think you may want to add -wsjtx to the pname or version

👍 Added to the version

Only question I would have is whether it would be better to implement hamlib/wsjtx.nix in terms of (import ./4.nix).overrideAttrs { ... } for reuse

Agree that this might make things more brittle. The main hamlib package builds from a tarball and doesn't need stuff like autoreconfHook, whereas we don't have that for the fork. There is also a difference in the version of libusb that is required, so it probably makes sense to keep them separate.

@dotemup Glad we are making some progress!

The one thing that is odd is in the hamlib-wsjtx the 4.3.1 version is called for from the SF repo, the superbuild tarball for 2.6.1 seems to have 4.5.4 if I'm not mistaken.

Yes, it's not clear where the wsjtx superbuild is getting that from. I've reached out to the wsjtx-devel list to see if there is a more up-to-date repo we can pull from.

@dotemup
Copy link
Contributor

dotemup commented Nov 7, 2023

@dotemup Glad we are making some progress!

Definitely heading the in the right direction @mattmelling

Yes, it's not clear where the wsjtx superbuild is getting that from. I've reached out to the wsjtx-devel list to see if there is a more up-to-date repo we can pull from.

Did some digging, the superbuild appears to be using the 'integration' tag/branch from bsomervi's git repo renamed to the 4.5.x version? There is the CMakeLists in the tarball for mostly how it was built still trying to piece it all together. I'll attach the file from the 2.6.1 superbuild version so maybe someone smarter than me can figure out the best path forward for a way to do it in nix.
CMakeLists.txt

There's definitely some oddities between static and shared libraries.

@mattmelling
Copy link
Contributor Author

According to wsjtx-devel:

Debian has been building wsjtx with the stock upstream hamlib for ages without issues.

This makes things easier for unvendoring hamlib, but doesn't address the issue with js8call. I will come back to this later and probably split this pr to separate the js8call stuff from wsjtx.

@mattmelling mattmelling mentioned this pull request Nov 8, 2023
13 tasks
@dotemup
Copy link
Contributor

dotemup commented Nov 9, 2023

Saw the wsjtx PR, will follow up with testing on that tomorrow.

I did some more poking around following the thread in the wsjtx-devel group. They linked the fedora spec for how they were building with latest hamlib which led me to look at how they're doing js8call.

A few patches, env vars and cmake flags to remove the static lib requirement and we should be able to js8call build work with latest hamlib 4.5.x. They've been on 4.x for a while...

https://src.fedoraproject.org/rpms/js8call/tree/rawhide

I also took a peak at the debian rpm and found their ci/cd for js8call which was ugly and filled with failed builds so quickly ran away from there. Wouldn't not recommend, they also have that weird wsjtx-data dependency that always caused me problems and ultimately led me to Nix so.... Fedora!

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.

None yet

4 participants