New version of mock working (I think)

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

All,

The inside-out version of mock (or upside-down, depending on your
perspective) seems to be working. For those that don't know what I'm
talking about, we've been looking at changing the way mock deals with
uid/gid manipulation to improve security a bit. The old way was for
/usr/bin/mock to be a link to mock.py and whenever mock wanted to do
something that required root privilege, it called a setuid root
program called /usr/sbin/mock-helper. This program knew how to do a
select few commands (chroot, mount, etc.) and did some argument
validation. Unfortunately mock-helper has some security issues and
extending it would require us to write more C code. While most of us
aren't afraid of writing C code, writing *secure* C code is not simple
and in this case it's probably not worth the effort.

Someone (Michael?) suggested that we turn everything around and write
a simple setuid root/setgid mock launcher program that would then
start mock.py and allow it to manipulate privilege from python code.
I wrote the first cut at a launcher and then added code to mock.py to
elevate and drop privileges around commands that needed it. I now have
a set of code that will build simple SRPMS (elinks, rsync, tar, etc.)
and would like to get some other eyeballs on this code. The "new"
organization is we have a /usr/bin/mock that is a setuid C program
which only knows how to exec /usr/bin/mock.py.

While none of the changes are massive, they are spread across a few
files (Makefile, mock.py, mock.spec, etc/default.cfg, src/Makefile,
src/mock.c) so I'm wondering if I should just blast out the files to
the list, or if I should cut a branch in CVS and let people look at it
from there? Or, should I just check it in and if the consensus is that
it sucks, we can change or revert?

Thoughts?

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEmZwMHyuj/+TTEp0RAsyIAJ9Q2p6qo4SDAc+Je8FAg6GvB6KwVACgum1b
> WZnKm0kdjPFob0k3aQQG8aU=
> =ry4/
> -----END PGP SIGNATURE-----

[Oliver Falk]

Hi Clark!

Clark Williams wrote:
[ ... ]
>  While none of the changes are massive, they are spread across a few
>  files (Makefile, mock.py, mock.spec, etc/default.cfg, src/Makefile,
>  src/mock.c) so I'm wondering if I should just blast out the files to
>  the list, or if I should cut a branch in CVS and let people look at it
>  from there? Or, should I just check it in and if the consensus is that
>  it sucks, we can change or revert?
>  
>  Thoughts? 

Thx for letting us know about your progress, Clark!

Best would be you send a patch/diff to the list - first of all, so all 
can investigate... At least I would appreciate!

Best,
  Oliver

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Oliver Falk wrote:
>  Hi Clark!
>
>  
Hi back Oliver!

>  Thx for letting us know about your progress, Clark!
>
>  Best would be you send a patch/diff to the list - first of all, so
>  all can investigate... At least I would appreciate! 

Good point. Attached is a 'cvs diff" of my tree, plus the new source
file src/mock.c

Comments welcome.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEmaB/Hyuj/+TTEp0RApGrAJ0YRlbn+ksIrZ9nKeDtun0GlEsrWQCgvrkm
> nQoGUnXgJ9XCxONZNluFVHU=
> =zP36
> -----END PGP SIGNATURE-----

[Oliver Falk]

Clark Williams wrote:
> > Thx for letting us know about your progress, Clark!
> >
> > Best would be you send a patch/diff to the list - first of all, so
> > all can investigate... At least I would appreciate!
>  
>  
>  Good point. Attached is a 'cvs diff" of my tree, plus the new source
>  file src/mock.c
>  
>  Comments welcome. 
I shouldn't worry about it in this context, but...


[ ... ]

>  --- Makefile	12 Apr 2006 14:23:17 -0000	1.8
>  +++ Makefile	21 Jun 2006 19:35:35 -0000
>  @@ -21,7 +21,7 @@
>   install:
>   	mkdir -p $(DESTDIR)/usr/bin/
>   	mkdir -p $(DESTDIR)/usr/libexec
>  -	install -m 755 mock.py $(DESTDIR)/usr/bin/mock
>  +	install -m 755 mock.py $(DESTDIR)/usr/bin
>   	install -m 755 mock-yum $(DESTDIR)/usr/libexec/mock-yum
>   	mkdir -p $(DESTDIR)/var/lib/mock 
Better written as:
install:
	install -D -m755 mock.py $(DESTDIR)/usr/bin/mock.py
         install -D -m755 mock-yum $(DESTDIR)/usr/libexec/mock-yum
         install -d -m775 $(DESTDIR)/var/lib/mock

This saves some bytes :-)

However, I really should worry 'bout other things in the world...

-of

[Enrico Scholz]

williams(a)redhat.com (Clark Williams) writes:

>           if os.path.exists(self.basedir):
>               cmd = '%s -rf %s' % (self.config['rm'], self.basedir)
wouldn't it be better to do such things with native syscalls instead of
invoking a command? You will never get this implemented race-free with
shell commands.


>   
>       def pack(self):
>           self.state('create cache')
>           self._ensure_dir(os.path.join(self.config['basedir'],
> self.config['cache_topdir'])) 
This seems to open an attack vector; attacker could create the
cache_topdir (basedir is writable by mock group) and create e.g. a
cache.tar -> /etc/nologin symlink.

Perhaps it suffices to remove group write-permissions from basedir. The
cleaner way would be to open the cache_file in a safe way and give only
the fd to tar. (ditto for unpack).


