Skip to content

Conversation

@mapitman
Copy link
Owner

@mapitman mapitman commented Dec 15, 2025

Changes

RPM Packaging

  • Add RPM spec file (mdview.spec) for Fedora packaging
  • Add Makefile targets for local and CI RPM builds
  • Sanitize VERSION for RPM builds (replace illegal characters)

CI Improvements

  • Add container-based CI simulation scripts for Ubuntu and Fedora builds
  • Update release workflow to build and publish both DEB and RPM packages
  • Remove duplicate rpm-build.yml workflow (consolidated into release workflow)
  • Stop building RPMs on every push (only on releases)
  • Fix git safe.directory issue in containerized builds

Scripts

  • Add scripts/ci-sim-ubuntu.sh for local Ubuntu build testing
  • Add scripts/ci-sim-fedora.sh for local Fedora RPM build testing
  • Add make ci-sim targets to simulate CI builds locally

Documentation

  • Create PACKAGING.md consolidating all packaging documentation (DEB + RPM)
  • Add RPM installation section to README.md
  • Update RELEASE_CHECKLIST.md to be package-agnostic
  • Delete 5 redundant RPM documentation files (~1000 lines of duplicate content)
  • Reduce total documentation from 9 markdown files to 4

Release Artifacts

Release workflow now publishes:

  • Platform binaries (Linux, Darwin, Windows, FreeBSD)
  • Debian packages (amd64, arm64, i386)
  • RPM packages (src.rpm, x86_64.rpm)

All changes have been tested with a temporary tag and verified that RPMs are correctly built and published as release assets.

Copilot AI review requested due to automatic review settings December 15, 2025 00:05
- Create PACKAGING.md consolidating 5 redundant RPM docs
- Add RPM installation section to README.md
- Update RELEASE_CHECKLIST.md to be package-agnostic
- Delete redundant files:
  - RPM_GUIDE.md
  - RPM_AUTOMATION_SUMMARY.md
  - RPM_BUILD_QUICK_REFERENCE.md
  - RPM_BUILD_AUTOMATION.md
  - FEDORA_PACKAGING.md

Result: 9 markdown files → 4 markdown files
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive RPM packaging support for Fedora Linux and significantly improves the CI/CD workflows by restructuring the release process into separate jobs and adding local CI simulation capabilities.

Key Changes:

  • Adds complete RPM packaging infrastructure with spec file, helper scripts, and Makefile targets for building Fedora packages
  • Restructures GitHub Actions workflows to use separate jobs for binaries and RPM builds, consolidating everything into the release workflow
  • Introduces Docker-based CI simulation scripts for local testing of Ubuntu and Fedora builds

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 20 comments.

