#1265 frontend: add support for runtime dependencies
Merged 2 years ago by praiskup. Opened 2 years ago by dturecek.
copr/ dturecek/copr runtime-dependencies  into  master

@@ -0,0 +1,22 @@ 

+ """

+ add runtime dependencies to copr

+ 

+ Revision ID: 6f83ea2ba416

+ Revises: 6d0a02dc7de4

+ Create Date: 2020-02-04 10:39:20.522370

+ """

+ 

+ import sqlalchemy as sa

+ from alembic import op

+ 

+ 

+ revision = '6f83ea2ba416'

+ down_revision = '6d0a02dc7de4'

+ 

+ 

+ def upgrade():

+     op.add_column('copr', sa.Column('runtime_dependencies', sa.Text()))

+ 

+ 

+ def downgrade():

+     op.drop_column('copr', 'runtime_dependencies')

@@ -322,6 +322,11 @@ 

                  validators=[UrlRepoListValidator()],

                  filters=[StringListFilter()])

  

+             runtime_dependencies = wtforms.TextAreaField(

+                 "Runtime dependencies",

+                 validators=[UrlRepoListValidator()],

+                 filters=[StringListFilter()])

+ 

              initial_pkgs = wtforms.TextAreaField(

                  "Initial packages to build",

                  validators=[

@@ -279,6 +279,8 @@ 

      multilib = db.Column(db.Boolean, default=False, nullable=False, server_default="0")

      module_hotfixes = db.Column(db.Boolean, default=False, nullable=False, server_default="0")

  

+     runtime_dependencies = db.Column(db.Text)

+ 

  

  class _CoprPrivate(db.Model, helpers.Serializer):

      """
@@ -548,6 +550,20 @@ 

                  mails.append(perm.user.mail)

          return mails

  

+     @property

+     def runtime_deps(self):

+         """

+         Return a list of runtime dependencies"

+         """

+         dependencies = set()

+         if self.runtime_dependencies:

+             for dep in self.runtime_dependencies.split(" "):

+                 if not dep:

+                     continue

+                 dependencies.add(dep)

+ 

+         return list(dependencies)

+ 

  class CoprPermission(db.Model, helpers.Serializer):

      """

      Association class for Copr<->Permission relation

@@ -153,6 +153,7 @@ 

              placeholder='Optional',

              info='Delete the project after the specfied period of time (empty = disabled)') }}

  

+     {{ render_field(form.runtime_dependencies, rows=5, cols=50, placeholder='Optional - URL to additional yum repos, which can be used as runtime dependencies. Space separated. This should be baseurl from .repo file. E.g.: http://copr-be.cloud.fedoraproject.org/results/rhughes/f20-gnome-3-12/fedora-$releasever-$basearch/') }}

      </div>

    </div>

  

@@ -1,5 +1,5 @@ 

  [{{ repo_id }}]

- name=Copr repo for {{ copr_dir.name }} owned by {{ copr_dir.copr.owner_name }}

+ name={{ name }}

  {{- " (" + arch + ")" if arch }}

  baseurl={{ url | fix_url_https_backend }}

  type=rpm-md

@@ -89,6 +89,16 @@ 

      </ul>

      {% endif %}

  

+     {% if copr.runtime_deps %}

+     <h3>Runtime Dependency List</h3>

+     <p> The following repositories are used as runtime dependencies </p>

+     <ul class=repos-list>

+       {% for repo in copr.runtime_deps %}

+         <li><a href="{{ repo|repo_url }}">{{ repo }}</a></li>

+       {% endfor %}

+     </ul>

+     {% endif %}

+ 

      {% if config.ENABLE_DISCUSSION %}

      {% if config.DISCOURSE_URL %}

      <div id='discourse-comments'></div>

@@ -0,0 +1,8 @@ 

+ [{{ repo_id }}]

+ name={{ name }}

+ baseurl={{ url }}

+ type=rpm-md

+ skip_if_unavailable=True

+ repo_gpgcheck=0

+ enabled=1

+ enabled_metadata=1

@@ -44,7 +44,9 @@ 

  

  from coprs.logic import builds_logic, coprs_logic, actions_logic, users_logic

  from coprs.helpers import generate_repo_url, CHROOT_RPMS_DL_STAT_FMT, \

-     url_for_copr_view, REPO_DL_STAT_FMT, CounterStatType

+     url_for_copr_view, REPO_DL_STAT_FMT, CounterStatType, generate_repo_name, \

+     WorkList

