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

Build and include notify_push for FreeBSD in every release too #153

Open
benbenik opened this issue Jan 13, 2022 · 28 comments
Open

Build and include notify_push for FreeBSD in every release too #153

benbenik opened this issue Jan 13, 2022 · 28 comments
Labels
enhancement New feature or request

Comments

@benbenik
Copy link

Is your feature request related to a problem? Please describe.
Nextcloud notify_push is great, but it is included for Linux aarch64, armv7 and x86_64 only. FreeBSD is missing, while it can easily be included in the Nextcloud release process. I feel a bit frustrated to build notify_push and testing with occ with every Netxcloud update myself; mundane work that can be automated to benefit all FreeBSD admins.

Describe the solution you'd like
Include fbsd_amd64 or fbsd_x86_64 in directory:
/NEXTCLOUD_ROOT/apps/notify_push/bin/
Execute in the release build process (generic FreeBSD fits-all):
cargo build --release --target=x86_64-unknown-freebsd

Describe alternatives you've considered
Manually running cargo build --release --target=x86_64-unknown-freebsd, moving compiled push_notify to correct location and re-running occ to check whether all is ok over-and-over again every update.

Additional context
No other context. Except a "thank you" for considering!

@szaimen szaimen transferred this issue from nextcloud/server Jan 13, 2022
@jSML4ThWwBID69YC
Copy link

I second this request.

@n-connect
Copy link
Contributor

+1

@benbenik
Copy link
Author

benbenik commented Aug 5, 2022

For anyone interested how to setup notify_push under FreeBSD manually:

Replace any NEXTCLOUD_ROOT with your own local path to Netxcloud.

Requirements:

  1. Redis is required for notify_push.
  2. If not installed yet: pkg install devel/php80-pcntl

Instructions:
pkg install lang/rust
fetch https://github.com/nextcloud/notify_push/archive/refs/tags/v0.4.0.tar.gz
tar zxvf v0.4.0.tar.gz
cd notify_push-0.4.0
cargo build --release --target=x86_64-unknown-freebsd
mkdir NEXTCLOUD_ROOT/apps/notify_push/bin/fbsd_amd64/
cp ./target/x86_64-unknown-freebsd/release/notify_push NEXTCLOUD_ROOT/apps/notify_push/bin/fbsd_amd64/
chown www:www NEXTCLOUD_ROOT/apps/notify_push/bin/fbsd_amd64/notify_push
chmod 0744 NEXTCLOUD_ROOT/apps/notify_push/bin/fbsd_amd64/notify_push

nano /usr/local/etc/rc.d/nextcloud_notify_push

#! /bin/sh
#
# $FreeBSD$
#

# PROVIDE: nextcloud_notify_push
# REQUIRE: NETWORKING DAEMON
# KEYWORD: shutdown

#
# Add the following lines to /etc/rc.conf to enable nextcloud_notify_push:
#
#nextcloud_notify_push_enable="YES"

. /etc/rc.subr

name="nextcloud_notify_push"
rcvar="nextcloud_notify_push_enable"

load_rc_config $name

: ${nextcloud_notify_push_enable:=NO}
: ${nextcloud_notify_push_config:=/NEXTCLOUD_ROOT/config/config.php}
: ${nextcloud_notify_push_flags:=-p 7867}

my_flags=${nextcloud_notify_push_flags}
nextcloud_notify_push_flags=

pidfile=/var/run/nextcloud_notify_push.pid
binary="/NEXTCLOUD_ROOT/apps/notify_push/bin/fbsd_amd64/notify_push"
command="/usr/sbin/daemon"
command_args="-S -u www -P ${pidfile} -- ${binary} \
	${my_flags} ${nextcloud_notify_push_config}"

run_rc_command "$1"

chmod +x /usr/local/etc/rc.d/nextcloud_notify_push
edit /etc/rc.conf
nextcloud_notify_push_enable="YES"
/usr/local/etc/rc.d/nextcloud_notify_push start

Check if notify_push is running:
ps ax | grep -i notify

