PR#532: The %forgeversion and other RPM macros cannot guess the commit date if they don’t have access to the corresponding tarball - FedoraReview

FedoraReview

#532 The %forgeversion and other RPM macros cannot guess the commit date if they don’t have access to the corresponding tarball

Opened 3 months ago by sergiomb. Modified 3 months ago

sergiomb/FedoraReview forgesource_fix into master

The %forgeversion and other RPM macros cannot guess the commit date if they don’t have access to the corresponding tarball

Sérgio M. Basto • 3 months ago

2bcc2c5

src/FedoraReview/checks.py

file modified

+2 -1

		`@@ -284,9 +284,10 @@`
		`unless invoked from ChecksLister.`
		`"""`
		`_ChecksLoader.__init__(self)`
		`- self.spec = SpecFile(spec_file, self.flags)`
		`+ # The %forgeversion and other RPM macros cannot guess the commit date if they don’t have access to the corresponding tarball`
		`self.srpm = SRPMFile(srpm_file)`
		`self.srpm.unpack()`
		`+ self.spec = SpecFile(spec_file, self.flags)`
		`self.data = self.Data()`
		`self.data.rpms = RpmDataSource(self.spec)`
		`self.data.buildsrc = BuildFilesSource()`

src/FedoraReview/spec_file.py

file modified

+10 -7

		`@@ -25,6 +25,7 @@`
		`import rpm`

		`from .mock import Mock`
		`+ from .review_dirs import ReviewDirs`
		`from .review_error import ReviewError, SpecParseReviewError`
		`from .rpm_compat import spec`
		`from .settings import Settings`
		`@@ -67,11 +68,20 @@`
		`if not expanded.startswith("%"):`
		`rpm.delMacro(macro[1:])`
		`rpm.addMacro(macro[1:], expanded)`
		`+ # The %forgeversion and other RPM macros cannot guess the commit date if they don’t have access to the corresponding tarball`
		`+ rpm.addMacro("_sourcedir", ReviewDirs.srpm_unpacked)`

		`def parse_spec():`
		`"""Let rpm parse the spec and build spec.spec (sic!)."""`
		`+ """Once the folder review-*/srpm-unpacked contains the tarball, running this function again will let RPM resolve %forgeversion properly. At that point, the version string will include the commit date and you can capture it into variables."""`
		`try:`
		`self.spec = spec(self.filename)`
		`+ self.name_vers_rel = [`
		`+ self.expand_tag(rpm.RPMTAG_NAME),`
		`+ self.expand_tag(rpm.RPMTAG_VERSION),`
		`+ "*",`
		`+ ]`
		`+ self.name_vers_rel[2] = self.expand_tag(rpm.RPMTAG_RELEASE)`
		`except Exception as ex:`
		`raise SpecParseReviewError("Can't parse specfile: " + ex.__str__())`

		`@@ -82,17 +92,10 @@`
		`self._process_fonts_pkg()`
		`stdout = sys.stdout`
		`sys.stdout = _Null()`
		`- parse_spec()`
		`sys.stdout = stdout`
		`self._packages = None`
		`- self.name_vers_rel = [`
		`- self.expand_tag(rpm.RPMTAG_NAME),`
		`- self.expand_tag(rpm.RPMTAG_VERSION),`
		`- "*",`
		`- ]`
		`update_macros()`
		`parse_spec()`
		`- self.name_vers_rel[2] = self.expand_tag(rpm.RPMTAG_RELEASE)`

		`name = property(lambda self: self.name_vers_rel[0])`
		`version = property(lambda self: self.name_vers_rel[1])`

sergiomb commented 3 months ago

So, _sourcedir must point to the directory where this tarball is located. RPM reads the filename, extracts the hash and the commit date (if present), and constructs the full version, that’s why correctly setting _sourcedir is crucial.
Without the tarball, %forgeversion can only include the hash, resulting in ERROR:
'No srpm found for (...)' (logs in /home/sergio/.cache/fedora-review.log)
or
'More than one srpm found for:

Fixes #519

sergiomb commented 3 months ago

@gotmax23 since you are the person which implemented the forge macros https://fedoraproject.org/wiki/Changes/Revitalize_Forge_Macros , you may help on validate this MR

Best regards, thank you

rebased onto 771afff

3 months ago