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.

Sign up for GitHub

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

Jump to bottom

Fedora/RHEL packaging #8144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fedora/RHEL packaging #8144

wants to merge 1 commit into from

Conversation

pgarciaq
Copy link

Add dependencies to build the RPM for Red Hat operating systems: Fedora, RHEL, etc.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 24, 2025
View reviewed changes
packaging/linux/rancher-desktop.spec Fixed Show fixed Hide fixed
packaging/linux/rancher-desktop.spec Fixed Show fixed Hide fixed
packaging/linux/rancher-desktop.spec Fixed Show fixed Hide fixed
@jandubois
Copy link
Member

Let's exclude the spec file from the spelling checker! Please add it to .github/actions/spelling/excludes.txt.

@jandubois
Copy link
Member

Or maybe add the 3 missing words to .github/actions/spelling/expect.txt. I thought there would be more.

@mook-as
Copy link
Contributor

mook-as commented Jan 25, 2025

I tried a test build, but that failed: https://build.opensuse.org/package/live_build_log/home:mook_work:branches:isv:Rancher:dev/rancher-desktop-main/Fedora_41/x86_64

That's mostly because the fedora rpmlint wants a build-id in our golang binaries, though, and we're not providing any:


[   41s] RPM build errors:
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/lima/bin/limactl
[   41s]     build-id found in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/docker-credential-secretservice too small
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/docker-credential-ecr-login
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/rdctl
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/kuberlr
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/docker-credential-none
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/docker-credential-pass
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/helm
[   41s]     build-id found in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/bin/docker too small
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/internal/trivy
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/internal/spin
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/internal/containerd-shim-spin-v2
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/internal/steve
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/docker-cli-plugins/docker-buildx
[   41s]     Missing build-id in /home/abuild/rpmbuild/BUILD/rancher-desktop-9.main.1737761866.6dd61f2-build/BUILDROOT/opt/rancher-desktop/resources/resources/linux/docker-cli-plugins/docker-compose
[   41s]     Generating build-id links failed

So this PR is probably okay as-is, but we can't fully test it (and we can't ship them).

@jandubois
Copy link
Member

That's mostly because the fedora rpmlint wants a build-id in our golang binaries, though, and we're not providing any:

How about just telling RPM to ignore them?

%global _missing_build_ids_terminate_build 0

Or does that have other problems?

@jandubois
Copy link
Member

@pgarciaq Your commit is also missing a DCO line: https://github.com/rancher-sandbox/rancher-desktop/pull/8144/checks?check_run_id=36153659244

jandubois
jandubois previously approved these changes Jan 25, 2025
View reviewed changes
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@mook-as Can you do another test build (and let me know where I can pick up the RPM, assuming it succeeds)?

@pgarciaq
Copy link
Author

@jandubois I think I've fixed the build error with this commit:
pgarciaq@4d5cda4

But I also think I've screwed up my git repository by inadvertently merging two branches, so maybe I should re-work my PR.

@jandubois
Copy link
Member

But I also think I've screwed up my git repository by inadvertently merging two branches, so maybe I should re-work my PR.

I was surprised to see you merge main into your branch, but it looks like it was a no-op, and the PR looks fine to me. Let's see what @mook-as says...

@mook-as
Copy link
Contributor

mook-as commented Jan 27, 2025 •
edited
Loading

The build succeeds, at least on commit 506edd7: https://build.opensuse.org/package/live_build_log/home:mook_work:branches:isv:Rancher:dev/rancher-desktop-main/Fedora_41/x86_64

(FWIW, there's no real need to be blocked on me making builds, since anybody can fork isv:Rancher:dev too…)

@jandubois
Copy link
Member

@pgarciaq Fails with one missing dependency:

Problem: conflicting requests
  - nothing provides password-store needed by rancher-desktop-9.main.1737770895.506edd78-2961.1.x86_64 from @commandline

I'm also wondering what the plan is to eventually publish this. Would you include it in the Fedora repos, or do you expect it to be published by SUSE or OBS?

@pgarciaq
Copy link
Author

But I also think I've screwed up my git repository by inadvertently merging two branches, so maybe I should re-work my PR.

I was surprised to see you merge main into your branch, but it looks like it was a no-op, and the PR looks fine to me. Let's see what @mook-as says...

That was a mistake. Developing at 2am is not a great idea O:-)

@pgarciaq Fails with one missing dependency:

Problem: conflicting requests
  - nothing provides password-store needed by rancher-desktop-9.main.1737770895.506edd78-2961.1.x86_64 from @commandline

I'm also wondering what the plan is to eventually publish this. Would you include it in the Fedora repos, or do you expect it to be published by SUSE or OBS?

I built the package here:
https://build.opensuse.org/package/show/home:pgarciaq:branches:isv:Rancher:stable/password-store

It'd be nice if Fedora/RHEL provided that but for now, I'd really appreciate if it would be available from the same repository where the Rancher Desktop RPM will be available from.

@mook-as
Copy link
Contributor

mook-as commented Jan 27, 2025

I believe the equivalent package is called pass. See also upstream installation instructions.

@pgarciaq
Copy link
Author

I believe the equivalent package is called pass. See also upstream installation instructions.

Ah indeed, great catch. I looked for it but didn't find it. I'll update my PR.

@pgarciaq
Fedora/RHEL packaging
c7f2ced 
Signed-off-by: Pau Garcia Quiles <[email protected]>
@pgarciaq
Copy link
Author

PR is updated:

  • Use the right package for password-store
  • Use Requires: gpg2 intead of BuildRequires, which I had previously mistakenly introduced
  • Got rid of the merge commit and squashed all my commits into a single one

This is cleanear now.

@mook-as mook-as mentioned this pull request Jan 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jandubois jandubois jandubois left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pgarciaq @jandubois @mook-as