Bug 2238191 - Review Request: golang-github-bazelbuild-bazelisk - A user-friendly launcher for Bazel
Summary: Review Request: golang-github-bazelbuild-bazelisk - A user-friendly launcher ...
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:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-09-09 23:44 UTC by Daniel Milnes
Modified: 2024-10-23 13:21 UTC (History)
4 users (show)

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


Attachments (Terms of Use)
The .spec file difference from Copr build 6835097 to 6840026 (1.08 KB, patch)
2023-12-31 19:21 UTC, Fedora Review Service
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Fedora Pagure releng issue 11668 0 None None None 2023-09-12 23:07:05 UTC

Description Daniel Milnes 2023-09-09 23:44:23 UTC
Spec URL: https://raw.githubusercontent.com/thebeanogamer/golang-github-bazelbuild-bazelisk/main/bazelisk.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/thebeanogamer/golang-github-bazelbuild-bazelisk/fedora-rawhide-x86_64/06390779-bazelisk/bazelisk-1.18.0-1.fc40.src.rpm
Description: A user-friendly launcher for Bazel
Fedora Account System Username: thebeanogamer
COPR Build: https://copr.fedorainfracloud.org/coprs/thebeanogamer/golang-github-bazelbuild-bazelisk/build/6390779/
Depends On: https://pagure.io/releng/issue/11668

This package is mostly generated by go2rpm, although I have had to add a small conditional to work around an upstream limitation.

I wasn't entirely sure about the package name, given that this is a command-line tool. Happy to revert to %{goname} if that's preferred.

Comment 1 Maxwell G 2023-09-11 17:00:24 UTC
I'll review this once the situation with the dependency is resolved.

Comment 3 Maxwell G 2023-10-04 04:10:44 UTC
I'm busy this week, but I'll try to take a look as soon as I can. Please avoid needinfo'ing me.

Comment 4 Tom Rix 2023-12-30 15:22:20 UTC
How is this useful if bazel is not itself a fedora package ?

There is a newer version.

Use a better %description

Bazelisk is a wrapper for Bazel written in Go. It automatically picks a good version of Bazel given your current working directory, downloads it from the official server (if required) and then transparently passes through all command-line arguments to the real Bazel binary. You can call it just like you would call Bazel.

There are several other issues raised by fedora-review.

Comment 5 Daniel Milnes 2023-12-30 17:30:09 UTC
Hey Tom, thanks for looking at this.

> How is this useful if bazel is not itself a fedora package ?

Bazelisk is effectively a package manager. It will download Bazel from the official sources and install it. Whilst I'd love to package Bazel itself for Fedora, that is a substantially more complex and brittle package to maintain. I'd personally argue this is equivalent to us packaging `npm`, as a way to download something that's not available as an RPM.

> There is a newer version.

Apologies, the new release didn't exist when I originally made this review. I've updated the package:

Spec URL: https://raw.githubusercontent.com/thebeanogamer/golang-github-bazelbuild-bazelisk/main/golang-github-bazelbuild-bazelisk.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/thebeanogamer/golang-github-bazelbuild-bazelisk/fedora-rawhide-x86_64/06834891-golang-github-bazelbuild-bazelisk/golang-github-bazelbuild-bazelisk-1.19.0-1.fc40.src.rpm

[fedora-review-service-build]

> Use a better %description

Updated, see spec above.

> There are several other issues raised by fedora-review.

Sorry, but I can't see these. The only failure I get is "Reviewer should test that the package builds in mock.", which has been broken for a long time. Could you link me a fedora-review output with the failures you're seeing so I can fix them please?

Having re-read the Golang packaging guide since I submitted this request, I've change the RPM name to golang-github-bazelbuild-bazelisk. Please let me know if this was in error.

Comment 6 Fedora Review Service 2023-12-30 20:54:55 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6835097
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2238191-golang-github-bazelbuild-bazelisk/fedora-rawhide-x86_64/06835097-golang-github-bazelbuild-bazelisk/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

Comment 7 Daniel Milnes 2023-12-30 22:06:29 UTC
> Sorry, but I can't see these. The only failure I get is "Reviewer should test that the package builds in mock.", which has been broken for a long time. Could you link me a fedora-review output with the failures you're seeing so I can fix them please?

Nevermind, I was being silly and missed the rpmlint errors. Here's fixed versions:

Spec URL: https://raw.githubusercontent.com/thebeanogamer/golang-github-bazelbuild-bazelisk/main/golang-github-bazelbuild-bazelisk.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/thebeanogamer/golang-github-bazelbuild-bazelisk/fedora-rawhide-x86_64/06835136-golang-github-bazelbuild-bazelisk/golang-github-bazelbuild-bazelisk-1.19.0-1.fc40.src.rpm

[fedora-review-service-build]

There are a few I can't fix:

> golang-github-bazelbuild-bazelisk.x86_64: W: no-manual-page-for-binary bazelisk

Upstream doesn't provide a manpage, and we can't run `help2man bazelisk` without an internet connection. Best I can do is `%doc README.md`.

> golang-github-bazelbuild-bazelisk-devel.noarch: W: hidden-file-or-dir /usr/share/gocode/src/github.com/bazelbuild/bazelisk/.goipath
> golang-github-bazelbuild-bazelisk-debugsource.x86_64: E: description-line-too-long This package provides debug sources for package golang-github-bazelbuild-bazelisk.

Every Go project seems to have these, though if you can point me at a fix I'd be happy to apply it.

