[buildd-tools-devel] Bug#546555: Bug#546555: Bug#546555: Bug#546555: Bug#546555: Updated patches for support of remote source (.dsc) files

Andres Mejia mcitadel at gmail.com
Tue Oct 20 22:00:11 UTC 2009


On Monday 19 October 2009 17:00:32 Andres Mejia wrote:
> On Monday 19 October 2009 15:46:19 Roger Leigh wrote:
> > On Sun, Oct 18, 2009 at 09:01:56PM -0400, Andres Mejia wrote:
> > > On Sunday 18 October 2009 18:02:02 Roger Leigh wrote:
> > > > On Sat, Oct 17, 2009 at 02:12:58PM -0400, Andres Mejia wrote:
> > > > > On Saturday 17 October 2009 01:24:24 Andres Mejia wrote:
> > > > > > Please ignore the two patches I submitted earlier. These new
> > > > > > patches will give a better progress output than what's currently
> > > > > > implemented for sbuild and will also avoid the issue that
> > > > > > backspaces can't be done in the log files, which is what done by
> > > > > > LWP::UserAgent progres() subroutine.
> > > > > >
> > > > > > The first patch will make using LWP::UserAgent optional via the
> > > > > > Module::Load::Conditional module. It also gives a better output
> > > > > > for the progress indicator.
> > > > > >
> > > > > > The second patch ensures all output from the Utility subroutines
> > > > > > show up in the log files.
> > > > >
> > > > > Could these patches be applied instead. The first patch is the same
> > > > > as the previous patch from the previous message except that this
> > > > > one keeps the implementation to print a total size of content
> > > > > that's downloaded. It also allows it to be printed in MB, KB, and
> > > > > B. It's also more careful not to bring any unnecessary changes like
> > > > > changes to comments and formatting changes.
> > > >
> > > > Applied.  Once thing I'm not sure I like (not in the patch here, but
> > > > from previously) is the use of STDERR for the progress indicator. 
> > > > This will cause the build log (rather than the package build log) to
> > > > fill up with junk, and result in spurious mails about sbuild problems
> > > > being sent).
> > > >
> > > > Why can't STDOUT be used in place?
> > >
> > > Well, STDOUT can be used, so long as STDOUT becomes unbuffered.
> >
> > Or do an explict flush at the end of the loop so it it flushed each
> > time through:
> >
> >   STDOUT->flush();
> 
> Ok.
> 
> > > > Do we actually /need/ a progress indicator?  Can't we mirror the APT
> > > > behaviour here?  Note: you could check if the stream is a TTY
> > > > (isatty) or that we're running interactively before enabling the
> > > > progress meter to avoid filling the logfile with junk.  It should
> > > > probably be disabled in buildd mode or if not run interactively.
> > >
> > > I would keep the progress indicator. There are various packages that
> > > are very large (> 100MB), some of which I work on.
> >
> > Sure, but it will make the log unreadable.  You don't have an
> > interactive progress bar for apt-get downloads in sbuild, so I don't
> > really see a compelling reason to do this for other cases either.
> >
> > To check if a progress indicator is needed:
> > 1) Has --nolog been specified?  If so, then display indicator.
> > 2) Are we running in buildd mode?  If so, then don't display.
> >    if ($conf->get('SBUILD_MODE') ne 'buildd')
> >    ...
> >
> > There may be better criteria for deciding this, but I think this is a
> > decent enough starting point to avoid messed up log files.  It's also
> > currently possible to log to $saved_stdout and avoid the logger, which
> > is another possibility and this could allow logging in all cases except
> > when in buildd mode.
> 
> Ok. I'll see about taking care of that so that they don't show up in any
>  log files.
> 
> > > > I think the best policy here would be for those functions to do what
> > > > the chroot code does.  See STREAMIN/STREAMOUT/STREAMERR in Chroot.pm
> > > > for how we pass open file handles around.  This means the code is
> > > > completely agnostic as to which streams it's using, and hence is
> > > > rather more flexible.
> > > >
> > > > Current consensus seems to be that sbuild should drop the multiple
> > > > package stuff in favour of buildd handling this.  This would allow
> > > > us to drop the two logs in favour of a single unified log, which
> > > > would make some of the logging code simpler.
> > >
> > > I think what's best is that there's a per-package build log from
> > > sbuild, regardless if sbuild drops support for processing multiple
> > > packages.
> >
> > It will continue to do this.
> >
> > To summarise the changes I'd like done:
> >
> > 1) Switch from STDERR to STDOUT
> > 2) Explict flush of STDOUT at loop end
> > 3) Display progress only if nolog is true and sbuild_mode is not buildd.
> >    Alternatively, log to the saved stdout stream
> >    $Sbuild::LogBase::saved_stdout.
> >
> > For the last, you'll need to pass in $conf as for other utility
> > functions to get access to the configuration state.
> 
> For 3, I think I'll use the saved stdout stream.
> 
> > Regards,
> > Roger
> 

Here's a new patch that addresses these concerns.

-- 
Regards,
Andres
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Print-to-the-saved-stdout-stream-from-Sbuild-Logbase.patch
Type: text/x-patch
Size: 4050 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20091020/33e7a605/attachment.bin>


More information about the Buildd-tools-devel mailing list