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

Fixed build and packaging issues #246

Merged
merged 12 commits into from
Jun 7, 2023
Merged

Fixed build and packaging issues #246

merged 12 commits into from
Jun 7, 2023

Conversation

4r7if3x
Copy link
Contributor

@4r7if3x 4r7if3x commented Jun 6, 2023

This patch addresses some cross-building and packaging issues, including: #47, #199, #220, #224, #225, #242, and rust-pcap/pcap#290.

Changes:

  • The executables are now available for both 32-bit and 64-bit processors based on both x86 and ARM architectures.1
  • The macOS package now contains a Universal-binary which is able to run on both Intel and Apple Silicon macs.
  • Due to cross-platform incompatibilities, the app is no longer dependent on the native SSL/TLS library ( libssl.so* ) and it utilizes the rust-tls instead.
  • The Windows executable now has an icon resource attached to it. This fixes the shortcut icons as well.
  • All packaging assets are now organized under resources/packaging/
  • The CI workflow is now split into a build and a packaging stage and the jobs on each side are processed simultaneously.
  • The packages will now provide a non-development dependency list.2
  • The Windows installer now installs the Npcap automatically displays a notice about the need for manual Npcap installation and opens its official website for the user.
  • The Linux app doesn't require escalated permissions anymore and can run without sudo.

+ Some adjustments on PowerShell scripts, WXS template of WixToolset, info files, etc.

Hacks:

  • Since we build the app on an Ubuntu Server which utilizes the v0.8 of libpcap, the linker sets this version of the library ( libpcap.so.0.8 ) as the dependency of the ELF executable. That won't work on Redhat-based distros as they usually come with v1.x. Therefore, I've changed it to libpcap.so manually which is symlinked to the actual installed version.
    • I didn't use the same approach for the SSL/TLS library due to the adoption of various major versions among different *nix systems and considering many other developers who had difficulties in this area.

To-Do:

  • Since the app doesn't have an icon in the Windows taskbar, I've tried to set the icon on the window settings of Iced, but I had to comment it out since it made the executable not work after compiling. I'm already in discussion with the Iced community to find a solution...
    • The Linux app has the same problem, but I believe that's a different issue. Iced doesn't use the native Window Manager ( you cannot fetch its class by xprop WM_CLASS ), hence the StartupWMClass=sniffnet that I had set in the .desktop file wouldn't work.
  • There is a command-line window opening and closing quickly at the end of the Windows setup wizard since no other WixToolset feature for running commands quietly worked as expected. I'll provide a patch on this later when I figured it out.
  • The sample graphics used for the Windows installer should be modified. Since transparency is supported, I suggest the dialog background be limited to the left side only to follow the standards and fix the visual issues there. The same can apply to the banner image, traditionally only an icon was shown on the right side of it.
  • The EULA of the Windows installer is generated based on the project's licensing, yet could be revised by other contributors for more accuracy.

Known Issues:

  • The cargo bundle command carries out an unavoidable extra build since it doesn't come with a --no-build flag, unlike other packaging tools. (already sent a feature request)
  • The minimize and maximize buttons of the window don't work on macOS, and probably the Quit as well.

Tests

Other than the minor issues mentioned above, both the installer and the app are tested on a fresh installation of Debian, Ubuntu, and Fedora Linux distros as well as macOS and Windows 11. Feedback from other testers is highly appreciated!

Screenshots:

image

Footnotes

  1. macOS and Redhat-based Linux distros only support 64-bit CPUs, and since most Windows machines are using x86 CPU families, we only provide packages for them.

  2. Developers would need build-essential libfreetype6-dev, libexpat1-dev, libpcap-dev, libasound2-dev, libfontconfig1-dev packages on Debian-based distros and @development-tools, expat-devel, libpcap-devel, alsa-lib-devel, fontconfig-devel on Redhat-based distros in order to develop this app on Linux. But end users only require the non-development version of the dependencies: libpcap0.8, libasound2, libfontconfig1 for Debiab-based distros and libpcap, alsa-lib, fontconfig for Redhat-based.

@GyulyVGC GyulyVGC added the enhancement New feature, request, or improvement label Jun 6, 2023
@GyulyVGC GyulyVGC added this to the v1.2.1 milestone Jun 6, 2023
Cargo.toml Outdated Show resolved Hide resolved
resources/packaging/linux/scripts/.gitkeep Outdated Show resolved Hide resolved
resources/packaging/macos/Info.plist Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
@GyulyVGC
Copy link
Owner

GyulyVGC commented Jun 6, 2023

A more in depth review will come soon.
If you make any change, please add commits without squashing.

Thank you so much for all the work!

resources/packaging/macos/wrapper.sh Show resolved Hide resolved
resources/packaging/macos/wrapper.sh Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
@GyulyVGC
Copy link
Owner

GyulyVGC commented Jun 6, 2023

A little recap: I've reviewed part of the PR but still a considerable part is missing and I'll do it in the next few days.

I've just opened a (still incomplete) issue to track all the missing steps related to packaging: #252

Since various problems are going to be solved by this PR, I'm looking forward to release a new version ASAP after merge (so don't worry to much about the other steps for the moment).

Thanks very much again 👍

.github/workflows/package.yml Outdated Show resolved Hide resolved
resources/packaging/windows/scripts/npcap.ps1 Outdated Show resolved Hide resolved
resources/packaging/windows/scripts/npcap.ps1 Outdated Show resolved Hide resolved
@GyulyVGC GyulyVGC changed the base branch from main to packages June 7, 2023 12:39
@GyulyVGC GyulyVGC merged commit 4ce21d9 into GyulyVGC:packages Jun 7, 2023
@GyulyVGC
Copy link
Owner

GyulyVGC commented Jun 7, 2023

It'll be merged in main soon.
It's just a matter of some more tests, (in this way I can run the workflow on dispatch 👍 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, request, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants