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

[Andres Mejia]

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.

> 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.

As far as when to display the progress indicator, I do see that there really 
isn't much use to display the progress when sbuild is not be run 
interactively. I'm not sure how to detect for this though, or how to detect if 
sbuild is being run in buildd mode.

> > The second patch is the same patch I supplied in the previous message.
> 
> The current design of the sbuild logging is that there are two logfiles:
> 
> 1) the main log which logs everything except package build stuff
>    (per-invocation)
> 2) the package build log (per-package)
> 
> STDOUT and STDERR are already redirected--to the main log.  See line
> 87 in open_log() in LogBase.pm.  Your patch would break this, and
> it is also not correct since it doesn't revert the stream state on
> package log close as the close_log() code does.
> 
> Now, I'm quite willing to revisit this design.  It might well make
> sense to divert STDOUT and STDERR to the build log for the duration of
> the build (while the package log is open).  However, it needs to use
> the same sort of logic open_log and close_log do already.
> 
> The reason we stopped doing this in the first place was to allow for
> multiple parallel builds, in which case each package needs its own
> separate streams; hence the reason why we stopped using global streams.
> 
> 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.

> 
> Regards,
> Roger
> 

-- 
Regards,
Andres