Conversation
There was a problem hiding this comment.
Pull request overview
Adds/updates RPM spec content under base/comps/, ostensibly to introduce a go-md2man spec and tweak a TODO comment in azurelinux-release.
Changes:
- Added a new local RPM spec file for
go-md2man. - Updated the
eol_dateTODO comment inazurelinux-release.spec.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| base/comps/go-md2man/go-md2man.spec | Introduces a new local spec for building/installing go-md2man. |
| base/comps/azurelinux-release/azurelinux-release.spec | Updates a TODO comment related to the eol_date macro. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Summary: Converts markdown into roff (man pages) | ||
| Name: go-md2man | ||
| Version: 2.0.2 | ||
| Release: 1%{?dist} |
There was a problem hiding this comment.
This spec file is added without any corresponding component definition (e.g., base/comps/go-md2man/go-md2man.comp.toml or an entry in base/comps/components.toml). As-is, go-md2man will not be picked up by includes = ["**/*.comp.toml"], so it won’t build as part of the distro. Add a .comp.toml for the component (or otherwise wire it into the component set), or move/remove this file if it’s only intended as a test artifact.
| Summary: Converts markdown into roff (man pages) | ||
| Name: go-md2man | ||
| Version: 2.0.2 | ||
| Release: 1%{?dist} | ||
| License: MIT | ||
| Vendor: Microsoft Corporation | ||
| Distribution: Azure Linux | ||
| Group: Tools/Container | ||
| URL: https://github.com/cpuguy83/go-md2man | ||
| Source0: https://github.com/cpuguy83/go-md2man/archive/v%{version}.tar.gz#/go-md2man-%{version}.tar.gz |
There was a problem hiding this comment.
Repository convention is to avoid forking/hand-coding specs for upstream packages and instead use upstream specs + overlays; local specs are currently only used for Azure Linux-originating components (e.g., azurelinux-release, azurelinux-repos). If go-md2man is not Azure Linux-originating, please switch this to an upstream component with overlays rather than introducing a local .spec.
| Vendor: Microsoft Corporation | ||
| Distribution: Azure Linux |
There was a problem hiding this comment.
Vendor: and Distribution: appear again after %description. At that point they will be treated as part of the package description text (not header tags), which is likely unintended and duplicates the header metadata. Remove these lines or move any intended tag definitions into the header section.
| Vendor: Microsoft Corporation | |
| Distribution: Azure Linux |
| URL: https://github.com/cpuguy83/go-md2man | ||
| Source0: https://github.com/cpuguy83/go-md2man/archive/v%{version}.tar.gz#/go-md2man-%{version}.tar.gz | ||
| BuildRequires: golang | ||
| BuildRequires: which |
There was a problem hiding this comment.
BuildRequires: which is not used anywhere in this spec (no invocation of which in build/install/check). Please drop the unused BuildRequires to keep the dependency set minimal.
| BuildRequires: which |
| %setup -q -n %{name}-%{version} -c | ||
|
|
||
| %build | ||
| export GOPATH=%{OUR_GOPATH} | ||
| export GOCACHE=%{OUR_GOPATH}/.cache | ||
| export CGO_ENABLED=0 | ||
| export GO111MODULE=on | ||
|
|
||
| cd %{_topdir}/BUILD/%{name}-%{version}/go-md2man-%{version} | ||
| go build -mod vendor -o go-md2man | ||
|
|
||
| %install | ||
| mkdir -p "%{buildroot}%{_bindir}" | ||
| cp -aT go-md2man-%{version}/go-md2man %{buildroot}%{_bindir}/go-md2man | ||
|
|
||
| # copy legal files | ||
| mkdir -p %{buildroot}%{_docdir}/%{name}-%{version} | ||
| cp go-md2man-%{version}/LICENSE.md %{buildroot}%{_docdir}/%{name}-%{version}/LICENSE.md |
There was a problem hiding this comment.
The %setup ... -c + absolute cd %{_topdir}/BUILD/... approach creates a nested source dir and hard-codes build paths, making the spec harder to maintain and more brittle across build environments. Prefer a single top-level source dir and relative/standard macros (e.g., %{_builddir}) so subsequent sections don’t depend on the nested layout.
| %setup -q -n %{name}-%{version} -c | |
| %build | |
| export GOPATH=%{OUR_GOPATH} | |
| export GOCACHE=%{OUR_GOPATH}/.cache | |
| export CGO_ENABLED=0 | |
| export GO111MODULE=on | |
| cd %{_topdir}/BUILD/%{name}-%{version}/go-md2man-%{version} | |
| go build -mod vendor -o go-md2man | |
| %install | |
| mkdir -p "%{buildroot}%{_bindir}" | |
| cp -aT go-md2man-%{version}/go-md2man %{buildroot}%{_bindir}/go-md2man | |
| # copy legal files | |
| mkdir -p %{buildroot}%{_docdir}/%{name}-%{version} | |
| cp go-md2man-%{version}/LICENSE.md %{buildroot}%{_docdir}/%{name}-%{version}/LICENSE.md | |
| %setup -q -n go-md2man-%{version} | |
| %build | |
| cd go-md2man-%{version} | |
| export GOPATH=%{OUR_GOPATH} | |
| export GOCACHE=%{OUR_GOPATH}/.cache | |
| export CGO_ENABLED=0 | |
| export GO111MODULE=on | |
| go build -mod vendor -o go-md2man | |
| %install | |
| cd go-md2man-%{version} | |
| mkdir -p "%{buildroot}%{_bindir}" | |
| cp -aT go-md2man %{buildroot}%{_bindir}/go-md2man | |
| # copy legal files | |
| mkdir -p %{buildroot}%{_docdir}/%{name}-%{version} | |
| cp LICENSE.md %{buildroot}%{_docdir}/%{name}-%{version}/LICENSE.md |
| %define is_development 1 | ||
|
|
||
| # TODO(azl): review | ||
| # TODO(azl): review -- set to a future date for now, fix before release |
There was a problem hiding this comment.
The comment says “set to a future date for now”, but eol_date is currently 1900-01-01 (a past date). Please update the comment to match the current placeholder strategy, or update eol_date if the intent really is to use a future placeholder.
| # TODO(azl): review -- set to a future date for now, fix before release | |
| # TODO(azl): review -- placeholder EOL date; 1900-01-01 indicates EOL not yet finalized |
4f86d31 to
55c64ba
Compare
❌ Spec Review Failed
🛠️ Debug locally: .github/workflows/scripts/README.md
|
Merge Checklist
All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)
*-staticsubpackages, etc.) have had theirReleasetag incremented../cgmanifest.json,./toolkit/scripts/toolchain/cgmanifest.json,.github/workflows/cgmanifest.json)./LICENSES-AND-NOTICES/SPECS/data/licenses.json,./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md,./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)*.signatures.jsonfilessudo make go-tidy-allandsudo make go-test-coveragepassSummary
What does the PR accomplish, why was it needed?
Change Log
Does this affect the toolchain?
YES/NO
Associated issues
Links to CVEs
Test Methodology