bug-make
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug #33138] .PARLLELSYNC enhancement with patch


From: Eli Zaretskii
Subject: Re: [bug #33138] .PARLLELSYNC enhancement with patch
Date: Tue, 16 Apr 2013 16:43:53 +0300

> From: Paul Smith <address@hidden>
> Cc: address@hidden, address@hidden, address@hidden
> Date: Tue, 16 Apr 2013 07:47:08 -0400
> 
> On Tue, 2013-04-16 at 11:30 +0300, Eli Zaretskii wrote:
> > > Date: Tue, 16 Apr 2013 05:54:13 +0000
> > > From: "Paul D. Smith" <address@hidden>
> > > 
> > > I did a little bit of code rearrangement, but I still think this code 
> > > will not
> > > work on Windows and might possibly not compile on Windows.
> > 
> > Indeed, it will not.  Some cursory comments below.
> 
> I was hoping that if OUTPUT_SYNC is not #defined, the Windows code would
> compile OK (although obviously without this feature), until we get it
> working.

That is indeed so; I was writing about what would happen if those
fragments were allowed to be compiled on Windows.

> >  . STREAM_OK uses fcntl and F_GETFD which both don't exist on Windows.
> 
> This is to test if somehow make's stdout or stderr has been closed (not
> just redirected to /dev/null, but closed).

Right, got that; but Windows needs a different test for that, as
there's no fcntl.

> I'm not sure what the semantics of tmpfile() are on Windows.

The file is automatically deleted when closed.  But the documentation
doesn't say what happens if it is open on more than one descriptor, or
what happens if the original descriptor is dup'ed.  I will need to
test that, and perhaps provide a work-around.

> >  . acquire_semaphore uses fcntl and F_SETLKW, which don't exist on
> >    Windows.  the commentary to that function is not revealing its
> >    purpose, so I'm unsure how to implement its equivalent on Windows.
> >    can someone explain why is this needed and what should it do?  the
> >    name seems to imply that using fcntl is an implementation detail,
> >    as a crude semaphore -- is that right?  similarly for
> >    release_semaphore.  (see also the next item.)
> 
> Yes, this is the guts of the feature.  It ensures that only one make
> process is writing output at a time.  On other systems like Windows a
> different method might be more appropriate.  Since the resource we're
> locking on is the output, on UNIX we lock the output fd.  This saves us
> from having to create a separate lock file, etc.
> 
> I'm pretty convinced that it works fine, even if stdout/stderr are
> redirected.  For example, a recursive make which is redirected to a
> different file will work OK; the locking for the sub-make will happen on
> that file which is different than the locking for other make instances,
> but that's OK because they're writing to different places anyway.  The
> sub-make's output will be internally consistent, and not interfere with
> the parent make's output which is what you want.
> 
> Windows has LockFileEx() but we'd need to examine the semantics to
> verify it will do what we want.

Do we even need to lock a file?  If all that's needed is a semaphore
(actually, a mutex, AFAICS), Windows has good support for that which
doesn't involve files at all.

> >  . is there any significance to the fact that sync_handle is either
> >    stdout or stderr?  is that important for the synchronization to
> >    work, or was that just a convenient handle around?  also, the
> >    documentation I have indicates that locking works only on files;
> >    is it guaranteed that these handles are redirected to files before
> >    acquire_semaphore is called?
> 
> They are definitely NOT guaranteed to be redirected to files.  The lock
> is taken on stdout (if open) before any redirection happens, so normally
> it would be taken on stdout going to the console.  On Linux it works
> fine.  I'll need to read the standard more closely and maybe do some
> testing on other systems.  It's just a convenient handle... but if it
> doesn't always work and we have to create our own handle then that's
> some extra work as we have to communicate the handle info to the child
> make processes.

This page:

   http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html

says, immediately prior to describing F_SETLKW and its friends:

   The following values for 'cmd' are available for advisory record
   locking. Record locking shall be supported for regular files, and
   may be supported for other files.

I don't know what is the de-facto situation in this regard on Posix
systems.

> >  . outputting stdout and stderr separately is IMO a misfeature: it
> >    breaks the temporal proximity of messages written to these streams,
> >    and this makes it harder to understand failures.
> 
> I agree 100%; that's what the combined output test above is supposed to
> handle.  If stdout and stderr are going to the same place then we
> redirect them to the same temporary file so they will ultimately appear
> in the same order as they would have on the terminal.  Only if they are
> not going to the same place anyway do we keep them separate.  This seems
> to always be the behavior you want.

But this redirection can be changed several times by the commands run
_after_ the initial decision described above was made: the shell
commands run by the job can do anything with these two handles, right?
So it could easily be the case that the output and error streams get
separated under OUTPUT_SYNC, where they originally appeared together,
interspersed.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]