+ 

  

  def url_for_copr_details(copr):

      return url_for_copr_view(
@@ -194,6 +196,7 @@ 

                  follow_fedora_branching=form.follow_fedora_branching.data,

                  delete_after_days=form.delete_after_days.data,

                  multilib=form.multilib.data,

+                 runtime_dependencies=form.runtime_dependencies.data.replace("\n", " "),

              )

  

              db.session.commit()
@@ -476,6 +479,7 @@ 

      copr.delete_after_days = form.delete_after_days.data

      copr.multilib = form.multilib.data

      copr.module_hotfixes = form.module_hotfixes.data

+     copr.runtime_dependencies = form.runtime_dependencies.data.replace("\n", " ")

      if flask.g.user.admin:

          copr.auto_prune = form.auto_prune.data

      else:
@@ -493,6 +497,29 @@ 

      else:

          flask.flash("Project has been updated successfully.", "success")

          db.session.commit()

+ 

+         copr_deps, _, non_existing = get_transitive_runtime_dependencies(copr)

+         deps_without_chroots = {}

+         for copr_dep in copr_deps:

+             for chroot in copr.active_chroots:

+                 if chroot not in copr_dep.active_chroots:

+                     if copr_dep in deps_without_chroots:

+                         deps_without_chroots[copr_dep].append(chroot.name)

+                     else:

+                         deps_without_chroots[copr_dep] = [chroot.name]

+ 

+         if non_existing:

+             flask.flash("Non-existing projects set as runtime dependencies: "

+                         "{0}.".format(", ".join(non_existing)), "warning")

+         for dep in deps_without_chroots:

+             flask.flash("Project {0}/{1} that is set as a dependency doesn't "

+                         "provide all the chroots enabled in this project: {2}."

+                         .format(

+                             dep.owner.name if isinstance(dep.owner, models.User)

+                             else "@" + dep.owner.name,

+                             dep.name, ", ".join(deps_without_chroots[dep])),

+                         "warning")

+ 

      _check_rpmfusion(copr.repos)

  

  
@@ -718,6 +745,46 @@ 

      return flask.redirect(url_for_copr_details(copr))

  

  

+ def get_transitive_runtime_dependencies(copr):

+     """Get a list of runtime dependencies (build transitively from

+     dependencies' dependencies). Returns three lists, one with Copr

+     dependencies, one with list of non-existing Copr dependencies

+     and one with URLs to external dependencies.

+ 

+     :type copr: models.Copr

+     :rtype: List[models.Copr], List[str], List[str]

+     """

+ 

+     if not copr:

+         return [], [], []

+ 

+     wlist = WorkList([copr])

+     internal_deps = set()

+     non_existing = set()

+     external_deps = set()

+ 

+     while not wlist.empty:

+         analyzed_copr = wlist.pop()

+ 

+         for dep in analyzed_copr.runtime_deps:

+             try:

+                 copr_dep = ComplexLogic.get_copr_by_repo_safe(dep)

+             except ObjectNotFound:

+                 non_existing.add(dep)

+                 continue

+ 

+             if not copr_dep:

+                 external_deps.add(dep)

+                 continue

+             if copr == copr_dep:

+                 continue

+             # check transitive dependencies

+             internal_deps.add(copr_dep)

+             wlist.schedule(copr_dep)

+ 

+     return list(internal_deps), list(external_deps), list(non_existing)

+ 

+ 

  @coprs_ns.route("/<username>/<copr_dirname>/repo/<name_release>/", defaults={"repofile": None})

  @coprs_ns.route("/<username>/<copr_dirname>/repo/<name_release>/<repofile>")

  @coprs_ns.route("/g/<group_name>/<copr_dirname>/repo/<name_release>/", defaults={"repofile": None})
@@ -731,19 +798,46 @@ 

      return render_generate_repo_file(copr_dir, name_release, arch)

  

  

- def render_repo_template(copr_dir, mock_chroot, arch=None, cost=None):

