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 -- [email protected] To unsubscribe send an email to [email protected]