>           self.debug("mounting proc in %s" % procdir)
>  -        command = '%s -t proc proc %s/proc' % (self.config['mount'], 
>  +        command = '%s -n -t proc proc %s/proc' % (self.config['mount'],
>
>                                                  self.rootdir) 
See below...

>           track.flush()
>           
>           if retval != 0:
>  @@ -427,9 +459,9 @@
>           devptsdir = os.path.join(self.rootdir, 'dev/pts') 
When you are about to increase security, you should fix such beginner
errors too. 'chroot' does not mean "prepend a path", but "change
/".


>           self._ensure_dir(devptsdir)
>           self.debug("mounting devpts in %s" % devptsdir)
>  -        command = '%s -t devpts devpts %s' % (self.config['mount'],
> devptsdir)
>  +        command = '%s -n -t devpts devpts %s' % (self.config['mount'],
> devptsdir) 
There is no reason to do this kind of mounting with the mount(8) shell
command. mount(2) is a much better (and easier) way.


>           item = '%s/%s' % (self.rootdir, path)
>           command = '%s %s' % (self.config['umount'], item) 
There is no reason for explicit unmounting. At least MNT_DETACH should
be set at least. Your (deprecated) shell command should get a '-n' too.


>  +	[ -d $(DESTDIR)/$(BINDIR) ] || \
>  +		$(MKDIR) -p $(DESTDIR)/$(BINDIR)
>  +	[ -d $(DESTDIR)/$(LIBDIR) ] || \
>  +		$(MKDIR) -p $(DESTDIR)/$(LIBDIR)
>  +	$(INSTALL) -o root -g $(MOCKGROUP) -m 4750 $(EXECUTABLE) $(DESTDIR)/$(BINDIR) 
   
               ~~~~~~~~~~~~~~~~~~~~~~~

please do not do such things. rpm packagers will hate you for that.


>  	memset(env, '\0', sizeof(char *) * (3 + ALLOWED_ENV_SIZE));
>  	env[0] = strdup(SAFE_PATH); 
plain assignment without strdup(3) suffices here. You could write

| char const *     env[ALLOWED_ENV_SIZE] = {
|   [0] = SAFE_PATH,
|   [1] = SAFE_HOME,
|   [2] = NSFLAG
| };

here.

>  	// set up a new argv/argc
>  	//     new argv[0] will be "/usr/bin/python"
>  	//     new argv[1] will be "/usr/bin/mock.py"
>  	//     remainder of new argv will be old argv[1:n]
>  	//     allocate one extra for null at end
>  	newargc = argc + 1;
>  	newargvsz = sizeof(char *) * (newargc + 1);
>  	newargv = malloc(newargvsz); 
why are you doing dynamic memory allocations here? Operating on stack is
much cheaper.


>  	if (newargv == NULL)
>  		error("malloc of argument vector (%d bytes) failed!\n", newargvsz);
>  	memset(newargv, '\0', newargvsz);
>  	newargv[0] = strdup(PYTHON_PATH); 
ditto; you are missing an error check here btw... (which would not be
needed by a simple 'newargv[0] = PYTHON_PATH;').



Enrico

[Dan Williams]

On Wed, 2006-06-21 at 14:39 -0500, Clark Williams wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Oliver Falk wrote:
>  > Hi Clark!
>  >
>  >
>  
>  Hi back Oliver!
>  
>  >
>  > Thx for letting us know about your progress, Clark!
>  >
>  > Best would be you send a patch/diff to the list - first of all, so
>  > all can investigate... At least I would appreciate!
>  
>  
>  Good point. Attached is a 'cvs diff" of my tree, plus the new source
>  file src/mock.c
>  
>  Comments welcome. 
Can we do just one-word state strings?  Plague tries to parse these from
the 'mock-state' file to figure out what stages mock is in (mainly for
when it's creating the chroot and when it's actually building).

Thanks,
Dan

>  Clark
>  
> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.3 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
> >  
> >  iD8DBQFEmaB/Hyuj/+TTEp0RApGrAJ0YRlbn+ksIrZ9nKeDtun0GlEsrWQCgvrkm
> >  nQoGUnXgJ9XCxONZNluFVHU=
> >  =zP36
> >  -----END PGP SIGNATURE----- 
>  
>  --
>  Fedora-buildsys-list mailing list
>  Fedora-buildsys-list(a)redhat.com
>  https://www.redhat.com/mailman/listinfo/fedora-buildsys-list

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Enrico Scholz wrote:
>  williams(a)redhat.com (Clark Williams) writes:
>
> >          if os.path.exists(self.basedir):
> >              cmd = '%s -rf %s' % (self.config['rm'], self.basedir)
>
>  wouldn't it be better to do such things with native syscalls instead of
>  invoking a command? You will never get this implemented race-free with
>  shell commands.
>
>  
Well, as an old-timey UNIX programmer, I was always taught (by even
older UNIX hackers) that you shouldn't use mount(2), that mount(8) is
preferred because all the corner-case logic for dealing with mount
points is encapsulated there; just use it.  Perhaps that isn't the
case anymore and I'm just falling back on old habits. In any case it's
much easier from a code maintenance perspective to just delegate the
responsibility of mounting filesystems to mount(8).

What race condition(s) would we avoid by using mount(2)?

> > 
> >      def pack(self):
> >          self.state('create cache')
> >          self._ensure_dir(os.path.join(self.config['basedir'],
self.config['cache_topdir']))
>  This seems to open an attack vector; attacker could create the
>  cache_topdir (basedir is writable by mock group) and create e.g. a
>  cache.tar -> /etc/nologin symlink.
>
>  Perhaps it suffices to remove group write-permissions from basedir. The
>  cleaner way would be to open the cache_file in a safe way and give only
>  the fd to tar. (ditto for unpack).
>
>  
With the change in managing uid/gids, I wonder if we even need the
mock group anymore? Currently we cycle between the users's real uid
and root (when a root action is required). I'm not sure we need to
setgid to mock any more.

> >          self.debug("mounting proc in %s" % procdir)
> > -        command = '%s -t proc proc %s/proc' %
> (self.config['mount'],
> > +        command = '%s -n -t proc proc %s/proc' %
> (self.config['mount'],
> >                                                 self.rootdir)
>
>  See below...
>
> >          track.flush()
> >         
> >          if retval != 0:
> > @@ -427,9 +459,9 @@
> >          devptsdir = os.path.join(self.rootdir, 'dev/pts')
>
>  When you are about to increase security, you should fix such beginner
>  errors too. 'chroot' does not mean "prepend a path", but "change
> /". 
Well, we have to prepend a path, since these mounts are occurring
before we chroot.  What would you suggest as an alternative?

> >          self._ensure_dir(devptsdir)
> >          self.debug("mounting devpts in %s" % devptsdir)
> > -        command = '%s -t devpts devpts %s' % (self.config['mount'],
devptsdir)
> > +        command = '%s -n -t devpts devpts %s' %
(self.config['mount'], devptsdir)
>  There is no reason to do this kind of mounting with the mount(8) shell
>  command. mount(2) is a much better (and easier) way.
>  
I disagree that it's easier to mount with mount(2). Are you suggesting
that it's less code than the above two lines (format and os.system)?

> >          item = '%s/%s' % (self.rootdir, path)
> >          command = '%s %s' % (self.config['umount'], item)
>
>  There is no reason for explicit unmounting. At least MNT_DETACH should
>  be set at least. Your (deprecated) shell command should get a '-n' too.
>
>  
No reason other than good programming practice perhaps.

Thanks for catching the -n.

> > +    [ -d $(DESTDIR)/$(BINDIR) ] || \
> > +        $(MKDIR) -p $(DESTDIR)/$(BINDIR)
> > +    [ -d $(DESTDIR)/$(LIBDIR) ] || \
> > +        $(MKDIR) -p $(DESTDIR)/$(LIBDIR)
> > +    $(INSTALL) -o root -g $(MOCKGROUP) -m 4750 $(EXECUTABLE)
$(DESTDIR)/$(BINDIR)
>                     ~~~~~~~~~~~~~~~~~~~~~~~
>
>  please do not do such things. rpm packagers will hate you for that.
>
>  
Please do not do *what* things? Create directories? Use the install
utility?

Please elaborate your concern.

> >     memset(env, '\0', sizeof(char *) * (3 +
> ALLOWED_ENV_SIZE));
> >     env[0] = strdup(SAFE_PATH);
>
>  plain assignment without strdup(3) suffices here. You could write
>
>  | char const *     env[ALLOWED_ENV_SIZE] = {
>  |   [0] = SAFE_PATH,
>  |   [1] = SAFE_HOME,
>  |   [2] = NSFLAG
>  | };
>
>  here.
>
> >     // set up a new argv/argc
> >     //     new argv[0] will be "/usr/bin/python"
> >     //     new argv[1] will be "/usr/bin/mock.py"
> >     //     remainder of new argv will be old argv[1:n]
> >     //     allocate one extra for null at end
> >     newargc = argc + 1;
> >     newargvsz = sizeof(char *) * (newargc + 1);
> >     newargv = malloc(newargvsz);
>
>  why are you doing dynamic memory allocations here? Operating on stack is
>  much cheaper.
>  
Well, I'd say that this is a holdover from kernel programming where
you don't put things other than trivial variables on the stack.
Probably not an issue in user-space, but I'm not going to change it
for a one-time exec. Who cares if it's a couple of micro-seconds
faster to assign to the stack as opposed to allocating a page off the
heap and duping a few strings? It's done once then and then we exec
python.

> >     if (newargv == NULL)
> >         error("malloc of argument vector (%d bytes) failed!\n",
newargvsz);
> >     memset(newargv, '\0', newargvsz);
> >     newargv[0] = strdup(PYTHON_PATH);
>
>  ditto; you are missing an error check here btw... (which would not be
>  needed by a simple 'newargv[0] = PYTHON_PATH;').
>  
Ah, I should have checked the strdup return.  Busted...



Clark
 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEnAvsHyuj/+TTEp0RAtQ9AKDPOVMS525eT3bKrTYS36fnJADyvACff2xR
> k76XCHVvjO4Ass30Sdgrj6s=
> =mUTh
> -----END PGP SIGNATURE-----

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dan Williams wrote:
> > Can we do just one-word state strings?  Plague tries to parse
>  these from
> > the 'mock-state' file to figure out what stages mock is in
> > (mainly for when it's creating the chroot and when it's actually
> > building).
>
> > Thanks, Dan
>  
Looks like the only two are "unpack cache" and "create cache".

Want to switch them to cache_create and cache_unpack?

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEnAxQHyuj/+TTEp0RAnyBAKCvZZ/MOU8W8jLwZ8WN2CEbPTEyuACdF4Tc
> uO3wCIsZp1slVvnea8uDXIU=
> =shhn
> -----END PGP SIGNATURE-----

[Enrico Scholz]

williams(a)redhat.com (Clark Williams) writes:

> >>          if os.path.exists(self.basedir):
> >>              cmd = '%s -rf %s' % (self.config['rm'],
> self.basedir)
> >
> > wouldn't it be better to do such things with native syscalls instead of
> > invoking a command? You will never get this implemented race-free with
> > shell commands.
>
>  Well, as an old-timey UNIX programmer, I was always taught (by even
>  older UNIX hackers) that you shouldn't use mount(2), that mount(8) is
>  preferred because all the corner-case logic for dealing with mount
>  points is encapsulated there; just use it. 
NFS & similar filesystem types which require e.g. RPC are the only
"corner-case" I am aware of. A native mount(2) would require

| chdir(new_chroot);
| chrootChdir("/dev/pts");
| mount("none", ".", "devpts", "gid=5,mode=620");

or

| chroot(new_chroot);
| mount("none", "/dev/pts", "devpts",
"gid=5,mode=620");

while mount(8) requires significantly more written and executed code
(think of cmdline escaping, loading the 9 libraries /bin/mount is linked
against, ...).


>  Perhaps that isn't the case anymore and I'm just falling back
> on old
>  habits. In any case it's much easier from a code maintenance perspective
>  to just delegate the responsibility of mounting filesystems to mount(8).
>
>  What race condition(s) would we avoid by using mount(2)? 
The comment above was about 'rm -rf'. After a quick look into its code,
I really can not tell whether it is implemented race-free. E.g. whether

   rm -rf chroot                       attacker
|  chdir("chroot");
|  lstat("bin");  --> is-dir
|                                      rmdir("bin");
|                                      symlink("/etc", "bin");
|  chdir("bin");
|  remove-all-what-I-can-find

will be prevented.


> >>          devptsdir = os.path.join(self.rootdir,
> 'dev/pts')
> >
> > When you are about to increase security, you should fix such beginner
> > errors too. 'chroot' does not mean "prepend a path", but
> "change /".
>
>  Well, we have to prepend a path, since these mounts are occurring
>  before we chroot.  What would you suggest as an alternative? 
do it in a safe manner...

| chdir(rootdir);
| chrootChdir("/dev/pts");
| mount("none", ".", "devpts", "gid=5,mode=620");

with

| chrootChdir(char const *dir)
| {
|   int root_fd = open("/", O_RDONLY);
|   chroot(".");
|   chdir(dir);
|   int tgt_fd  = open(".", O_RDONLY);
|   fchdir(root_fd);
|   chroot(".");
|   fchdir(tgt_fd);
|   close(tgt_fd);
|   close(root_fd);
| }

[obviously, error-checking must be added at every line]


> >> +        command = '%s -n -t devpts devpts %s' %
> (self.config['mount'], devptsdir)
> >
> > There is no reason to do this kind of mounting with the mount(8)
> > shell command. mount(2) is a much better (and easier) way.
>
>  I disagree that it's easier to mount with mount(2). Are you suggesting
>  that it's less code than the above two lines (format and os.system)? 
As shown above, mount(2) results in significantly less written and
executed code than mount(8). This is resulting both in faster and more
secure code. And I really do not know how people came to the idea to
use system(3) at this place. Even prototyping languages like perl or
python know execve(2) equivalences.


> >> +    [ -d $(DESTDIR)/$(BINDIR) ] || \
> >> +        $(MKDIR) -p $(DESTDIR)/$(BINDIR)
> >> +    [ -d $(DESTDIR)/$(LIBDIR) ] || \
> >> +        $(MKDIR) -p $(DESTDIR)/$(LIBDIR)
> >> +    $(INSTALL) -o root -g $(MOCKGROUP) -m 4750 $(EXECUTABLE)
> $(DESTDIR)/$(BINDIR)
> >                  ~~~~~~~~~~~~~~~~~~~~~~~
> >
> > please do not do such things. rpm packagers will hate you for that.
>
>  Please do not do *what* things? Create directories? 
Sorry; my comment was not about the mkdir stuff with the (IMO unneeded)
test-if-exists. It just stepped in more or less inadvertently.


>  Use the install utility? 
no; only the underlined part. When 'install' was compiled with (or
without ... I can not remember the exact conditions) AFS support, the
'make install' will fail as non-root user.


> >>     newargc = argc + 1;
> >>     newargvsz = sizeof(char *) * (newargc + 1);
> >>     newargv = malloc(newargvsz);
> >
> > why are you doing dynamic memory allocations here? Operating on stack
> > is much cheaper.
> >
>
>  Well, I'd say that this is a holdover from kernel programming where
>  you don't put things other than trivial variables on the stack. 
We are in userspace where we have 8MB stack or so by default... And I
do not think that somebody wants to implement mock-helper as a kernel
module with only 4KB stack... ;)


>  Probably not an issue in user-space, but I'm not going to change
> it
>  for a one-time exec. Who cares if it's a couple of micro-seconds
>  faster to assign to the stack as opposed to allocating a page off the
>  heap and duping a few strings? 
We are speaking about SUID programs. Every additional and unneeded line
of code increases complexity and complicates an audit. Your original
code showed that the unneeded memory-allocating is error-prone too.


>  It's done once then and then we exec python. 
Why write complicated code, when same thing can be done shorter, faster
and less error-prone?




Enrico

[Michael E Brown]

Any chance we could get a better defined method of mock<->plague
communication? Parsing output is bound to be fragile and hard to
maintain.
--
Michael 

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Clark Williams
>  Sent: Friday, June 23, 2006 10:44 AM
>  To: Discussion of Fedora build system
>  Subject: Re: New version of mock working (I think)
>  
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Dan Williams wrote:
>  >
>  >> Can we do just one-word state strings?  Plague tries to parse
>  > these from
>  >> the 'mock-state' file to figure out what stages mock is in (mainly 
>  >> for when it's creating the chroot and when it's actually building).
>  >
>  >> Thanks, Dan
>  >
>  
>  Looks like the only two are "unpack cache" and "create cache".
>  
>  Want to switch them to cache_create and cache_unpack?
>  
>  Clark
>  
> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.3 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
> >  
> >  iD8DBQFEnAxQHyuj/+TTEp0RAnyBAKCvZZ/MOU8W8jLwZ8WN2CEbPTEyuACdF4Tc
> >  uO3wCIsZp1slVvnea8uDXIU=
> >  =shhn
> >  -----END PGP SIGNATURE----- 
>  
>  --
>  Fedora-buildsys-list mailing list
>  Fedora-buildsys-list(a)redhat.com
>  https://www.redhat.com/mailman/listinfo/fedora-buildsys-list
>

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Enrico Scholz wrote:
>  williams(a)redhat.com (Clark Williams) writes:
>
> >>> if os.path.exists(self.basedir): cmd = '%s -rf %s' %
> >>> (self.config['rm'], self.basedir)
> >> wouldn't it be better to do such things with native syscalls
> >> instead of invoking a command? You will never get this
> >> implemented race-free with shell commands.
> > Well, as an old-timey UNIX programmer, I was always taught (by
> > even older UNIX hackers) that you shouldn't use mount(2), that
> > mount(8) is preferred because all the corner-case logic for
> > dealing with mount points is encapsulated there; just use it.
>
>  NFS & similar filesystem types which require e.g. RPC are the only
>  "corner-case" I am aware of. A native mount(2) would require
>
>  | chdir(new_chroot); | chrootChdir("/dev/pts"); | mount("none",
>  ".", "devpts", "gid=5,mode=620");
>
>  or
>
>  | chroot(new_chroot); | mount("none", "/dev/pts",
> "devpts",
>  "gid=5,mode=620");
>
>  while mount(8) requires significantly more written and executed
>  code (think of cmdline escaping, loading the 9 libraries /bin/mount
>  is linked against, ...).
>  
It took me a little while to track down the mount interface (I've
never used diskutils), so I feel better about it now.  I'm not against
using the mount(2)/umount(2) calls to manage stuff in our chroot.
Also, I haven't heard anyone jump in here screaming that we shouldn't
do that (could be you, me, Michael and Dan are the only ones on this
channel :).  I'll look at modifying things to use the diskutils mount.

I have some reservations about chroot though...

<snip>

>  do it in a safe manner...
>
>  | chdir(rootdir); | chrootChdir("/dev/pts"); | mount("none",
> ".",
>  "devpts", "gid=5,mode=620");
>
>  with
>
>  | chrootChdir(char const *dir) | { |   int root_fd = open("/",
>  O_RDONLY); |   chroot("."); |   chdir(dir); |   int tgt_fd  =
>  open(".", O_RDONLY); |   fchdir(root_fd); |   chroot("."); |
>  fchdir(tgt_fd); |   close(tgt_fd); |   close(root_fd); | }
>
>  [obviously, error-checking must be added at every line]
>
>  
I'd like to hold off a bit on attempting to use os.chroot() rather
than using the chroot program.  As I'm sure you are aware, moving to
os.chroot() entails a fairly big philosophical shift from a design
perspective. Currently we only format commands to be run within the
chroot and feed them to the chroot program. Shifting over to
os.chroot() means that we'll need to os.fork() and wait for child
completion.  I'd rather take this in small steps, that is release a
version of mock that uses the new launcher mechanism, let that settle,
then consider getting more fine grained by actually crossing the
chroot barrier in python code.

I'm not saying I'm against doing it, rather I'd like to get this first
step out of the way before we try. In the mean time we can argue about
it to our hearts content :).