Read how to setup reverse proxy (configure webserver): see section "Reserve proxy" for Apache and Nginx:
https://github.com/nextcloud/notify_push

su -m www -c "php occ app:enable notify_push"
su -m www -c "php NEXTCLOUD_ROOT/occ notify_push:setup https://nextcloud.example.com/push"

All green? Enjoy!

See current metrics:
su -m www -c "php occ notify_push:metrics"
Example output:

Active connection count: 3
Total connection count: 6
Total database query count: 2
Events received: 15
Messages sent: 2

@alerque
Copy link

alerque commented Aug 5, 2022

The best place for this request would actually be the FreeBSD project itself. By far the best home for binary builds for specific systems are the packaging systems of those distros.

@benbenik
Copy link
Author

benbenik commented Aug 5, 2022

The best place for this request would actually be the FreeBSD project itself. By far the best home for binary builds for specific systems are the packaging systems of those distros.

Curious, why isn't building for specific Linux distro's outsourced to the packaging systems of the Linux distro's?

What is the harm in adding FreeBSD, while 3 Linux distro's are already compiled for?:
cargo build --release --target=x86_64-unknown-freebsd

With the above included, it would require zero effort after every Nextcloud upgrade. No action needed, because the compiled version for FreeBSD just exists, like it would for the Linux distro's. Wouldn't you agree @alerque?

@alerque
Copy link

alerque commented Aug 5, 2022

Curious, why isn't building for specific Linux distro's outsourced to the packaging systems of the Linux distro's?

They should be. Indeed for many it already is: Arch Linux, Gentoo, LiGurOS, Parabola, Manjaro, and probably others.

Notably OpenBSD packages this too.

What is the harm in adding FreeBSD, while 3 Linux distro's are already compiled for?:

Because cross compiling is full of pitfalls: it takes different expertise, different dependencies, and it doesn't integrate well with target OS release workflows. The OS has a better idea when something needs rebuilding (e.g. for glibc updates) than downstream projects like this.

@benbenik
Copy link
Author

benbenik commented Aug 7, 2022

I contemplated the direction you're proposing. However, I see issues by not using the vanilla process with built-in NC updater, like troubleshooting. Likewise, the package has its own pitfalls: dependency conflicts to consider and manual checking of .htacess.dist with every upgrade. Still, notify_push is not included and needs to be built anyway. Also the backup from a non-standard webdata dir that the package uses needs consideration. And all FreeBSD users need to install and migrate to the new solution. So, after considering some examples, if I do as proposed: I keep my original issue, plus I get some extra issues.

Concerning compiling: generically I understand, and from a C-perspective I understand, but with this use case with Rust, I don't. One compiler to rule all OSes. I fail to see the pitfalls, the needed expertise, the different dependencies and any integration issues in this Rust notify_push use case: Cross OS build and it works; the power of Rust. One build, help many. Where the Nextcloud release process itself knows when a new version is shipped.

At least it would be helpful if a Nextcloud upgrade doesn't remove the current manual build of notify_push; prevents the need of rebuilding, at least keeps everything working.
But this probably requires more effort than just including one extra 'cargo build' in the release process; which circles back to my original request.

@n-connect
Copy link
Contributor

@benbenik, PR #254 just merged, check it out.
Another PR #256 is being raised to have the BSD style rc.d script part of the codebase, not in Readme.md

Once a new release comes out, and the build works, this issue can be closed.

@GuillaumedeVolpiano
Copy link
Contributor

@benbenik, PR #254 just merged, check it out. Another PR #256 is being raised to have the BSD style rc.d script part of the codebase, not in Readme.md

Once a new release comes out, and the build works, this issue can be closed.

As someone who has been using benbenik's workaround for a while now, many thanks for the merge.

I still have an issue, though: when I run occ:notify_push setup, the script reports

"your system architecture(amd64) is not supported by the bundled binaries."

FreeBSD reports its architecture as amd64 and not x86_64, which I guess is the issue here.

@n-connect
Copy link
Contributor

n-connect commented Jun 2, 2023