-     repo_id = "copr:{0}:{1}:{2}{3}".format(

+ def render_repo_template(copr_dir, mock_chroot, arch=None, cost=None, runtime_dep=None, dependent=None):

+     repo_id = "{0}:{1}:{2}:{3}{4}".format(

+         "coprdep" if runtime_dep else "copr",

          app.config["PUBLIC_COPR_HOSTNAME"].split(":")[0],

          copr_dir.copr.owner_name.replace("@", "group_"),

          copr_dir.name,

          ":ml" if arch else ""

      )

+ 

+     if runtime_dep and dependent:

+         name = "Copr {0}/{1}/{2} runtime dependency #{3} - {4}/{5}".format(

+             app.config["PUBLIC_COPR_HOSTNAME"].split(":")[0],

+             dependent.copr.owner_name, dependent.name, runtime_dep,

+             copr_dir.copr.owner_name, copr_dir.name

+         )

+     else:

+         name = "Copr repo for {0} owned by {1}".format(copr_dir.name, copr_dir.copr.owner_name)

+ 

      url = os.path.join(copr_dir.repo_url, '') # adds trailing slash

      repo_url = generate_repo_url(mock_chroot, url, arch)

      pubkey_url = urljoin(url, "pubkey.gpg")

+ 

      return flask.render_template("coprs/copr_dir.repo", copr_dir=copr_dir,

                                   url=repo_url, pubkey_url=pubkey_url,

-                                  repo_id=repo_id, arch=arch, cost=cost) + "\n"

+                                  repo_id=repo_id, arch=arch, cost=cost,

+                                  name=name)

+ 

+ 

+ def _render_external_repo_template(dep, copr_dir, mock_chroot, dep_idx):

+     repo_name = "coprdep:{0}".format(generate_repo_name(dep))

+     baseurl = helpers.pre_process_repo_url(mock_chroot.name, dep)

+ 

+     name = "Copr {0}/{1}/{2} external runtime dependency #{3} - {4}".format(

+         app.config["PUBLIC_COPR_HOSTNAME"].split(":")[0],

+         copr_dir.copr.owner_name, copr_dir.name, dep_idx,

+         generate_repo_name(dep)

+     )

+ 

+     return flask.render_template("coprs/external_dependency.repo", repo_id=repo_name,

+                                  name=name, url=baseurl) + "\n"

  

  

  @cache.memoize(timeout=5*60)
@@ -783,6 +877,31 @@ 

              models.MockChroot.multilib_pairs[mock_chroot.arch],

              cost=1100)

  

+     internal_deps, external_deps, non_existing = get_transitive_runtime_dependencies(copr)

+     dep_idx = 1

+ 

+     for runtime_dep in internal_deps:

+         owner_name = runtime_dep.owner.name

+         if isinstance(runtime_dep.owner, models.Group):

+             owner_name = "@{0}".format(owner_name)

+         copr_dep_dir = ComplexLogic.get_copr_dir_safe(owner_name, runtime_dep.name)

+         response_content += "\n" + render_repo_template(copr_dep_dir, mock_chroot,

+                                                         runtime_dep=dep_idx,

+                                                         dependent=copr_dir)

+         dep_idx += 1

+ 

+     dep_idx = 1

+     for runtime_dep in external_deps:

+         response_content += "\n" + _render_external_repo_template(runtime_dep, copr_dir,

+                                                                   mock_chroot, dep_idx)

+         dep_idx += 1

+ 

+     for dep in non_existing:

+         response_content += (

+             "\n\n# This repository is configured to have a runtime dependency "

+             "on a Copr project {0} but that doesn't exist.".format(dep[7:])

+         )

+ 

      response = flask.make_response(response_content)

  

      response.mimetype = "text/plain"

@@ -1,3 +1,4 @@ 

+ # pylint: disable=attribute-defined-outside-init

  import base64

  import json

  import os
@@ -138,6 +139,18 @@ 

              self.user_api_creds[u.username] = {"login": u.api_login, "token": u.api_token}

  

      @pytest.fixture

+     def f_groups(self):

+         """ Group for creating group projects."""

+         self.g1 = models.Group(

+             name=u"group1",

+             fas_name="fas_1",

+         )

+ 

+         self.basic_group_list = [self.g1]

+ 

+         self.db.session.add_all(self.basic_group_list)

+ 

+     @pytest.fixture

      def f_coprs(self):

          self.c1 = models.Copr(name=u"foocopr", user=self.u1, repos="", description="""

  ```python
@@ -147,7 +160,11 @@ 

      return 1

  ```""")

          self.c2 = models.Copr(name=u"foocopr", user=self.u2, repos="")

-         self.c3 = models.Copr(name=u"barcopr", user=self.u2, repos="")

+         self.c3 = models.Copr(name=u"barcopr", user=self.u2, repos="",

+                               runtime_dependencies=(

+                                   "copr://user1/foocopr "

+                                   "https://url.to/external/repo"

+                               ))

          self.basic_coprs_list = [self.c1, self.c2, self.c3]

          self.db.session.add_all(self.basic_coprs_list)

  
@@ -195,13 +212,107 @@ 

  

              cc4 = models.CoprChroot()

              cc4.mock_chroot = self.mc4