> >>> +    [ -d $(DESTDIR)/$(BINDIR) ] || \ +        $(MKDIR) -p
> >>> $(DESTDIR)/$(BINDIR) +    [ -d $(DESTDIR)/$(LIBDIR) ] || \ +
> >>> $(MKDIR) -p $(DESTDIR)/$(LIBDIR) +    $(INSTALL) -o root -g
> >>> $(MOCKGROUP) -m 4750 $(EXECUTABLE) $(DESTDIR)/$(BINDIR)
> >> ~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> please do not do such things. rpm packagers will hate you for
> >> that.
> > Please do not do *what* things? Create directories?
>
>  Sorry; my comment was not about the mkdir stuff with the (IMO
>  unneeded) test-if-exists. It just stepped in more or less
>  inadvertently.
>
>
> > Use the install utility?
>
>  no; only the underlined part. When 'install' was compiled with (or
>  without ... I can not remember the exact conditions) AFS support,
>  the 'make install' will fail as non-root user.
>
>  
You know, I really didn't mean for those spec file modifications to
see the light of day. That's what I get for being lazy and doing a
'cvs diff'.


> >>> newargc = argc + 1; newargvsz = sizeof(char *) * (newargc
> +
> >>> 1); newargv = malloc(newargvsz);
> >> why are you doing dynamic memory allocations here? Operating on
> >> stack is much cheaper.
> >>
> > Well, I'd say that this is a holdover from kernel programming
> > where you don't put things other than trivial variables on the
> > stack.
>
>  We are in userspace where we have 8MB stack or so by default... And
>  I do not think that somebody wants to implement mock-helper as a
>  kernel module with only 4KB stack... ;)
>
>  
Bah! Child's play! Let's implement mock as a kernel thread!

> > Probably not an issue in user-space, but I'm not going to
> change
> > it for a one-time exec. Who cares if it's a couple of
> > micro-seconds faster to assign to the stack as opposed to
> > allocating a page off the heap and duping a few strings?
>
>  We are speaking about SUID programs. Every additional and unneeded
>  line of code increases complexity and complicates an audit. Your
>  original code showed that the unneeded memory-allocating is
>  error-prone too.
>
>
> > It's done once then and then we exec python.
>
>  Why write complicated code, when same thing can be done shorter,
>  faster and less error-prone?
>
>  
Hoist on my own petard. When it comes down to it, I'm all about less
code. I'll go rework mock.c (the mock-helper replacement) and post a
new one for further review.

Clark

P.S.  I was really kidding about the kernel thread thing...
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEnEekHyuj/+TTEp0RArdXAKCOrjnKxMDlS+GBiaV987fbQCjZBACg3zZ6
> lrT/VY4GFXGkj6glnBhcSzs=
> =AES+
> -----END PGP SIGNATURE-----

[Dan Williams]

On Fri, 2006-06-23 at 13:40 -0500, Michael_E_Brown(a)Dell.com wrote:
>  Any chance we could get a better defined method of
> mock<->plague
>  communication? Parsing output is bound to be fragile and hard to
>  maintain. 
Yup.  Quite right.  And I've had problems in the past with the reads of
the file blocking (since you evidently can't do NONBLOCK on filesystem
file sockets or something either?) and hanging the plague-builder
process until more data gets written to the file.  So as a quick hack
plague-builder will only read & match the first 4 characters of the
status string anyway.

I'd rather not read the status file either.  Any ideas that don't suck
would be greatly appreciated.

Dan

>  --
>  Michael 
>  
>  > -----Original Message-----
>  > From: fedora-buildsys-list-bounces(a)redhat.com 
>  > [mailto:[e-mail]] On Behalf Of 
>  > Clark Williams
>  > Sent: Friday, June 23, 2006 10:44 AM
>  > To: Discussion of Fedora build system
>  > Subject: Re: New version of mock working (I think)
>  > 
>  > -----BEGIN PGP SIGNED MESSAGE-----
>  > Hash: SHA1
>  > 
>  > Dan Williams wrote:
>  > >
>  > >> Can we do just one-word state strings?  Plague tries to parse
>  > > these from
>  > >> the 'mock-state' file to figure out what stages mock is in (mainly
>
>  > >> for when it's creating the chroot and when it's actually
> building).
>  > >
>  > >> Thanks, Dan
>  > >
>  > 
>  > Looks like the only two are "unpack cache" and "create cache".
>  > 
>  > Want to switch them to cache_create and cache_unpack?
>  > 
>  > Clark
>  > 
> >  > -----BEGIN PGP SIGNATURE-----
> >  > Version: GnuPG v1.4.3 (GNU/Linux)
> >  > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
> >  > 
> >  > iD8DBQFEnAxQHyuj/+TTEp0RAnyBAKCvZZ/MOU8W8jLwZ8WN2CEbPTEyuACdF4Tc
> >  > uO3wCIsZp1slVvnea8uDXIU=
> >  > =shhn
> >  > -----END PGP SIGNATURE----- 
>  > 
>  > --
>  > Fedora-buildsys-list mailing list
>  > Fedora-buildsys-list(a)redhat.com
>  > https://www.redhat.com/mailman/listinfo/fedora-buildsys-list
>  > 
>  
>  --
>  Fedora-buildsys-list mailing list
>  Fedora-buildsys-list(a)redhat.com
>  https://www.redhat.com/mailman/listinfo/fedora-buildsys-list

[Jesse Keating]

On Sat, 2006-06-24 at 11:26 -0400, Dan Williams wrote:
>  
>  I'd rather not read the status file either.  Any ideas that don't suck
>  would be greatly appreciated.  
Wild/Crazy idea:

Why not use xml-rpc calls between the two?  If mock sent out status
information via xml-rpc, any other wrapper around mock would be able to
gather that information up and act upon it.

I have no idea what amount of work this would entail.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jesse Keating wrote:
>  On Sat, 2006-06-24 at 11:26 -0400, Dan Williams wrote:
> > I'd rather not read the status file either.  Any ideas that don't suck
> > would be greatly appreciated.
>
>  Wild/Crazy idea:
>
>  Why not use xml-rpc calls between the two?  If mock sent out status
>  information via xml-rpc, any other wrapper around mock would be able to
>  gather that information up and act upon it.
>
>  I have no idea what amount of work this would entail. 
Hmm, that would mean that mock would have to be an XML-RPC server.  I
like XML-RPC, but that might be a bit more complicated than is
strictly needed.

How about shared memory? mock just dumps the state into it; anyone
that wants to know state just reads it? We might even be able to do it
through mmap'ing the state file...

Or a UNIX-domain socket?

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEnactHyuj/+TTEp0RAtJ5AJ40sHAUix+cZfb5Z64cY8PG6W3F9QCgujmo
> clQDEysMpj1vj2JlVocFuzQ=
> =agsL
> -----END PGP SIGNATURE-----

[Jesse Keating]

On Sat, 2006-06-24 at 15:57 -0500, Clark Williams wrote:
>  
>  Hmm, that would mean that mock would have to be an XML-RPC server.  I
>  like XML-RPC, but that might be a bit more complicated than is
>  strictly needed.  
Does it really have to be a server?  Could it be a push rather than a
poll?  Mock could make xml-rpc calls to whatever wraps around it.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jesse Keating wrote:
>  On Sat, 2006-06-24 at 15:57 -0500, Clark Williams wrote:
> > Hmm, that would mean that mock would have to be an XML-RPC server.  I
> > like XML-RPC, but that might be a bit more complicated than is
> > strictly needed.
>
>  Does it really have to be a server?  Could it be a push rather than a
>  poll?  Mock could make xml-rpc calls to whatever wraps around it. 
I've used XML-RPC to implement services a couple of times now. Both
times it's been python to python, with a python client talking to a
python server running under mod_python in Apache.  I'm not sure how
you would implement XML-RPC in a push mode, since someone has to
provide the remote procedure call service and someone has to call it.

I really like XML-RPC as a an RPC mechanism, but I think it's a bit of
overkill for this. To do it as an RPC, we would have to listen on a
socket for someone to ask "what's your state?" and reply with the
current state of the build. I'd say it's much simpler to "publish" the
state to somewhere that anyone who cares (e.g. plague) can read it.
I'm still thinking shared memory, but there may be another mechanism
(a simpler one) that I'm missing.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEn/JNHyuj/+TTEp0RAvkfAKCtVH1PFgjZpiRjoxUN39OgmixgRgCfYdjc
> thQQ7taYOGJ8KYST6KKRf3U=
> =m26z
> -----END PGP SIGNATURE-----

[Jesse Keating]

On Mon, 2006-06-26 at 09:42 -0500, Clark Williams wrote:
>  I've used XML-RPC to implement services a couple of times now.
> Both
>  times it's been python to python, with a python client talking to a
>  python server running under mod_python in Apache.  I'm not sure how
>  you would implement XML-RPC in a push mode, since someone has to
>  provide the remote procedure call service and someone has to call it.
>  
>  I really like XML-RPC as a an RPC mechanism, but I think it's a bit of
>  overkill for this. To do it as an RPC, we would have to listen on a
>  socket for someone to ask "what's your state?" and reply with the
>  current state of the build. I'd say it's much simpler to "publish" the
>  state to somewhere that anyone who cares (e.g. plague) can read it.
>  I'm still thinking shared memory, but there may be another mechanism
>  (a simpler one) that I'm missing.
>   
What about using dbus?  Its not "simple" per se, but should scale pretty
well and allow for pretty good interaction.  

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[seth vidal]