Which NC version are you on? If the occ command gives you this error, maybe it need be handled within the NC codebase. Older versions did not gave this error that's for sure, but I used manual compiled binary.
The latest GitHub release gives me this info:
file notify_push-x86_64-unknown-freebsd notify_push-x86_64-unknown-freebsd: ELF 64-bit LSB pie executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, for FreeBSD 12.3, FreeBSD-style, with debug_info, not stripped

I'll need to do multiple NC version upgrades on servers, so I'll check the releases to find where(when actually) it is error rooted.

@GuillaumedeVolpiano
Copy link
Contributor

GuillaumedeVolpiano commented Jun 2, 2023

I'm running 26.0.1 (26.0.2 hasn't hit the ports yet) on FreeBSD 13.2 (so, basically, the latest RELEASE and the CURRENT port).

After some peering through, I think I get the problem. In Command/Setup, the line

$output->writeln(" And pre-build binaries for x86_64, armv7 and aarch64 in the github actions."); (line 74)
is present if there are no bundled binaries (i.e. building from source) but not if there are (i.e. pulling the app from the Nextcloud AppStore).

It should appear also between lines 105 and 106, and also mention the existence of the freebsd binary.

I've created a pull request (PR #273 ). Note that this doesn't solve all the issues of using the setup wizard on FreeBSD as it then checks for Systemd, which is absent from FreeBSD. But once PR #256 is merged, this could be done

@n-connect
Copy link
Contributor

n-connect commented Jun 6, 2023

I've checked #273 , it is handling the console outputs with additional text to include the FreeBSD (amd64) which is good to have. I think, what we need is to modify some functions in SetupWizard.php. At least hasBinary(), hasBundledBinaries(), which today directly sends the code to the "../bin/x86_64" directory. I believe we need to start from getArch(). I'll check the code, altought I'm not a coder for a living. Perhaps we can extend your PR to include the modified hasBundledBinaries() - the FreeBSD binary is under "../bin/fbsd_amd64" - , or at least my own previous builds were there. I'll crosscheck it

@GuillaumedeVolpiano
Copy link
Contributor

I'll look into the code too. Thanks for checking.

@GuillaumedeVolpiano
Copy link
Contributor

GuillaumedeVolpiano commented Jun 6, 2023

I have now updated getArch() and hasBundledBinaries() to report FreeBSD and find the binary under "../bin/fbsd_amd64".

The next step for a fully working wizard would be to look into the systemd roadblock, but that depends on the merging of PR #256.

@n-connect
Copy link
Contributor

n-connect commented Jun 6, 2023

Great, it seems to be good. Were you able to test it?
Found one typo, with php_uname('s') result check at line 36. I marked in the #273, you can check it there: 'FreBSD' vs 'FreeBSD'

@GuillaumedeVolpiano
Copy link
Contributor

I did test it. As mentioned, the setup daemon now fails with
your system doesn't seem to be using systemd.
I'm not sure it's worth fixing that, as I'm assuming that anyone running Nextcloud on FreeBSD would be comfortable enough to drop in an rc script, but who knows (by the way, I did suggest a modification on your rc script ;) ), but I might be wrong.

Anyhow, the weird thing is it went well, despite the typo, which has me puzzled.

@n-connect
Copy link
Contributor

You're right, sorry. Good, I'm still working. I'll do test it too the weekend
If you give me some time I'll work on it (systemd function or its original intent) during tonight, so it would not fail, while the rc script gets its way into the main repo. From there the CI/CD jobs needs some tweaks, then again mod the systemd function for good - with that the FB support should be done.

@icewind1991
Copy link
Member

Note that it's unlikely that the release archives will include bsd binaries because the extra size that everyone will have to download doesn't seem worth it for the bsd install base.

With updates to the setup wizard/instructions and pre-build binaries available for download it should still be doable to create a fairly fool proof experience.

I also have no experience with any bsd (nor any desire to start), so while I'm open to merging bsd specific improvements I have no means of testing them.

@n-connect
Copy link
Contributor

n-connect commented Jun 8, 2023