-             # c3 barcopr with fedora-rawhide-i386

+             cc5 = models.CoprChroot()

+             cc5.mock_chroot = self.mc1

+             # c3 barcopr with fedora-rawhide-i386 fedora-18-x86_64

+             cc5.copr = self.c3

              self.c3.copr_chroots.append(cc4)

-             self.db.session.add_all([cc1, cc2, cc3, cc4])

+ 

+             self.db.session.add_all([cc1, cc2, cc3, cc4, cc5])

  

          self.db.session.add_all([self.mc1, self.mc2, self.mc3, self.mc4])

  

      @pytest.fixture

+     def f_group_copr(self, f_groups, f_mock_chroots):

+         """ Two group Coprs, the first being a dependency for the second."""

+         _side_effects = (f_groups, f_mock_chroots)

+ 

+         self.gc1 = models.Copr(name=u"groupcopr1", user=self.u1, group=self.g1,

+                                repos="")

+         self.gc2 = models.Copr(name=u"groupcopr2", user=self.u1, group=self.g1,

+                                repos="",

+                                runtime_dependencies="copr://@group1/groupcopr1")

+         self.gc1_dir = models.CoprDir(name=u"groupcopr1", copr=self.gc1,

+                                       main=True)

+         self.gc2_dir = models.CoprDir(name=u"groupcopr2", copr=self.gc2,

+                                       main=True)

+         self.group_coprs_list = [self.gc1, self.gc2]

+         self.db.session.add_all(self.group_coprs_list)

+         self.group_copr_dir_list = [self.gc1_dir, self.gc2_dir]

+         self.db.session.add_all(self.group_copr_dir_list)

+ 

+         cc1 = models.CoprChroot()

+         cc1.mock_chroot = self.mc1

+         self.gc1.copr_chroots.append(cc1)

+         cc2 = models.CoprChroot()

+         cc2.mock_chroot = self.mc1

+         self.gc2.copr_chroots.append(cc2)

+         self.db.session.add_all([cc1, cc2])

+ 

+     @pytest.fixture

+     def f_group_copr_dependent(self, f_coprs, f_mock_chroots, f_group_copr):

+         """ Copr dependent on a group copr."""

+         _side_effects = (f_coprs, f_mock_chroots, f_group_copr)

+ 

+         self.c_gd = models.Copr(name=u"depcopr", user=self.u2, repos="",

+                                 runtime_dependencies="copr://@group1/groupcopr1")

+         self.basic_coprs_list.append(self.c_gd)

+         self.db.session.add_all(self.basic_coprs_list)

+ 

+         self.c_gd_dir = models.CoprDir(name=u"depcopr", copr=self.c_gd,

+                                        main=True)

+         self.basic_copr_dir_list.append(self.c_gd_dir)

+         self.db.session.add_all(self.basic_copr_dir_list)

+ 

+         cc = models.CoprChroot()

+         cc.mock_chroot = self.mc1

+         cc.copr = self.c_gd

+ 

+         self.db.session.add_all([cc])

+ 

+ 

+     @pytest.fixture

+     def f_copr_transitive_dependency(self, f_coprs, f_mock_chroots):

+         """ Coprs that have transitive runtime dependency on each other."""

+         _side_effects = (f_coprs, f_mock_chroots)

+ 

+         self.c_td1 = models.Copr(name=u"depcopr1", user=self.u2, repos="",

+                                  runtime_dependencies=(

+                                      "copr://user2/depcopr1 "

+                                      "copr://user2/depcopr2"

+                                  ))

+         self.c_td2 = models.Copr(name=u"depcopr2", user=self.u2, repos="",

+                                  runtime_dependencies=(

+                                      "copr://user2/depcopr3 "

+                                      "http://some.url/"

+                                  ))

+         self.c_td3 = models.Copr(name=u"depcopr3", user=self.u2, repos="",

+                                  runtime_dependencies=(

+                                      "copr://user2/depcopr1 "

+                                      "copr://user2/nonexisting"

+                                  ))

+         self.basic_coprs_list.extend([self.c_td1, self.c_td2, self.c_td3])

+         self.db.session.add_all(self.basic_coprs_list)

+ 

+         self.c_td1_dir = models.CoprDir(name=u"depcopr1", copr=self.c_td1, main=True)

+         self.c_td2_dir = models.CoprDir(name=u"depcopr2", copr=self.c_td2, main=True)

+         self.c_td3_dir = models.CoprDir(name=u"depcopr3", copr=self.c_td3, main=True)