On Mon, 2006-06-26 at 11:33 -0400, Jesse Keating wrote:
>  On Mon, 2006-06-26 at 09:42 -0500, Clark Williams wrote:
>  > I've used XML-RPC to implement services a couple of times now. Both
>  > times it's been python to python, with a python client talking to a
>  > python server running under mod_python in Apache.  I'm not sure how
>  > you would implement XML-RPC in a push mode, since someone has to
>  > provide the remote procedure call service and someone has to call it.
>  > 
>  > I really like XML-RPC as a an RPC mechanism, but I think it's a bit of
>  > overkill for this. To do it as an RPC, we would have to listen on a
>  > socket for someone to ask "what's your state?" and reply with the
>  > current state of the build. I'd say it's much simpler to "publish"
> the
>  > state to somewhere that anyone who cares (e.g. plague) can read it.
>  > I'm still thinking shared memory, but there may be another mechanism
>  > (a simpler one) that I'm missing.
>  > 
>  
>  What about using dbus?  Its not "simple" per se, but should scale pretty
>  well and allow for pretty good interaction.   
gonna play hell if our builders are rhel4.



-sv

[Andreas Thienemann]

On Mon, 26 Jun 2006, Jesse Keating wrote:

>  What about using dbus?  Its not "simple" per se, but should
> scale pretty
>  well and allow for pretty good interaction.   
Please don't.

As Seth correctly stated, there are people using RHEL4 as the buildserver. 
We do.

As far as I understood dbus, I need a daemon running for the message 
passing to work.
This will not do on a buildserver. 

I don't really care _how_ communication is done, but please do not require 
additional software running for that. And no dependencies on gnome-* or 
xorg-* stuff as well. Please.


regards,
 andreas

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Thienemann wrote:
>  On Mon, 26 Jun 2006, Jesse Keating wrote:
>
> > What about using dbus?  Its not "simple" per se, but should scale pretty
> > well and allow for pretty good interaction. 
>  Please don't.
>
>  As Seth correctly stated, there are people using RHEL4 as the buildserver.
>  We do.
>
>  As far as I understood dbus, I need a daemon running for the message
>  passing to work.
>  This will not do on a buildserver.
>
>  I don't really care _how_ communication is done, but please do not require
>  additional software running for that. And no dependencies on gnome-* or
>  xorg-* stuff as well. Please.
>  
Well, I actually have builders running RHEL3, so I'll try not to do
anything that breaks that. :)

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoAb3Hyuj/+TTEp0RArlEAJ0TqIDAh08psjFq/I/3PXHBp+9oXQCeLhxA
> Tx7Yt/jFC9G1xXmKp6A9lO0=
> =HDUQ
> -----END PGP SIGNATURE-----

[Jesse Keating]

On Mon, 2006-06-26 at 11:10 -0500, Clark Williams wrote:
>  
>  Well, I actually have builders running RHEL3, so I'll try not to do
>  anything that breaks that. :)
>   
That's what branching the tree is for no?  We don't want to stifle
advancements in mock because it has to continue to run on RHEL3.  A
stable version of Mock could continue to run there, while developmental
versions of mock march forward into the current century.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Andreas Thienemann]

On Mon, 26 Jun 2006, Clark Williams wrote:

>  Well, I actually have builders running RHEL3, so I'll try not to
> do
>  anything that breaks that. :) 
They do work? Interesting.

During some testing of plague at the end of last year I decided to ditch 
the RHEL3 builders as they were too problematic.

regards,
 andreas

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Jesse Keating
>  Sent: Saturday, June 24, 2006 4:48 PM
>  To: fedora-buildsys-list(a)redhat.com
>  Subject: Re: New version of mock working (I think)
>  
>  On Sat, 2006-06-24 at 15:57 -0500, Clark Williams wrote:
>  > 
>  > Hmm, that would mean that mock would have to be an XML-RPC 
>  server.  I 
>  > like XML-RPC, but that might be a bit more complicated than is 
>  > strictly needed.
>  
>  Does it really have to be a server?  Could it be a push 
>  rather than a poll?  Mock could make xml-rpc calls to 
>  whatever wraps around it. 
Just catching up on weekend traffic here. XMLRPC is a great idea. I have
implemented several xmlrpc servers in the past, and this is an idea that
could work out well. Given the nature of mock, it would probably be best
if it made calls into plague, rather than the other way around, but it
wouldn't be _too_ much overhead to have a server that could be polled
for status.

I have mock hacking on my todo list for this week, so after I finish
catching up with list traffic and get the latest prototype downloaded
and building, I'll take a look at this.
--
Michael

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Jesse Keating
>  Sent: Monday, June 26, 2006 11:17 AM
>  To: fedora-buildsys-list(a)redhat.com
>  Subject: Re: New version of mock working (I think)
>  
>  On Mon, 2006-06-26 at 11:10 -0500, Clark Williams wrote:
>  > 
>  > Well, I actually have builders running RHEL3, so I'll try not to do 
>  > anything that breaks that. :)
>  > 
>  
>  That's what branching the tree is for no?  We don't want to 
>  stifle advancements in mock because it has to continue to run 
>  on RHEL3.  A stable version of Mock could continue to run 
>  there, while developmental versions of mock march forward 
>  into the current century. 
Correct. Current mock 0.6 works for RHEL3 and RHEL4 builds. No need to
hold back development for that. If you want to continue using RHEL3,
mock 0.6 will continue to work and you can stick with that.

That said, we should think carefully before adding nifty features that
break RHEL3.
--
Michael

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
> >
> > Does it really have to be a server?  Could it be a push
> > rather than a poll?  Mock could make xml-rpc calls to
> > whatever wraps around it.
>
>  Just catching up on weekend traffic here. XMLRPC is a great idea. I have
>  implemented several xmlrpc servers in the past, and this is an idea that
>  could work out well. Given the nature of mock, it would probably be best
>  if it made calls into plague, rather than the other way around, but it
>  wouldn't be _too_ much overhead to have a server that could be polled
>  for status.
>
>  I have mock hacking on my todo list for this week, so after I finish
>  catching up with list traffic and get the latest prototype downloaded
>  and building, I'll take a look at this. 
Ugh. I get the feeling that this is a case of "Oooo shiny!".

What benefit is it to mock to add either client or server RPC
capabilities so that someone can determine mock's state? Are we
talking about communicating something besides a string? XML-RPC works
GREAT as a marshaling/unmarshaling mechanism to describe complex data
structures. In this case however we're talking about telling someone
that we're in the "prep" state, or in the "cache_unpack" state.

If there is a wealth of information that we're not making available to
the outside world (i.e. distributed build systems) then yeah, lets set
up a series of remote procedure calls and publish them; I'll gladly
work on client/server code in the mock codebase if it's justified. But
if we're only changing state and not allowing the outside world to
interact with us to change our behavior while building, then let's
keep it simple.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoCznHyuj/+TTEp0RAptnAKCaXCE2IAF7W/M8beGRkJQw+aajgQCg2jw4
> U5Z6cwHzbLGQWcoZZuALPNc=
> =rYkJ
> -----END PGP SIGNATURE-----

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Clark Williams
>  Sent: Monday, June 26, 2006 1:52 PM
>  To: Discussion of Fedora build system
>  Subject: Re: New version of mock working (I think)
>  
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Michael_E_Brown(a)Dell.com wrote:
>  >>
>  >> Does it really have to be a server?  Could it be a push 
>  rather than a 
>  >> poll?  Mock could make xml-rpc calls to whatever wraps around it.
>  >
>  > Just catching up on weekend traffic here. XMLRPC is a great idea. I 
>  > have implemented several xmlrpc servers in the past, and this is an 
>  > idea that could work out well. Given the nature of mock, it would 
>  > probably be best if it made calls into plague, rather than 
>  the other 
>  > way around, but it wouldn't be _too_ much overhead to have a server 
>  > that could be polled for status.
>  >
>  > I have mock hacking on my todo list for this week, so after 
>  I finish 
>  > catching up with list traffic and get the latest prototype 
>  downloaded 
>  > and building, I'll take a look at this.
>  
>  Ugh. I get the feeling that this is a case of "Oooo shiny!".
>  
>  What benefit is it to mock to add either client or server RPC 
>  capabilities so that someone can determine mock's state? Are 
>  we talking about communicating something besides a string? 
>  XML-RPC works GREAT as a marshaling/unmarshaling mechanism to 
>  describe complex data structures. In this case however we're 
>  talking about telling someone that we're in the "prep" state, 
>  or in the "cache_unpack" state.
>  
>  If there is a wealth of information that we're not making 
>  available to the outside world (i.e. distributed build 
>  systems) then yeah, lets set up a series of remote procedure 
>  calls and publish them; I'll gladly work on client/server 
>  code in the mock codebase if it's justified. But if we're 
>  only changing state and not allowing the outside world to 
>  interact with us to change our behavior while building, then 
>  let's keep it simple. 
It is simple, we can do an xmlrpc client in about 7 lines of code and
add it to the status() function. Here are all the lines that I can think
to add that would fully enable this:

In setup_default_config_opts():
	config["status_server"] = None

In command_parse():
      parser.add_option("--statusserver", action="store",
type="string",
dest="status_server",
           default=None, help="server to contact for status info.")

In set_config_opts_per_cmdline():
	if options.status_server is not None: config["status_server"] =
options.status_server

In state():
	if self.config["status_server"]:
		import xmlrpclib
		server = xmlrpclib.Server(config["status_server"])
		server.set_mock_status(some_string)
	
A server would probably take another 10-20 lines of code.

An additional argument could probably be made for shared mem or a named
pipe, but I havent coded those, so I don't know. As for simplicity,
another file descriptor pre-opened by plague, or a named pipe passed via
command line would probably be "simpler", but probably not in terms of
lines of code.
--
Michael

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Clark Williams wrote:
>  Enrico Scholz wrote:
> >> Why write complicated code, when same thing can be done
> >> shorter, faster and less error-prone?
> >>
> >>
>  Hoist on my own petard. When it comes down to it, I'm all about
>  less code. I'll go rework mock.c (the mock-helper replacement) and
>  post a new one for further review. 
In between whining about complexity, I modified the new launcher to
use stack instead of heap for the argument vector. Attached is the
source for the modified launcher, mock.c

