Bug 1332211 - Exim: remove conditional source includes
Summary: Exim: remove conditional source includes
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: exim
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jaroslav Škarvada
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-02 14:00 UTC by Felix Schwarz
Modified: 2016-05-04 14:12 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-05-04 14:12:57 UTC
Type: Bug


Attachments (Terms of Use)

Description Felix Schwarz 2016-05-02 14:00:39 UTC
There are conditionals for clamav and spamassassin sources in the Exim spec file.

This means I can not build the SRPM without spamassassin (in mock) and take generated srpm to produce RPMs with spamassassin. Even worse I have to remember to disable spamassassin/clamav explicitely if building for a platform where they are enabled by default.

Please always include all source files and just use the conditionals later (applying patches, configuration, building).

Comment 1 Jaroslav Škarvada 2016-05-03 18:14:44 UTC
Are you using sa-exim? I noticed we have lost the sources around F12 from dist-git and nobody have complained so far. Also it seems it doesn't build with recent gcc. I am probably going to drop it completely as it is deprecated in favour of the built-in ACL support for content scanning.

Comment 2 Felix Schwarz 2016-05-03 18:31:02 UTC
(In reply to Jaroslav Škarvada from comment #1)
> Are you using sa-exim? I noticed we have lost the sources around F12 from
> dist-git and nobody have complained so far. 

lol :-)

> Also it seems it doesn't build
> with recent gcc. I am probably going to drop it completely as it is
> deprecated in favour of the built-in ACL support for content scanning.

No, I'm neither using sa-exim nor the clamav integration. However I rebuilt the Fedora RPM on EPEL7 but could not rebuild the resulting srpm (from mock) with Fedora again (had to use explicit "--without ...").

Comment 3 Jaroslav Škarvada 2016-05-03 18:50:48 UTC
Sorry, I still haven't got it. Could you post the steps you do?

- sa-exim is disabled since very long, so it shouldn't cause problems
- clamav sources are included in Fedora SRPM, so it shouldn't be problem to rebuilt with/without clamav and by default it builds without clamav in EPEL

Comment 4 Felix Schwarz 2016-05-03 19:53:38 UTC
(on a F23 machine)
fedpkg clone exim
fedpkg srpm
mock  --rebuild -r epel-7-x86_64 exim-4.87-2.fc25.src.rpm

=> /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm does not contain clamd.exim.service and exim-clamav-tmpfiles.conf anymore even though they were present in the fc25.src.rpm

mock --rebuild -r fedora-23-x86_64 /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm

=> this fails because the aforementioned two files are not present anymore.

Comment 5 Felix Schwarz 2016-05-03 19:55:24 UTC
Actually I care neither about spamassassin nor clamav but so far (with other packages) I just used a single SRPM for Fedora and CentOS - provided that it was buildable/installable for both distros.

