PR#710: rpmbuild: download srpm/spec if url contains query string - copr/copr

copr / copr

#710 rpmbuild: download srpm/spec if url contains query string

Merged 4 years ago by praiskup. Opened 4 years ago by praiskup.

Unknown source fix-issue-705 into master

[rpmbuild] use six.moves.urllib.parse

Pavel Raiskup • 4 years ago

053c4a5

[rpmbuild] download srpm/spec if url contains query string

Pavel Raiskup • 4 years ago

7dfb269

rpmbuild/copr-rpmbuild.spec

file modified

		`@@ -26,6 +26,10 @@`
		`Source0: {{{ git_dir_archive }}}`

		`BuildRequires: %{python}-devel`
		`+ BuildRequires: %{python}-distro`
		`+ %if 0%{?rhel} == 0 \|\| 0%{?rhel} != 6`
		`+ BuildRequires: %{python}-httmock`
		`+ %endif`
		`BuildRequires: %{rpm_python}`
		`BuildRequires: asciidoc`
		`BuildRequires: %{python}-setuptools`

rpmbuild/copr_rpmbuild/helpers.py

file modified

+1 -4

		`@@ -11,10 +11,7 @@`
		`import pipes`
		`from threading import Timer`

		`- try:`
		`- from urllib.parse import urlparse`
		`- except ImportError:`
		`- from urlparse import urlparse`
		`+ from six.moves.urllib.parse import urlparse`

		`log = logging.getLogger("__main__")`

rpmbuild/copr_rpmbuild/providers/scm.py

file modified

+2 -4

		`@@ -9,10 +9,8 @@`
		`from ..helpers import run_cmd, CONF_DIRS, get_mock_uniqueext`
		`from .base import Provider`

		`- try:`
		`- from urllib.parse import urlparse`
		`- except ImportError:`
		`- from urlparse import urlparse`
		`+ from six.moves.urllib.parse import urlparse`
		`+`

		`log = logging.getLogger("__main__")`

rpmbuild/copr_rpmbuild/providers/spec.py

file modified

+7 -4

		`@@ -3,6 +3,8 @@`
		`import requests`
		`from ..helpers import run_cmd`
		`from .base import Provider`
		`+ from six.moves.urllib.parse import urlparse`
		`+`

		`log = logging.getLogger("__main__")`

		`@@ -11,10 +13,11 @@`
		`def __init__(self, source_json, outdir, config=None):`
		`super(UrlProvider, self).__init__(source_json, outdir, config)`
		`self.url = source_json["url"]`
		`+ self.parsed_url = urlparse(self.url)`

		`def save_spec(self):`
		`response = requests.get(self.url)`
		`- path = os.path.join(self.workdir, self.url.split("/")[-1])`
		`+ path = os.path.join(self.workdir, self.parsed_url.path.split("/")[-1])`
		`with open(path, "w") as spec:`
		`spec.write(response.text)`
		`return path`
		`@@ -25,7 +28,7 @@`
		`return run_cmd(cmd, cwd=self.workdir)`

		`def download_srpm(self):`
		`- basename = os.path.basename(self.url)`
		`+ basename = os.path.basename(self.parsed_url.path)`
		`filename = os.path.join(self.outdir, basename)`
		`response = requests.get(self.url, stream=True)`
		`if response.status_code != 200:`
		`@@ -38,8 +41,8 @@`
		`f.write(chunk)`

		`def produce_srpm(self):`
		`- if self.url.endswith(".spec"):`
		`+ if self.parsed_url.path.endswith(".spec"):`
		`return self.build_srpm_from_spec()`
		`- if self.url.endswith(".src.rpm"):`
		`+ if self.parsed_url.path.endswith(".src.rpm"):`
		`return self.download_srpm()`
		`raise RuntimeError("Url is not a path to .spec nor .src.rpm file")`

rpmbuild/main.py

file modified