Comments please.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoDPgHyuj/+TTEp0RAouKAKDEH6y79paANJ9h6iIaTZZdnCmdRACgtcRI
> 40ONAAZz+eNuTo76nqIEOy0=
> =jvIT
> -----END PGP SIGNATURE-----

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
> >
> > Ugh. I get the feeling that this is a case of "Oooo shiny!".
> >
> > What benefit is it to mock to add either client or server RPC
> > capabilities so that someone can determine mock's state? Are
> > we talking about communicating something besides a string?
> > XML-RPC works GREAT as a marshaling/unmarshaling mechanism to
> > describe complex data structures. In this case however we're
> > talking about telling someone that we're in the "prep" state,
> > or in the "cache_unpack" state.
> >
> > If there is a wealth of information that we're not making
> > available to the outside world (i.e. distributed build
> > systems) then yeah, lets set up a series of remote procedure
> > calls and publish them; I'll gladly work on client/server
> > code in the mock codebase if it's justified. But if we're
> > only changing state and not allowing the outside world to
> > interact with us to change our behavior while building, then
> > let's keep it simple.
>
>  It is simple, we can do an xmlrpc client in about 7 lines of code and
>  add it to the status() function. Here are all the lines that I can think
>  to add that would fully enable this:
>
>  In setup_default_config_opts():
>      config["status_server"] = None
>
>  In command_parse():
>        parser.add_option("--statusserver", action="store",
> type="string",
>  dest="status_server",
>             default=None, help="server to contact for status info.")
>
>  In set_config_opts_per_cmdline():
>      if options.status_server is not None: config["status_server"] =
>  options.status_server
>
>  In state():
>      if self.config["status_server"]:
>          import xmlrpclib
>          server = xmlrpclib.Server(config["status_server"])
>          server.set_mock_status(some_string)
>     
>  A server would probably take another 10-20 lines of code.
>
>  An additional argument could probably be made for shared mem or a named
>  pipe, but I havent coded those, so I don't know. As for simplicity,
>  another file descriptor pre-opened by plague, or a named pipe passed via
>  command line would probably be "simpler", but probably not in terms of
>  lines of code.
>  --
>  Michael 
I can see I'm on the losing end of this argument.  My first suggestion
would have been to use mmap to map the status file, but re-reading
Dan's email, my guess is that's not significantly different from
reading the file. And on the plus side, we can pass structured data
using XML-RPC, so that might "suck less" :).

Let me just state two things and then I'll shut up on this issue:

1. I think that adding XML-RPC has great potential to be a Pandora's
Box, in terms of potential code bloat.  We need to be on guard to
avoid the "Now that we have XML-RPC, let's add this feature!". Please
remember that the intent behind mock is to rebuild an SRPM inside a
chroot. Everything else is secondary.

2. We need to hear from Dan as to whether the likes the idea. My guess
is he will since plague already has XML-RPC elements.

Clark "Back in my day we didn't need fancy XML-RPCs!  We had DCE!"
Williams

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoDlfHyuj/+TTEp0RAjfhAKCeX6yfVFTudHZ9ox1JTU4vwLXGWgCggQbu
> vOpb6nSP6QQJgK4jCd4tvk0=
> =0Wvu
> -----END PGP SIGNATURE-----

[seth vidal]

On Mon, 2006-06-26 at 14:45 -0500, Clark Williams wrote:

>  I can see I'm on the losing end of this argument.  My first
> suggestion
>  would have been to use mmap to map the status file, but re-reading
>  Dan's email, my guess is that's not significantly different from
>  reading the file. And on the plus side, we can pass structured data
>  using XML-RPC, so that might "suck less" :).
>  
>  Let me just state two things and then I'll shut up on this issue:
>  
>  1. I think that adding XML-RPC has great potential to be a Pandora's
>  Box, in terms of potential code bloat.  We need to be on guard to
>  avoid the "Now that we have XML-RPC, let's add this feature!". Please
>  remember that the intent behind mock is to rebuild an SRPM inside a
>  chroot. Everything else is secondary.
>  
>  2. We need to hear from Dan as to whether the likes the idea. My guess
>  is he will since plague already has XML-RPC elements.
>  
>  Clark "Back in my day we didn't need fancy XML-RPCs!  We had DCE!"
>  Williams 
Just for 2cents, I'd like to make sure we don't forget the other use
cases of mock:

1. in brew
2. standalone with just a person in front of a terminal


-sv

[Jesse Keating]

Good points about other uses.  I'll comment on how this particular issue
effects these uses:

On Mon, 2006-06-26 at 15:55 -0400, seth vidal wrote:
>  1. in brew 
Brew likes this because brew is already an xml-rpc server.

>  2. standalone with just a person in front of a terminal  
I would think that mock would default to having the notification turned
off, since it requires configuration anyway, so standalone users
shouldn't be affected (hopefully).

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Michael E Brown]

>  > 2. We need to hear from Dan as to whether the likes the 
>  idea. My guess 
>  > is he will since plague already has XML-RPC elements. 
Yes, feedback from Dan would be good. My initial thoughts are that a
client implementation would be best at this point, due to the security
implications of a server. Something where we call a server API provided
by plague like:
	server.begin_mock_status( "my.src.rpm", MY_PID)
	server.set_mock_status( "my.src.rpm", "status_string", MY_PID)
	...
	server.end_mock_status( "my.src.rpm", return_code, MY_PID)

>  > 
>  > Clark "Back in my day we didn't need fancy XML-RPCs!  We had DCE!"
>  > Williams
>  
>  Just for 2cents, I'd like to make sure we don't forget the 
>  other use cases of mock:
>  
>  1. in brew 
I don't know what brew is. Link?

>  2. standalone with just a person in front of a terminal 
Xmlrpc would be an optional behaviour, invoked via something like:
	mock -r CFG --status_server=http://localhost:8080/  my.src.rpm

Default behaviour would be no change.
--
Michael

[Jesse Keating]

On Mon, 2006-06-26 at 15:55 -0400, seth vidal wrote:
>  Just for 2cents, I'd like to make sure we don't forget the
> other use
>  cases of mock:
>  
>  1. in brew
>  2. standalone with just a person in front of a terminal  
I suppose I should add a #3 here.

In Red Hat land we use mock to run commands in a chroot.  Examples would
be buildinstall from anaconda, pkgorder from anaconda, createrepo,
repoview, things of this nature that should be ran in an environment it
is trying to build and in some cases on a particular arch it is trying
to build.

This is IMHO outside the original scope of Mock, and it is something
we're just making use of as it is convenient to use the same code paths
for generating a chroot, installing a package set into said chroot, and
doing something inside that chroot.  In our case that something isn't
building a package, its doing something else.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[seth vidal]

On Mon, 2006-06-26 at 15:55 -0400, Jesse Keating wrote:
>  Good points about other uses.  I'll comment on how this
> particular issue
>  effects these uses:
>  
>  On Mon, 2006-06-26 at 15:55 -0400, seth vidal wrote:
>  > 1. in brew
>  
>  Brew likes this because brew is already an xml-rpc server.
>  
>  > 2. standalone with just a person in front of a terminal 
>  
>  I would think that mock would default to having the notification turned
>  off, since it requires configuration anyway, so standalone users
>  shouldn't be affected (hopefully).
>   
sounds perfect. I just wanted to mention it. I'm sure you understand. :)

-sv

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Jesse Keating
>  Sent: Monday, June 26, 2006 2:59 PM
>  To: fedora-buildsys-list(a)redhat.com
>  Subject: Re: New version of mock working (I think)
>  
>  In Red Hat land we use mock to run commands in a chroot.  
>  Examples would be buildinstall from anaconda, pkgorder from 
>  anaconda, createrepo, repoview, things of this nature that 
>  should be ran in an environment it is trying to build and in 
>  some cases on a particular arch it is trying to build.
>  
>  This is IMHO outside the original scope of Mock, and it is  
Agreed.

>  something we're just making use of as it is convenient to use 
>  the same code paths for generating a chroot, installing a 
>  package set into said chroot, and doing something inside that 
>  chroot.  In our case that something isn't building a package, 
>  its doing something else. 
For security implications, there is a push to make mock 'safe to run by
semi- or non-trusted users'. The chroot option is not ever going to be
safe, from what I can tell, so we might have to make a two-level scheme,
or a privleged config option for enabling/disabling this.

The 'mach' project has much greater ambitions on this front, and might
be a better choice for you.
--
Michael

[Jesse Keating]

On Mon, 2006-06-26 at 14:57 -0500, Michael_E_Brown(a)Dell.com wrote:
>  I don't know what brew is. Link? 
Brew is the Red Hat buildsystem that replaces Beehive.  Work on brew
started at last years summit, before Extras was really off the ground,
and before plague had seen the light of day.  Brew does a lot of things
around package and chroot tracking that plague doesn't, and it was
thought best to keep the two separate.  Brew still uses mock as the
workhorse though.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
>  Yes, feedback from Dan would be good. My initial thoughts are that a
>  client implementation would be best at this point, due to the security
>  implications of a server. Something where we call a server API provided
>  by plague like:
>      server.begin_mock_status( "my.src.rpm", MY_PID)
>      server.set_mock_status( "my.src.rpm", "status_string", MY_PID)
>      ...
>      server.end_mock_status( "my.src.rpm", return_code, MY_PID)
>  
Ah, and you thought I really *meant* it when I said I'd shut up. Ha!

Don't you think that a simple server where we just have a listen
socket  and respond to "what's your status?" would be more straight
forward?

> >
> > Just for 2cents, I'd like to make sure we don't forget the
> > other use cases of mock:
> >
> > 1. in brew
>
>  I don't know what brew is. Link?
>  
I wasn't sure if brew used mock. It's the RHEL build system.


Clark
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoD5fHyuj/+TTEp0RAtqWAKC0ltOzM0+GBTb1Ss9GR23XHqVMUwCfdi/d
> uBGXAWEzercmDocQlYBwT1M=
> =lANN
> -----END PGP SIGNATURE-----

[Jesse Keating]

On Mon, 2006-06-26 at 15:02 -0500, Michael_E_Brown(a)Dell.com wrote:
>  For security implications, there is a push to make mock 'safe to
> run by
>  semi- or non-trusted users'. The chroot option is not ever going to be
>  safe, from what I can tell, so we might have to make a two-level scheme,
>  or a privleged config option for enabling/disabling this. 
I'm not sure how that will effect us (Red Hat).  The user that calls our
mock is always trusted I suppose, in our locked down build environment.

>  The 'mach' project has much greater ambitions on this front,
> and might
>  be a better choice for you.  
I'm not sure if 'mach' was reviewed for use, or if we just went straight
to mock, since plague did.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[seth vidal]

On Mon, 2006-06-26 at 15:02 -0500, Michael_E_Brown(a)Dell.com wrote:
>  > -----Original Message-----
>  > From: fedora-buildsys-list-bounces(a)redhat.com 
>  > [mailto:[e-mail]] On Behalf Of 
>  > Jesse Keating
>  > Sent: Monday, June 26, 2006 2:59 PM
>  > To: fedora-buildsys-list(a)redhat.com
>  > Subject: Re: New version of mock working (I think)
>  > 
>  > In Red Hat land we use mock to run commands in a chroot.  
>  > Examples would be buildinstall from anaconda, pkgorder from 
>  > anaconda, createrepo, repoview, things of this nature that 
>  > should be ran in an environment it is trying to build and in 
>  > some cases on a particular arch it is trying to build.
>  > 
>  > This is IMHO outside the original scope of Mock, and it is 
>  
>  Agreed.
>  
>  > something we're just making use of as it is convenient to use 
>  > the same code paths for generating a chroot, installing a 
>  > package set into said chroot, and doing something inside that 
>  > chroot.  In our case that something isn't building a package, 
>  > its doing something else.
>  
>  For security implications, there is a push to make mock 'safe to run by
>  semi- or non-trusted users'. The chroot option is not ever going to be
>  safe, from what I can tell, so we might have to make a two-level scheme,
>  or a privleged config option for enabling/disabling this.
>  
>  The 'mach' project has much greater ambitions on this front, and might
>  be a better choice for you. 
If converting things back over to 'mach' makes more sense then I'd
suggest going that path for all of it.

At the time we put mock together b/c we needed an immediate solution to
a specific problem with a certain set of prereqs.

Thomas has made a number of changes to mach, if it is happy for our use
case then go with it.