+         self.basic_copr_dir_list.extend([self.c_td1_dir, self.c_td2_dir, self.c_td3_dir])

+         self.db.session.add_all(self.basic_copr_dir_list)

+ 

+         cc1 = models.CoprChroot()

+         cc2 = models.CoprChroot()

+         cc3 = models.CoprChroot()

+         cc1.mock_chroot = self.mc1

+         cc2.mock_chroot = self.mc1

+         cc3.mock_chroot = self.mc1

+         cc1.copr = self.c_td1

+         cc2.copr = self.c_td2

+         cc3.copr = self.c_td3

+ 

+         self.db.session.add_all([cc1, cc2, cc3])

+ 

+     @pytest.fixture

      def f_mock_chroots_many(self):

          """

          Adds more chroots to self.c1

@@ -1,15 +1,18 @@ 

  import json

- import flask

- import pytest

  import re

+ from configparser import ConfigParser

  

  from unittest import mock

  

+ import pytest

+ import flask

  from sqlalchemy import desc

  

+ 

  from copr_common.enums import ActionTypeEnum, ActionPriorityEnum

  from coprs import app, cache, models

  

+ from coprs.helpers import generate_repo_name

  from coprs.logic.coprs_logic import CoprsLogic, CoprDirsLogic

  from coprs.logic.actions_logic import ActionsLogic

  
@@ -700,7 +703,6 @@ 

  

      def test_works_on_older_builds(self, f_users, f_coprs, f_mock_chroots,

                                     f_custom_builds, f_db):

-         from coprs import app

          orig = app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"]

          app.config["ENFORCE_PROTOCOL_FOR_BACKEND_URL"] = "https"

          r = self.tc.get(
@@ -799,6 +801,108 @@ 

          assert normal_baseurl == baseurls[0]

          assert normal_baseurl.rsplit('-', 1)[0] == baseurls[1].rsplit('-', 1)[0]

  

+     @new_app_context

+     def test_repofile_copr_runtime_deps(self, f_users, f_coprs, f_mock_chroots):

+         """

+         Test that a repofile for a project that has runtime dependencies was

+         generated correctly.

+         """

+         _side_effects = (f_users, f_coprs, f_mock_chroots)

+ 

+         repofile = self.tc.get(

+             "/coprs/{0}/{1}/repo/fedora-18/some.repo?arch=x86_64".format(

+                 self.u2.name, self.c3.name))

+ 

+         config = ConfigParser()

+         config.read_string(repofile.data.decode("utf-8"))

+ 

+         name1 = "Copr localhost/user2/barcopr runtime dependency #1 - user1/foocopr"

+         name2 = "Copr localhost/user2/barcopr external runtime dependency #1 - https_url_to_external_repo"

+ 

+         assert len(config.sections()) == 3

+         assert name1 in config.get(config.sections()[1], "name")

+         assert name2 in config.get(config.sections()[2], "name")

+         assert "{0}:{1}".format(self.u1.name, self.c1.name) in config.sections()[1]

+ 

+         url = "https://url.to/external/repo"

+         repo_id = "coprdep:{0}".format(generate_repo_name(url))

+         assert repo_id == config.sections()[2]

+         assert config.get(repo_id, "baseurl") == url

+ 

+     @new_app_context

+     def test_repofile_group_copr_runtime_deps(self, f_users, f_coprs,

+                                               f_mock_chroots, f_group_copr,

+                                               f_group_copr_dependent):

+         """

+         Test that repofiles for a project that has runtime dependency on

+         a group project and a group project with runtime dependency were

+         generated correctly.

+         """

+         _side_effects = (f_users, f_coprs, f_mock_chroots, f_group_copr,

+                          f_group_copr_dependent)

+ 

+         repofile = self.tc.get(

+             "/coprs/{0}/{1}/repo/fedora-18/some.repo?arch=x86_64".format(

+                 self.u2.name, self.c_gd.name))

+ 

+         config = ConfigParser()

+         config.read_string(repofile.data.decode("utf-8"))

+ 

+         name = (

+             "Copr localhost/user2/depcopr runtime dependency #1 - "

+             "@group1/groupcopr"

+         )

+ 

+         assert len(config.sections()) == 2

+         assert name in config.get(config.sections()[1], "name")

+ 

+         repofile = self.tc.get(

+             "/coprs/g/{0}/{1}/repo/fedora-18/some.repo?arch=x86_64".format(

+                 self.g1.name, self.gc2.name))

+ 

+         config = ConfigParser()

+         config.read_string(repofile.data.decode("utf-8"))

+ 

+         name = (

+             "Copr localhost/@group1/groupcopr2 runtime dependency #1 - "

+             "@group1/groupcopr1"

+         )

+ 

+         assert len(config.sections()) == 2

+         assert name in config.get(config.sections()[1], "name")

+ 

+ 

+     @new_app_context

+     def test_repofile_transitive_runtime_deps(self, f_users,

+                                               f_copr_transitive_dependency):

+         """