No worries, we'll cover the FreeBSD tests. Maybe this direction can be used to stip the actual release archive to have only one rust binary included (or none, it the setupwizard turns to be good enough for an included wget/curl step)

@GuillaumedeVolpiano
Copy link
Contributor

@n-connect let me know if you want me to delve into the Setup wizard on my side. I haven't been touching it more for now as you said you'd delve into it, but I'm obviously happy to take my share.

@icewind1991
Copy link
Member

I've been working on changing the cross-compile setup used to build the binaries, can somebody verify that the freebsd binaries from https://github.com/nextcloud/notify_push/actions/runs/5377720649 work as expected

@GuillaumedeVolpiano
Copy link
Contributor

It's starting and passing all tests in occ notify_push:self-test. Anything else to check?

@icewind1991
Copy link
Member

Nothing else, thanks for the verification

@thstyl2000
Copy link

Only asking just to make sure I am not doing sth wrong: the updated wizard is not yet available on the app store, is it?

@joshtrichards joshtrichards added the enhancement New feature or request label Oct 5, 2023
@thstyl2000
Copy link

Sorry to bump, was just wandering if there is anything new....thank you very much for your work!

@benbenik
Copy link
Author

First of, @n-connect: thank you for your effort! @thstyl2000 / @n-connect : I recently updated to Nextcloud 27 and have manually installed notify_push binary 0.6.6 ( notify_push-x86_64-unknown-freebsd ) from https://github.com/nextcloud/notify_push/releases after updating.

The binary itself is working; great! I will in the future explicitly check whether notify_push binary remains after an update.

However:
In the Admin panel > Overview, Nextcloud 27.1.5 doesn't like the binary:
Some files have not passed the integrity check. Further information on how to resolve this issue can be found in the [documentation ↗](https://docs.nextcloud.com/server/27/go.php?to=admin-code-integrity). ([List of invalid files…] / [Rescan…])

Rescan did result in the same notice.

I read about possibly including the /usr/local/rc.d/ service script in releases in issue #256 in the future: nice progress!

@icewind1991
Copy link
Member

I've updated the build setup which required some changes to how the FreeBSD binaries are built and would appreciate it if somebody can test the binaries from https://github.com/nextcloud/notify_push/actions/runs/9407127052?pr=466

@n-connect
Copy link
Contributor

n-connect commented Jun 8, 2024

@icewind1991

It seems fine!

Binary build is the same as previous using file, and runs:

# file notify_push
notify_push: ELF 64-bit LSB pie executable, x86-64, version 1 (FreeBSD), dynamically linked, interpreter /libexec/ld-elf.so.1, FreeBSD-style, not stripped
# chmod +x ./notify_push
# chmod +x ./notify_push
# ./notify_push
Error:   × No nextcloud server is configured

Replacing the binary the service runs, it listening on the port:

# service nextcloud_notify_push start
Starting nextcloud_notify_push.
# service nextcloud_notify_push status
nextcloud_notify_push is running as pid 13509.
# sockstat -l4 | grep 7867
www      notify_pus 13510 12  tcp4   *:7867                *:*

Log before nextcloud app update:

WARN [notify_push] src/lib.rs:130: push server (version 0.6.12) is not the same version as the app (version 0.6.9)

After:

# sudo -u www php ./occ notify_push:self-test
✓ redis is configured
✓ push server is receiving redis messages
✓ push server can load mount info from database
✓ push server can connect to the Nextcloud server
✓ push server is a trusted proxy
✓ push server is running the same version as the app

# sudo -u www php ./occ notify_push:metrics
Active connection count: 2
Active user count: 1
Total connection count: 2
Total database query count: 1
Events received: 2
Messages sent: 0

One question though: what to do, if I'm running the test_client-x86_64-unknown-freebsd and get back
/storage/backup/test_client-x86_64-unknown-freebsd: No match. But the <nextcloud url> <username> <password> parameters are fine and cross-checked. There's 2FA switched on over logins in browser, can it be connected?

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

No branches or pull requests

8 participants