On Tuesday, October 18, 2016 4:01:10 PM CEST Michal Novotny wrote: > The code implements support for building from Fedora DistGit. The > subcommand of copr-cli is called `buildfedpkg` to suggest that. > > Naming inside COPR code was chosen as such because the idea is to make > DistGit source general.
To make this general, you need `git url` and `lookaside cache` info. Now you have only `clone url`, which is neither general, nor Fedora specific (there's no need for clone url in case of fedpkg). Pavel > clime > > On Tue, Oct 18, 2016 at 11:44 AM, Pavel Raiskup <prais...@redhat.com> wrote: > > > Thanks for the dist-git patch [1]! > > > > There is an issue with naming, what does "dist-git" mean? Is that copr's > > dist-git? Is that Fedora's dist-git? Is that Red Hat Internal dist-git? > > > > Can each package be built from different dist-git instance? If not, why? > > > > +class DistGitProvider(SrpmBuilderProvider): > > + def get_srpm(self): > > + cmd = ['git', 'clone', self.task.distgit_clone_url, self.tmp_dest] > > + output, error = VM.run(cmd, dst_dir=self.tmp_dest, > > name=self.task.task_id) > > + log.info(output) > > + > > + if self.task.distgit_branch: > > + cmd = ['fedpkg', '--dist', self.task.distgit_branch, 'srpm'] > > + else: > > + cmd = ['fedpkg', 'srpm'] > > > > Please consider naming this Provider like FedoraDistGit, and rename > > everything API related into Fedora namespace, because this is not generic > > "dist-git". If we have it "renamed", this would be fine contribution ... > > but otherwise we waste namespace in api without a way back, and forever -- > > DON'T RELEASE YET. > > > > Internally (RHEL) we also have dist-git, and it would be really nice to > > have > > this configurable so we can reuse this, or add RHELDistGit, too.. > > > > + output, error = VM.run(cmd, dst_dir=self.tmp_dest, > > name=self.task.task_id, cwd=self.tmp_dest) > > > > Please consider moving all this expensive tasks onto builder. That's the > > only > > _secure_ way to add "SRPM providers" and it is the only scalable way (koji > > does > > this right, we can possibly copy code from there - or at least study). > > Also, > > VM != virtualization in this case, which is confusing. > > > > + log.info(output) > > + self.copy() > > ... > > > > Seriously, this is basically good code, but start doing reviews so people > > involved can discuss design issues. > > > > [1] https://github.com/fedora-copr/copr/commit/308daed9235cb > > > > Pavel > > > > _______________________________________________ > > copr-devel mailing list -- copr-devel@lists.fedorahosted.org > > To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org > > > _______________________________________________ copr-devel mailing list -- copr-devel@lists.fedorahosted.org To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org
