https://bugzilla.redhat.com/show_bug.cgi?id=2292255 --- Comment #6 from Neal Gompa <ngompa13@xxxxxxxxx> --- (In reply to Carl George 🤠 from comment #5) > ============================================================================= > === > > This spec installs a sysconfig file. This type of file is a holdover from > the sysvinit days, when it was necessary to change a daemon's command > without editing the script itself. These days, systemd handles that much > more gracefully with unit file overrides. My recommendation is to remove > this file, the EnvironmentFile directive in the unit file, and just specify > the port explicitly in the ExecStart line. This isn't forbidden in the > guidelines, so this isn't a blocker to the review, just a suggestion. If > you insist on keeping it, you'll need to at least fix the EnvironmentFile > path in the unit file, as it currently reads /etc/default/tailscaled, while > your file is installed to /etc/sysconfig/tailscaled. > As this would be the only way to configure the daemon for various non-default modes, I agree with fixing the environmentfile path. > ============================================================================= > === > > This spec file contains both tailscale and tailscaled binaries. Should > those be placed into separate subpackages? I'm not very familiar with how > tailscale works, so disregard this if it is expected for both of these > binaries to always be installed together. > tailscale is the client for the tailscaled daemon, so they should be together. > ============================================================================= > === > > The PATENTS and cmd/tailscale/cli/licenses.go files are not license files, > and shouldn't be marked as such in %files. The PATENTS file may make sense > as a %doc file, but the go source file probably shouldn't be included at > all. There are also several files included as %doc that don't seem > appropriate (k8s, sysv, derp, windows, etc). Please review these and only > include relevant documentation. > > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation > It's probably fine to put PATENTS in licenses, but otherwise this is correct. > ============================================================================= > === > > The way this is currently being built, the version subcommand has faulting > output. > > 1.68.0-ERR-BuildInfo > go version: go1.22.3 > > Upstream specficially requests that distributions build in a way that > preserves the version info. > > https://github.com/tailscale/tailscale/blob/v1.68.1/README.md#building > > If I'm reading their source correctly, I think you can embed the version > correctly by putting this in %build before the compilation. > > export LDFLAGS="-X tailscale.com/version.longStamp=%{version} -X > tailscale.com/version.shortStamp=%{version}" > Please use "%{version}-%{release}" so that each build is easily identified. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2292255 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202292255%23c6 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
