Bug 782449 - mock ccache plugin is broken by environment variable settings.
Summary: mock ccache plugin is broken by environment variable settings.
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora EPEL
Classification: Fedora
Component: mock
Version: el6
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Clark Williams
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-17 14:17 UTC by Kirby Zhou
Modified: 2012-05-26 07:37 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-05-26 07:37:20 UTC
Type: ---


Attachments (Terms of Use)

Description Kirby Zhou 2012-01-17 14:17:59 UTC
Description of problem:

Mock-1.1.17 and later version reset environment when do chroot, it brokes ccache plugin. Because ccache plugin want CCACHE_DIR and CCACHE_UMASK to be inherited with chroot.

Version-Release number of selected component (if applicable):

Mock-1.1.17 
Mock-1.1.18

How reproducible:

100%

Steps to Reproduce:
1. enabling ccache pluging
2. build a rpm through mock
3. check the ccache cache dir
  
Actual results:

Nothing cached by ccache

Expected results:


Additional info:

Comment 1 Kirby Zhou 2012-01-17 14:29:28 UTC
By default, ccache plugin want to bind its cache to ${CHROOT}/tmp/ccache, and set CCACHE_DIR=/tmp/ccache.
But actually, CCACHE_DIR is cleared by ChildPreExec, so ccache takes /builddir/.ccache as its cache directory, which is cleaned everytime you want to build a rpm. So ccache takes no effect.

Comment 2 Rex Dieter 2012-01-17 14:32:32 UTC
Isn't that what mock's
 config_opts['plugin_conf']['ccache_opts']['dir'] = "%(cache_topdir)s/%(root)s/ccache/"
cfg option is for?

Comment 3 Kirby Zhou 2012-01-17 16:18:24 UTC
config_opts['plugin_conf']['ccache_opts']['dir'] is the directory outside the mock, it will be bind to '/tmp/ccache' by ccache plugin. The code is line 33 below.


 21 class CCache(object):
 22     """enables ccache in buildroot/rpmbuild"""
 23     decorate(traceLog())
 24     def __init__(self, rootObj, conf):
 25         self.rootObj = rootObj
 26         self.ccache_opts = conf
 27         self.ccachePath = self.ccache_opts['dir'] % self.ccache_opts
 28         rootObj.ccacheObj = self
 29         rootObj.preExistingDeps.append("ccache")
 30         rootObj.addHook("prebuild", self._ccacheBuildHook)
 31         rootObj.addHook("preinit",  self._ccachePreInitHook)
 32         rootObj.umountCmds.append('umount -n %s' % rootObj.makeChrootPath("/tmp/ccache"))
 33         rootObj.mountCmds.append('mount -n --bind %s  %s' % (self.ccachePath, rootObj.makeChrootPath("/tmp/ccache")))

Comment 4 Kirby Zhou 2012-01-17 16:25:53 UTC
The function mockbuild.backend.doChroot() has a arugment 'env', and pass it to mockbuild.util.do(), if env is None, mockbuild.util.do() calls clean_env() to get a very very clean env which contanis only a few env vars without 'CCACHE_DIR'.

So I try

Function mockbuild.backend.shell() and mockbuild.backend.chroot() pass self.env to mockbuild.backend.doChroot, both of the two can handle ccache well if we write "config_opts['environment']['CCACHE_DIR'] = '/tmp/ccache'" in the configuration file. 

But mockbuild.backend.build and other methods DO NOT takes self.env, so nothing I can do without modifying the code of mock to resolve the issue.

Comment 5 Rex Dieter 2012-01-17 16:30:37 UTC
ah, thanks for clearing up my naivety.

Comment 6 Clark Williams 2012-01-27 19:46:25 UTC
Ok, let me see what I can do about getting the CCACHE_* environment variables into the chroot environment

Comment 7 Kirby Zhou 2012-02-03 04:31:05 UTC
It seems mock-1.1.19 partially fixed the bug by both ccache.py:CCache._ccachePreInitHook and backend.py:Root.doChroot.

ccache.py:CCache._ccachePreInitHook updates the rootObj.env, and backend.py:Root.doChroot passes self.env to util.do.

And there is also a bug in uid.py:uidManager.changeOwner which is called by ccache.py:
    def changeOwner(self, path, uid=None, gid=None):
        self._elevatePrivs()
        if uid is None:
            uid = self.unprivUid
        if gid is None:
            gid = uid
        os.chown(path, uid, gid)
...
     it is called by 'self.rootObj.uidManager.changeOwner(self.ccachePath)'

There is no any reason to asssume a gid==uid exist or related to uid.
Maybe we can set gid=-1 or gid=self.unprivGid.

Comment 8 Clark Williams 2012-02-29 23:52:57 UTC
I have no idea why we're setting gid = uid. I'm going to change that to be gid = self.unprivGid.

Comment 9 Clark Williams 2012-03-01 01:05:45 UTC
commit 3c26c9eea88de8f0b6840bd379d78f56aa70bb6f
Author: Clark Williams <williams>
Date:   Wed Feb 29 19:03:32 2012 -0600

    fix incorrect setting of gid in UidManager.changeOwner() [BZ# 782449]
    
    For some reason we were setting gid to uid if gid was not
    passed in. Instead set it to self.unprivGid.
    
    Signed-off-by: Clark Williams <williams>

diff --git a/py/mockbuild/uid.py b/py/mockbuild/uid.py
index 9066fd8..b054f69 100644
--- a/py/mockbuild/uid.py
+++ b/py/mockbuild/uid.py
@@ -73,7 +73,7 @@ class uidManager(object):
         if uid is None:
             uid = self.unprivUid
         if gid is None:
-            gid = uid
+            gid = self.unprivGid
         os.chown(path, uid, gid)
 
 # python doesnt have native versions of these. :(


Applied and queued for next release

Comment 10 Fedora Update System 2012-03-30 18:31:41 UTC
mock-1.1.22-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/mock-1.1.22-1.el6

Comment 11 Fedora Update System 2012-03-30 18:32:13 UTC
mock-1.1.22-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc15

Comment 12 Fedora Update System 2012-03-30 18:32:40 UTC
mock-1.0.29-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/mock-1.0.29-1.el5

Comment 13 Fedora Update System 2012-03-30 18:33:04 UTC
mock-1.1.22-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc17

Comment 14 Fedora Update System 2012-03-30 18:33:50 UTC
mock-1.1.22-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc16

Comment 15 Fedora Update System 2012-05-04 14:31:22 UTC
mock-1.1.22-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mock-1.1.22-2.fc17

Comment 16 Fedora Update System 2012-05-04 22:18:05 UTC
Package mock-1.1.22-2.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing mock-1.1.22-2.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-7324/mock-1.1.22-2.fc17
then log in and leave karma (feedback).

Comment 17 Fedora Update System 2012-05-26 07:37:20 UTC
mock-1.1.22-2.1.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.