The %ansible_collection_build_install macro defined in this package is also problematic. Building should occur in %build and installation should occur in %install. Please use %ansible_collection_build and %ansible_collection_install in the appropriate places.

(In reply to Maxwell G from comment #0)
> I already fixed most of Fedora's ansible collection packages to BuildRequire
> ansible-packaging (see https://bugzilla.redhat.com/2126893), but I was
> unable to fix this package given the complexity and illegibility of its
> specfile.
> 
> Please consider removing the RHEL conditionals from this package's Fedora
> specfile or otherwise making it more legible. For the EPEL packages I
> maintain, I also prefer to use the same specfile for both Fedora and EPEL,
> but I avoid doing so when it would require too many conditionals or other
> complexity. 

How many conditionals is too many?
How complex is too complex?
Is this one of those things that "you can't explain it, but you know it when you see it"?

If you maintain the same package in Fedora and EPEL, but require two different spec files - how do you keep the spec files in sync?

> 
> From the Packaging Guidelines:
> 
> > All spec files MUST be legible and maintained in such a way that the community of packagers is able to understand and work with them.
> 
> --
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility
> 
> You can look at
> https://src.fedoraproject.org/rpms/ansible-collection-community-rabbitmq for
> an example of a collection package. I am working on[1] writing official
> packaging guidelines for Ansible collections, as well.
> 
> [1]: https://pagure.io/packaging-committee/pull-request/1201

some comments

> + == Naming
> + Collection packages MUST be named `+ansible-collection-NAMESPACE-NAME+`.

The linux-system-roles package antedates this guideline - are we going to be required to change the name of the package?  Note that the linux-system-roles package includes the roles in both the legacy role format and as the fedora.linux_system_roles collection.

> + == Collection Source
> + Collection source code MUST be downloaded from the collection's respective
> + Git forge/other SCM repository.

This is problematic because the system roles collection
* is spread out among 20 or so repositories under https://github.com/linux-system-roles/, and includes https://github.com/willshersystems/ansible-sshd which is in a different organization
* has repos that are in the legacy role format instead of the collection format because a) we still have to support the legacy role format both in Galaxy and for customers b) it is much easier to do roles -> collection than vice versa (which we do during the RPM packaging which increases the complexity of the spec file)

So we could possibly do it, but IMO it is not any better than source tarfiles

Thank you for your response, and for working with me! I'll respond inline to your comments.