-sv

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Clark Williams
>  Sent: Monday, June 26, 2006 3:07 PM
>  To: Discussion of Fedora build system
>  Subject: Re: New version of mock working (I think)
>  
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Michael_E_Brown(a)Dell.com wrote:
>  >
>  > Yes, feedback from Dan would be good. My initial thoughts 
>  are that a 
>  > client implementation would be best at this point, due to 
>  the security 
>  > implications of a server. Something where we call a server API 
>  > provided by plague like:
>  >     server.begin_mock_status( "my.src.rpm", MY_PID)
>  >     server.set_mock_status( "my.src.rpm", "status_string",
> MY_PID)
>  >     ...
>  >     server.end_mock_status( "my.src.rpm", return_code, MY_PID)
>  >
>  
>  Ah, and you thought I really *meant* it when I said I'd shut up. Ha!
>  
>  Don't you think that a simple server where we just have a 
>  listen socket  and respond to "what's your status?" would be 
>  more straight forward? 
I would like to foster discussion. It would be bad if one of the
co-maintainers failed to voice their concerns over project direction, so
no, I would hope that you never shut up.

My initial thoughts around this is that having an xmlrpc server exposes
too much of our (running with elevated privs) internals to random,
untrusted strangers. There has already been one reported vulnerability
in the python xmlrpc code.

Both code-wise (7-10 lines for client, vs at least 20 for server), and
security-wise, I think a client would be 'simpler'.

Also, adding a server entices future additions along the lines you
already said we don't want to go, and starts to supplant plague/brews
role.
--
Michael

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
>  
>
> > -----Original Message-----
> > From: fedora-buildsys-list-bounces(a)redhat.com
> > [mailto:[e-mail]] On Behalf Of
> > Clark Williams
> > Sent: Monday, June 26, 2006 3:07 PM
> > To: Discussion of Fedora build system
> > Subject: Re: New version of mock working (I think)
> >
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Michael_E_Brown(a)Dell.com wrote:
> >> Yes, feedback from Dan would be good. My initial thoughts
> > are that a
> >> client implementation would be best at this point, due to
> > the security
> >> implications of a server. Something where we call a server API
> >> provided by plague like:
> >>     server.begin_mock_status( "my.src.rpm", MY_PID)
> >>     server.set_mock_status( "my.src.rpm", "status_string",
> MY_PID)
> >>     ...
> >>     server.end_mock_status( "my.src.rpm", return_code, MY_PID)
> >>
> > Ah, and you thought I really *meant* it when I said I'd shut up. Ha!
> >
> > Don't you think that a simple server where we just have a
> > listen socket  and respond to "what's your status?" would be
> > more straight forward?
>
>  I would like to foster discussion. It would be bad if one of the
>  co-maintainers failed to voice their concerns over project direction, so
>  no, I would hope that you never shut up. 
<aside>

/me hopes everyone takes note of the above.

Please don't construe my opposition to a proposal on the list as
anything personal.  It's just my opinions based on a couple of decades
of programming experience.  My programming philosophy will pretty much
always boil down to: simpler is better.

I can be talked around to solution that I initially oppose. I just
need some encouragement :)

</aside>
>  My initial thoughts around this is that having an xmlrpc server exposes
>  too much of our (running with elevated privs) internals to random,
>  untrusted strangers. There has already been one reported vulnerability
>  in the python xmlrpc code.
>
>  Both code-wise (7-10 lines for client, vs at least 20 for server), and
>  security-wise, I think a client would be 'simpler'.
>
>  Also, adding a server entices future additions along the lines you
>  already said we don't want to go, and starts to supplant plague/brews
>  role. 
Ok, I can go with that. Probably the most convincing portion of the
above argument is the idea of implementing a root-privilege server as
opposed to pushing that back to the build system maintainers.

So I suppose it means that we should publish a call-out specification
that says: if you invoke mock with the  --server-mumble=X on the
command line, mock will use X as a URL to make an XML-RPC call with
the state  as an argument in <as yet to be determined> form.  We will
then have to wrap that in a try/except and possibly some timeout logic
to make sure that if the other end isn't speaking our language, we
continue or fail in a controlled fashion.

Clark


> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoEyNHyuj/+TTEp0RAoToAJ9jNUmUqLkyxsAh0HoUNEu94lEq8wCffdIh
> YTkO7OshBbqn1eymgtrMP3c=
> =pl/z
> -----END PGP SIGNATURE-----

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Clark Williams
>  Sent: Monday, June 26, 2006 4:07 PM
>  To: Discussion of Fedora build system
>  Subject: Re: New version of mock working (I think)
>  
>  Ok, I can go with that. Probably the most convincing portion 
>  of the above argument is the idea of implementing a 
>  root-privilege server as opposed to pushing that back to the 
>  build system maintainers.
>  
>  So I suppose it means that we should publish a call-out 
>  specification that says: if you invoke mock with the  
>  --server-mumble=X on the command line, mock will use X as a 
>  URL to make an XML-RPC call with the state  as an argument in 
>  <as yet to be determined> form.  We will then have to wrap 
>  that in a try/except and possibly some timeout logic to make 
>  sure that if the other end isn't speaking our language, we 
>  continue or fail in a controlled fashion. 
I have some stock timeout and exception handling code that we can cut
and paste in. It adds another 5-10 lines of code, and should make the
code robust against what you describe.

First order of business would be to get Dan's opinion on the matter,
then settle on an API. I can code this up, along with a test harness.
--
Michael

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dan Williams wrote:
>  On Fri, 2006-06-23 at 13:40 -0500, Michael_E_Brown(a)Dell.com wrote:
> > Any chance we could get a better defined method of mock<->plague
> > communication? Parsing output is bound to be fragile and hard to
> > maintain.
>
>  Yup.  Quite right.  And I've had problems in the past with the reads of
>  the file blocking (since you evidently can't do NONBLOCK on filesystem
>  file sockets or something either?) and hanging the plague-builder
>  process until more data gets written to the file.  So as a quick hack
>  plague-builder will only read & match the first 4 characters of the
>  status string anyway.
>
>  I'd rather not read the status file either.  Any ideas that don't suck
>  would be greatly appreciated.
>  
After being convinced that calling out with XML-RPC from mock wouldn't
trigger a nuclear winter, I went back to read Dan's request and think
about it in the context of making an XML-RPC to the build-server.

If we presuppose that mock will make XML-RPC's to a URL supplied by
the build system to indicate status changes, would it make sense to
send the complete list of status's to the build system before we
start? In other words, when we crank up, make an initial call to the
status URL with "Here is the list of status values you can receive",
maybe as a list of strings or a hash with descriptive text.

