#38 Fixes needed for Copr
Merged by clime. Opened by praiskup.
praiskup/rpkg-util copr-fixes  into  master

Download 38.patch
no initial comment

I suggest using this snippet from rpkg 2:

        repo_path_parts = subs['repo_path'].split('/')
        subs['name'] = repo_path_parts[-1]
        ns_index = 1
        for part in reversed(repo_path_parts[:-1]):
            key = 'ns'+str(ns_index)
            subs[key] = part
            ns_index += 1

It is general (any number of namespaces) and uses name instead of module which is quite an overloaded word :).

This should definitely be possible (to download from src.fp.o forks) but it shouldn't be the default in rpkg.conf which uses also variable domain name (${git_props:remote_netloc}) and is thus non-specific to src.fp.o.

So what should stay here? I'm a bit lost ... What DistGit instance is this useful for?

So what should stay here? I'm a bit lost ... What DistGit instance is this useful for?

Please, keep rpkg.conf file as it is in master branch. it's the default the version 3 was released with and it cannot really be changed without a new major version bump.

What DistGit instance is this useful for?

Probably copr-dist-git which has multi-level namespaces.

Just to be sure - you don't expect that rpkg-util will work locally "by default" for builds
like those?
https://copr.fedorainfracloud.org/coprs/praiskup/ping/build/3258338/

I mean, unless we change the default - rpkg-util has no chance to guess where to download the sources from.

Just to be sure - you don't expect that rpkg-util will work locally "by default" for builds
like those?
https://copr.fedorainfracloud.org/coprs/praiskup/ping/build/3258338/

I mean, unless we change the default - rpkg-util has no chance to guess where to download the sources from.

Yes, I don't expect it to work for src.fp.o forks by default. If this proves to be a problem, we can:
1) --repo-path param can be added to srpm subcommand (it probably should anyway as it is already present for sources) which will set the appropriate segment of download URL and this will be visible in the copr srpm build log.
2) the whole rpkg.conf which is now dynamically generated in Copr can be converted to command-line args through -c params (can be mentioned multiple times but probably only setting download_url dynamically would be enough)
3) perhaps some extension of configuration file that would fix this for src.fp.o specifically but left the current default untouched otherwise (would need some thinking how to do this properly though)

We can do one those even now if you think it should be done (3 might be a bit difficult).

rebased onto c3842d999bfca25c756711a8480054d24a8bf7bb

Ok as long as it is acceptable to not support forks by default upstream, I think we are OK
to drop this functionality from copr long-term as well. I still updated this pull
request, as I find it useful. PTAL

2) the whole rpkg.conf which is now dynamically generated in Copr

But you simply cannot tell the users "install rpkg-util and try locally".

This really, really complicates the whole logic - and long therm I wish we could
drop it (and have just /etc/rpkg.conf). The way copr-distgit-client works is
much more convenient.

Ok as long as it is acceptable to not support forks by default upstream, I think we are OK
to drop this functionality from copr long-term as well.

Well, of course I would like the functionality to work/stay in Copr but it depends on you guys.

I couidn't simply support all the dist-gits by default (by a single generic configuration) so I picked what seemed to be the most sensible default to me but I think, if needed, there might be good ways to extend that support if needed.

I still updated this pull

request, as I find it useful. PTAL

Thank you.

2) the whole rpkg.conf which is now dynamically generated in Copr

But you simply cannot tell the users "install rpkg-util and try locally".

This really, really complicates the whole logic - and long therm I wish we could
drop it (and have just /etc/rpkg.conf). The way copr-distgit-client works is
much more convenient.

Well, you can have just the default rpkg.conf and generate the --repo-path param accordingly to the target dist-git while everything else staying the same (%(repo_path) should be the only differentiaing part in the URL) (this was basically proposal 1))

But you simply cannot tell the users "install rpkg-util and try locally".

I think you can, if you tell them to follow the build log which is necessary even today.

This really, really complicates the whole logic - and long therm I wish we could
drop it (and have just /etc/rpkg.conf).

I am not sure I understand what logic it complicates and what should be dropped. Sorry.

The way copr-distgit-client works is much more convenient.

Where is the project hosted so that I can take a look? Also feel free the to explain.

I am not sure I understand what logic it complicates and what should be dropped. Sorry.