Comment 8 Tom Rix 2023-12-31 13:05:28 UTC
(In reply to Daniel Milnes from comment #5)
> Hey Tom, thanks for looking at this.
> 
> > How is this useful if bazel is not itself a fedora package ?
> 
> Bazelisk is effectively a package manager. It will download Bazel from the
> official sources and install it. Whilst I'd love to package Bazel itself for
> Fedora, that is a substantially more complex and brittle package to
> maintain. I'd personally argue this is equivalent to us packaging `npm`, as
> a way to download something that's not available as an RPM.

How does this help with packaging projects in fedora that use bazel ?

Comment 9 Daniel Milnes 2023-12-31 16:02:43 UTC
(In reply to Tom Rix from comment #8)
> (In reply to Daniel Milnes from comment #5)
> > Hey Tom, thanks for looking at this.
> > 
> > > How is this useful if bazel is not itself a fedora package ?
> > 
> > Bazelisk is effectively a package manager. It will download Bazel from the
> > official sources and install it. Whilst I'd love to package Bazel itself for
> > Fedora, that is a substantially more complex and brittle package to
> > maintain. I'd personally argue this is equivalent to us packaging `npm`, as
> > a way to download something that's not available as an RPM.
> 
> How does this help with packaging projects in fedora that use bazel ?

It doesn't, Bazelisk is an end-user tool. It's designed to allow people who have created repos that use Bazel to easily manage multiple versions of it. Without Bazelisk, the only real way to install Bazel is `curl -O /usr/local/bin/bazel https...`.

This is why I think it's comparable to `npm`. We can't use it as part of an RPM spec, but it's very helpful to end-users who are working with Bazel.

I don't think there's too much we can do to ease packaging projects which use Bazel for RPM. Bazel is designed to be hermetically-sealed and downloads its project dependencies to vendored directories, which is the opposite of Fedora's "Everything should come through the package manager" approach. I'm happy to have a discussion about that, but I think it's better suited to Matrix (thebeanogamer:fedora.im).

Comment 10 Fedora Review Service 2023-12-31 19:21:33 UTC
Created attachment 2006689 [details]
The .spec file difference from Copr build 6835097 to 6840026

Comment 11 Fedora Review Service 2023-12-31 19:21:36 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6840026
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2238191-golang-github-bazelbuild-bazelisk/fedora-rawhide-x86_64/06840026-golang-github-bazelbuild-bazelisk/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

Comment 12 Tom Rix 2024-01-02 14:27:46 UTC
why was the specfile name changed ?

why is there a devel package when no libraries are distributed ?

There is this lingering rpmlint warning
golang-github-bazelbuild-bazelisk.x86_64: W: unused-direct-shlib-dependency /usr/bin/bazelisk /lib64/libresolv.so.2

Comment 13 Daniel Milnes 2024-01-14 15:57:29 UTC
(In reply to Tom Rix from comment #12)
> why was the specfile name changed ?
Having re-read the Golang Packaging Guidelines, I think I was wrong with my original name for this package. I've renamed it to the version generated by the %{goname} macro.

> why is there a devel package when no libraries are distributed ?
As far as I can tell, this is the case for all Golang packages (for example: https://koji.fedoraproject.org/koji/buildinfo?buildID=2085464). I can't see any way to remove the -devel package outside of rewriting the spec file to not use any of the %go macros. All the examples around go2rpm and go-rpm-macros I can see have this behaviour. 

> golang-github-bazelbuild-bazelisk.x86_64: W: unused-direct-shlib-dependency /usr/bin/bazelisk /lib64/libresolv.so.2
This also appears to happen for all Golang packages. Interestingly, rpmlint doesn't flag this for me, but manually running `ldd -u` does. I'm not entirely sure why that happens and will research further.

Comment 14 Daniel Milnes 2024-01-14 16:07:44 UTC
(In reply to Daniel Milnes from comment #13)
> (In reply to Tom Rix from comment #12)
> > why was the specfile name changed ?
> Having re-read the Golang Packaging Guidelines, I think I was wrong with my
> original name for this package. I've renamed it to the version generated by
> the %{goname} macro.

Just to add to this, the packaging guide says the following:

> Source packages that provide a well-known application such as etcd MUST be named after the application. End users do not care about the language their applications are written in. But do not name packages after an obscure utility binary that happens to be built by the package.

I don't think this qualifies as "well-known", as even Prometheus doesn't have its package named like this. I'm happy to change it back through if you feel it does meet this definition.

Comment 15 Tom Rix 2024-01-14 18:07:57 UTC
(In reply to Daniel Milnes from comment #14)
> (In reply to Daniel Milnes from comment #13)
> > (In reply to Tom Rix from comment #12)
> > > why was the specfile name changed ?
> > Having re-read the Golang Packaging Guidelines, I think I was wrong with my
> > original name for this package. I've renamed it to the version generated by
> > the %{goname} macro.
> 
> Just to add to this, the packaging guide says the following:
> 
> > Source packages that provide a well-known application such as etcd MUST be named after the application. End users do not care about the language their applications are written in. But do not name packages after an obscure utility binary that happens to be built by the package.
> 
> I don't think this qualifies as "well-known", as even Prometheus doesn't
> have its package named like this. I'm happy to change it back through if you
> feel it does meet this definition.

As someone that has tried to find the tools to build TensorFlow, I think the earlier name is better.
I will leave this up to you.

Comment 16 Petr Pisar 2024-10-23 13:21:54 UTC
The reviewer's account was closed.


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