This would give the build system a mechanism to quickly detect when it
and mock are out-of-sync with respect to state changes (i.e.
brew/plague could receive the state list and abort if it sees a state
change it doesn't understand). Of course it can do that anyway, but
that might be a long while into a glibc build before the build system
sees an unknown state, as opposed to 3 seconds into startup.

Just a thought.

Clark


> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoFAFHyuj/+TTEp0RAqn5AJ92YcCIj0fJcLpzhY6E7VfNWK7ohwCeKbKk
> bug2S0DNv892S46r/MKbkhg=
> =MkFS
> -----END PGP SIGNATURE-----

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Clark Williams
>  Sent: Monday, June 26, 2006 4:22 PM
>  To: Discussion of Fedora build system
>  Subject: Re: New version of mock working (I think) 
>  
>  If we presuppose that mock will make XML-RPC's to a URL 
>  supplied by the build system to indicate status changes, 
>  would it make sense to send the complete list of status's to 
>  the build system before we start? In other words, when we 
>  crank up, make an initial call to the status URL with "Here 
>  is the list of status values you can receive", maybe as a 
>  list of strings or a hash with descriptive text. 
All good ideas. That was sort of an implicit placeholder in the
server.begin_mock_status(...) API. We can munge that call to do whatever
pre-processing that Dan would like to see implemented. I'm sort of a
"implement what we are going to actually use _now_" kind of guy, so I
like to build the API to be minimal but expandable. Something like
python named args we can add later if plague/brew request them.

>  This would give the build system a mechanism to quickly 
>  detect when it and mock are out-of-sync with respect to state 
>  changes (i.e.
>  brew/plague could receive the state list and abort if it sees 
>  a state change it doesn't understand). Of course it can do 
>  that anyway, but that might be a long while into a glibc 
>  build before the build system sees an unknown state, as 
>  opposed to 3 seconds into startup. 
We probably also want to include our API version expectations in the
initial call. Revised proposed API:

	server.begin_mock_status(api_ver=1, srpm="my.src.rpm",
pid=MY_PID)
	server.set_mock_status(srpm="my.src.rpm", status_str="status",
pid=MY_PID)
	server.end_mock_status(srpm="my.src.rpm", return_code=retcode,
pid=MY_PID)

With Clark's suggestion, begin would look like this:
	server.begin_mock_status(api_ver=1, srpm="my.src.rpm",
pid=MY_PID, expected_status_codes=[codes,...])

--
Michael

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
> > -----Original Message-----
> > From: fedora-buildsys-list-bounces(a)redhat.com
> > [mailto:[e-mail]] On Behalf Of
> > Clark Williams
> > Sent: Monday, June 26, 2006 4:07 PM
> > To: Discussion of Fedora build system
> > Subject: Re: New version of mock working (I think)
> >
> > Ok, I can go with that. Probably the most convincing portion
> > of the above argument is the idea of implementing a
> > root-privilege server as opposed to pushing that back to the
> > build system maintainers.
> >
> > So I suppose it means that we should publish a call-out
> > specification that says: if you invoke mock with the 
> > --server-mumble=X on the command line, mock will use X as a
> > URL to make an XML-RPC call with the state  as an argument in
> > <as yet to be determined> form.  We will then have to wrap
> > that in a try/except and possibly some timeout logic to make
> > sure that if the other end isn't speaking our language, we
> > continue or fail in a controlled fashion.
>
>  I have some stock timeout and exception handling code that we can cut
>  and paste in. It adds another 5-10 lines of code, and should make the
>  code robust against what you describe.
>
>  First order of business would be to get Dan's opinion on the matter,
>  then settle on an API. I can code this up, along with a test harness.
>  --
>  Michael 
I have some changes to mock.py that go with the new launcher
philosophy that should probably go in at the same time. We probably
want to tag and branch before we go off in this new direction.

Shouldn't we do something like the following:

$ cd mock
$ cvs tag r0_6-branchpoint
$ cvs tag -r r0_6-branchpoint -b r0_6-branch

I believe that this creates a branchpoint, then creates a branchtag
and leaves our head alone. We could then go merrily on our way with
the new launcher and the XML-RPC stuff while leaving 0.6 safe and sound.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoFaLHyuj/+TTEp0RAsReAJ9OKB2DUQpw8zsKletz+qOW7wlZ2QCg3DGY
> GBHpYn6aEUOYp/Jy3i4SBSw=
> =Wxt5
> -----END PGP SIGNATURE-----

[Michael E Brown]

>  -----Original Message-----
>  From: fedora-buildsys-list-bounces(a)redhat.com 
>  [mailto:[e-mail]] On Behalf Of 
>  Jesse Keating
>  Sent: Monday, June 26, 2006 3:10 PM
>  To: fedora-buildsys-list(a)redhat.com
>  Subject: RE: New version of mock working (I think)
>  
>  On Mon, 2006-06-26 at 15:02 -0500, Michael_E_Brown(a)Dell.com wrote:
>  > For security implications, there is a push to make mock 
>  'safe to run 
>  > by
>  > semi- or non-trusted users'. The chroot option is not ever 
>  going to be 
>  > safe, from what I can tell, so we might have to make a two-level 
>  > scheme, or a privleged config option for enabling/disabling this.
>  
>  I'm not sure how that will effect us (Red Hat).  The user 
>  that calls our mock is always trusted I suppose, in our 
>  locked down build environment.
>  
>  > The 'mach' project has much greater ambitions on this 
>  front, and might 
>  > be a better choice for you.
>  
>  I'm not sure if 'mach' was reviewed for use, or if we just 
>  went straight to mock, since plague did. 
One of the signs of project maturity and good leadership is knowing when
to say 'No', and not be afraid to point potential users to better
alternativest to solve their problem. At this point, your request is
right on the border, but I think we can accommodate so far. What I don't
want to run into, though, is the point where we use mock for things like
this: http://thomas.apestaart.org/log/?p=360. That is what 'mach' is
for, and I don't care to try to compete.

--
Michael

[Mike McLean]

Jesse Keating wrote:
>  On Mon, 2006-06-26 at 15:55 -0400, seth vidal wrote:
>  
> >1. in brew
>  
>  Brew likes this because brew is already an xml-rpc server. 
Actually no. Brew's use of xmlrpc does not imply that this particular 
suggestion will be easy to integrate with Brew. We'd want to be pushing 
data back to the brew-builder process which does not run an xmlrpc server.

Note I'm not necessarily against the idea, but I am wary.

I'd almost rather handle this with a standard format state file (say 
xml) and file locking.

[Jesse Keating]

On Mon, 2006-06-26 at 17:07 -0500, Michael_E_Brown(a)Dell.com wrote:
>  One of the signs of project maturity and good leadership is knowing
>  when
>  to say 'No', and not be afraid to point potential users to better
>  alternativest to solve their problem. At this point, your request is
>  right on the border, but I think we can accommodate so far. What I
>  don't
>  want to run into, though, is the point where we use mock for things
>  like
>  this: http://thomas.apestaart.org/log/?p=360. That is what 'mach' is
>  for, and I don't care to try to compete.  
heh, i don't think we'll be doing anything crazy like that.

-- 
Jesse Keating RHCE      (geek.j2solutions.net)
Fedora Legacy Team      (www.fedoralegacy.org)
GPG Public Key          (geek.j2solutions.net/jkeating.j2solutions.pub)

[Andreas Thienemann]

On Mon, 26 Jun 2006, Mike McLean wrote:

>  I'd almost rather handle this with a standard format state file
> (say 
>  xml) and file locking. 
+1

Communication between plague-builder and mock is local. They are both 
running on the same machine.
Using XMLRPC sounds to me as if some people here are suffering from "If 
all I have is a hammer, everything looks like a nail"-syndrome.
It happened to me as well. ;D

An added benefit of just keeping state in a simple textfile is the ability 
to easily parse this with a shellscript.

I've used mock in the past to just rebuild a bunch of rpms and used the 
state file to keep track.

Having to suddenly add xmlrpc processing or even xml just processing to 
the shellscripts is a nuisance.

Furthermore, who is going to parse these state files? Right now, it's just 
plague, as brew is designed differently. Do we really need a extendable 
meta format for exchanging data?
IMHO, a simple textfile with a simple string would suffice for most uses.

If you want special status queries, why not just create a pipe which 
another tool can fopen() and fwrite() "status" and fget() "Building 
foo.src.rpm".
We do not have a large bunch of different states, we don't really have to 
encapsulate these in some meta-format.

regards,
 andreas

[Dan Williams]

On Mon, 2006-06-26 at 16:19 -0500, Michael_E_Brown(a)Dell.com wrote:
>  > -----Original Message-----
>  > From: fedora-buildsys-list-bounces(a)redhat.com 
>  > [mailto:[e-mail]] On Behalf Of 
>  > Clark Williams
>  > Sent: Monday, June 26, 2006 4:07 PM
>  > To: Discussion of Fedora build system
>  > Subject: Re: New version of mock working (I think)
>  > 
>  > Ok, I can go with that. Probably the most convincing portion 
>  > of the above argument is the idea of implementing a 
>  > root-privilege server as opposed to pushing that back to the 
>  > build system maintainers.
>  > 
>  > So I suppose it means that we should publish a call-out 
>  > specification that says: if you invoke mock with the  
>  > --server-mumble=X on the command line, mock will use X as a 
>  > URL to make an XML-RPC call with the state  as an argument in 
>  > <as yet to be determined> form.  We will then have to wrap 
>  > that in a try/except and possibly some timeout logic to make 
>  > sure that if the other end isn't speaking our language, we 
>  > continue or fail in a controlled fashion.
>  
>  I have some stock timeout and exception handling code that we can cut
>  and paste in. It adds another 5-10 lines of code, and should make the
>  code robust against what you describe.
>  
>  First order of business would be to get Dan's opinion on the matter,
>  then settle on an API. I can code this up, along with a test harness. 
I hate to throw water on the fire, but to me XML-RPC seems like a
sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
this specific instance of pushing status information back to a
controlling process, I think it's somewhat heavy-handed.

Stepping back a second, what are we trying to do with this?  We're only
trying to push a status string, and possibly a UID/PID, back to some
other process.  I don't have a good idea what might be functionality
might be wanted in the future here though, but having mock as a discrete
program that's fairly uncomplicated seems like a good thing to me.

XML-RPC is quite nice, especially from Python, but it doesn't feel like
the solution that fits the problem here.

So, either of the two other proposed solutions would work for me:

1) Status file with locking.  If there's locking, I can stat() the file
and find out how big it is before trying to read it.  Hence, no need for
non-blocking IO here, though the previous blocking problems could be a
stupid problem on my part in plague.

2) Unix socket provided by plague-builder, which mock writes to.  Pass
mock a --status-socket argument pointing to the location of a status
socket.  We can also use Unix socket peer credentials (like dbus and
gnome-keyring) here to verify that the writer to the socket is actually
the 'mock' user or whatever, in addition to setting permissions on the
socket itself.

I'd vote to #1 to start with since it's the simplest, then #2 if we have
issues with #1.

Either of these starts falling down fairly quickly if we want to push
more than a few items of status information to the controlling process
from mock.  But in that case, we should ask _why_ we're trying to push
so much info back, and if we need to.  If we decide we need to, then I
think we should investigate XML-RPC, possibly over TCP/IP, but most
likely over a local unix socket.  I just don't think mock needs to be
network-enabled.

Dan

[Jeremy Katz]

On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
>  I hate to throw water on the fire, but to me XML-RPC seems like a
>  sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
>  this specific instance of pushing status information back to a
>  controlling process, I think it's somewhat heavy-handed. 
And I'll just pick this point in the thread to say that I agree with
dcbw here....   KISS goes a long way.  And while we can say that it's
configurable and you enable it if you want or don't if not -- that's
then two code paths for status to have to keep working and ensure we
don't break as well as maintaining them for some time

Jeremy

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeremy Katz wrote:
>  On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
> > I hate to throw water on the fire, but to me XML-RPC seems like a
> > sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
> > this specific instance of pushing status information back to a
> > controlling process, I think it's somewhat heavy-handed.
>
>  And I'll just pick this point in the thread to say that I agree with
>  dcbw here....   KISS goes a long way.  And while we can say that it's
>  configurable and you enable it if you want or don't if not -- that's
>  then two code paths for status to have to keep working and ensure we
>  don't break as well as maintaining them for some time
>  
So where were you guys when I was playing Horatius at the bridge? :)

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoUsnHyuj/+TTEp0RAq4XAKDMeKTgIn1TkGA1ikNq9CzETi054ACfZF8L
> t2OhL8a/aKJ1QmxHpTq8jA0=
> =qSJp
> -----END PGP SIGNATURE-----

[Greg DeKoenigsberg]

On Tue, 27 Jun 2006, Clark Williams wrote:

>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Jeremy Katz wrote:
>  > On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
>  >> I hate to throw water on the fire, but to me XML-RPC seems like a
>  >> sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
>  >> this specific instance of pushing status information back to a
>  >> controlling process, I think it's somewhat heavy-handed.
>  >
>  > And I'll just pick this point in the thread to say that I agree with
>  > dcbw here....   KISS goes a long way.  And while we can say that it's
>  > configurable and you enable it if you want or don't if not -- that's
>  > then two code paths for status to have to keep working and ensure we
>  > don't break as well as maintaining them for some time
>  >
>  
>  So where were you guys when I was playing Horatius at the bridge? :) 
They were letting you win the argument yourself, to prove how gosh-darned 
smart you are, Clark.  ;-)

--g

-------------------------------------------------------------
Greg DeKoenigsberg || Fedora Project || fedoraproject.org
Be an Ambassador || http://fedoraproject.org/wiki/Ambassadors
-------------------------------------------------------------

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Greg DeKoenigsberg wrote:
>  On Tue, 27 Jun 2006, Clark Williams wrote:
>
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Jeremy Katz wrote:
> >> On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
> >>> I hate to throw water on the fire, but to me XML-RPC seems like a
> >>> sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
> >>> this specific instance of pushing status information back to a
> >>> controlling process, I think it's somewhat heavy-handed.
> >> And I'll just pick this point in the thread to say that I agree with
> >> dcbw here....   KISS goes a long way.  And while we can say that it's
> >> configurable and you enable it if you want or don't if not -- that's
> >> then two code paths for status to have to keep working and ensure we
> >> don't break as well as maintaining them for some time
> >>
> > So where were you guys when I was playing Horatius at the bridge? :)
>
>  They were letting you win the argument yourself, to prove how gosh-darned
>  smart you are, Clark.  ;-)
>  
Not smart enough to argue that we should wait for our build system
clients to weigh in. Sigh...

Dan/Jeremy/Mike/Andreas/et al

How are you going to use file locking? Do you just want mock to lock
the status file when we're changing state, or is there something else
I'm missing?

Clark

P.S. Who let the marketing dweeb on this channel? :)


Clark
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoU7VHyuj/+TTEp0RAkrtAKDhdyQHZxmW+8ilshXePqcgJNGigACgtqgn
> fcPt8h7/oZ/Irw5Jv4mrNnI=
> =HYDS
> -----END PGP SIGNATURE-----

[Mike McLean]

Clark Williams wrote:
>  Not smart enough to argue that we should wait for our build system
>  clients to weigh in. Sigh...
>  
>  Dan/Jeremy/Mike/Andreas/et al
>  
>  How are you going to use file locking? Do you just want mock to lock
>  the status file when we're changing state, or is there something else
>  I'm missing? 
Pretty much. The lock is to prevent the watcher from reading a partially 
written status file.

[seth vidal]

On Tue, 2006-06-27 at 10:13 -0500, Clark Williams wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Jeremy Katz wrote:
>  > On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
>  >> I hate to throw water on the fire, but to me XML-RPC seems like a
>  >> sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
>  >> this specific instance of pushing status information back to a
>  >> controlling process, I think it's somewhat heavy-handed.
>  >
>  > And I'll just pick this point in the thread to say that I agree with
>  > dcbw here....   KISS goes a long way.  And while we can say that it's
>  > configurable and you enable it if you want or don't if not -- that's
>  > then two code paths for status to have to keep working and ensure we
>  > don't break as well as maintaining them for some time
>  >
>  
>  So where were you guys when I was playing Horatius at the bridge? :)
>   
I'd like to take this opportunity to point to lesscode.org

thanks,
-sv

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

