search

sclorg / rpm-list-builder

RPM List Builder helps you to build a list of defined RPM packages including Software Collection from the recipe file
GNU General Public License v2.0
4 stars 8 forks source link

Remove hardcoded rhpkg #83

Closed junaruga closed 7 years ago

junaruga commented 7 years ago

Issue

This issue is from

https://github.com/sclorg/rpm-list-builder/issues/66#issuecomment-316033852

The specific problem is that -b mock calls rhpkg srpm to create the SRPM. As a result, it doesn't work on Fedora, which only has fedpkg.

Right now rhpkg is hardcoded. It's time to fix it.

$ grep -r rhpkg rpmlb/
rpmlb/cli.py:    type=click.Choice('none local rhpkg custom'.split()),
rpmlb/cli.py:    help='Git branch for downloaders that use it (rhpkg).',
rpmlb/builder/mock.py:        utils.run_cmd('rhpkg srpm')
rpmlb/builder/copr.py:        utils.run_cmd('rhpkg srpm')
rpmlb/downloader/rhpkg.py:    """A downloader class to get a pacakge with rhpkg command."""
rpmlb/downloader/rhpkg.py:        self.do_rhpkg_and_checkout(package_dict, branch)
rpmlb/downloader/rhpkg.py:    def do_rhpkg_and_checkout(self, package_dict, branch):
rpmlb/downloader/rhpkg.py:        LOG.debug('rhpkg co %s', package)
rpmlb/downloader/rhpkg.py:        subprocess.check_call(['rhpkg', 'co', package])

Possible solution

  1. Add --pkg-cmd [fedpkg|rhpkg] option (default: fedpkg). The option name is inspired from mock --dnf-cmd.
  2. Add --rhpkg option. If it is not specified, it is run as fedpkg.

But if --download rhpkg is specified, the package cmd is run as rhpkg. But if --download fedpkg (future new feature) is specified, the package cmd is run as fedpkg.

Right now I prefer the solution 1.

hroncok commented 7 years ago

What if we have something like BaseRpkgDownloader and inherit FedPkgDownloader and RhPkgDownloader from it?

class FedPkgDownloader(BaseRpkgDownloader):
    command = 'fedpkg'
junaruga commented 7 years ago

What if we have something like BaseRpkgDownloader and inherit FedPkgDownloader and RhPkgDownloader from it?

That's great idea. I am going to use the way. Thanks.

hroncok commented 7 years ago

Should be fixed by 9f74474044e34f835313420f1bfe3f8c6932ed2c https://github.com/sclorg/rpm-list-builder/pull/88