The way we configure rpkg.conf, using faked $HOME, and this:
https://pagure.io/copr/copr/blob/00bd99399d0694bebfc3141e3875c1e79b07500e/f/rpmbuild/copr_rpmbuild/providers/scm.py#_65-70

Where is the project hosted so that I can take a look? Also feel free the to explain.

dnf install -y copr-distgit-client

Just clone the repo (any dist-git instance), checkout branch, and hit copr-distgit-client.

The way we configure rpkg.conf, using faked $HOME,

I mean :-) in ansible.git we have two rpkg.conf (one replacing the system default -ie this pull request, and one replacing the /etc/copr-rpmbuild/rpkg.conf.j2) and no-one really fully understands how to configure it properly. Multiply the problems with v2 vs. v3 now.

dnf install -y copr-distgit-client

Just clone the repo (any dist-git instance), checkout branch, and hit copr-distgit-client.

Right, ok, cool.

I mean :-) in ansible.git we have two rpkg.conf (one replacing the system default -ie this pull request, and one replacing the /etc/copr-rpmbuild/rpkg.conf.j2) and no-one really fully understands how to configure it properly. Multiply the problems with v2 vs. v3 now.

Only the jinja template should be needed. The one in the ansible shouldn't. I think the ansible one was needed when rpkg was cloning repos from copr-dist-git but that's not the case for quite some time.

As for the fake "$HOME" and generating the config - it would be better to generate instead everything as command-line args (i.e. -c 'lookaside.download_url=' or just the --repo-path proposal) so no rpkg.conf.j2 would be needed either.

But here: https://pagure.io/copr/copr/blob/main/f/rpmbuild/main.ini#_48, there should be the %(ns1)/%(name) stuff instead of %(repo_url) so currently cloning from forks doesn't work?

(i.e. this log is still rpkg 2: https://download.copr.fedorainfracloud.org/results/praiskup/ping/srpm-builds/03258338/builder-live.log.gz ?)

That might be something I might have overlooked.

There's no %(ns1) (till this PR is merged). So cloning from forks doesn't work, I applied
the patch here in production (and we have ansible.git patch for rpkg.conf, too).

And yeah, we switched to F35 later today:
https://pagure.io/fedora-infra/ansible/c/90cc3214648338d3b224a9c7c1c5aa59f63f883d?branch=main

There's no %(ns1) (till this PR is merged). So cloning from forks doesn't work, I applied
the patch here in production (and we have ansible.git patch for rpkg.conf, too).

I see but whatever is in /etc/rpkg.conf will get overrwritten by the config generated from rpkg.conf.j2 because that contains download_url and it has precedence over /etc/rpkg.conf so I wonder how did you actually manage to fix it.

And yeah, we switched to F35 later today:
https://pagure.io/fedora-infra/ansible/c/90cc3214648338d3b224a9c7c1c5aa59f63f883d?branch=main

Cool!

how did you actually manage to fix it.

Well "fix" ... it seems to work at least :-)
https://pagure.io/fedora-infra/ansible/blob/90cc3214648338d3b224a9c7c1c5aa59f63f883d/f/roles/copr/backend/files/provision/provision_builder_tasks.yml#_266-268
https://pagure.io/fedora-infra/ansible/blob/90cc3214648338d3b224a9c7c1c5aa59f63f883d/f/roles/copr/backend/templates/provision/copr-rpmbuild/main.ini.j2#_12

how did you actually manage to fix it.

Well "fix" ... it seems to work at least :-)
https://pagure.io/fedora-infra/ansible/blob/90cc3214648338d3b224a9c7c1c5aa59f63f883d/f/roles/copr/backend/files/provision/provision_builder_tasks.yml#_266-268
https://pagure.io/fedora-infra/ansible/blob/90cc3214648338d3b224a9c7c1c5aa59f63f883d/f/roles/copr/backend/templates/provision/copr-rpmbuild/main.ini.j2#_12

Ahh...okay :). Thanks a lot for fixing it.

Ok, I will merge this and release new rpkg-util today so that it gets to repos asap.

rebased onto 25d3c2ce0ee6f074dfe39eba513aac2bddeee8b9

Pull-Request has been merged by clime

This PR has been migrated to GitHub as a placeholder issue:
https://github.com/fedora-copr/rpkg-util/issues/38

Metadata