Show a summary per file
File Description
mdview.spec RPM package specification defining build requirements, dependencies, and installation rules
build-rpm.sh Interactive helper script for building RPMs locally with dependency checking and colored output
Makefile Adds RPM build targets (rpm, rpm-local, rpm-setup, rpm-clean) and CI simulation targets with version sanitization
.github/workflows/release.yml Refactored into three jobs (build-binaries, build-rpm, release) for parallel builds and artifact collection
.github/workflows/build.yml Updated to use new version determination logic (though only builds binaries, not RPMs)
scripts/ci-sim-ubuntu.sh CI simulation script for testing Ubuntu builds locally in Docker containers
scripts/ci-sim-fedora.sh CI simulation script for testing Fedora RPM builds locally in Docker containers
RPM_GUIDE.md Comprehensive guide for building and installing RPM packages
RPM_BUILD_AUTOMATION.md Detailed documentation of local and GitHub Actions RPM build automation
RPM_BUILD_QUICK_REFERENCE.md Quick reference for common RPM build commands and workflows
RPM_AUTOMATION_SUMMARY.md High-level summary of the RPM automation system
RELEASE_CHECKLIST.md Step-by-step checklist for releasing new versions with RPM packages
FEDORA_PACKAGING.md Complete guide for Fedora-specific packaging requirements and processes
mdview.1.md Updated version placeholder from hardcoded "1.4.0" to generic "XXX"
.gitignore Added dist/ directory to ignore build artifacts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Makefile Outdated
Comment on lines 97 to 102
RPM_VERSION=$(shell echo "$(VERSION)" | sed 's/[^A-Za-z0-9.+~_:]/./g') ; \
echo "Using RPM version: $$RPM_VERSION (from $(VERSION))" ; \
git archive --prefix=mdview-$$RPM_VERSION/ -o $(HOME)/rpmbuild/SOURCES/mdview-$$RPM_VERSION.tar.gz HEAD ; \
cp mdview.spec $(HOME)/rpmbuild/SPECS/ ; \
rpmbuild -ba -D "_version $$RPM_VERSION" $(HOME)/rpmbuild/SPECS/mdview.spec ; \
echo "RPM build complete. Output in $(HOME)/rpmbuild/RPMS/"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The RPM_VERSION sanitization is performed in a shell subshell and is only available within that line. This means the sanitized version is not consistently available for use in subsequent commands like the rpm-local target. Consider exporting RPM_VERSION as a variable that can be reused across targets to ensure consistency between build and copy operations.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +16
# For manual runs (workflow_dispatch) allow explicit version; otherwise use the ref name
if [ "${{ github.event_name }}" == "workflow_dispatch" ]; then
VERSION="${{ github.event.inputs.version }}"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The workflow checks for workflow_dispatch event but the workflow does not declare workflow_dispatch in its triggers (only push is declared). This means the condition will never be true, and the github.event.inputs.version will never be used. Either add workflow_dispatch to the trigger list or remove this unused conditional logic.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

build-rpm.sh Outdated
Comment on lines 49 to 53
echo -e "${RED}Error: Missing dependencies: ${missing[*]}${NC}"
echo -e "${YELLOW}Install on Fedora:${NC}"
echo " sudo dnf install -y rpm-build golang pandoc make git"
exit 1
fi
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The error message lists missing dependencies but doesn't provide clear guidance when the dependency is available but not detected. For example, if rpm-build is installed but the rpmbuild command is not in PATH, users might be confused. Consider checking for the actual command names (like rpmbuild instead of rpm-build) and providing more specific error messages.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback


bin/linux-amd64/mdview: manpage
env GOOS=linux GOARCH=amd64 go build -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/linux-amd64/mdview
env GOOS=linux GOARCH=amd64 go build -buildvcs=false -ldflags "-X main.appVersion=$(VERSION)" -o ./bin/linux-amd64/mdview
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The -buildvcs=false flag is added to all go build commands. While this is useful for reproducible builds in containerized environments, it removes valuable version control information from the binary. Consider documenting why this flag is necessary or making it conditional based on the build environment (e.g., only in CI or when VCS info is unavailable).

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Copy link
Contributor

Copilot AI commented Dec 15, 2025

@mapitman I've opened a new pull request, #44, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits December 15, 2025 00:28
…rkflow

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Dec 15, 2025

@mapitman I've opened a new pull request, #45, to work on those changes. Once the pull request is ready, I'll request review from you.

mapitman and others added 3 commits December 14, 2025 16:33
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Dec 15, 2025

@mapitman I've opened a new pull request, #46, to work on those changes. Once the pull request is ready, I'll request review from you.

mapitman and others added 11 commits December 14, 2025 16:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Changed from checking package names (rpm-build) to actual commands (rpmbuild)
- Added mapping between command names and package names for Fedora
- Enhanced error messages to be more specific about missing commands
- Added guidance for when packages are installed but commands not in PATH

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Resolved conflicts in Makefile by combining VERSION_SAFE approach
from base branch with corrected tar transform syntax.

Changes:
- Keep VERSION_SAFE variable definition for sanitizing VERSION
- Use VERSION_SAFE in all tar/zip filenames
- Use corrected tar transform syntax with proper sed-style delimiters
- Merge other changes from fedora-package branch
VERSION_SAFE in Makefile now handles slash sanitization, making
the workflow-level sanitization redundant. This approach is cleaner
as the Makefile is the single source of truth for filename sanitization.

Co-authored-by: mapitman <4205286+mapitman@users.noreply.github.com>
Fix build workflow failures, add workflow_dispatch trigger, and resolve merge conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants