> To fix this, dnf needs to be rewritten so that it unpacks the src.rpm to extract the spec file, and then re-use its existing logic to pull BuildRequires from the spec file. This will ensure it evaluates the conditional BuildRequires according to the current host architecture.

This is totally wrong approach. But we will check why it doesn't install all builddeps.

(In reply to Igor Gnatenko from comment #1)
> > To fix this, dnf needs to be rewritten so that it unpacks the src.rpm to extract the spec file, and then re-use its existing logic to pull BuildRequires from the spec file. This will ensure it evaluates the conditional BuildRequires according to the current host architecture.
> 
> This is totally wrong approach. But we will check why it doesn't install all
> builddeps.

It doesn't install all builddeps simply because the information source it is using (Requires from src.rpm) is incorrect. This can be clearly seen by just querying the src.rpm directly with rpm. AFAIK, there's nothing dnf can do to fix this lack of information in src.rpm deps as RPM does not embed conditional logic in its binary src.rpm Requires. The spec file inside the src.rpm is only place dnf can get the correct information.

(In reply to Daniel Berrange from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> > > To fix this, dnf needs to be rewritten so that it unpacks the src.rpm to extract the spec file, and then re-use its existing logic to pull BuildRequires from the spec file. This will ensure it evaluates the conditional BuildRequires according to the current host architecture.
> > 
> > This is totally wrong approach. But we will check why it doesn't install all
> > builddeps.
> 
> It doesn't install all builddeps simply because the information source it is
> using (Requires from src.rpm) is incorrect. This can be clearly seen by just
> querying the src.rpm directly with rpm. AFAIK, there's nothing dnf can do to
> fix this lack of information in src.rpm deps as RPM does not embed
> conditional logic in its binary src.rpm Requires. The spec file inside the
> src.rpm is only place dnf can get the correct information.
then it's clearly bug in RPM.

(In reply to Igor Gnatenko from comment #3)
> (In reply to Daniel Berrange from comment #2)
> > It doesn't install all builddeps simply because the information source it is
> > using (Requires from src.rpm) is incorrect. This can be clearly seen by just
> > querying the src.rpm directly with rpm. AFAIK, there's nothing dnf can do to
> > fix this lack of information in src.rpm deps as RPM does not embed
> > conditional logic in its binary src.rpm Requires. The spec file inside the
> > src.rpm is only place dnf can get the correct information.
> then it's clearly bug in RPM.

It really isn't - its an inherent limitation of the src.rpm Requires information. When creating the src.rpm RPM has to evaluate the spec file to expand the various %ifdef conditionals to determin which BuildRequires are relevant for the src.rpm Requires. It can only evaluate this based on the host environment it is using to create the src.rpm. There's no practical way to evaluate what such conditionals would produce for an x86_64 host, when running on an arm7 host, or vica-verca.

If dnf really wants to use src.rpms as the basis for builddep, then Fedora would have to ship src.rpms per-architecture in its repos, so it could fetch the src.rpm corresponding to the current architecture its installing on.

If Requires from src.rpm does not work for you use

 # dnf builddep libvirt.spec 

to install BuildRequires from the spec.

Here we go again... :) Igor, I suggest you see bug 554854 and bug 771938 and bug 664427 for some background.

The short version is that mostly everybody assumes an src.rpm to be an arch-independent construct when it is not, never was. It even says so if you look at the ARCH tag. Using dependencies from src.rpm works just fine but ONLY if that src.rpm was generated on the same architecture, distro version etc.

So with infra like Fedora where one src.rpm is shared for all architectures, you have two choices:
- download the src.rpm and regenerate it with eg 'rpmbuild -rs --nodeps foo.src.rpm' 
- download the src.rpm and fish the spec out of it

For Fedora packages a third possibility exists as well: fetching the spec out of dist-git to avoid having to download the entire src.rpm. It's a well-known location with easy nvr -> tag mapping so it wouldn't even be hard.

Anyway, back to dnf-plugins-core.

But is it possible to get correct BuildRequires from spec if there are macros and/or %ifs used?

Well, where do you think the dependencies in src.rpm come from? By parsing the spec with all its macros and %ifs etc - of course you need to use rpm API to do that but then builddep already supports parsing the spec, doesn't it?

Generally speaking its easy to create a spec that is completely unparseable without its build-requires installed but that'd make the spec broken for any machine processing anyway.

(In reply to Panu Matilainen from comment #8)
> Well, where do you think the dependencies in src.rpm come from? By parsing
> the spec with all its macros and %ifs etc - of course you need to use rpm
> API to do that but then builddep already supports parsing the spec, doesn't
> it?

Sure, but macro on build host can vary from macros on my workstation so parsing spec locally I'll see a different output than build host did.

E.g.: My system is F25 and in addition to F25 repos I have also F24 repos configured and enabled. Now when I parse a spec from F24 src.rpm package its Release says 1%{dist} which will be wrongly expanded to 1.f25 on my system...

> Generally speaking its easy to create a spec that is completely unparseable
> without its build-requires installed but that'd make the spec broken for any
> machine processing anyway.

(In reply to Michael Mráka from comment #9)
> (In reply to Panu Matilainen from comment #8)
> > Well, where do you think the dependencies in src.rpm come from? By parsing
> > the spec with all its macros and %ifs etc - of course you need to use rpm
> > API to do that but then builddep already supports parsing the spec, doesn't
> > it?
> 
> Sure, but macro on build host can vary from macros on my workstation so
> parsing spec locally I'll see a different output than build host did.
> 
> E.g.: My system is F25 and in addition to F25 repos I have also F24 repos
> configured and enabled. Now when I parse a spec from F24 src.rpm package its
> Release says 1%{dist} which will be wrongly expanded to 1.f25 on my system...

dnf builddep doesn't need to care about that. Its job is merely to get build deps installed suitable for the environment in which dnf builddep is run. If you want the macros to evaluate in a particular manner then you need to run dnf builddep in a suitable environment for your needs. ie if you're trying to rebuild a f24 RPM on f25, then you likely *want* %dist to evaluate to f25. If you want it to evaluate differently then use dnf builddep inside a chroot containing a suitable environment.  This is all better that what it does today wrt to reading src.rpm instead of the spec file.

Yes, macros and conditionals can and do vary between hosts due to a number of reasons and that's how it should be. Many specs have conditionals to support building with different distros and versions and the only way to get the correct dependencies and other settings is to parse the spec on the targeted distro/version - it's not rare to have a build-dependency on version X+1 which doesn't even exist on X, etc.

Random drive-by comment: Many of those 'BuildRequires: foo-devel' in libvirt.spec look like they're violating the packaging guidelines, and should be 'BuildRequires: pkgconfig(foo)' instead.

(In reply to David Woodhouse from comment #12)
> Random drive-by comment: Many of those 'BuildRequires: foo-devel' in
> libvirt.spec look like they're violating the packaging guidelines, and
> should be 'BuildRequires: pkgconfig(foo)' instead.
Not really. you use pkgconfig(foo) when package wants foo.pc, otherwise you use foo-devel. Anyway it's not mandatory.

Trying to work-around the builddep brokeness with src.rpm using the .spec file does not work. #1404281 relates to this one.

Sorry, but this needs to be worked by release engineering to provide arch-dependent SRPMs. DNF will *not* download srpms, unpack them and so on.