[PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH

Neal Gompa posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201026220820.60737-1-ngompa13@gmail.com
libvirt.spec.in | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
[PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Neal Gompa 3 years, 6 months ago
Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
based on the changelog entry date stamp. In scenarios where it already
is defined, we do not want to redefine it.

Additionally, when building the libvirt package in an Open Build Service
instance, the spec file is not present in %_specdir, but instead in %_sourcedir.

Signed-off-by: Neal Gompa <ngompa13@gmail.com>
---
 libvirt.spec.in | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2a4324b974..eb0fba4b5a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1128,7 +1128,16 @@ exit 1
 
 # place macros above and build commands below this comment
 
-export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec)
+if [ -z "${SOURCE_DATE_EPOCH}" ]; then
+# If SOURCE_DATE_EPOCH is not defined, set it based on spec file
+    if [ ! -f "%{_specdir}/%{name}.spec" ]; then
+        # OBS does not install the spec file into the specdir
+        SPECFILE_PATH="%{_sourcedir}/%{name}.spec"
+    else
+        SPECFILE_PATH="%{_specdir}/%{name}.spec"
+    fi
+    export SOURCE_DATE_EPOCH=$(stat --printf='%Y' ${SPECFILE_PATH})
+fi
 
 %meson \
            -Drunstatedir=%{_rundir} \
-- 
2.28.0

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Michal Privoznik 3 years, 6 months ago
On 10/26/20 11:08 PM, Neal Gompa wrote:
> Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> based on the changelog entry date stamp. In scenarios where it already
> is defined, we do not want to redefine it.
> 

This part is okay.

> Additionally, when building the libvirt package in an Open Build Service
> instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> 

But this looks fishy. Is the %_specdir defined in that case?

Michal

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Neal Gompa 3 years, 6 months ago
On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> On 10/26/20 11:08 PM, Neal Gompa wrote:
> > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> > based on the changelog entry date stamp. In scenarios where it already
> > is defined, we do not want to redefine it.
> >
>
> This part is okay.
>
> > Additionally, when building the libvirt package in an Open Build Service
> > instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> >
>
> But this looks fishy. Is the %_specdir defined in that case?
>

It is (that comes from RPM itself), however the directory is empty.



--
真実はいつも一つ!/ Always, there's only one truth!


Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Michal Privoznik 3 years, 6 months ago
On 10/27/20 1:06 PM, Neal Gompa wrote:
> On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>>
>> On 10/26/20 11:08 PM, Neal Gompa wrote:
>>> Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
>>> based on the changelog entry date stamp. In scenarios where it already
>>> is defined, we do not want to redefine it.
>>>
>>
>> This part is okay.
>>
>>> Additionally, when building the libvirt package in an Open Build Service
>>> instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
>>>
>>
>> But this looks fishy. Is the %_specdir defined in that case?
>>
> 
> It is (that comes from RPM itself), however the directory is empty.

That feels like a bug in OBS then. IIUC this macro can be specified on 
the rpmbuild's cmd line. Can't it set the %_specdir to be the same as 
%_sourcedir?

Michal

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Neal Gompa 3 years, 6 months ago
On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> On 10/27/20 1:06 PM, Neal Gompa wrote:
> > On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> >>
> >> On 10/26/20 11:08 PM, Neal Gompa wrote:
> >>> Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> >>> based on the changelog entry date stamp. In scenarios where it already
> >>> is defined, we do not want to redefine it.
> >>>
> >>
> >> This part is okay.
> >>
> >>> Additionally, when building the libvirt package in an Open Build Service
> >>> instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> >>>
> >>
> >> But this looks fishy. Is the %_specdir defined in that case?
> >>
> >
> > It is (that comes from RPM itself), however the directory is empty.
>
> That feels like a bug in OBS then. IIUC this macro can be specified on
> the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
> %_sourcedir?
>

Nothing about RPM mandates that %_specdir is actually *used* for anything.

-- 
真実はいつも一つ!/ Always, there's only one truth!


Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Michal Privoznik 3 years, 6 months ago
On 10/28/20 9:47 PM, Neal Gompa wrote:
> On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>>
>> On 10/27/20 1:06 PM, Neal Gompa wrote:
>>> On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
>>>>
>>>> On 10/26/20 11:08 PM, Neal Gompa wrote:
>>>>> Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
>>>>> based on the changelog entry date stamp. In scenarios where it already
>>>>> is defined, we do not want to redefine it.
>>>>>
>>>>
>>>> This part is okay.
>>>>
>>>>> Additionally, when building the libvirt package in an Open Build Service
>>>>> instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
>>>>>
>>>>
>>>> But this looks fishy. Is the %_specdir defined in that case?
>>>>
>>>
>>> It is (that comes from RPM itself), however the directory is empty.
>>
>> That feels like a bug in OBS then. IIUC this macro can be specified on
>> the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
>> %_sourcedir?
>>
> 
> Nothing about RPM mandates that %_specdir is actually *used* for anything.
> 

But this is not the case, is it? %_specdir is defined and points to an 
actual directory. Having said that, I am not against the change, but 
maybe we can document this weirdness somewhere? Also, with the latest 
specfile discussion I'll let Andrea take a look.

Michal

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Daniel P. Berrangé 3 years, 6 months ago
On Thu, Oct 29, 2020 at 12:27:16PM +0100, Michal Privoznik wrote:
> On 10/28/20 9:47 PM, Neal Gompa wrote:
> > On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > 
> > > On 10/27/20 1:06 PM, Neal Gompa wrote:
> > > > On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > > 
> > > > > On 10/26/20 11:08 PM, Neal Gompa wrote:
> > > > > > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> > > > > > based on the changelog entry date stamp. In scenarios where it already
> > > > > > is defined, we do not want to redefine it.
> > > > > > 
> > > > > 
> > > > > This part is okay.
> > > > > 
> > > > > > Additionally, when building the libvirt package in an Open Build Service
> > > > > > instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> > > > > > 
> > > > > 
> > > > > But this looks fishy. Is the %_specdir defined in that case?
> > > > > 
> > > > 
> > > > It is (that comes from RPM itself), however the directory is empty.
> > > 
> > > That feels like a bug in OBS then. IIUC this macro can be specified on
> > > the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
> > > %_sourcedir?
> > > 
> > 
> > Nothing about RPM mandates that %_specdir is actually *used* for anything.
> > 
> 
> But this is not the case, is it? %_specdir is defined and points to an
> actual directory. Having said that, I am not against the change, but maybe
> we can document this weirdness somewhere? Also, with the latest specfile
> discussion I'll let Andrea take a look.

I'm inclined to just delete all the source epoch stuff from the spec and
rely on the build environment set it if they want reproducable builds.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Neal Gompa 3 years, 6 months ago
On Thu, Oct 29, 2020 at 7:39 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Oct 29, 2020 at 12:27:16PM +0100, Michal Privoznik wrote:
> > On 10/28/20 9:47 PM, Neal Gompa wrote:
> > > On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > >
> > > > On 10/27/20 1:06 PM, Neal Gompa wrote:
> > > > > On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > > >
> > > > > > On 10/26/20 11:08 PM, Neal Gompa wrote:
> > > > > > > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> > > > > > > based on the changelog entry date stamp. In scenarios where it already
> > > > > > > is defined, we do not want to redefine it.
> > > > > > >
> > > > > >
> > > > > > This part is okay.
> > > > > >
> > > > > > > Additionally, when building the libvirt package in an Open Build Service
> > > > > > > instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> > > > > > >
> > > > > >
> > > > > > But this looks fishy. Is the %_specdir defined in that case?
> > > > > >
> > > > >
> > > > > It is (that comes from RPM itself), however the directory is empty.
> > > >
> > > > That feels like a bug in OBS then. IIUC this macro can be specified on
> > > > the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
> > > > %_sourcedir?
> > > >
> > >
> > > Nothing about RPM mandates that %_specdir is actually *used* for anything.
> > >
> >
> > But this is not the case, is it? %_specdir is defined and points to an
> > actual directory. Having said that, I am not against the change, but maybe
> > we can document this weirdness somewhere? Also, with the latest specfile
> > discussion I'll let Andrea take a look.
>

RPM internally populates a specfile variable that is the true pointer
to the spec file, regardless of path. This is not exposed as a macro
despite attempts to do so[1]. There is no rule that the spec file
*must* come from %_specdir, and it's really only used for the location
to store the specfile when extracting an SRPM.

[1]: https://github.com/rpm-software-management/rpm/pull/202

> I'm inclined to just delete all the source epoch stuff from the spec and
> rely on the build environment set it if they want reproducable builds.
>

Considering that RHEL 8 doesn't have the change to turn on
SOURCE_DATE_EPOCH[2], I am inclined to keep it.

[2]: https://src.fedoraproject.org/rpms/redhat-rpm-config/c/86aae600e62fadc18760d95d1fddd323cf9e9a86


-- 
真実はいつも一つ!/ Always, there's only one truth!


Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Daniel P. Berrangé 3 years, 6 months ago
On Thu, Oct 29, 2020 at 09:01:05AM -0400, Neal Gompa wrote:
> On Thu, Oct 29, 2020 at 7:39 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Oct 29, 2020 at 12:27:16PM +0100, Michal Privoznik wrote:
> > > On 10/28/20 9:47 PM, Neal Gompa wrote:
> > > > On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > >
> > > > > On 10/27/20 1:06 PM, Neal Gompa wrote:
> > > > > > On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > > > >
> > > > > > > On 10/26/20 11:08 PM, Neal Gompa wrote:
> > > > > > > > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> > > > > > > > based on the changelog entry date stamp. In scenarios where it already
> > > > > > > > is defined, we do not want to redefine it.
> > > > > > > >
> > > > > > >
> > > > > > > This part is okay.
> > > > > > >
> > > > > > > > Additionally, when building the libvirt package in an Open Build Service
> > > > > > > > instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> > > > > > > >
> > > > > > >
> > > > > > > But this looks fishy. Is the %_specdir defined in that case?
> > > > > > >
> > > > > >
> > > > > > It is (that comes from RPM itself), however the directory is empty.
> > > > >
> > > > > That feels like a bug in OBS then. IIUC this macro can be specified on
> > > > > the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
> > > > > %_sourcedir?
> > > > >
> > > >
> > > > Nothing about RPM mandates that %_specdir is actually *used* for anything.
> > > >
> > >
> > > But this is not the case, is it? %_specdir is defined and points to an
> > > actual directory. Having said that, I am not against the change, but maybe
> > > we can document this weirdness somewhere? Also, with the latest specfile
> > > discussion I'll let Andrea take a look.
> >
> 
> RPM internally populates a specfile variable that is the true pointer
> to the spec file, regardless of path. This is not exposed as a macro
> despite attempts to do so[1]. There is no rule that the spec file
> *must* come from %_specdir, and it's really only used for the location
> to store the specfile when extracting an SRPM.
> 
> [1]: https://github.com/rpm-software-management/rpm/pull/202
> 
> > I'm inclined to just delete all the source epoch stuff from the spec and
> > rely on the build environment set it if they want reproducable builds.
> >
> 
> Considering that RHEL 8 doesn't have the change to turn on
> SOURCE_DATE_EPOCH[2], I am inclined to keep it.

We can just wrap the current setting in "%if 0%{?rhel}" then, so we
honour the setting from Fedora.

> 
> [2]: https://src.fedoraproject.org/rpms/redhat-rpm-config/c/86aae600e62fadc18760d95d1fddd323cf9e9a86

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Posted by Neal Gompa 3 years, 6 months ago
On Thu, Oct 29, 2020 at 9:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Oct 29, 2020 at 09:01:05AM -0400, Neal Gompa wrote:
> > On Thu, Oct 29, 2020 at 7:39 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Thu, Oct 29, 2020 at 12:27:16PM +0100, Michal Privoznik wrote:
> > > > On 10/28/20 9:47 PM, Neal Gompa wrote:
> > > > > On Wed, Oct 28, 2020 at 7:49 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > > >
> > > > > > On 10/27/20 1:06 PM, Neal Gompa wrote:
> > > > > > > On Tue, Oct 27, 2020 at 6:24 AM Michal Privoznik <mprivozn@redhat.com> wrote:
> > > > > > > >
> > > > > > > > On 10/26/20 11:08 PM, Neal Gompa wrote:
> > > > > > > > > Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
> > > > > > > > > based on the changelog entry date stamp. In scenarios where it already
> > > > > > > > > is defined, we do not want to redefine it.
> > > > > > > > >
> > > > > > > >
> > > > > > > > This part is okay.
> > > > > > > >
> > > > > > > > > Additionally, when building the libvirt package in an Open Build Service
> > > > > > > > > instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
> > > > > > > > >
> > > > > > > >
> > > > > > > > But this looks fishy. Is the %_specdir defined in that case?
> > > > > > > >
> > > > > > >
> > > > > > > It is (that comes from RPM itself), however the directory is empty.
> > > > > >
> > > > > > That feels like a bug in OBS then. IIUC this macro can be specified on
> > > > > > the rpmbuild's cmd line. Can't it set the %_specdir to be the same as
> > > > > > %_sourcedir?
> > > > > >
> > > > >
> > > > > Nothing about RPM mandates that %_specdir is actually *used* for anything.
> > > > >
> > > >
> > > > But this is not the case, is it? %_specdir is defined and points to an
> > > > actual directory. Having said that, I am not against the change, but maybe
> > > > we can document this weirdness somewhere? Also, with the latest specfile
> > > > discussion I'll let Andrea take a look.
> > >
> >
> > RPM internally populates a specfile variable that is the true pointer
> > to the spec file, regardless of path. This is not exposed as a macro
> > despite attempts to do so[1]. There is no rule that the spec file
> > *must* come from %_specdir, and it's really only used for the location
> > to store the specfile when extracting an SRPM.
> >
> > [1]: https://github.com/rpm-software-management/rpm/pull/202
> >
> > > I'm inclined to just delete all the source epoch stuff from the spec and
> > > rely on the build environment set it if they want reproducable builds.
> > >
> >
> > Considering that RHEL 8 doesn't have the change to turn on
> > SOURCE_DATE_EPOCH[2], I am inclined to keep it.
>
> We can just wrap the current setting in "%if 0%{?rhel}" then, so we
> honour the setting from Fedora.
>
> >
> > [2]: https://src.fedoraproject.org/rpms/redhat-rpm-config/c/86aae600e62fadc18760d95d1fddd323cf9e9a86
>

That doesn't really change the need for this. I'm building this in an
OBS system, and I want that property to take effect properly.


-- 
真実はいつも一つ!/ Always, there's only one truth!