Comment 6 Jaroslav Škarvada 2016-05-04 10:40:07 UTC
(In reply to Felix Schwarz from comment #4)
> (on a F23 machine)
> fedpkg clone exim
> fedpkg srpm
> mock  --rebuild -r epel-7-x86_64 exim-4.87-2.fc25.src.rpm
> 
> => /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm does
> not contain clamd.exim.service and exim-clamav-tmpfiles.conf anymore even
> though they were present in the fc25.src.rpm
> 
> mock --rebuild -r fedora-23-x86_64
> /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm
> 
> => this fails because the aforementioned two files are not present anymore.

This worked for me as I expected and I cannot see why it shouldn't work, i.e.:

$ fedpkg co exim
$ cd exim
$ fedpkg srpm
$ mock -r epel-7-x86_6 exim-4.87-2.fc25.src.rpm

Also to proof that's my EPEL-7 mock config is correct:
$ koji build epel7-candidate --scratch ./exim-4.87-2.fc25.src.rpm

Result:
http://koji.fedoraproject.org/koji/taskinfo?taskID=13916809

It's because clamav is not by default build for EPEL-7, but it's build for the Fedora, so the sources are already included in the SRPM made on Fedora and simply not used during the EPEL-7 build from SRPM.

The scenario which will probably fail is:
- create SRPM on RHEL-7/CentOS machine / chroot
- build for Fedora from such SRPM
The sources will be not included in such case and the build will fail. But I can't imagine why anybody would need to do this.

And the sa-exim conditionals shouldn't matter in any case.

Comment 7 Felix Schwarz 2016-05-04 10:47:21 UTC
(In reply to Jaroslav Škarvada from comment #6)
> The scenario which will probably fail is:
> - create SRPM on RHEL-7/CentOS machine / chroot
> - build for Fedora from such SRPM
> The sources will be not included in such case and the build will fail.

Which is exactly what I did (if you check the sequence of actions I posted in #5).

> But I can't imagine why anybody would need to do this.

I'm using RHEL/CentOS on our servers. Sometimes I need newer versions of some software so usually I can just take the Fedora RPM and recompile it for RHEL 7 (sometimes with custom patches). The resulting SRPM is stored together with the binary RPMs in our internal repos.

Now it might happen that I have to debug a failure. Sometimes problems arise because of mismatched components (e.g. a newer version of a dependency resolved a particular bug which is only triggered by our rebuilt package) and the easiest way to check this is to rebuild our (modified) SRPM in Fedora.

This was exactly what I tried to do and I was surprised to see that this didn't work with the exim rpm.

I guess the conditional source includes also contributed to loosing the sa-exim files during the git migration, right? (I didn't see this effect with other packages.)

Comment 8 Jaroslav Škarvada 2016-05-04 10:55:32 UTC
(In reply to Felix Schwarz from comment #7)
> (In reply to Jaroslav Škarvada from comment #6)
> > The scenario which will probably fail is:
> > - create SRPM on RHEL-7/CentOS machine / chroot
> > - build for Fedora from such SRPM
> > The sources will be not included in such case and the build will fail.
> 
> Which is exactly what I did (if you check the sequence of actions I posted
> in #5).
> 
Sorry, there are no steps in comment 5.

In comment 4 you wrote:
> (on a F23 machine)
> fedpkg clone exim
> fedpkg srpm
> mock  --rebuild -r epel-7-x86_64 exim-4.87-2.fc25.src.rpm

So I don't think it is exactly what you did.

> > But I can't imagine why anybody would need to do this.
> 
> I'm using RHEL/CentOS on our servers. Sometimes I need newer versions of
> some software so usually I can just take the Fedora RPM and recompile it for
> RHEL 7 (sometimes with custom patches). The resulting SRPM is stored
> together with the binary RPMs in our internal repos.
> 
> Now it might happen that I have to debug a failure. Sometimes problems arise
> because of mismatched components (e.g. a newer version of a dependency
> resolved a particular bug which is only triggered by our rebuilt package)
> and the easiest way to check this is to rebuild our (modified) SRPM in
> Fedora.
> 
> This was exactly what I tried to do and I was surprised to see that this
> didn't work with the exim rpm.
> 
> I guess the conditional source includes also contributed to loosing the
> sa-exim files during the git migration, right? (I didn't see this effect
> with other packages.)

Sure it makes sense. I will check with guidelines but I think there is no problem to unconditionalize the clamav sources.

Comment 9 Felix Schwarz 2016-05-04 11:00:21 UTC
(In reply to Jaroslav Škarvada from comment #8)
> (In reply to Felix Schwarz from comment #7)
> > Which is exactly what I did (if you check the sequence of actions I posted
> > in #5).
> > 
> Sorry, there are no steps in comment 5.

Yes, sorry. As you figured I meant step 4. I really wished bugzilla had some edit functionality for comments... :-/

> In comment 4 you wrote:
> > (on a F23 machine)
> > fedpkg clone exim
> > fedpkg srpm
> > mock  --rebuild -r epel-7-x86_64 exim-4.87-2.fc25.src.rpm
> 
> So I don't think it is exactly what you did.

I think I can see where the misunderstanding comes from. The first rebuild for EPEL7 works as expected. But then I'm continuing the sequence:

> > mock --rebuild -r fedora-23-x86_64
> > /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm

So I'm taking the new SRPM generated by the EPEL7 build process to build a Fedora RPM which fails. Sorry for being unclear.

> Sure it makes sense. I will check with guidelines but I think there is no
> problem to unconditionalize the clamav sources.

Thank you.

Comment 10 Jaroslav Škarvada 2016-05-04 11:08:57 UTC
(In reply to Felix Schwarz from comment #9)
> (In reply to Jaroslav Škarvada from comment #8)
> > (In reply to Felix Schwarz from comment #7)
> > > Which is exactly what I did (if you check the sequence of actions I posted
> > > in #5).
> > > 
> > Sorry, there are no steps in comment 5.
> 
> Yes, sorry. As you figured I meant step 4. I really wished bugzilla had some
> edit functionality for comments... :-/
> 
> > In comment 4 you wrote:
> > > (on a F23 machine)
> > > fedpkg clone exim
> > > fedpkg srpm
> > > mock  --rebuild -r epel-7-x86_64 exim-4.87-2.fc25.src.rpm
> > 
> > So I don't think it is exactly what you did.
> 
> I think I can see where the misunderstanding comes from. The first rebuild
> for EPEL7 works as expected. But then I'm continuing the sequence:
> 
> > > mock --rebuild -r fedora-23-x86_64
> > > /var/lib/mock/epel-7-x86_64/result/exim-4.87-2.el7.centos.src.rpm
> 
> So I'm taking the new SRPM generated by the EPEL7 build process to build a
> Fedora RPM which fails. Sorry for being unclear.
> 

Ah, sorry, my fault. I didn't read it all. I was missing the 'cd exim' step so I thought it's not the 1:1 reproducer and it looked to me quite weird that somebody is building RHEL-7 SRPM on the F23 machine and reusing it again for the F23 build.

Comment 11 Felix Schwarz 2016-05-04 11:14:05 UTC
(In reply to Jaroslav Škarvada from comment #10)
> Ah, sorry, my fault. I didn't read it all. I was missing the 'cd exim' step
> so I thought it's not the 1:1 reproducer 

To be honest it is also no 1:1 reproducer (well, for you it is probably) because the plain "fedpkg clone exim" will throw an error as I don't have commit rights on Fedora's exim repo ;-)

Anyway thank you again for your time.

Comment 12 Jaroslav Škarvada 2016-05-04 13:58:01 UTC
It also seems that the current state is against the Fedora Packaging Guidelines [1]:

> Packages MUST NOT have SourceN: or PatchN: tags which are conditionalized on architecture. 

[1] https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Conditionalizing_Builds_for_Architecture


Note You need to log in before you can comment on or make changes to this bug.