seth vidal wrote:
>  On Tue, 2006-06-27 at 10:13 -0500, Clark Williams wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Jeremy Katz wrote:
> >> On Tue, 2006-06-27 at 10:12 -0400, Dan Williams wrote:
> >>> I hate to throw water on the fire, but to me XML-RPC seems like a
> >>> sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
> >>> this specific instance of pushing status information back to a
> >>> controlling process, I think it's somewhat heavy-handed.
> >> And I'll just pick this point in the thread to say that I agree with
> >> dcbw here....   KISS goes a long way.  And while we can say that it's
> >> configurable and you enable it if you want or don't if not -- that's
> >> then two code paths for status to have to keep working and ensure we
> >> don't break as well as maintaining them for some time
> >>
> > So where were you guys when I was playing Horatius at the bridge? :)
> >
>
>  I'd like to take this opportunity to point to lesscode.org
>
>  thanks,
>  -sv 
Thanks for that link.

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoVjaHyuj/+TTEp0RAudxAJ49NekRv0HFiauK1OP7ya1+MQD0twCdHLS7
> ymeHY1y8p9TE6179JYvhcjY=
> =Zums
> -----END PGP SIGNATURE-----

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mike McLean wrote:
>  Clark Williams wrote:
> > Not smart enough to argue that we should wait for our build system
> > clients to weigh in. Sigh...
> >
> > Dan/Jeremy/Mike/Andreas/et al
> >
> > How are you going to use file locking? Do you just want mock to lock
> > the status file when we're changing state, or is there something else
> > I'm missing?
>
>  Pretty much. The lock is to prevent the watcher from reading a
>  partially written status file. 
Something like :

    flock(fileno, LOCK_EX)
    <write new status>
    flock(fileno, LOCK_UN)

That what you're looking for?

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoVmjHyuj/+TTEp0RAlGBAKDJNG/nSNZX0CGMLgWZjuXuVrwrmwCghKPn
> JcaPW7HZCR00+Q7Genx7luA=
> =IdtH
> -----END PGP SIGNATURE-----

[Dan Williams]

On Tue, 2006-06-27 at 11:15 -0500, Clark Williams wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  Mike McLean wrote:
>  > Clark Williams wrote:
>  >> Not smart enough to argue that we should wait for our build system
>  >> clients to weigh in. Sigh...
>  >>
>  >> Dan/Jeremy/Mike/Andreas/et al
>  >>
>  >> How are you going to use file locking? Do you just want mock to lock
>  >> the status file when we're changing state, or is there something else
>  >> I'm missing?
>  >
>  > Pretty much. The lock is to prevent the watcher from reading a
>  > partially written status file.
>  
>  Something like :
>  
>      flock(fileno, LOCK_EX)
>      <write new status>
>      flock(fileno, LOCK_UN)
>  
>  That what you're looking for? 
In python:

import fcntl
lkfd = open(<lockfile_path>, 'w')
rc = fcntl.flock(lkfd, fcntl.LOCK_EX)
<write new status>
fcntl.flock(lkfd, fcntl.LOCK_UN)
close(lkfd)

Yes.

Dan

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dan Williams wrote:
>  On Tue, 2006-06-27 at 11:15 -0500, Clark Williams wrote:
> > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >
> > Mike McLean wrote:
> >> Clark Williams wrote:
> >>> Not smart enough to argue that we should wait for our build
> >>> system clients to weigh in. Sigh...
> >>>
> >>> Dan/Jeremy/Mike/Andreas/et al
> >>>
> >>> How are you going to use file locking? Do you just want mock
> >>> to lock the status file when we're changing state, or is
> >>> there something else I'm missing?
> >> Pretty much. The lock is to prevent the watcher from reading a
> >> partially written status file.
> > Something like :
> >
> > flock(fileno, LOCK_EX) <write new status> flock(fileno, LOCK_UN)
> >
> > That what you're looking for?
>
>  In python:
>
>  import fcntl lkfd = open(<lockfile_path>, 'w') rc =
>  fcntl.flock(lkfd, fcntl.LOCK_EX) <write new status>
>  fcntl.flock(lkfd, fcntl.LOCK_UN) close(lkfd)
>
>  Yes.
>
>  Dan
>  
What, you couldn't tell that was pseudo-python? :)

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoXbRHyuj/+TTEp0RAnUlAJ9HfAgDZP+teHPSeQNdNcCoAWcKuACgslBO
> aeOz/hsxMBY6afa7COrXu+I=
> =HiV2
> -----END PGP SIGNATURE-----

[Michael E Brown]

Ok, I didnt see the implications of the status file before, but it looks fine to me. The
only problem with that approach that I see is that the caller needs to poll the file to
observe status changes. But if that is fine with the buildsystem guys, it works for me.

We shouldnt really need locking on that file. Writes <4k are atomic. Since we are only
writing one line at a time to the status file, there is no danger of a reading process
seeing an inconsistent file. We should probably just add a flush() statement to make sure
that we are not buffering anything past close() (which I think is probably illegal,
anyways)
--
Michael


-----Original Message-----
From: fedora-buildsys-list-bounces(a)redhat.com on behalf of Clark Williams
Sent: Tue 6/27/2006 1:20 PM
To: Discussion of Fedora build system
Cc: mikem(a)redhat.com
Subject: Re: New version of mock working (I think)
 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Dan Williams wrote:
>  On Tue, 2006-06-27 at 11:15 -0500, Clark Williams wrote:
> > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >
> > Mike McLean wrote:
> >> Clark Williams wrote:
> >>> Not smart enough to argue that we should wait for our build
> >>> system clients to weigh in. Sigh...
> >>>
> >>> Dan/Jeremy/Mike/Andreas/et al
> >>>
> >>> How are you going to use file locking? Do you just want mock
> >>> to lock the status file when we're changing state, or is
> >>> there something else I'm missing?
> >> Pretty much. The lock is to prevent the watcher from reading a
> >> partially written status file.
> > Something like :
> >
> > flock(fileno, LOCK_EX) <write new status> flock(fileno, LOCK_UN)
> >
> > That what you're looking for?
>
>  In python:
>
>  import fcntl lkfd = open(<lockfile_path>, 'w') rc =
>  fcntl.flock(lkfd, fcntl.LOCK_EX) <write new status>
>  fcntl.flock(lkfd, fcntl.LOCK_UN) close(lkfd)
>
>  Yes.
>
>  Dan
>  
What, you couldn't tell that was pseudo-python? :)

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEoXbRHyuj/+TTEp0RAnUlAJ9HfAgDZP+teHPSeQNdNcCoAWcKuACgslBO
> aeOz/hsxMBY6afa7COrXu+I=
> =HiV2
> -----END PGP SIGNATURE----- 
--
Fedora-buildsys-list mailing list
Fedora-buildsys-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/fedora-buildsys-list

[Michael E Brown]

<aside>Sorry for the format of the posts today, forced to use stupid webmail which
is slow and ugly.</aside>

How about the old unix everything is a file concept? We could just have a --status_file
parameter that could take a file or a named pipe as input, it would look the same to us,
whether it was a pipe or a regular file. Either that, or we could even have a --status_fd
parameter for people that might want to pre-open a file and pass it to us as an fd when
they fork/exec.

We might want to consider removing the open()/close() and instead just flush() it. We
still shouldnt really need locking.
--
Michael

-----Original Message-----
From: fedora-buildsys-list-bounces(a)redhat.com on behalf of Dan Williams
Sent: Tue 6/27/2006 9:12 AM
To: Discussion of Fedora build system
Subject: RE: New version of mock working (I think)
 
On Mon, 2006-06-26 at 16:19 -0500, Michael_E_Brown(a)Dell.com wrote:
>  > -----Original Message-----
>  > From: fedora-buildsys-list-bounces(a)redhat.com 
>  > [mailto:[e-mail]] On Behalf Of 
>  > Clark Williams
>  > Sent: Monday, June 26, 2006 4:07 PM
>  > To: Discussion of Fedora build system
>  > Subject: Re: New version of mock working (I think)
>  > 
>  > Ok, I can go with that. Probably the most convincing portion 
>  > of the above argument is the idea of implementing a 
>  > root-privilege server as opposed to pushing that back to the 
>  > build system maintainers.
>  > 
>  > So I suppose it means that we should publish a call-out 
>  > specification that says: if you invoke mock with the  
>  > --server-mumble=X on the command line, mock will use X as a 
>  > URL to make an XML-RPC call with the state  as an argument in 
>  > <as yet to be determined> form.  We will then have to wrap 
>  > that in a try/except and possibly some timeout logic to make 
>  > sure that if the other end isn't speaking our language, we 
>  > continue or fail in a controlled fashion.
>  
>  I have some stock timeout and exception handling code that we can cut
>  and paste in. It adds another 5-10 lines of code, and should make the
>  code robust against what you describe.
>  
>  First order of business would be to get Dan's opinion on the matter,
>  then settle on an API. I can code this up, along with a test harness. 
I hate to throw water on the fire, but to me XML-RPC seems like a
sledgehammer.  There's actually a ton of code in xmlrpclib.py and for
this specific instance of pushing status information back to a
controlling process, I think it's somewhat heavy-handed.

Stepping back a second, what are we trying to do with this?  We're only
trying to push a status string, and possibly a UID/PID, back to some
other process.  I don't have a good idea what might be functionality
might be wanted in the future here though, but having mock as a discrete
program that's fairly uncomplicated seems like a good thing to me.

XML-RPC is quite nice, especially from Python, but it doesn't feel like
the solution that fits the problem here.

So, either of the two other proposed solutions would work for me:

1) Status file with locking.  If there's locking, I can stat() the file
and find out how big it is before trying to read it.  Hence, no need for
non-blocking IO here, though the previous blocking problems could be a
stupid problem on my part in plague.

2) Unix socket provided by plague-builder, which mock writes to.  Pass
mock a --status-socket argument pointing to the location of a status
socket.  We can also use Unix socket peer credentials (like dbus and
gnome-keyring) here to verify that the writer to the socket is actually
the 'mock' user or whatever, in addition to setting permissions on the
socket itself.

I'd vote to #1 to start with since it's the simplest, then #2 if we have
issues with #1.

Either of these starts falling down fairly quickly if we want to push
more than a few items of status information to the controlling process
from mock.  But in that case, we should ask _why_ we're trying to push
so much info back, and if we need to.  If we decide we need to, then I
think we should investigate XML-RPC, possibly over TCP/IP, but most
likely over a local unix socket.  I just don't think mock needs to be
network-enabled.

Dan


--
Fedora-buildsys-list mailing list
Fedora-buildsys-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/fedora-buildsys-list

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael_E_Brown(a)Dell.com wrote:
>  <aside>Sorry for the format of the posts today, forced to use
> stupid 
webmail which is slow and ugly.</aside>
>  How about the old unix everything is a file concept? We could just have 
a
--status_file parameter that could take a file or a named pipe as
input, it would look the same to us, whether it was a pipe or a regular
file. Either that, or we could even have a --status_fd parameter for
people that might want to pre-open a file and pass it to us as an fd
when they fork/exec.
>  We might want to consider removing the open()/close() and instead just 
flush() it.
We still shouldnt really need locking.

I'd just as soon put the flock's around the write for explicitness's
sake.  It's an import of fcntl and then two calls to fcntl.flock()...

Clark

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFEobgsHyuj/+TTEp0RAsztAJ9nezD09sNR0lyA/BBp3kFJHgji0ACg2utd
> j598GRugOIiLB/9iYBTWLCU=
> =X3dv
> -----END PGP SIGNATURE-----