+         Test that a repofile for a project that has multiple transitive

+         runtime dependencies was generated correctly.

+         """

+         _side_effects = (f_users, f_copr_transitive_dependency)

+ 

+         repofile = self.tc.get(

+             "/coprs/{0}/{1}/repo/fedora-18/some.repo?arch=x86_64".format(

+                 self.u2.name, self.c_td1.name))

+ 

+         config = ConfigParser()

+         config.read_string(repofile.data.decode("utf-8"))

+ 

+         warning = (

+             "# This repository is configured to have a runtime dependency on "

+             "a Copr project user2/nonexisting but that doesn't exist."

+         )

+         assert warning in repofile.data.decode("utf-8")

+ 

+         assert len(config.sections()) == 4

+         assert "coprdep:localhost:{0}:{1}".format(self.u2.name, self.c_td2.name) in config.sections()

+         assert "coprdep:localhost:{0}:{1}".format(self.u2.name, self.c_td3.name) in config.sections()

+ 

+         url = "http://some.url/"

+         repo_id = "coprdep:{0}".format(generate_repo_name(url))

+         assert repo_id == config.sections()[3]

+         assert config.get(repo_id, "baseurl") == url

+ 

  

  class TestSearch(CoprsTestCase):

  

no initial comment

Metadata Update from @dturecek:
- Pull-request tagged with: needs-work

2 years ago

rebased onto 80d3bbf6f56e0fefdef7b96deb52370c4a79a7af

2 years ago

rebased onto 6a1b74ab3fb2bab6ae629f9505e42e9bd32fbb26

2 years ago

If there are multiple dependencies, we need to distinguish between them in dnf log output...
I'd like to at least see Runtime dependency #1 of @copr/copr, or something like that.

The fix_url_https_backend IMO isn't correct. That is for links hosted on backend, but this template is for any external repository (not only copr://).

Shouldn't this be unified with the build-time external repo dependency form handler?

  • can you please attach screenshot of the form?
  • we absolutely need to have new version of dnf-plugins-core released first for this

Long shot comment:
It feels pretty painful to rely on external dnf-plugins-core for such changes,
shouldn't we have some library (say copr-cli-repo package) that would provide
the necessary logic related to enable/disable copr repository? And update
the dnf-plugins-core logic to use our library? That would btw. allow us to have
something like copr-cli enable ... as well.

rebased onto 7d9d746ac283a6aa274c883bb66ba027f189fc79

2 years ago

rebased onto 238ae12497c7186380f1d8335a7a12488b8f5eb3

2 years ago

rebased onto fdaf91d40830e4e393c59de48939fb6e86c07639

2 years ago

Fixed, PTAL.

And the screenshot is here.

rebased onto 6eda926e1179f0d4dc8d21a292c58e0dba631186

2 years ago

Changed repo ID to star with coprdep: for runtime dependencies so it is easier to detect in the dnf copr plugin.

This looks very interesting. Thanks for the update. I need to take a closer look,
so let's concentrate on the dnf-plugins-core proposal for now and I will provide
feedback this week.

  1. I tried this eventually... and there are several problems. E.g. copr://@group/project can
    not be specified, if it is -> the repo file url leads to 404.
  2. the repo file is still cached; we should put the external repos to the repo cache arguments
    so that any change in the repo creates different cache?
  3. copr:// creates coprdep: in repo file, nice! The same needs to be done for (external)

All those ^^ need to have a testcase. Example:

[copr:localhost:group_copr:copr-dev]
name=Copr repo for copr-dev owned by @copr
baseurl=http://copr-be.cloud.fedoraproject.org/results/@copr/copr-dev/fedora-rawhide-$basearch/
type=rpm-md
skip_if_unavailable=True
gpgcheck=1
gpgkey=http://copr-be.cloud.fedoraproject.org/results/@copr/copr-dev/pubkey.gpg
repo_gpgcheck=0
enabled=1
enabled_metadata=1


[coprdep:localhost:praiskup:ping]
name=Runtime dependency #1 praiskup/ping for copr project @copr/copr-dev
baseurl=http://copr-be.cloud.fedoraproject.org/results/praiskup/ping/fedora-rawhide-$basearch/
type=rpm-md
skip_if_unavailable=True
gpgcheck=1
gpgkey=http://copr-be.cloud.fedoraproject.org/results/praiskup/ping/pubkey.gpg
repo_gpgcheck=0
enabled=1
enabled_metadata=1


[https_example_com_test_releasever_basearch_os]
name=Runtime dependency #2 (external) for copr project @copr/copr-dev
baseurl=https://example.com/test/$releasever/$basearch/os/
type=rpm-md
skip_if_unavailable=True
repo_gpgcheck=0
enabled=1
enabled_metadata=1

What about even better spelling:

- Runtime dependency #1 praiskup/ping for copr project @copr/copr-dev
+ Copr hostname/praiskup/ping dependancy #1 - @copr/copr-dev

- Runtime dependency #2 (external) for copr project @copr/copr-dev
+ Copr hostname/praiskup/ping external dependancy #1 - https_example_com_test_releasever_basearch_os

I added non-existing copr://foo/bar project to one copr, and the
repofile started to throw 404 error. Can you please keep the repo file
working in such case - and just put there some kind of comment:
# this repo is configured to depend on foo/bar, but that doesn't exist

Can you please add test for transitive dependenciy, e.g. if
coprA depends on coprB which depends on coprC?

rebased onto b044c29d83c9a4c94907de4b68a80261da7aa858

2 years ago

rebased onto 25e052164a7c6829aba900ed4df95b65e9c647c1

2 years ago

I've fixed the transitive dependency and addressed your comments. Only thing still missing is the test for dependency on a group project as I was not able to figure out how to do the test (yet).

rebased onto 8558a8b8bbcbbf7940616e878982fc2dcdd55596

2 years ago

I've figured out the group project for the test, I think it's ready for review.

Metadata Update from @dturecek:
- Pull-request untagged with: needs-work

2 years ago

Please document the input/output of this method (format of output isn't clear from quickly looking at it).

What if project B depends on project C which depends on .... this is far from
complete. Consider this pseudocode:

deps = {}
coprs_todo = {copr}
while coprs_todo:
    analyzed_copr = coprs_todo.pop()
    for dep in analyzed_copr.runtime_deps():
        if dep in deps or dep_is_this_copr:
            # close the loops
            continue

        deps.add(dep)
        if dep_is_copr(dep):
            coprs_todo.add(query_db_for_dep(dep))

I wouldn't be overoptimistic :-) and I'd rather expect that this field is misformated in DB.
I.e. at least consider that the repo could be separated by multiple spaces.

Perhaps we could return two sets? One with already resolved coprs, and
one with generic strings with baseurls? That would allow us to not query
the database once more for the same thing.

We could have something like Copr.repo_deps property in models.py returning
the parsed list of urls. That would make the code much more obvious.

My code does the same thing (with the exception on checking if the dependency is the copr itself). So for example, if I have project A depending on B, B depending on C, C depending on D and D depending on A, I will have repofile with A that has dependencies on B, C, D , A.

My code does the same thing

I missed this command, you are right. I was distracted by otherwise pretty complicated reading. The rest of comments still apply.

Can you please use this abstraction? https://pagure.io/copr/copr/pull-request/1359 ?
The overall code (not pseudocode) should be much more readable:

deps = set()
copr_deps = set()

wlist = WorkList([copr])
while not wlist.empty:
    analyzed_copr = wlist.pop()

    # runtime_deps is property of 'class Copr:'
    for dependancy in analyzed_copr.runtime_deps:
        copr_dep = ComplexLogic.get_copr_by_repo_safe(dependancy)
        if not copr_dep:
            deps.add(dependancy)
            continue
        if copr == copr_dep:
            continue
        # transitive deps
        copr_deps.add(copr_dep)
        wlist.schedule(copr_dep)

# return tuple, so we don't have to re-query?
return (copr_deps, deps)

Can you please use this abstraction?

I will.

rebased onto c1732a7d9b8739330a394eeedb9d146466daab46

2 years ago

Fixed the code. I've also changed the test for transitive dependencies a little to make it more robust.

Still have some pylint issues to solve.

I thought that _safe suffix means that we don't have to have try/except wrapper. But it isn't truth, can anyone tell me what it is for originally?

I usually wrap the super-long lines to 80 characters (I know, I know ... we have 120...) by

response_content += (
    "Hello "
    "pandemic "
    "world!"
) 

trick.

I'd slightly prefer (copr, external, non_existing) variant) ... but this is not a huge requirement.

This looks really nice! Thank you for the update.

Still have some pylint issues to solve.

Ping me when you see me online, I have some tips.

praiskup-blah-fedora-30.repo.xz

Some OCD nitpicks... can we have the same amount (single?) spaces between
all the repositories? Also, having the warning # This repository is configured to have ... could be at the end of the file.

I tried "user project" -> "group project" -> "normal project" now, and
the group project -> normal project dependency doesn't work.

Some UI hints:
- we should render the list of dependencies on project home page, same as we do for build-time only repositories
- Runtime dependencies: field should have the same comment as External Repositories: field (that the copr:// is allowed, $distname as well, ...

pretty please pagure-ci rebuild

2 years ago

rebased onto ad5b42401babd66e80fa35062d509d37f8fee9f9

2 years ago

rebased onto 5252f9fc4fdb08367bf346ef25de67895bf83bc4

2 years ago

rebased onto d623b8f0817188f08ae1412c9e681d685063ccd4

2 years ago

This is extremely fishy call. I can return None, it can return copr, or it can raise exception. :-) But that's not about to be fixed here, /me shrugs

Looks good, just please rebase against master .. for alembic migration.

Ah, I forgot about copr project homepage update...

rebased onto 9ec42464db0d3cb39b5ce85a1d4388fceeea1501

2 years ago

Fixed the migration and rebased.

Ah, I forgot about copr project homepage update...

I mean, "External repositories" are listed on project homepage. Can you
list (not necessarily transitively) list of runtime dependent projects, too?

rebased onto 808012516dc8c74b252c0f957fd73f217ba5df58

2 years ago

Sure, I've added the list as well as a flash warning when you try to add non-existing dependency or a dependency that doesn't have all chroots enabled.

Screenshot

I'm not too happy with the wording of the second warning message though, so I'd be glad for any tip to make it better.

I'm not too happy with the wording of the second warning message though

The dependancy 'copr://some/thing' doesn't exist, ignoring.?

This is the flash I got when testing:

These projects set as runtime dependencies don't have all the chroots enabled as this project does: copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr, copr/copr.

Perhaps nicer wording:
Project @copr/copr you started to depend on doesn't provide all the chroots enabled in this project: epel-7-x86_64, epel-8-x86_64, ...

rebased onto ffb144119771a2c7b47f3572e92497a463876166

2 years ago

I've changed the flash message: Screenshot.

Looks very nice, thank you. +1

rebased onto ed651ba

2 years ago

Pull-Request has been merged by praiskup

2 years ago
Metadata
Flags
jenkins
failure
Build #131 failed (commit: ed651bae)
2 years ago
Copr build
success (100%)
#1373968
2 years ago
jenkins
failure
Build #124 failed (commit: ffb14411)
2 years ago
Copr build
success (100%)
#1371883
2 years ago
jenkins
failure
Build #115 failed (commit: 80801251)
2 years ago
Copr build
success (100%)
#1369283
2 years ago
jenkins
failure
Build #112 failed (commit: 9ec42464)
2 years ago
Copr build
success (100%)
#1369053
2 years ago
jenkins
failure
Build #94 failed (commit: d623b8f0)
2 years ago
Copr build
success (100%)
#1366230
2 years ago
jenkins
failure
Build #93 failed (commit: 5252f9fc)
2 years ago
Copr build
success (100%)
#1366212
2 years ago
jenkins
failure
Build #92 failed (commit: ad5b4240)
2 years ago
Copr build
success (100%)
#1366201
2 years ago
jenkins
failure
Build #91 failed (commit: c1732a7d)
2 years ago
jenkins
failure
Build #83 failed (commit: c1732a7d)
2 years ago
Copr build
success (100%)
#1354107
2 years ago
jenkins
failure
Build #66 failed (commit: 8558a8b8)
2 years ago
Copr build
failure
#1351101
2 years ago
jenkins
failure
Build #59 failed (commit: 25e05216)
2 years ago
Copr build
failure
#1346046
2 years ago
jenkins
failure
Build #58 failed (commit: b044c29d)
2 years ago
Copr build
failure
#1345834
2 years ago
Copr build
success (100%)
#1326094
2 years ago
Copr build
success (100%)
#1323760
2 years ago
Copr build
success (100%)
#1323759
2 years ago
Copr build
success (100%)
#1323606
2 years ago
Copr build
failure
#1257974
2 years ago
Copr build
success (100%)
#1242198
2 years ago