(In reply to Rich Megginson from comment #2)
> If you maintain the same package in Fedora and EPEL, but require two
> different spec files - how do you keep the spec files in sync?

Well, they don't need to be perfectly in sync. If you're updating both versions, you change the Version, reset the Release, and add the changelog entry. You just do the same thing twice. If you need to make other changes, you'd have to adapt them to the new specfile, but it shouldn't be too difficult.


> How many conditionals is too many?
> How complex is too complex?

`%endif` appears 34 times in the specfile. The package build behaves significantly differently between RHEL and Fedora. I'd define that as too much.

Excluding the changelog, ansible-collection-community-rabbitmq.spec is 67 lines. linux-system-roles is 747. I know this is not an apples to apples comparison due to the way linux-system-roles is composed, but there is still a lot that can be cut out.

If you had two separate specfiles, it should be *more* maintainable and *less* error prone. It would be clearer to you and other developers what is actually happening if they don't have to sort through a bunch of conditionals and macros. Both specfiles would be more concise.

Here are some more specific comments about the package.  Some of this should also apply to `ansible-pcp` and `ansible-collection-microsoft-sql`. I'm not saying all of this needs to be done immediately, but hopefully, some of my suggestions could be applied incrementally.


```
# NOTE: Even though ansible-core is in 8.6, it is only available
# at *runtime*, not at *buildtime* - so we can't have
# ansible-core as a build_dep on RHEL8
%if 0%{?fedora} || 0%{?rhel} >= 9
%bcond_without ansible
%global ansible_build_dep ansible-core >= 2.11.0
%else
%if 0%{?rhel} && ! 0%{?epel}
%bcond_with ansible
%else
%bcond_without ansible
%global ansible_build_dep ansible >= 2.9.10
%endif
%endif
```

This is very confusing. It obfuscates what dependencies are used where and is completely irrelevant to Fedora.

```
%if 0%{?rhel} && ! 0%{?epel}
%bcond_with ansible
%else
%bcond_without ansible
%global ansible_build_dep ansible >= 2.9.10
%endif
```

The EPEL part is wrong, as well. ansible 2.9.10 has been retired from all current EPEL releases. Collections packaged for EPEL should also depend on ansible-packaging. I'm not sure why this is here, anyways. The package is part of RHEL.

In the Fedora specfile, you can remove all of this and just have `BuildRequires: ansible-packaging`. In the RHEL specfile, you can either simplify this conditional or remove the macros and just conditionalalize the BuildRequires themselves.

The `%ansible_collection_*` definitions should also be removed. You should use `%ansible_collection_build` and `%ansible_collection_install` from ansible-packaging in the Fedora specfile. In the RHEL specfile, I wouldn't use macros here at all; just include the proper commands inline.

Ditto for the logic to work around RHEL not having the dependency generator.

```
for file in %_sourcedir/*.tar.gz; do
    if [[ "$file" =~ %_sourcedir/([^-]+)-([^-]+)-(.+).tar.gz ]]; then
        ns=${BASH_REMATCH[1]}
        name=${BASH_REMATCH[2]}
        ver=${BASH_REMATCH[3]}
        mkdir -p .external/$ns/$name
        pushd .external/$ns/$name > /dev/null
        tar xfz "$file"
        popd > /dev/null
    fi
done
```

Packages must not reference %_sourcedir. It has potential to cause hard to debug issues when %_sourcedir is not empty and is forbidden by the Packaging Guidelines[2].

[2] https://docs.fedoraproject.org/en-US/packaging-guidelines/RPM_Source_Dir/

The following logic could also be removed from the Fedora specfile:

Some of these conditionals would be okay in isolation, but all together, it's just way too confusing.

- Bundling of ansible.posix and community.general could removed..

Side note: For the RHEL package where bundling is enabled, you should include `bundled()` Provides. This type of bundling isn't allowed in Fedora, but in cases where it is, you need to mark what's being bundled[1]. I'd assume that translates to RHEL.

[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling

- The conditional docs stuff could be removed.

- The conditional naming could be removed from Fedora. It adds a lot of complexity. I'd also argue that this should be removed entirely. What about if I test my playbooks on Fedora and run them in production on RHEL. Would I have to change every module name? I guess

```
---
- hosts: all
  collections:
    - fedora.linux_system_roles
    - redhat.rhel_system_roles

  roles:
    - role_name_here
```

would work, but it feels gross.


> Is this one of those things that "you can't explain it, but you know it when
> you see it"?

I am an experienced packager and the main person who works on the ansible stack in Fedora, and I have a hard time understanding what's going on.


> > From the Packaging Guidelines:
> > 
> > > All spec files MUST be legible and maintained in such a way that the community of packagers is able to understand and work with them.
> > 
> > --
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility
> > 
> > You can look at
> > https://src.fedoraproject.org/rpms/ansible-collection-community-rabbitmq for
> > an example of a collection package. I am working on[1] writing official
> > packaging guidelines for Ansible collections, as well.
> > 
> >z [1]: https://pagure.io/packaging-committee/pull-request/1201
> 
> some comments
> 
> > + == Naming
> > + Collection packages MUST be named `+ansible-collection-NAMESPACE-NAME+`.
> 
> The linux-system-roles package antedates this guideline - are we going to be
> required to change the name of the package?  Note that the
> linux-system-roles package includes the roles in both the legacy role format
> and as the fedora.linux_system_roles collection.

Nothing is finalized yet, and we can always make exceptions. I agree that this package is a bit special and not everything is applicable. One solution would be to create a separate `ansible-collection-fedora-linux_system_roles` subpackage that the main one depends on., but I worry that would make this package even more complex.

> > + == Collection Source
> > + Collection source code MUST be downloaded from the collection's respective
> > + Git forge/other SCM repository.
> 
> This is problematic because the system roles collection
> * is spread out among 20 or so repositories under
> https://github.com/linux-system-roles/, and includes
> https://github.com/willshersystems/ansible-sshd which is in a different
> organization

Yes, I understand. Those repositories *could* be considered the "collection's respective Git forge."

> * has repos that are in the legacy role format instead of the collection
> format because a) we still have to support the legacy role format both in
> Galaxy and for customers b) it is much easier to do roles -> collection than
> vice versa (which we do during the RPM packaging which increases the
> complexity of the spec file)
> z
> So we could possibly do it, but IMO it is not any better than source tarfiles

I personally don't like this model, but I understand the reasoning for it. I don't intend these guidelines to require maintainers to radically change upstream development models.

Perhaps some of the build process could be moved into a Makefile or script upstream? It looks like some of the build scripts comes from https://github.com/linux-system-roles/auto-maintenance, but there's still a lot done in the specfile. You could keep the tarball downloading and unpacking in %prep, run `%ansible_collection_build` and `%ansible_collection_install` in the specfile, and move at least some of the rest.

FEDORA-2022-e46f931651 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-e46f931651

FEDORA-2022-e46f931651 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.