This should allow people to build from git repositories that have a different structure than Fedora (they still need some magic with source_cmd). The patch is not too intrusive and can be set per repository.
Let me know if it can be considered for inclusion. I am happy to rework it.
--- diff --git a/builder/kojid b/builder/kojid index b3d0d1c..89f9474 100755 --- a/builder/kojid +++ b/builder/kojid @@ -3292,7 +3292,7 @@ class BuildSRPMFromSCMTask(BaseBuildTask): self.patch_scm_source(sourcedir, logfile, opts)
# Find and verify that there is only one spec file. - spec_files = glob.glob("%s/*.spec" % sourcedir) + spec_files = glob.glob("%s/%s*.spec" % (sourcedir, scm.spec_path)) if len(spec_files) == 0: raise koji.BuildError("No spec file found") elif len(spec_files) > 1: diff --git a/koji/daemon.py b/koji/daemon.py index 205f62e..68af212 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -200,6 +200,7 @@ class SCM(object): - use_common (defaults to True, may be set by assert_allowed()) - source_cmd (defaults to ['make', 'sources'], may be set by assert_allowed()) - scmtype + - spec_path
The exact format of each attribute is SCM-specific, but the structure of the url must conform to the template above, or an error will be raised. @@ -220,6 +221,7 @@ class SCM(object): self.revision = fragment self.use_common = True self.source_cmd = ['make', 'sources'] + self.spec_path = '/'
for scmtype, schemes in SCM.types.items(): if self.scheme in schemes: @@ -311,6 +313,10 @@ class SCM(object): else: # there was nothing after the trailing :, so they don't want to run a source_cmd at all self.source_cmd = None + # check if spec path is defined + if len(scm_tuple) >= 5: + if scm_tuple[4]: + self.spec_path = scm_tuple[4] + '/' break else: self.logger.warn('Ignoring incorrectly formatted SCM host:repository: %s' % allowed_scm)
Disclaimer: I'm not a Koji maintainer
On Wed, 2014-08-06 at 17:42 +0200, Thomas wrote:
# check if spec path is defined
if len(scm_tuple) >= 5:
if scm_tuple[4]:
self.spec_path = scm_tuple[4] + '/'
Not commenting on whether this is desired or acceptable, but if you're adding a new part to the allowed_scms tuple, could you also document it in the kojid.conf file, to help admins know about this parameter?
Right now there already is an undocumented option (source_cmd) which admins can only discover by reading the source code. I just sent a patch to fix that, but it would be nice if we didn't add one more undocumented option. :)
Good point I'll add your patch and update the kojid.conf. thanks.
On Thu, Aug 7, 2014 at 10:01 AM, Mathieu Bridon bochecha@fedoraproject.org wrote:
Disclaimer: I'm not a Koji maintainer
On Wed, 2014-08-06 at 17:42 +0200, Thomas wrote:
# check if spec path is defined
if len(scm_tuple) >= 5:
if scm_tuple[4]:
self.spec_path = scm_tuple[4] + '/'
Not commenting on whether this is desired or acceptable, but if you're adding a new part to the allowed_scms tuple, could you also document it in the kojid.conf file, to help admins know about this parameter?
Right now there already is an undocumented option (source_cmd) which admins can only discover by reading the source code. I just sent a patch to fix that, but it would be nice if we didn't add one more undocumented option. :)
-- Mathieu
-- buildsys mailing list buildsys@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/buildsys
On 08/06/2014 11:42 AM, Thomas wrote:
This should allow people to build from git repositories that have a different structure than Fedora (they still need some magic with source_cmd). The patch is not too intrusive and can be set per repository.
I really hate to add yet another flag to the already tedious scm config format, especially when I really doubt we'll have cause to set it to anything other than these two values. Probably better just to do something like this:
@@ -3417,6 +3417,9 @@ class BuildSRPMFromSCMTask(BaseBuildTask):
# Find and verify that there is only one spec file. spec_files = glob.glob("%s/*.spec" % sourcedir) + if not spec_files: + # also check SPECS dir + spec_files = glob.glob("%s/SPECS/*.spec" % sourcedir) if len(spec_files) == 0: raise koji.BuildError("No spec file found") elif len(spec_files) > 1:
This much gets you halfway there, but it looks like the system is just going to choke later on having the sources under SOURCES instead of the git root dir.
I guess you could set up a source_cmd to move or link the sources around, but that's hardly a clean solution.
I wouldn't mind teaching koji to handle this layout, but it looks like it might be a little more involved than just the spec portion (unless I'm missing something). Did you have a solution for the SOURCES dir?
Let me know if it can be considered for inclusion. I am happy to rework it.
diff --git a/builder/kojid b/builder/kojid index b3d0d1c..89f9474 100755 --- a/builder/kojid +++ b/builder/kojid @@ -3292,7 +3292,7 @@ class BuildSRPMFromSCMTask(BaseBuildTask): self.patch_scm_source(sourcedir, logfile, opts)
# Find and verify that there is only one spec file.
spec_files = glob.glob("%s/*.spec" % sourcedir)
spec_files = glob.glob("%s/%s*.spec" % (sourcedir, scm.spec_path)) if len(spec_files) == 0: raise koji.BuildError("No spec file found") elif len(spec_files) > 1:
diff --git a/koji/daemon.py b/koji/daemon.py index 205f62e..68af212 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -200,6 +200,7 @@ class SCM(object): - use_common (defaults to True, may be set by assert_allowed()) - source_cmd (defaults to ['make', 'sources'], may be set by assert_allowed()) - scmtype
- spec_path The exact format of each attribute is SCM-specific, but the
structure of the url must conform to the template above, or an error will be raised. @@ -220,6 +221,7 @@ class SCM(object): self.revision = fragment self.use_common = True self.source_cmd = ['make', 'sources']
self.spec_path = '/' for scmtype, schemes in SCM.types.items(): if self.scheme in schemes:
@@ -311,6 +313,10 @@ class SCM(object): else: # there was nothing after the trailing :, so they don't want to run a source_cmd at all self.source_cmd = None
# check if spec path is defined
if len(scm_tuple) >= 5:
if scm_tuple[4]:
self.spec_path = scm_tuple[4] + '/' break else: self.logger.warn('Ignoring incorrectly formatted SCM
host:repository: %s' % allowed_scm)
buildsys mailing list buildsys@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/buildsys
On 08/13/2014 06:05 PM, Mike McLean wrote:
This much gets you halfway there, but it looks like the system is just going to choke later on having the sources under SOURCES instead of the git root dir.
I guess you could set up a source_cmd to move or link the sources around, but that's hardly a clean solution.
I wouldn't mind teaching koji to handle this layout, but it looks like it might be a little more involved than just the spec portion (unless I'm missing something). Did you have a solution for the SOURCES dir?
Actually not that complicated after all. I did add an option to turn the support off in case it causes a problem (possible that someone could have a SOURCES dir in their checkout for some other reason).
Anyway, here is where I'm at. Seems to be working.
Hi All,
I updated your patch so it does not assume sourcedir=chrootdir But it's not yet the perfect solution.
For reference the layout is: SPEC/ SOURCES/ .metadata-file
diff --git a/builder/kojid b/builder/kojid index 821f19d..4210823 100755 --- a/builder/kojid +++ b/builder/kojid @@ -455,6 +455,10 @@ class BuildRoot(object):
def build_srpm(self, specfile, sourcedir, source_cmd): self.session.host.setBuildRootState(self.id,'BUILDING') + if self.options.support_rpm_source_layout: + sources_dir = "%s/SOURCES" % sourcedir + else: + sources_dir = sourcedir if source_cmd: # call the command defined by source_cmd in the chroot so any required files not stored in # the SCM can be retrieved @@ -466,7 +470,7 @@ class BuildRoot(object): self.expire() raise koji.BuildError, "error retrieving sources, %s" % self._mockResult(rv)
- args = ['--no-clean', '--buildsrpm', '--spec', specfile, '--sources', sourcedir, + args = ['--no-clean', '--buildsrpm', '--spec', specfile, '--sources', sources_dir, '--target', 'noarch']
rv = self.mock(args) @@ -3793,10 +3797,7 @@ class BuildSRPMFromSCMTask(BaseBuildTask):
#build srpm self.logger.debug("Running srpm build") - sources_dir = "%s/SOURCES" % sourcedir - if not self.options.support_rpm_source_layout or not os.path.isdir(sources_dir): - sources_dir = sourcedir - broot.build_srpm(spec_file, sources_dir, scm.source_cmd) + broot.build_srpm(spec_file, sourcedir, scm.source_cmd)
srpms = glob.glob('%s/*.src.rpm' % broot.resultdir()) if len(srpms) == 0:
On Fri, Aug 15, 2014 at 12:10 AM, Mike McLean mikem@redhat.com wrote:
On 08/13/2014 06:05 PM, Mike McLean wrote:
This much gets you halfway there, but it looks like the system is just going to choke later on having the sources under SOURCES instead of the git root dir.
I guess you could set up a source_cmd to move or link the sources around, but that's hardly a clean solution.
I wouldn't mind teaching koji to handle this layout, but it looks like it might be a little more involved than just the spec portion (unless I'm missing something). Did you have a solution for the SOURCES dir?
Actually not that complicated after all. I did add an option to turn the support off in case it causes a problem (possible that someone could have a SOURCES dir in their checkout for some other reason).
Anyway, here is where I'm at. Seems to be working.
On 10/22/2014 03:55 AM, Thomas wrote:
Hi All,
I updated your patch so it does not assume sourcedir=chrootdir But it's not yet the perfect solution.
This looks reasonable, except that you dropped the directory existence check, which makes the code /require/ the new layout if support_rpm_source_layout is enabled. This is something of an interim solution and I'd rather not break things for other folks (esp since support_rpm_source_layout defaults to True).
I think the fix is something like this:
For reference the layout is: SPEC/ SOURCES/ .metadata-file
diff --git a/builder/kojid b/builder/kojid index 821f19d..4210823 100755 --- a/builder/kojid +++ b/builder/kojid @@ -455,6 +455,10 @@ class BuildRoot(object):
def build_srpm(self, specfile, sourcedir, source_cmd): self.session.host.setBuildRootState(self.id,'BUILDING')
if self.options.support_rpm_source_layout:
sources_dir = "%s/SOURCES" % sourcedir
else:
sources_dir = sourcedir if source_cmd: # call the command defined by source_cmd in the chroot so
any required files not stored in # the SCM can be retrieved @@ -466,7 +470,7 @@ class BuildRoot(object): self.expire() raise koji.BuildError, "error retrieving sources, %s" % self._mockResult(rv)
args = ['--no-clean', '--buildsrpm', '--spec', specfile,
'--sources', sourcedir,
args = ['--no-clean', '--buildsrpm', '--spec', specfile,
'--sources', sources_dir, '--target', 'noarch']
rv = self.mock(args)
@@ -3793,10 +3797,7 @@ class BuildSRPMFromSCMTask(BaseBuildTask):
#build srpm self.logger.debug("Running srpm build")
sources_dir = "%s/SOURCES" % sourcedir
if not self.options.support_rpm_source_layout or not
os.path.isdir(sources_dir):
sources_dir = sourcedir
broot.build_srpm(spec_file, sources_dir, scm.source_cmd)
broot.build_srpm(spec_file, sourcedir, scm.source_cmd) srpms = glob.glob('%s/*.src.rpm' % broot.resultdir()) if len(srpms) == 0:
On Fri, Aug 15, 2014 at 12:10 AM, Mike McLean mikem@redhat.com wrote:
On 08/13/2014 06:05 PM, Mike McLean wrote:
This much gets you halfway there, but it looks like the system is just going to choke later on having the sources under SOURCES instead of the git root dir.
I guess you could set up a source_cmd to move or link the sources around, but that's hardly a clean solution.
I wouldn't mind teaching koji to handle this layout, but it looks like it might be a little more involved than just the spec portion (unless I'm missing something). Did you have a solution for the SOURCES dir?
Actually not that complicated after all. I did add an option to turn the support off in case it causes a problem (possible that someone could have a SOURCES dir in their checkout for some other reason).
Anyway, here is where I'm at. Seems to be working.
buildsys@lists.fedoraproject.org