+1 -9

		`@@ -26,15 +26,7 @@`
		`from copr_rpmbuild.helpers import read_config, extract_srpm, locate_srpm, \`
		`SourceType, parse_copr_name, dump_live_log, copr_chroot_to_task_id`

		`- try:`
		`- from urllib.parse import urlparse, urljoin`
		`- except ImportError:`
		`- from urlparse import urlparse, urljoin`
		`-`
		`- try:`
		`- from urllib.parse import urlencode`
		`- except ImportError:`
		`- from urllib import urlencode`
		`+ from six.moves.urllib.parse import urlparse, urljoin, urlencode`

		`log = logging.getLogger(__name__)`
		`log.setLevel(logging.INFO)`

rpmbuild/tests/__init__.py

file modified

		`@@ -27,8 +27,13 @@`
		`"""`

		`class TestCase(unittest.TestCase):`
		`+ def test_setup(self):`
		`+ # to be defined in child class`
		`+ pass`
		`+`
		`def setUp(self):`
		`self.config_path, self.config = self.read_config_data(CONFIG)`
		`+ self.test_setup()`

		`def tearDown(self):`
		`os.unlink(self.config_path)`

rpmbuild/tests/test_spec.py

file modified

+52 -2

		`@@ -1,5 +1,21 @@`
		`import configparser`

		`+ import os`
		`+ import distro`
		`+ import pytest`
		`+ import tempfile`
		`+`
		`+ try:`
		`+ from httmock import urlmatch, HTTMock`
		`+`
		`+ @urlmatch(netloc=r'(.*\.)?example\.com$')`
		`+ def example_com_match(url, request):`
		`+ return 'some-content'`
		`+`
		`+ except:`
		`+ pass`
		`+`
		`+`
		`from copr_rpmbuild.providers.spec import UrlProvider`
		`from . import TestCase`

		`@@ -13,8 +29,7 @@`


		`class TestUrlProvider(TestCase):`
		`- def setUp(self):`
		`- super(TestUrlProvider, self).setUp()`
		`+ def test_setup(self):`
		`self.source_json = {"url": u"http://foo.ex/somepackage.spec"}`
		`self.resultdir = "/path/to/resultdir"`

		`@@ -38,3 +53,38 @@`
		`provider = UrlProvider(self.source_json, self.resultdir, self.config)`
		`provider.save_spec()`
		`mock_open.assert_called_with("{0}/somepackage.spec".format(provider.workdir), "w")`
		`+`
		`+`
		`+ class TestUrlProviderQueryString(TestCase):`
		`+ def test_setup(self):`
		`+ self.json_1 = {`
		`+ 'url': "http://example.com/"`
		`+ "srelay-0.4.8p3-0.20181224.git688764b.fc10.3sunshine.src.rpm?dl=1",`
		`+ }`
		`+ self.json_2 = { 'url': "http://example.com/test.spec?a=1&b=2" }`
		`+ self.resultdir = tempfile.mkdtemp()`
		`+`
		`+ @pytest.mark.skipif(distro.id() in ['rhel', 'centos'] and`
		`+ distro.major_version() == '6',`
		`+ reason='on httmock on rhel6')`
		`+ def test_srpm_query_string(self):`
		`+ with HTTMock(example_com_match):`
		`+ provider = UrlProvider(self.json_1, self.resultdir, self.config)`
		`+ provider.produce_srpm()`
		`+ file = os.path.join(`
		`+ self.resultdir,`
		`+ "srelay-0.4.8p3-0.20181224.git688764b.fc10.3sunshine.src.rpm",`
		`+ )`
		`+ with open(file, 'r') as f:`
		`+ assert f.read() == 'some-content'`
		`+`
		`+ @pytest.mark.skipif(distro.id() in ['rhel', 'centos'] and`
		`+ distro.major_version() == '6',`
		`+ reason='on httmock on rhel6')`
		`+ def test_spec_query_string(self):`
		`+ with HTTMock(example_com_match):`
		`+ provider = UrlProvider(self.json_2, self.resultdir, self.config)`
		`+ filename = provider.save_spec()`
		`+ with open(filename, 'r') as f:`
		`+ assert f.read() == 'some-content'`
		`+ assert filename.endswith('.spec')`

praiskup commented 4 years ago

Fixes: #705, PR#710

rebased onto 69bd388aae67d101c582385df2639208f48872e0

4 years ago

rebased onto 94353a2be6102dfebab87b30a1fce846bdd2a156

4 years ago

frostyx commented 4 years ago

Do I understand this correctly, that the issue is not with redirects as it was suggested in the #705, but rather with downloading the SRPM/spec files properly, but storing them with a wrong filename (ending with foo-1.0-1.src.rpm?dl=1, etc)?

The solution looks really good, but I have some notes about tests.

frostyx commented 4 years ago

The solution looks really good, but I have some notes about tests.

Hmm, maybe I don't, I just didn't understand some things.

1 new commit added

[rpmbuild] use six.moves.urllib.parse

4 years ago

rebased onto b6ca2408c7ab3cd90af7efeabd3d70fe3af1a45f

4 years ago

praiskup commented 4 years ago

but I have some notes about tests.

That part isn't really calligraphy, so feel free to suggest changes.. In the mean time I added a commit which makes at lest imports of urlparse methods nicer.

rebased onto 7dfb269

4 years ago

Pull-Request has been merged by praiskup

4 years ago