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

Add documentation on how to build a hotfix #505

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

agrare
Copy link
Member

@agrare agrare commented Sep 6, 2024

TODO:

  • Example of creating a patch with git format-patch and fixing up the directory prefixes to work with the builddir

build a hotfix against, for example `manageiq-release-18.0-1.el9.src.rpm`

```sh
docker run --rm -v `pwd`/OPTIONS:/root/OPTIONS -v `pwd`/BUILD:/root/BUILD $USER/rpm_build:radjabov-hotfix build_hotfix
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE instead of a separate volume map for the hotfix dir it seemed easier to just map ./BUILD so the artifacts and the srpm are in one place but if this doesn't work for some reason I can change it

Copy link
Member

Choose a reason for hiding this comment

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

I thought we already had this documented, but maybe not. @bdunne Please review.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I didn't see any so this started as me just taking notes on what I had to do so we had something to reference in the future

Copy link
Member

@kbrock kbrock Sep 13, 2024

Choose a reason for hiding this comment

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

Having the docker command here is helpful

So much time goes between needing to build rpms that I often forget.

Re: "I thought we already had this documented"

On the mac, mapping the BUILD volume was horribly slow. Think I mapped BUILD/rpm to speed it up.

I remember putting together documentation on this a while back but people seemed to disagree on the best approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

On the mac, mapping the BUILD volume was horribly slow. Think I mapped BUILD/rpm to speed it up.

I remember putting together documentation on this a while back but people seemed to disagree on the best approach.

Oh did you have hotfix documentation? Or you mean documentation on how to build rpms on a mac?

Copy link
Member

@jrafanie jrafanie Jan 2, 2025

Choose a reason for hiding this comment

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

My only addition is that I needed to manually clean the hotfix directory of everything but the srpm. It was listing the patch file multiple times in the rpm spec. I'll open a PR. We should just clean it before we copy over things.

EDIT: Shockingly, I didn't get it to patch the files the first time, so I was getting:

error: patch 1 defined multiple times

I cleaned up the directory manually but then decided to fix the code so I'll open a PR with my changes. We can discuss it there.

EDIT 2:

Opened a PR to clean the hotfix directory to resolve this problem: #537

Copy link
Member

Choose a reason for hiding this comment

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

sorry, one last thing...

Note, I couldn't actually build a hotfix patch manageiq srpm.

Everything else seems to be working, although the manageiq release value of 1 in the spec doesn't line up with the expectation of a date format seen here. I had to use the product srpm to actually test building a hotfix.

@Fryguy Fryguy added documentation Improvements or additions to documentation wip labels Sep 6, 2024
@agrare agrare changed the title [WIP] Add documentation on how to build a hotfix Add documentation on how to build a hotfix Sep 9, 2024

Which will create a patch file: `0001-Merge-pull-request-23123-from-agrare-fix_miq_request.patch`

Next we have to change the file location to match the RPM BUILDDIR, for a patch in the core rpm this will be e.g. `manageiq-core-18.0-1`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not totally sure what you did here.

Can you show us a diff of the fix_miq_request.patch?
Was it just a change to first line?

Yes, meta, a diff of a diff, but just trying to understand what was done here.
or maybe show it as a sed command?

Copy link
Member

Choose a reason for hiding this comment

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

We also need to mention that you should remove any spec files from the patches.

Copy link
Member

Choose a reason for hiding this comment

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

@agrare Can you add the note about removing specs from the patch file?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 done

README.md Outdated Show resolved Hide resolved
@miq-bot miq-bot added the stale label Dec 30, 2024
@miq-bot
Copy link
Member

miq-bot commented Dec 30, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@agrare agrare force-pushed the add_hotfix_documentation branch from f665fcd to 0c18222 Compare December 30, 2024 13:36
@Fryguy Fryguy removed the wip label Jan 2, 2025
@Fryguy
Copy link
Member

Fryguy commented Jan 2, 2025

@bdunne Please review.

@Fryguy Fryguy removed the stale label Jan 2, 2025
@agrare agrare force-pushed the add_hotfix_documentation branch from 0c18222 to 466224a Compare January 2, 2025 16:49
This ensures that the hotfix patch will apply correctly when it is run. Then move that patch
file to `rpm_spec/patches/` in the rpm_build repo.

`docker build --pull --tag $USER/rpm_build:radjabov-hotfix .`
Copy link
Member

Choose a reason for hiding this comment

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

FYI, I needed to supply the arch when building on my mac since we don't have aarch64 rpms in our manageiq rpm repository and my platform isn't the most common target arch: x86_64:

 podman build --pull --arch=x86_64 --tag $USER/rpm_build:radjabov-hotfix .

Copy link
Member Author

Choose a reason for hiding this comment

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

That's probably an issue for all of the docker examples in this README. Maybe we split out a PR to update all of them to work on a mac?

Copy link
Member

Choose a reason for hiding this comment

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

oh for sure... I'm trying to validate it since I'm in this neck of the woods anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is podman the way to go on mac silicon macs now? Maybe we can drop the docker commands here now too

Copy link
Member

Choose a reason for hiding this comment

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

Not sure. That's what I'm using. 😉

Copy link
Member

Choose a reason for hiding this comment

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

FYI, the next command also worked with podman:

podman run --rm -v `pwd`/OPTIONS:/root/OPTIONS -v `pwd`/BUILD:/root/BUILD $USER/rpm_build:radjabov-hotfix build_hotfix

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah would be awesome if we could get one set of instructions that work for all, don't want to end up with another developer_setup with mac+ubuntu+centos yum+centos dnf for everything

Copy link
Member

Choose a reason for hiding this comment

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

I'm using podman desktop, but docker is still "common"

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

I'm good with this documentation. It was very helpful for testing it.

podman or docker, it doesn't matter, it's easy enough to replace them vice versa. We can decide on that later if needed.

@Fryguy Fryguy merged commit 5906272 into ManageIQ:master Jan 3, 2025
4 checks passed
@Fryguy Fryguy assigned Fryguy and unassigned bdunne Jan 3, 2025
@agrare agrare deleted the add_hotfix_documentation branch January 3, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants