Bug 2277423 - Review Request: nextcloud-spreed-signaling - Standalone signaling server which can be used for Nextcloud Talk
Summary: Review Request: nextcloud-spreed-signaling - Standalone signaling server whic...
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 2277416 2277418 2277421 2277422
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-04-26 20:41 UTC by Renich Bon Ciric
Modified: 2025-11-20 14:48 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Renich Bon Ciric 2024-04-26 20:41:39 UTC
Spec URL: https://fedorapeople.org/~renich/signaling/nextcloud-spreed-signaling.spec
SRPM URL: https://fedorapeople.org/~renich/signaling/nextcloud-spreed-signaling-1.2.4-1.fc39.src.rpm
Description: Standalone signaling server which can be used for Nextcloud Talk
Fedora Account System Username: renich

Comment 1 Mikel Olasagasti Uranga 2024-04-27 13:44:35 UTC
- This one can be removed: docker/README.md

> %global godocs          docs CHANGELOG.md README.md docker/README.md


- Although correct, I think this time these can be conveted to "BuildRequires: /usr/bin/easyjson" type of BR.

> BuildRequires:  golang-github-mailru-easyjson
> BuildRequires:  golang-google-grpc
> BuildRequires:  golang-google-protobuf


- If you plan to use `make` you need a BR on it.


- Doesn't setting GO* env vars before make work?

> make GOPROXY=off GO111MODULE=off GOPATH=%{gobuilddir}:%{gopath} common


- Is client required? Makefile's build section only builds server and proxy.

> %gobuild -o %{gobuilddir}/bin/client %{goipath}/client

Documentation refers to it as "A simple client exists to benchmark the server."


- Change from this:

> %{_bindir}/*

to: 

%{_bindir}/signaling
%{_bindir}/proxy
%{_bindir}/client


- You can remove these to create a binary only package:

> %gopkginstall
> (...)
> %gopkgfiles


- IIUC this package is meant to be used as a daemon by default? If so systemd definition and user creation is missing.

https://github.com/strukturag/nextcloud-spreed-signaling/blob/master/README.md#running-as-daemon


- Does it make sense to split the package into signaling and proxy subpackages? Or add the proxy subpackage?

Comment 2 Renich Bon Ciric 2024-04-29 16:54:41 UTC
(In reply to Mikel Olasagasti Uranga from comment #1)
> - This one can be removed: docker/README.md
> 
> > %global godocs          docs CHANGELOG.md README.md docker/README.md

Done.

> - Although correct, I think this time these can be conveted to
> "BuildRequires: /usr/bin/easyjson" type of BR.
> 
> > BuildRequires:  golang-github-mailru-easyjson
> > BuildRequires:  golang-google-grpc
> > BuildRequires:  golang-google-protobuf

Done.

> - If you plan to use `make` you need a BR on it.

Done.
 
> - Doesn't setting GO* env vars before make work?
> 
> > make GOPROXY=off GO111MODULE=off GOPATH=%{gobuilddir}:%{gopath} common

No; still getting build errors:

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.mz5mbd
+ umask 022
+ cd /builddir/build/BUILD
+ cd nextcloud-spreed-signaling-1.2.4
+ GOPROXY=off
+ GO111MODULE=off
+ GOPATH=/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/_build:/usr/share/gocode
+ make common
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_backend.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_internal.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_mcu.proto
protoc \
        --go_out=. --go_opt=paths=source_relative \
        grpc_sessions.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_backend.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_internal.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_mcu.proto
protoc \
        --go-grpc_out=. --go-grpc_opt=paths=source_relative \
        grpc_sessions.proto
rm -f easyjson-bootstrap*.go
easyjson -all api_async.go
Error parsing api_async.go: file '/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/api_async.go' is not in GOPATH '/builddir/build/BUILD/nextcloud-spreed-signaling-1.2.4/_build:/usr/share/gocode'
make: *** [Makefile:102: api_async_easyjson.go] Error 1

The full log is at: https://paste.centos.org/view/08c1fffe

> - Is client required? Makefile's build section only builds server and proxy.
> 
> > %gobuild -o %{gobuilddir}/bin/client %{goipath}/client
> 
> Documentation refers to it as "A simple client exists to benchmark the
> server."

It is needed for the tests. I don't think it is needed in the actual package. I don't think proxy is required at all either. 
 
> 
> - Change from this:
> 
> > %{_bindir}/*
> 
> to: 
> 
> %{_bindir}/signaling
> %{_bindir}/proxy
> %{_bindir}/client

Done.

> - You can remove these to create a binary only package:
> 
> > %gopkginstall
> > (...)
> > %gopkgfiles

Done.

> - IIUC this package is meant to be used as a daemon by default? If so
> systemd definition and user creation is missing.
> 
> https://github.com/strukturag/nextcloud-spreed-signaling/blob/master/README.
> md#running-as-daemon

You are right. I haven't included the systemd service unit. I will do this as soon as I can make the package build.

> - Does it make sense to split the package into signaling and proxy
> subpackages? Or add the proxy subpackage?

I think it is worth splitting. You don't always need a proxy. I will do this as soon as I can make the package build.

Comment 3 Renich Bon Ciric 2024-04-30 02:14:29 UTC
OK, package built thanks to Mikel's help. Thanks a lot. I have implemented all your indications.

Comment 4 Renich Bon Ciric 2024-05-14 05:34:53 UTC
I have added a sysusers configuration file.

Comment 5 Renich Bon Ciric 2024-06-03 17:15:28 UTC
OK, so, what's next?

Comment 6 Package Review 2025-06-04 00:45:32 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 7 Renich Bon Ciric 2025-06-13 18:18:07 UTC
Yep. I'm interested.

Comment 8 Renich Bon Ciric 2025-06-15 02:41:53 UTC
I've updated to v2.0.3 and we have a new dependency:

SPEC: https://renich.fedorapeople.org/signaling/golang-github-pquerna-cachecontrol.spec
SPRM: https://renich.fedorapeople.org/signaling/golang-github-pquerna-cachecontrol-0.2.0-1.fc42.src.rpm

I will open up a separate ticket for this one. 

The updated SPEC and SRPM for signaling are:

SPEC: https://renich.fedorapeople.org/signaling/nextcloud-spreed-signaling.spec
SRPM: https://renich.fedorapeople.org/signaling/nextcloud-spreed-signaling-2.0.3-1.fc42.src.rpm

If anyone reviewing, please, review.

Comment 9 Benson Muite 2025-06-15 06:36:28 UTC
Consider contacting people in the Go SIG https://fedoraproject.org/wiki/SIGs/Go

Comment 10 Renich Bon Ciric 2025-06-18 18:08:00 UTC
Will do. Thanks, Benson.

Comment 11 Fedora Review Service 2025-11-20 14:46:18 UTC
Hello,
I do realize that this is possibly an old ticket. I am sorry that it hasn't been
reviewed yet. Let me trigger the Fedora Review Service to see if the package
builds successfully. Hopefully, a green check mark will attract some reviewer.

If I am resurrecting an old ticket that you are not interested in anymore, my
apologies, feel free to close it.

[fedora-review-service-build]

Comment 12 Fedora Review Service 2025-11-20 14:48:15 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9819681
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2277423-nextcloud-spreed-signaling/fedora-rawhide-x86_64/09819681-nextcloud-spreed-signaling/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.


Note You need to log in before you can comment on or make changes to this bug.