Bug 1473671

I've got the same issue:
http://copr-dist-git.fedorainfracloud.org/per-task-logs/582235.log

It seems the spec parsing has changed recently, because I managed to build the previous version of the package (with a similar spec file) in the past without issues.

It is IMO mistake to parse the spec file on dist-git side (where we can not
guarantee all the dependencies in general) to get the package name.

When we already have spec_path, why not to get the package name from spec file?

(In reply to Pavel Raiskup from comment #2)
> It is IMO mistake to parse the spec file on dist-git side (where we can not
> guarantee all the dependencies in general) to get the package name.
Exactly!
> 
> When we already have spec_path, why not to get the package name from spec
> file?
you mean to s/\.spec$// ? I think there are problems in mock-scm / tito / whatsoever. But in case of plain SRPMs, it is correct approach.

(In reply to Igor Gnatenko from comment #3)
> you mean to s/\.spec$// ?

Yes.

> I think there are problems in mock-scm / tito / whatsoever. But in case of
> plain SRPMs, it is correct approach.

Well, I don't know what problems.  All the methods should still have specfile,
or?

(In reply to Julius Schwartzenberg from comment #1)
> I've got the same issue:
> http://copr-dist-git.fedorainfracloud.org/per-task-logs/582235.log
> 
> It seems the spec parsing has changed recently, because I managed to build
> the previous version of the package (with a similar spec file) in the past
> without issues.

The problem with the spec in build 582235 is that in the spec, there are 'if' conditions like 

if %{rhel}

or if 0%{rhel}

It will work if the conditions are changed to: if 0%{?rhel}

(In reply to Igor Gnatenko from comment #0)
> https://copr.fedorainfracloud.org/coprs/g/rust/playground/build/582231/
> 
> ---
> 
> Downloading
> https://copr.fedorainfracloud.org/tmp/tmpY4pqmB/rust-bytecount-0.1.7-1.fc27.
> src.rpm
> https://copr.fedorainfracloud.org/tmp/tmpY4pqmB/rust-bytecount-0.1.7-1.fc27.
> src.rpm
> Extracting srpm
> package_content: PackageContent({'spec_path':
> '/tmp/tmpGj3KZs/rust-bytecount.spec', 'extra_content':
> ['/tmp/tmpGj3KZs/bytecount-0.1.7.crate',
> '/tmp/tmpGj3KZs/bytecount-0.1.7-fix-metadata.diff'], 'source_paths':
> ['/tmp/tmpGj3KZs/rust-bytecount-0.1.7-1.fc27.src.rpm']})
> Could not parse /tmp/tmpGj3KZs/rust-bytecount.spec with error can't parse
> specfile
> 
> Exception raised during package import.
> Traceback (most recent call last):
>   File "/usr/share/copr/dist_git/importer.py", line 94, in do_import
>     package_content
>   File "/usr/share/copr/dist_git/helpers.py", line 40, in wrapper
>     return f(*args, **kwargs)
>   File "/usr/share/copr/dist_git/package_import.py", line 156, in
> import_package
>     pkg_name = helpers.get_package_name(package_content.spec_path)
>   File "/usr/share/copr/dist_git/helpers.py", line 311, in get_package_name
>     "Could not get package name from {}.".format(spec_path))
> PackageNameCouldNotBeObtainedException: Could not get package name from
> /tmp/tmpGj3KZs/rust-bytecount.spec.
> sending a responses for branches master, f26, f25
> Sending back: 
> {"branch": "master", "task_id": "582231", "error": "Could not import this
> branch."}
> Sending back: 
> {"branch": "f26", "task_id": "582231", "error": "Could not import this
> branch."}
> Sending back: 
> {"branch": "f25", "task_id": "582231", "error": "Could not import this
> branch."}
> 
> ---
> 
> I guess it is failing on parsing rich dependencies, so it is regression and
> blocks Rust SIG to build packages in COPR (again).

The spec file is not provided as an attachment but you probably use a macro in Name: and rpm experimental features at the same time.

It must always be possible to acquire package name. This is done by parsing spec file.

There is a regular expression fallback for cases when the spec file is not parsable but it does not do macro substitutions. The solution is not to use macro in Name: when you supply unparsable specfile. I understand this is a limitation but I think it's an acceptable one.

Can you, please, provide the specfile here so that I can see your use-case for using macros in name?

> It must always be possible to acquire package name.

Accepted.

> This is done by parsing spec file.

This is not clear.  Spec file name is name of package.  If not from the builder
POV, then from the dist-git POV.  In other words, even if I submit package
sclo-postgresql96-postgresql.src.rpm for build I want to import into
postgresql module name (because there's postgresql.spec).

I haven't studied this particular spec file (maybe that build failed
anyways)...  but as I claimed in https://pagure.io/copr/copr/issue/102 --
having such details like git import hardwired directly in dist-git code
makes it very, very hard to reproduce outside copr; and it is almost
impossible to debug issues _in specfiles_ for users.  IMVHO, to avoid all
of this burden, everything else than glob('*.spec')[0] should go out from
copr; either into rpkg, pyrpkg or at least separate and reproducible
script for the time being.

(In reply to Pavel Raiskup from comment #7)
> > This is done by parsing spec file.
> 
> This is not clear.  Spec file name is name of package.  If not from the
> builder
> POV, then from the dist-git POV.  In other words, even if I submit package
> sclo-postgresql96-postgresql.src.rpm for build I want to import into
> postgresql module name (because there's postgresql.spec).

Actually, it doesn't have to be (though enforced by Fedora Packaging Guidelines).

> Actually, it doesn't have to be

I don't understand.  What could happen if we imported 'foo.spec'
into foo.git, even when it generated 'Name: %{?scl_prefix}baz'?

That's what we do everywhere else...

> I don't understand.  What could happen if we imported 'foo.spec'
> into foo.git, even when it generated 'Name: %{?scl_prefix}baz'?

I mean, I hope you don't you want to mismatch dist-git for this package
with completely different package 'baz' coming from spec file named
'baz.spec'.

(In reply to Pavel Raiskup from comment #9)
> > Actually, it doesn't have to be
> 
> I don't understand.  What could happen if we imported 'foo.spec'
> into foo.git, even when it generated 'Name: %{?scl_prefix}baz'?
> 
> That's what we do everywhere else...

Nothing bad, I'm fine with this behavior. @clime didn't like this approach ;)

In theory, SRPM names might collide after all.

And also, please keep SCL stuff as much away as possible from me ;)

(In reply to Pavel Raiskup from comment #10)
> > I don't understand.  What could happen if we imported 'foo.spec'
> > into foo.git, even when it generated 'Name: %{?scl_prefix}baz'?
> 
> I mean, I hope you don't you want to mismatch dist-git for this package
> with completely different package 'baz' coming from spec file named
> 'baz.spec'.

depends, I don't know what are the reasons behind. Moreover, I don't want to know. myself I would prefer to have one single dist-git repository for one COPR repository.

So since @clime wanted me to file my use-case here:

We have everywhere Name: rust-%{crate} and we don't care how it will be stored in COPR. We use some special RPM features in BuildRequires/Requires, so Fedora's RPM can't parse such specfiles.

Our typical specfile look like:

# Generated by rust2rpm
%bcond_without check

%global crate ripgrep

Name:           rust-%{crate}
Version:        0.5.2
Release:        3%{?dist}
Summary:        Line oriented search tool using Rust's regex library

License:        Unlicense or MIT
URL:            https://crates.io/crates/ripgrep
Source0:        https://crates.io/api/v1/crates/%{crate}/%{version}/download#/%{crate}-%{version}.crate
# Initial patched metadata
# * No simd
# * No paths
# * Bump encoding_rs to 0.6, https://github.com/BurntSushi/ripgrep/pull/518
Patch0:         ripgrep-0.5.2-fix-metadata.diff

ExclusiveArch:  %{rust_arches}

BuildRequires:  rust-packaging
# [dependencies]
BuildRequires:  (crate(atty) >= 0.2.2 with crate(atty) < 0.3.0)
BuildRequires:  (crate(bytecount) >= 0.1.4 with crate(bytecount) < 0.2.0)
BuildRequires:  (crate(clap) >= 2.24.1 with crate(clap) < 3.0.0)
BuildRequires:  (crate(encoding_rs) >= 0.6.0 with crate(encoding_rs) < 0.7.0)
BuildRequires:  (crate(env_logger) >= 0.4.0 with crate(env_logger) < 0.5.0)
BuildRequires:  (crate(grep) >= 0.1.5 with crate(grep) < 0.2.0)
BuildRequires:  (crate(ignore) >= 0.2.0 with crate(ignore) < 0.3.0)
BuildRequires:  (crate(lazy_static) >= 0.2.0 with crate(lazy_static) < 0.3.0)
BuildRequires:  (crate(libc) >= 0.2.0 with crate(libc) < 0.3.0)
BuildRequires:  (crate(log) >= 0.3.0 with crate(log) < 0.4.0)
BuildRequires:  (crate(memchr) >= 1.0.0 with crate(memchr) < 2.0.0)
BuildRequires:  (crate(memmap) >= 0.5.0 with crate(memmap) < 0.6.0)
BuildRequires:  (crate(num_cpus) >= 1.0.0 with crate(num_cpus) < 2.0.0)
BuildRequires:  (crate(regex) >= 0.2.1 with crate(regex) < 0.3.0)
BuildRequires:  (crate(same-file) >= 0.1.1 with crate(same-file) < 0.2.0)
BuildRequires:  (crate(termcolor) >= 0.3.0 with crate(termcolor) < 0.4.0)
# [build-dependencies]
BuildRequires:  (crate(clap) >= 2.24.1 with crate(clap) < 3.0.0)
BuildRequires:  (crate(lazy_static) >= 0.2.0 with crate(lazy_static) < 0.3.0)

%description
%{summary}.

%package     -n %{crate}
Summary:        %{summary}

%description -n %{crate}
Line oriented search tool using Rust's regex library. Combines
the raw performance of grep with the usability of the silver searcher.

%prep
%autosetup -n %{crate}-%{version} -p1
%cargo_prep

%build
%cargo_build

%install
%cargo_install
install -D -p -m0644 doc/rg.1 %{buildroot}%{_mandir}/man1/rg.1

%if %{with check}
%check
%cargo_test
%endif

%files       -n %{crate}
%license LICENSE-MIT UNLICENSE COPYING
%doc README.md CHANGELOG.md
%{_bindir}/rg
%{_mandir}/man1/rg.1*

%changelog

Fixed by https://pagure.io/copr/copr/c/fc3cd9784ac19e0ee202e417c964000300b5100f.

New COPR version has been released.